Skip to content

Commit 7f82a23

Browse files
docs: sync tier 2 review doc with actual GetLeftPart(Authority) fix
Gemini round 2 caught that the review doc still showed the original scheme/host/port triple-comparison snippet, while the merged code uses Uri.GetLeftPart(Authority). Updated the snippet and explanation to match the actual implementation, and bumped the test-count count from 4 to 5 (the round-1 reply added an explicit-default-port edge-case regression test). Gemini's other round-2 comment is a repeat of the round-1 finding against the pre-fix code — acknowledged but no action needed; the fix from 0d65d7a is correct.
1 parent 0d65d7a commit 7f82a23

1 file changed

Lines changed: 18 additions & 10 deletions

File tree

docs/superpowers/reviews/2026-04-18-tier2-security-review.md

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,27 +107,35 @@ Net: **HIGH** due to impact + defense-in-depth principle.
107107
**Fix applied:**
108108

109109
```powershell
110-
$nextBuilder = [System.UriBuilder]::new($nextUrl)
111-
if ($nextBuilder.Scheme -ne $URI.Scheme -or
112-
$nextBuilder.Host -ne $URI.Host -or
113-
$nextBuilder.Port -ne $URI.Port) {
110+
$nextBuilder = [System.UriBuilder]::new($nextUrl)
111+
$originalOrigin = $URI.Uri.GetLeftPart([System.UriPartial]::Authority)
112+
$nextOrigin = $nextBuilder.Uri.GetLeftPart([System.UriPartial]::Authority)
113+
if ($nextOrigin -ne $originalOrigin) {
114114
throw "Refusing to follow pagination 'next' URL to a different origin..."
115115
}
116116
$nextBuilder
117117
```
118118

119-
The validation compares scheme + host + port against the *original*
120-
request URI. A response attempting to redirect pagination off-origin
121-
throws a clear error instead of being silently followed. All modern
122-
browsers apply this same rule to `Authorization` headers across redirects
123-
— we match that posture explicitly for an HTTP client library.
119+
`Uri.GetLeftPart([System.UriPartial]::Authority)` normalises both URIs
120+
to a canonical `scheme://host[:port]` form. The default port is
121+
automatically omitted when it matches the scheme's default (443 for
122+
https, 80 for http), so an original URI constructed with explicit
123+
`Port = 443` compares equal to a server-returned `next` that omits the
124+
port. The normalisation also avoids any edge cases with
125+
`UriBuilder.Port == -1` when a port is not explicitly set.
124126

125-
**Tests added** (`Tests/Helpers.Tests.ps1`, 4 new cases):
127+
A response attempting to redirect pagination off-origin throws a clear
128+
error instead of being silently followed. All modern browsers apply
129+
this same rule to `Authorization` headers across redirects — we match
130+
that posture explicitly for an HTTP client library.
131+
132+
**Tests added** (`Tests/Helpers.Tests.ps1`, 5 new cases):
126133

127134
1. Different host → throws
128135
2. Scheme downgrade https→http → throws
129136
3. Port change → throws
130137
4. Matching scheme/host/port → follows normally (regression test)
138+
5. Explicit default port in original vs omitted default port in `next` → follows (edge-case regression for the port normalisation)
131139

132140
### [MEDIUM — documented] DS-1. PSGallery publish runner installs Pester / PSScriptAnalyzer without upper version bound
133141

0 commit comments

Comments
 (0)