Strip rel-link authorization only across origins - #27981
Open
Chris Peterson (chris-peterson) wants to merge 1 commit into
Open
Chris Peterson (chris-peterson) wants to merge 1 commit into
Chris Peterson (chris-peterson) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Chris Peterson (chris-peterson)
force-pushed
the
restore-authenticated-paging
branch
from
September 6, 2026 16:52
1fb569b to
4203c04
Compare
Chris Peterson (chris-peterson)
marked this pull request as ready for review
September 6, 2026 17:13
Chris Peterson (chris-peterson)
requested review from
a team and
Justin Chung (jshigetomi)
as code owners
September 6, 2026 17:13
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Copilot started reviewing on behalf of
Chris Peterson (chris-peterson)
September 6, 2026 17:13
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to origin-based stripping logic and is backed by targeted tests for same-origin and cross-origin rel-link behavior.
Pull request overview
This PR fixes Invoke-RestMethod -FollowRelLink incorrectly stripping the caller-provided Authorization header on subsequent rel-link page fetches by treating them like redirects, restoring expected behavior for authenticated pagination while still stripping credentials when a rel-link crosses origins.
Changes:
- Normalize the initially requested
-Uriand compare each followed rel-link’s origin (scheme/host/port) against the originally requested origin to decide whether to stripAuthorization. - Extend the WebListener
Linktest controller to emitrel="next"links pointing at an alternate origin via anextoriginquery parameter. - Update and expand Pester coverage to assert same-origin rel-links keep
Authorization, cross-origin rel-links strip it, and-PreserveAuthorizationOnRedirectoverrides stripping across origins.
File summaries
| File | Description |
|---|---|
src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs |
Fixes rel-link paging auth behavior by stripping Authorization only when the followed rel-link leaves the originally requested origin. |
test/tools/WebListener/Controllers/LinkController.cs |
Adds nextorigin support so the Link controller can generate cross-origin rel="next" links for tests. |
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 |
Updates/extends -FollowRelLink tests to cover same-origin retention, cross-origin stripping, and override behavior. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 10, 2026
Chris Peterson (chris-peterson)
added a commit
to chris-peterson/pwsh-gitlab
that referenced
this pull request
Sep 12, 2026
## Context `Invoke-GitlabApi` pages by handing `Invoke-RestMethod` the `-FollowRelLink` flag and letting it walk the `rel=next` chain GitLab puts in its `Link` header. PowerShell 7.6 drops the `Authorization` header when it follows one of those links, so the request for page 2 goes out unauthenticated. Anything using `-All` over a result longer than one page breaks — GitLab serves 20 per page by default, so that is most of them. `-PreserveAuthorizationOnRedirect` keeps the header attached. The upstream bug is [PowerShell#27861](PowerShell/PowerShell#27861); the properly scoped fix is in flight as [PowerShell#27981](PowerShell/PowerShell#27981) (with [#27873](PowerShell/PowerShell#27873) as the forward-port). This comes back out once that ships — hence the comment pointing at the issue rather than a bare flag. ## Review guide **Core change, and the whole diff** — [`src/GitlabCli/Utilities.psm1`](https://github.com/chris-peterson/pwsh-gitlab/pull/167/changes#diff-bcad0c863b627b9e28cb755bd24ac1be9ab715a01e6750820f02009a80ec2c3cR103) ## Approach & trade-offs The flag is broader than what this deserves: it also preserves the header across *cross-origin* redirects, which is the case the strip was added to close. Every `rel=next` GitLab emits here points back at the same host the request already went to — measured, not assumed — so the exposure it reopens is theoretical while the breakage it repairs is not. The narrower alternative was switching the header to `PRIVATE-TOKEN`, which PowerShell doesn't strip. It works, but it's GitLab-specific and swapping the auth scheme to dodge a client bug is a bigger change to reason about than one flag with an expiry date on it. ## Validation Measured against `gitlab.getty.cloud` with a raw `Invoke-RestMethod`, so the result isn't shaped by this module. Both endpoints page at 20 per request: | Endpoint | without the flag | with the flag | | --- | --- | --- | | `issues?assignee_username=…` (112 items) | **401 on page 2** | 112 items | | `merge_requests?…&scope=all` (55 items) | **21 items, exit 0, no error** | 55 items | The second row is the one worth pausing on. The same defect does not always surface as a failure — that call returned a partial result and reported success, and a short list looks like an answer. This module's callers have no way to tell 21 from 55. The effect is endpoint-specific rather than uniform: on GitHub's API the plain `/issues` list endpoint throws the same 401 while `/search/issues` walks its chain intact. So which symptom you meet, or whether you meet one at all, depends on the call you reach for first. [`pwsh-github#11`](chris-peterson/pwsh-github#11) carries the identical one-line fix. The two are independent — neither has to merge first.
-FollowRelLink exists so Invoke-RestMethod can page an API that advertises its next page in a Link header, and those APIs want the caller's Authorization header on every page. Since 91448ff the rel-link loop passed isRedirect: followedRelLink > 0, so GetRequest dropped that header on every page after the first. That reads as truncation rather than an error. Against a server that permits anonymous reads the unauthenticated follows still return 200, with a visibility-filtered result set: a GitLab group of 43 projects came back as 39 at every page size, with no warning and no non-zero status. A rel link is not a redirect. It is a client-initiated GET to a URL the same server advertised in its own Link header, so the credentials the caller supplied for that API still apply. Compare the followed link's scheme, host and port against the origin the caller requested and pass isRedirect only when they differ; CheckProtocol normalizes the requested URI so a -Uri given without a scheme has an origin to read, which GetRequest applies per request anyway. Genuine 3xx redirects and -PreserveAuthorizationOnRedirect are untouched. 91448ff's rel-link assertion is replaced. WebListener generated every link from the request's own URL, so that case could only ever prove the same-origin behavior this changes. Four cases take its place: the header reaching every same-origin page, with and without a scheme on -Uri; every page past a cross-origin link still stripping; and -PreserveAuthorizationOnRedirect still overriding across origins. The cross-origin cases walk three pages, the last two served by the second origin, so they hold only while each hop is compared against the requested origin rather than the hop before it. LinkController gains a nextorigin query parameter so a rel="next" can point at another listener port.
Chris Peterson (chris-peterson)
force-pushed
the
restore-authenticated-paging
branch
from
September 13, 2026 17:12
daedf2b to
fcecebe
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
Fixes #27861.
-FollowRelLinkexists soInvoke-RestMethodcan page an API that advertises its next page in aLinkheader — GitHub's and GitLab's paginated endpoints are the canonical case, and they want the caller'sAuthorizationheader on every page, not just the first. Since91448ff15the rel-link loop passesisRedirect: followedRelLink > 0, soGetRequestdrops that header on every page after the first.Against a server that permits anonymous reads, that reads as truncation rather than an error: the unauthenticated follows still return 200, with a visibility-filtered result set. A GitLab group of 43 projects came back as 39 at every page size tried, reached through the
GitlabClimodule'sGet-GitlabProject -Recurse -All, with no warning and no non-zero status.PR Context
A rel link is not a redirect. It is a client-initiated GET to a URL the same server advertised in its own
Linkheader, so the credentials the caller supplied for that API still apply. Stripping on every hop removes them from the one case-FollowRelLinkwas added to serve.This compares the followed link's scheme, host and port against the origin the caller requested and passes
isRedirectonly when they differ. Genuine 3xx redirects still strip unconditionally, and-PreserveAuthorizationOnRedirectstill overrides both.The test contract this changes.
91448ff15added five test cases; this contradicts exactly one, the rel-link one. WebListener generated every link from the request's own display URL, so nothing in that suite could reach a second origin — which is why "strips the authorization header on followed relation links" was only ever proven same-origin. The four-PreserveHttpMethodOnRedirectcases it added exercise the 3xx path, for both cmdlets, and are untouched.masteris the target because 7.7 and later inherit from it. The same call site is live onrelease/v7.4.20,release/v7.5.11,release/v7.6.6andrelease/v7.7.0-preview.5, and shipped in v7.4.19, v7.5.10, v7.6.5 and v7.7.0-preview.4 — backports follow this.Review guide
Start here — the fix.
WebRequestPSCmdlet.Common.csis the origin comparison, and the call it feeds is the only behavior change in the product code.Uri.ComparewithUriComponents.SchemeAndServeris the framework's own origin comparison, which normalizes a default port away rather than leaving that to string handling. Note thatrequestedUriis fixed at what the caller asked for, so every hop is measured against that rather than against the hop before it — a walk that leaves the origin stays unauthenticated for the rest of its pages.CheckProtocol(Uri)is load-bearing, not tidying: a-Urigiven without a scheme parses as a relative URI,Uri.Comparethen reports it as a mismatch against the absolute followed link, and the header would be stripped on every page.GetRequestapplies the same normalization per request, so passing it on changes nothing about the request itself.The contract change.
keeps the authorization header on relation links within the originis91448ff15's assertion inverted, now covering both the scheme and scheme-less forms of-Uri— the second case is what makes the normalization above provable. Two more cases pin the other side: every page past a cross-origin link still strips, and-PreserveAuthorizationOnRedirectstill overrides across origins. Both cross-origin cases walk three pages, the last two served by the second origin, so they hold only while the comparison stays anchored to the requested origin.Test-tool support.
LinkController.cstakes anextoriginquery parameter so arel="next"can point at another listener port. Without it no case in this suite can leave its origin — the redirect tests included.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header