Skip to content

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

Merged
dirtybits merged 3 commits into
mainfrom
fix/reject-private-connected-repos
Nov 4, 2026
Merged

dirtybits merged 3 commits into
mainfrom
fix/reject-private-connected-repos

Conversation

@dirtybits

@dirtybits dirtybits commented Oct 12, 2026 •

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; private/404 responses receive the public-source message, while transient lookup failures receive retry guidance.

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.

Review follow-up (2026-11-04)

Signed commit 65b9ec3 distinguishes private/404 from unavailable GitHub metadata. The focused suite passes 42 tests on Node 24; independent review confirmed fail-closed behavior and the review thread is resolved. The branch includes current main and its root npm ci deployment setup.

@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.

Comment thread web/lib/mirror/connectedRepos.ts Resolved
@dirtybits
dirtybits merged commit de339a9 into main Nov 4, 2026
3 checks passed

This branch was successfully deployed

1 active deployment
Preview — 65b9ec39 Deployed Nov 4, 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