Skip to content

fix(web): require public connected repositories - #238

Open
dirtybits wants to merge 1 commit into
mainfrom
fix/reject-private-connected-repos
Open

dirtybits wants to merge 1 commit into
mainfrom
fix/reject-private-connected-repos

Conversation

@dirtybits

Copy link
Copy Markdown
Owner

Summary

Connected repositories are documented and implemented as public sources: the mirror downloads their content from unauthenticated raw.githubusercontent.com. Before this change, a wallet with a matching linked GitHub login could connect any owner/repo name without verifying that the repository existed or was public. That could save a private or nonexistent repository as active, then immediately hand it to a sync path that cannot fetch its content.

This PR:

  • queries the public GitHub repository-metadata endpoint before accepting the linked-login ownership path;
  • accepts only an explicit private: false response;
  • sends no mirror service token or user OAuth credential on that request, so private access can never satisfy the public-source requirement; and
  • adds direct coverage for public, private, unavailable, malformed, and network-failure outcomes.

The change does not alter the existing public verify-file ownership path, database schema, existing connected-repo rows, or sync behavior.

Test Plan

  • npm test --workspace @agentvouch/web -- __tests__/lib/mirror-connectedRepos.test.ts --maxWorkers=1 --no-fileParallelism — 31 passed. The new assertions failed first: private/unavailable/malformed/error metadata responses were previously accepted and the endpoint was never queried.
  • npm run format:check — passed.
  • npm run lint:web — passed.
  • npm run typecheck — passed.
  • npm test --workspace @agentvouch/web -- --maxWorkers=1 --no-fileParallelism — 152 files / 1,316 tests passed.
  • git diff --check — passed.
  • npm exec --workspace @agentvouch/web -- next build --webpack compiled and completed TypeScript, but failed only during /sitemap.xml prerender because this cron worktree has no web/.env.local / DATABASE_URL. The existing ox/viem dynamic-import warning remained.

Not verified

  • The cron shell exposed Node v22.23.1; Node 24 validation was not run locally.
  • No live GitHub ownership lookup, database write, sync, wallet, or chain flow was run.

Plan

.agents/plans/reject-private-connected-repos.plan.md records the verified gap, implementation, rollback, and verification results.

@dirtybits

Copy link
Copy Markdown
Owner Author

Holding this one. The idea is right, but the unauthenticated lookup is likely to reject real users.

Problem: isPublicGithubRepo calls api.github.com with no token on purpose. GitHub allows only 60 unauthenticated requests per hour per source IP, and Vercel's outgoing IPs are shared with many other customers. Once that limit is used up, GitHub returns 403, the helper returns false, and a public repo owned by the wallet's linked login gets "must be public". That fails safe, but legitimate connects will fail for reasons the user can't see.

The reason for skipping the token doesn't hold. The code comment says the server token "can read private repositories". It can, but GET /repos/{owner}/{repo} with a token still returns "private": true for a private repo, and this check rejects anything that isn't private === false. A token only lets the call see the repo; it can't make a private repo look public. The token should only stay out of the actual content fetch, which already goes to unauthenticated raw.githubusercontent.com.

Suggested fix:

  • Send the same token as githubHeaders() in web/lib/mirror/github.ts (GITHUB_TOKEN / GH_TOKEN / GITHUB_PAT). Ideally export a shared helper so both use the same header logic. Keep the strict body.private === false check.
  • Change the test that checks for no Authorization header to check that { private: true } is rejected even with a token set (stub the env).
  • Optional: tell a 403/429 from GitHub apart from a real private/missing repo, and return a "try again later" error instead of "must be public".
  • Optional: leave the .agents/plans/reject-private-connected-repos.plan.md file out of the PR, or update its "Blockers" section, which currently forbids sending the token.

This branch was successfully deployed

1 active deployment
Preview — fb461d8b Deployed Oct 12, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant