Skip to content

fix(server): repository lookups no longer send GitHub tokens to arbitrary hosts - #26176

Merged
juliusmarminge merged 1 commit into
mainfrom
fix/github-selector-host-validation
Oct 27, 2026
Merged

juliusmarminge merged 1 commit into
mainfrom
fix/github-selector-host-validation

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Any client with only orchestration:read could call sourceControl.lookupRepository with a selector such as evil.com#.ghe.com/o/n. The host passed the .ghe.com check, the server chose GH_TOKEN/GITHUB_TOKEN, and the request with the bearer token went to https://api.evil.com#.ghe.com, which is api.evil.com. Characters like ?, @ and / worked the same way.

The fix rejects any host that is not a plain hostname (with an optional port):

  • parseGitHubRepositorySelector treats a host/owner/name selector with an invalid host as unparseable.
  • As a second check, GitHubCredentials.get and environmentToken refuse to hand out a token for such a host (saved, env, or gh), whatever path it arrived by.

Checked and unchanged:

  • GH_HOST: comes from the server's own environment, and env-token matching now also requires a valid host.
  • parsePullRequestReference and URL selectors take the host from URL.host, which is already safe.
  • Remote-derived hosts go through new URL(...).host.
  • GitCafe has a fixed host allowlist.
  • Forgejo picks a credential only by exact match against stored tea/fj logins.
  • Bitbucket has a fixed API base.
  • GitLab and Azure DevOps leave host and credential selection to their CLIs.

Tests: new failing-first cases in gitHubRepositoryResolution.test.ts and GitHubCredentials.test.ts.

Claude Opus 5.5 via Claude Code (in T3 Code)

🤖 Generated with Claude Code

…rary hosts

A host/owner/name selector took its host verbatim, so `evil.com#.ghe.com/o/n`
looked like a GHE.com host, got GH_TOKEN, and the request went to api.evil.com.
Selectors and credential lookups now accept only a plain hostname with an
optional port.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 27, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 04a7143

Macroscope's review found this PR approvable — Repository selectors and credential lookups now reject malformed host text before retrieving tokens. This is a contained security hardening change that preserves normal GitHub and enterprise-host credential behavior.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.6 KiB 21.6 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.1 KiB 5.0 KiB −35 B (−0.7%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.2 KiB −35 B (−2.7%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB −41 B (−0.2%) 24.4 KiB ✅
Claude Live turn messages 2 1 −1 (−50.0%) 8 ✅

Baseline: e66ffcc · PR result: 04a7143 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between e66ffcc and 04a7143.

Files selected for processing: 4
  • packages/source-control-github/src/server/GitHubCredentials.test.ts
  • packages/source-control-github/src/server/GitHubCredentials.ts
  • packages/source-control-github/src/server/gitHubRepositoryResolution.test.ts
  • packages/source-control-github/src/server/gitHubRepositoryResolution.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

The change adds a shared GitHub host validator and applies it to environment-token lookup, credential lookup, and three-part repository selectors. Tests cover invalid host inputs and a selector with a port.

Changes: GitHub host validation

Layer / File(s) Summary
Host validation for credentials
packages/source-control-github/src/server/GitHubCredentials.ts, packages/source-control-github/src/server/GitHubCredentials.test.ts
A hostname pattern and exported validator are added. Environment-token lookup returns null for invalid hosts, and credential lookup returns GitHubNotSignedInError before checking host settings or credentials. Tests check that invalid inputs do not invoke the process runner.
Repository selector host validation
packages/source-control-github/src/server/gitHubRepositoryResolution.ts, packages/source-control-github/src/server/gitHubRepositoryResolution.test.ts
Three-part selectors are accepted only when their lowercased host passes the validator. Tests cover a host with a port and selectors with suspicious characters or a leading hyphen.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

  • pingdotgg/t3code#23995: Both changes affect GH_REPO parsing and handling of repository selectors with invalid hosts.
  • pingdotgg/t3code#21581: Both changes affect repository-specific GitHub credential selection through repository selectors.
  • pingdotgg/t3code#21265: The PR adds repository-scoped credentials; this change validates hosts before credential lookup.

Suggested reviewers: maria-rcks, macroscopeapp

Merge Risk: Minimal · up to 04a71

No concrete merge-blocking defect is established; the host-validation change is ready for normal checks.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the vulnerability, the fix, affected credential paths, and added tests. It does not provide the required scope and approval information, and the verification section does not … Add explicit Problem, Change, Scope and approval, and Verification sections. Provide the triaged issue or maintainer approval, or explain why this focused obvious bug qualifies without one. Report the exact tests run and their observed resu…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary security fix: preventing repository lookups from sending GitHub tokens to arbitrary hosts.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the vulnerability, the fix, affected credential paths, and added tests. It does not provide the required scope and approval information, and the verification section does not report observed test results or unverified checks.

Resolution

Add explicit Problem, Change, Scope and approval, and Verification sections. Provide the triaged issue or maintainer approval, or explain why this focused obvious bug qualifies without one. Report the exact tests run and their observed results, and state what was not checked.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge
juliusmarminge merged commit 80b7b0f into main Oct 27, 2026
30 checks passed
@juliusmarminge
juliusmarminge deleted the fix/github-selector-host-validation branch October 27, 2026 23:23
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Oct 28, 2026
## What's Changed
* fix(mobile): center Android sheets and fit book-mode sheets in the left pane by @PixPMusic in pingdotgg/t3code#18492
* fix(mobile): keep the collapsed Android rail square by @PixPMusic in pingdotgg/t3code#26039
* fix(web): closing the last Browser tab hides the right panel again by @juliusmarminge in pingdotgg/t3code#26058
* refactor(web): pull requests Cmd+F focuses the topbar search directly by @juliusmarminge in pingdotgg/t3code#26045
* refactor(web): install draft model reconciliation at app start by @juliusmarminge in pingdotgg/t3code#26046
* perf(mobile): project favicons mount their image in one commit by @juliusmarminge in pingdotgg/t3code#26047
* refactor(web): the upload queue alone records finished file uploads on drafts by @juliusmarminge in pingdotgg/t3code#26050
* feat(mobile): Usage uses the width on iPad and unfolded phones by @juliusmarminge in pingdotgg/t3code#26061
* fix(gitcafe): PR panel follows renumbered pull requests by @juliusmarminge in pingdotgg/t3code#26079
* chore(infra): bump Alchemy to 2.0.0-beta.84 by @juliusmarminge in pingdotgg/t3code#26057
* refactor(mobile): stop writing the resolved project filter back from an effect by @juliusmarminge in pingdotgg/t3code#26049
* fix(mobile): a pairing link fills the open Add environment form instead of stacking another by @juliusmarminge in pingdotgg/t3code#26052
* fix(mobile): snooze menu times advance with the clock by @juliusmarminge in pingdotgg/t3code#26043
* fix(clients): prompt links find projects cloned from git.cafe by @juliusmarminge in pingdotgg/t3code#26042
* fix(server): retry Codex capacity failures with empty input by @Yash-Singh1 in pingdotgg/t3code#25611
* fix(server): recover handoffs when target context is exhausted by @Yash-Singh1 in pingdotgg/t3code#25610
* fix(web): chips drop their pill and stay readable on every theme by @flamboh in pingdotgg/t3code#25066
* fix(mobile): switching pull request tabs no longer jumps the page by @flamboh in pingdotgg/t3code#26064
* feat(mobile): report crashes to Axiom with symbolicated stacks by @gabrielelpidio in pingdotgg/t3code#19093
* fix(web): chip palette test follows the single design by @juliusmarminge in pingdotgg/t3code#26131
* feat(server): /mcp narrows a client's modes from T3-Mode-Limit by @juliusmarminge in pingdotgg/t3code#16653
* feat(server): t3_thread_launch takes clientRequestId by @juliusmarminge in pingdotgg/t3code#16654
* feat(server): link to another environment by @juliusmarminge in pingdotgg/t3code#16655
* feat(server): agents use linked environments through any T3 tool by @juliusmarminge in pingdotgg/t3code#16684
* feat(server): work a linked environment starts stays within its link by @juliusmarminge in pingdotgg/t3code#16695
* feat(web,mobile): linked environments in Connections by @juliusmarminge in pingdotgg/t3code#16698
* feat(server): delegate_task to a linked environment by @juliusmarminge in pingdotgg/t3code#16719
* feat(server,web,mobile): an agent sets up a link to another environment by @juliusmarminge in pingdotgg/t3code#17129
* feat(server,web,mobile): @-mention a machine, and linked environments become a Labs beta by @juliusmarminge in pingdotgg/t3code#17130
* fix(mobile): dragging a microphone or environment route now saves its new place by @juliusmarminge in pingdotgg/t3code#26096
* feat(web): channel artwork choice on Latest, plus nightly or dev art on any channel by @maria-rcks in pingdotgg/t3code#26138
* feat(mobile): browse ZIP attachments with the shared archive reader by @Bil0000 in pingdotgg/t3code#19463
* fix(server): Claude --settings launch args merge with T3's settings instead of being dropped by @t3dotgg in pingdotgg/t3code#22864
* feat(server,web): see and stop the dev servers a thread started by @t3dotgg in pingdotgg/t3code#22542
* fix(web): composer no longer jumps when a second line is added by @maria-rcks in pingdotgg/t3code#26155
* feat(web,mobile): queued messages show who sent them by @juliusmarminge in pingdotgg/t3code#26159
* fix(preview): agent and browser panel show the same page after a late desktop attach by @t3dotgg in pingdotgg/t3code#22877
* fix(web): pairing, connect, error, and Clerk sign-in surfaces match the app's dialog style by @maria-rcks in pingdotgg/t3code#25281
* fix(server): parallel worktree launches no longer fail on .git/config lock by @juliusmarminge in pingdotgg/t3code#25700
* fix(task-cache): the hook test waits for every flusher it starts by @juliusmarminge in pingdotgg/t3code#26148
* chore(lint): every React effect states its reason at the call site by @juliusmarminge in pingdotgg/t3code#26161
* test(server): the link-card retry test names a new MCP credential by @juliusmarminge in pingdotgg/t3code#26143
* perf(web): stop measuring table cells while replies stream by @t3dotgg in pingdotgg/t3code#26152
* feat(desktop): add a remote server by clicking its t3code://pair link by @t3dotgg in pingdotgg/t3code#24111
* fix(server): agent questions stay answerable after a server restart by @SunkenInTime in pingdotgg/t3code#19229
* chore(lint): the effect lint error walks the alternatives before the suppression by @juliusmarminge in pingdotgg/t3code#26181
* fix(web): settings avatar no longer hugs the modal corner by @t3dotgg in pingdotgg/t3code#26180
* fix(mobile): shared file names can no longer write outside the Android cache by @juliusmarminge in pingdotgg/t3code#26175
* fix(server): repository lookups no longer send GitHub tokens to arbitrary hosts by @juliusmarminge in pingdotgg/t3code#26176
* fix(github): long period runs no longer freeze reference autolinking by @juliusmarminge in pingdotgg/t3code#26177
* fix(mobile): Android diff ellipsis no longer measures text quadratically by @juliusmarminge in pingdotgg/t3code#26174
* fix(server): Codex works again after its app-server is killed by @t3dotgg in pingdotgg/t3code#26188


**Full Changelog**: pingdotgg/t3code@v0.1.4-nightly.20261027.3783...v0.1.4-nightly.20261027.3796

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.1.4-nightly.20261027.3796
Pipyakas pushed a commit to Pipyakas/t3code that referenced this pull request Oct 28, 2026
…rary hosts (pingdotgg#26176)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 80b7b0f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant