Allow public extension installs past SAML enforcement - #14108
Open
loganrosen wants to merge 3 commits into
Open
Conversation
Retry read-only extension release requests without authentication when a SAML-protected token blocks a public repository. Preserve private repository errors and propagate upgrade metadata failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve retry behavior for Git extension version checks while caching binary release results for install and upgrade reuse. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cover anonymous tag metadata and asset download while preserving the pinned manifest state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
Thanks for your pull request! This is a large change (764 lines across 10 files) that doesn't reference a Large feature PRs require prior discussion in an issue before implementation — this helps the team assess whether the feature aligns with the project's direction before significant effort is invested. Please open an issue to discuss this feature first. This PR will be automatically closed in 2 days if requirements are not met. Full contribution requirements
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates the extension installation and upgrade flows to recover from organization SAML enforcement that blocks authenticated API requests for public extension repositories, by retrying specific release/repo lookups without authentication and reusing the selected client for subsequent release/asset operations. It also makes SSO challenge URL handling effectively request-scoped for extension traffic, avoiding stale SSO guidance leaking into unrelated errors.
Changes:
- Add a SAML-enforced 403 fallback path for extension release lookups that retries anonymously only after confirming the repo is publicly accessible.
- Propagate and reuse the selected HTTP client (authenticated vs anonymous) across release metadata reads and asset downloads, including upgrades and git→binary migration.
- Make SSO URL extraction prefer per-error response headers and introduce a transport wrapper to skip global header extraction for extension requests.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmd/factory/default.go | Uses an extension-scoped HTTP client that skips global X-GitHub-SSO extraction; adds SSOURLFromHeader helper and wires a plain (unauthenticated) client into the extension manager. |
| pkg/cmd/extension/manager.go | Threads plain client support through install/upgrade paths; reuses selected client and cached release data to avoid repeated SAML failures and duplicate requests. |
| pkg/cmd/extension/manager_test.go | Adds coverage for SAML fallback behavior across install/upgrade, including public vs private behavior and git-version retry behavior. |
| pkg/cmd/extension/http.go | Introduces fetch*WithFallback helpers and SAML-protected detection to retry release fetches anonymously after confirming public accessibility. |
| pkg/cmd/extension/http_test.go | Adds focused tests for release fallback selection plus a reusable SAML-protected responder. |
| pkg/cmd/extension/extension.go | Caches latest release + selected client and preserves retry semantics for git extensions while avoiding repeated binary lookup failures. |
| internal/ghcmd/cmd.go | Derives SSO recovery URL from the error’s response headers when available, avoiding reliance on global extracted state for HTTP errors. |
| internal/ghcmd/cmd_test.go | Adds test coverage for the new ssoRecoveryURL behavior. |
| api/http_client.go | Adds SkipHeaderExtraction and updates ExtractHeader to honor request-scoped skipping via context. |
| api/http_client_test.go | Adds test ensuring SkipHeaderExtraction prevents extraction without removing the response header itself. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 10/10 changed files
- Comments generated: 0
- Review effort level: Lite
6 tasks
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.
Fixes #6675
Description
Organization SAML enforcement can make an otherwise valid token return a 403 when GitHub CLI reads release metadata for a public extension. This prevents commands such as
gh extension install github/gh-net, even though the same release is publicly downloadable without authentication.Extension release requests now remain authenticated by default, but retry without authentication when the response is specifically a SAML-enforced 403 and the repository is confirmed to be public. The selected client is then reused for release metadata and asset downloads. Private repositories and non-SAML failures continue to return the original authenticated error.
The change covers binary and script installs, pinned releases, upgrades, and Git-to-binary migration. It also keeps SSO response-header handling request-scoped so a recovered extension request cannot produce stale authorization guidance elsewhere.
How did you test this change?
go test ./pkg/cmd/extension/...- passedgo test -race ./pkg/cmd/extension/...- passedenv -u GH_TOKEN -u GH_HOST -u GIT_CONFIG_COUNT -u GIT_CONFIG_KEY_0 -u GIT_CONFIG_VALUE_0 go test ./...- passedgo run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.0 run ./...-0 issues.I also reproduced the same failure mode with an active token:
GET /repos/github/gh-stack/releases/latestreturned 403 withX-GitHub-SSO. Using the built binary and that same token, an isolatedgh extension install github/gh-stackinstalled v0.1.0 successfully, andgh stack --helpran successfully.Key points
X-GitHub-SSOand only proceeds after an unauthenticated repository check succeeds.Notes for reviewers
Start with
pkg/cmd/extension/http.gofor fallback selection, thenpkg/cmd/extension/manager.gofor client and release reuse.pkg/cmd/factory/default.goandinternal/ghcmd/cmd.gocontain the request-scoped SSO handling.Issue #6675 contains the original
github/gh-netreproduction.github/gh-stackwas used to validate the same failure mode and fix against the current implementation.Authorship and follow-up
Who wrote this:
Who answers review comments: