Stop leaking GITHUB_TOKEN to redirect targets in the skill installer - #225
Open
Chirag6722 wants to merge 1 commit into
Open
Stop leaking GITHUB_TOKEN to redirect targets in the skill installer#225Chirag6722 wants to merge 1 commit into
Chirag6722 wants to merge 1 commit into
Conversation
github_request() set an Authorization header on the request and handed it to urlopen, which follows redirects. urllib's redirect handler copies every header except Content-Length and Content-Type onto the new request, so a redirect served on an install URL received the user's GitHub token - any host, including a plain-http downgrade. Both installer entry points take the repo, ref and path from user-supplied arguments and the README invites running them against third-party repositories, so the leak was reachable by following an install one-liner from someone else's README. Scope the credential to the destination instead of the request: - Only attach the token when the URL is https and the host is github.com or githubusercontent.com (or a subdomain), so archive downloads that redirect to codeload/objects still authenticate for private repos. - Re-check on every hop through a redirect handler that strips Authorization when the target fails that test, and open requests through an opener built with it. - Refuse non-https URLs outright rather than sending credentials in cleartext, and apply a 30s timeout so a hung host cannot block forever. - Percent-encode owner, repo, ref and path when building GitHub URLs, and reject refs containing empty or dot segments. tests/test_github_utils.py covers the host and scheme rules, which header is sent where, and an end-to-end redirect chain over real sockets that asserts the second host sees no Authorization header. That test fails against the previous implementation. Closes composio-community#224 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #224
The bug
skill-installer/scripts/github_utils.pyattached the user's token to the request and then leturlopenfollow redirects.urllib'sHTTPRedirectHandlercopies every header exceptContent-Length/Content-Typeonto the redirect request, soAuthorizationtravelled to whatever host the server pointed at — a different domain, or a downgrade to plainhttp://.Reproduced with two local servers, the first redirecting to the second:
Both entry points are reachable with user-supplied arguments (
--repo,--url,--ref,--path), and the README plus many catalog entries tell users to run the installer against third-party repositories. A redirect anywhere on that path silently exfiltrates a token that usually carriesreposcope, while the install still succeeds.The fix
Treat authentication as a property of the destination, re-evaluated on every hop:
is_trusted_url()— the token is attached only when the URL ishttpsand the host isgithub.comorgithubusercontent.com, or a subdomain. GitHub archive downloads redirect tocodeload.github.com/objects.githubusercontent.com, so private-repo installs keep working; a look-alike such asgithub.com.evil.comdoes not match._AuthScopingRedirectHandlerstripsAuthorization(from bothheadersandunredirected_hdrs) whenever a redirect target fails that check. Requests now go through an opener built with it rather than the module-levelurlopen.httpsURLs are refused outright instead of sending credentials in cleartext, and a 30s timeout replaces the previous unbounded wait../..segments are rejected. Slashes are preserved in ref names, so--ref refs/heads/masterstill works.A non-GitHub URL is still fetched — just anonymously — so nothing that worked without a token breaks.
Tests
tests/test_github_utils.py, 24 cases: the host/scheme matrix (including thegithub.com.evil.comlook-alike and host casing), redirect stripping in both directions, which header is sent forGITHUB_TOKEN/GH_TOKEN/ neither / untrusted host, non-https refusal, URL encoding, and ref validation — plus an end-to-end redirect chain over real sockets asserting the second host receives noAuthorization. That last test fails against the previous implementation:Manually verified against the real repo with no token set:
--repo/--pathinstall,--urltree-URL install,--ref refs/heads/master, andlist-curated-skills.pyall still succeed.Note on overlap
This branch adds
tests/and.gitignore, which #223 also adds (identical.gitignore, different test file). Whichever merges first, the other should merge cleanly; #223 is what wirestests/into CI.🤖 Generated with Claude Code