fix(api,cli): gate first-claim of a GitHub repo binding on entitlement (issue #297)#356
Conversation
…t (issue #297) Closes a live cross-tenant gap: with the App installed org-wide, any workspace could implicitly bind (via /github/comment or /github/promote) or explicitly claim (via `uploads github link`) an unbound repo belonging to a different org, then post or deface the uploads-sh[bot] comment there. There was no check that the calling workspace had any real relationship to the repo it was about to own. A workspace may now make the FIRST claim on a repo only when its linked GitHub account (issue #340/#344 attribution) has push/maintain/admin access to that repo, verified live against GitHub via the App's own installation token (github-claim-authz.ts). Legacy/enrollment/shared tokens (including the communal `default` workspace's) have no linked identity and can never claim a new repo, though they keep working on repos already bound to them. Already-bound repos are grandfathered and never re-checked. Every decline is a soft `{ posted: false / claimed: false, reason: "not_authorized" }`, never a 5xx, and the CLI never falls back to `gh` for it.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUnbound GitHub repository claims now require live verification that the caller’s linked GitHub account has push-level or higher access. Unauthorized claims return soft ChangesRepository claim authorization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GithubCommentRoute
participant isEntitledToClaimRepo
participant collaboratorPermission
participant GitHubAPI
Client->>GithubCommentRoute: post comment for repository
GithubCommentRoute->>isEntitledToClaimRepo: verify unbound repository claim
isEntitledToClaimRepo->>collaboratorPermission: check linked login permission
collaboratorPermission->>GitHubAPI: request collaborator permission
GitHubAPI-->>collaboratorPermission: permission response
collaboratorPermission-->>isEntitledToClaimRepo: entitlement result
isEntitledToClaimRepo-->>GithubCommentRoute: allow or not_authorized
GithubCommentRoute-->>Client: comment result
Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-api | aa35f16 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 08:54 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-auth | c08cfa8 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 08:31 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-web | c08cfa8 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 08:31 PM |
Zach Dunn (zachdunn)
added
the
coderabbit:review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/api/src/github-app.ts (1)
172-195: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd defense-in-depth validation for
repohere too.
loginis guarded byLOGIN_REbefore interpolation, butrepois interpolated into the same GitHub API path (Line 186) with no shape check in this function itself — it relies entirely on every caller (github-link.ts,github-promote.ts,github-comment.ts) pre-validating withREPO_RE/DOTS_ONLY_RE. That's safe today, but this is an exported, security-critical helper (the sole GitHub-side signal for cross-tenant claim authorization) — a future caller that skips that validation could let a craftedrepovalue (e.g. containing..segments) get dot-segment-normalized by the URL/fetch layer into an unintendedapi.github.compath using the installation token.🛡️ Proposed defensive check
+const REPO_RE = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/; + export async function collaboratorPermission( env: Env, cfg: GithubAppConfig, installationId: number, repo: string, login: string, fetchImpl: typeof fetch = fetch, ): Promise<string | null> { - if (!LOGIN_RE.test(login)) return null; + if (!LOGIN_RE.test(login) || !REPO_RE.test(repo)) return null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/github-app.ts` around lines 172 - 195, Add defense-in-depth validation for the repo parameter at the start of collaboratorPermission, using the existing repository-shape validation symbol (REPO_RE or the established equivalent), and return null before obtaining an installation token or constructing the GitHub URL when validation fails. Preserve the existing LOGIN_RE check and valid-repository behavior.apps/api/src/routes/github-comment-route.test.ts (1)
11-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the
mintingUserIdD1-stub pattern into one shared test helper.The same "fake an
auth_tokensrow withminting_user_id" stub is copy-pasted across three test files. A shared helper (e.g. alongsideRepoLinksTable/FakeKvinapps/api/test/helpers/) would remove the drift risk if theauth_tokensshape ever changes.
apps/api/src/routes/github-comment-route.test.ts#L11-L54: extractclaimTestDb's minting-user stub logic into the shared helper and have this file call it.apps/api/src/routes/github-link-route.test.ts#L26-L93: replace the inlinedb.prepareoverride inseededEnvwith a call to the same shared helper.apps/api/src/routes/github-promote-route.test.ts#L42-L161: replace the inlinedb.prepareoverride inseededEnvwith a call to the same shared helper (keeping it distinct from the adjacentscopedTokenoverride).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/github-comment-route.test.ts` around lines 11 - 54, Extract the shared auth_tokens/minting_user_id D1 stub into a helper alongside RepoLinksTable/FakeKv, preserving the existing token-row shape and lookup behavior. In apps/api/src/routes/github-comment-route.test.ts#11-54, update claimTestDb to use the helper; in apps/api/src/routes/github-link-route.test.ts#26-93 and apps/api/src/routes/github-promote-route.test.ts#42-161, replace the inline seededEnv prepare overrides with the same helper, keeping promote’s adjacent scopedToken override separate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/src/github-app.ts`:
- Around line 172-195: Add defense-in-depth validation for the repo parameter at
the start of collaboratorPermission, using the existing repository-shape
validation symbol (REPO_RE or the established equivalent), and return null
before obtaining an installation token or constructing the GitHub URL when
validation fails. Preserve the existing LOGIN_RE check and valid-repository
behavior.
In `@apps/api/src/routes/github-comment-route.test.ts`:
- Around line 11-54: Extract the shared auth_tokens/minting_user_id D1 stub into
a helper alongside RepoLinksTable/FakeKv, preserving the existing token-row
shape and lookup behavior. In
apps/api/src/routes/github-comment-route.test.ts#11-54, update claimTestDb to
use the helper; in apps/api/src/routes/github-link-route.test.ts#26-93 and
apps/api/src/routes/github-promote-route.test.ts#42-161, replace the inline
seededEnv prepare overrides with the same helper, keeping promote’s adjacent
scopedToken override separate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f80081c2-b11b-499f-bbbf-593ad54b286c
📒 Files selected for processing (16)
.changeset/cross-tenant-repo-claim-authz.mdapps/api/src/github-app.tsapps/api/src/github-claim-authz.test.tsapps/api/src/github-claim-authz.tsapps/api/src/routes/github-comment-route.test.tsapps/api/src/routes/github-comment.tsapps/api/src/routes/github-link-route.test.tsapps/api/src/routes/github-link.tsapps/api/src/routes/github-promote-route.test.tsapps/api/src/routes/github-promote.tsapps/api/src/uploader-identity.tsapps/web/src/pages/docs/github-app.astropackages/uploads/src/client.tspackages/uploads/src/commands.tspackages/uploads/test/commands-github-link.test.tsskills/uploads-cli/SKILL.md
- collaboratorPermission now validates repo shape (REPO_RE) up front, before minting an installation token or building the GitHub URL, matching the existing LOGIN_RE defense-in-depth guard. - Dedupe the auth_tokens/minting_user_id D1 stub used across github-comment-route.test.ts, github-link-route.test.ts, and github-promote-route.test.ts into a shared test/helpers/fake-minting-user-token.ts helper; promote's adjacent scopedToken override is untouched.
1 participant
In plain terms
Any workspace could become the "owner" of any other org's GitHub repo, just by being the first to call the bot-comment/promote endpoints (or
uploads github link) against it — the App is installed org-wide, so uploads.sh holds a write token for every customer org, and nothing checked that the calling workspace actually had a real relationship to the repo it was claiming. Once claimed, that workspace could post — or overwrite — theuploads-sh[bot]comment on someone else's PR.This closes that gap: a workspace can only make the first claim on a repo when its linked GitHub account actually has push access there, checked live against GitHub itself.
The gap (post-#327/#346)
PR #327 added the baseline binding gate (
github_repo_links): once a repo is bound, a different workspace'scommentcall declines. PR #346 let the communaldefaultworkspace claim repos like any other workspace. Neither addressed the actual claim step —installationForReporesolves any org's installation for any workspace's token, so the very first call from any workspace against an unbound repo silently became that repo's permanent binding (recordRepoLink, first-claim-wins). Three routes could create that first binding with zero entitlement check:/github/comment,/github/promote, and the explicitPOST /github/linkbehinduploads github link.Exploit: org A uploads its own image under a crafted key
gh/orgB/repo/pull/N/x.png, then calls its own workspace's/github/commentwith{repo: "orgB/repo", num: N}. The server resolves org B's installation, posts asuploads-sh[bot]into org B's PR, and records org A's workspace asorgB/repo's owner going forward — blocking org B's own legitimate calls from then on.Design
New module
apps/api/src/github-claim-authz.ts(isEntitledToClaimRepo) gates every first-claim call site.WorkspaceRecordhas no GitHub org/owner field, and the workspace↔org mapping is a Better Auth organization slug — it says nothing about GitHub repo access — so the strongest signal actually available is: the calling token's minting user's linked GitHub login (issue #340/#344 attribution) must have push/maintain/admin permission on the target repo, verified viaGET /repos/:repo/collaborators/:login/permissionusing the App's own installation token. This is GitHub's own authorization data, not anything self-reported by the workspace.Degrade-safe by construction: no minting user, no linked account, App not installed/configured, or a failed lookup all resolve to "not entitled" — never a 5xx. Legacy/enrollment/shared tokens (including
default's) can never claim a new repo, though they keep working normally on repos already bound to them.Grandfathering: existing
github_repo_linksrows are never re-validated — the gate only runs when a repo is unbound. Already-bound legitimate repos are unaffected.Webhook path:
github-webhook.ts's auto-promotion only reads existing bindings (findRepoLink) and never creates one, so no change was needed there.Every decline is a soft
{ posted: false, reason: "not_authorized" }/{ claimed: false, reason: "not_authorized" }, same shape as the existing "bound to someone else" decline — never a server error, and the CLI (uploads github link/ the bot-comment fallback logic) never falls back to posting via localghfor it.What it does / what it is not
/github/commentand/github/promote, and the explicit claim inPOST /github/link(uploads github link).How to try it
If your linked GitHub account doesn't have push access to that repo, you'll get:
Technical notes
apps/api/src/github-claim-authz.ts,apps/api/src/github-claim-authz.test.ts.apps/api/src/github-app.ts(newcollaboratorPermission),apps/api/src/uploader-identity.ts(extractedresolveUploaderLogin),apps/api/src/routes/github-comment.ts,apps/api/src/routes/github-promote.ts,apps/api/src/routes/github-link.ts.packages/uploads/src/client.ts(newreasonfield onGithubLinkClaimResult),packages/uploads/src/commands.ts(new decline message).apps/web/src/pages/docs/github-app.astro(Repo bindings section),skills/uploads-cli/SKILL.md.reasonand message).Test plan
pnpm test— 2087 tests pass, incl. newgithub-claim-authz.test.ts(entitled/not-entitled matrix) and updated route tests for/github/comment,/github/promote,/github/linkcovering: cross-tenant claim refused, legitimate same-identity claim allowed, grandfathered already-bound repos unaffected, soft-decline shape, and the CLI's new decline message.pnpm typecheckpnpm check(oxlint + oxfmt)Summary by CodeRabbit
Security
User Experience
not_authorizedresult instead of an error.Documentation