From fb461d8be03c00ba68668162661becfaa4ab0420 Mon Sep 17 00:00:00 2001 From: dirtybits Date: Mon, 12 Oct 2026 09:42:33 -0700 Subject: [PATCH 1/2] fix(web): require public connected repos --- .../reject-private-connected-repos.plan.md | 79 ++++++++++++++++ .../lib/mirror-connectedRepos.test.ts | 90 ++++++++++++++++++- web/lib/mirror/connectedRepos.ts | 33 ++++++- 3 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 .agents/plans/reject-private-connected-repos.plan.md diff --git a/.agents/plans/reject-private-connected-repos.plan.md b/.agents/plans/reject-private-connected-repos.plan.md new file mode 100644 index 00000000..c2abc6ff --- /dev/null +++ b/.agents/plans/reject-private-connected-repos.plan.md @@ -0,0 +1,79 @@ +--- +name: reject-private-connected-repos +overview: "Require linked-login connected repositories to be publicly visible before marking them active, preserving the unauthenticated raw-content sync contract." +todos: + - id: prove-public-visibility + content: Add a public GitHub repository visibility check to the linked-login ownership path before a connected repository can be created + status: completed + - id: cover-visibility-boundary + content: Add focused behavioral regressions for public, private, unavailable, and GitHub lookup failure outcomes + status: completed + - id: verify-visibility-hardening + content: Run focused mirror tests, formatting, lint, typecheck, full web Vitest, webpack build, and whitespace checks — build attempted; blocked only by absent DATABASE_URL during sitemap prerender + status: completed +isProject: false +--- + +# Reject Private Connected Repositories + +## Goal + +Keep connected-repository registration aligned with its public-source contract: a wallet whose linked GitHub login matches the requested owner may connect only repositories that GitHub exposes publicly, so an unavailable or private repository is not saved as active and immediately handed to the unauthenticated raw-content sync path. + +## Scope + +- In scope: the linked-login branch of `verifyRepoOwnership` and direct unit coverage for the GitHub public-visibility boundary. +- Out of scope: GitHub OAuth, database schema, existing repository rows, community mirrors, sync transport, and changes to ownership proof semantics for the existing public verify-file path. + +## Verified Gap (2026-10-12) + +- `web/lib/mirror/connectedRepos.ts:147-150` treats a linked GitHub login matching `githubOwner` as sufficient ownership proof without checking that `githubRepo` exists or is public. +- The same module documents at line 12 that connected repositories must be public because `raw.githubusercontent.com` fetches their content without credentials; `web/lib/mirror/github.ts:157-162` confirms raw content is fetched unauthenticated. +- Consequently, a linked owner can receive an active connection response for a private or nonexistent repository, after which initial sync cannot retrieve its raw content. The server GitHub token used by tree/ref helpers must not be treated as permission to mirror private content. +- Current `web/__tests__/lib/mirror-connectedRepos.test.ts` covers coordinate validation and SQL conflict behavior but has no `verifyRepoOwnership` coverage. Open PRs #235 and #236 address coordinate type validation and SSRF blocklist coverage, respectively, not repository visibility (verified 2026-10-12). + +## Files To Change + +- `web/lib/mirror/connectedRepos.ts`: use an unauthenticated GitHub repository metadata lookup on the linked-login path and only accept an explicit public result; return a clear ownership failure otherwise. +- `web/__tests__/lib/mirror-connectedRepos.test.ts`: mock fetch and add public/private/unavailable/error behavior tests around `verifyRepoOwnership`. + +## Implementation Steps + +1. Add a small helper that reads `https://api.github.com/repos/{owner}/{repo}` without the server mirror token and returns true only when a successful response explicitly reports `private: false`. +2. Make the linked-login ownership branch wait for that check. Preserve the existing linked-login success result for public repositories and return an actionable refusal for private, missing, or lookup-failed repositories. +3. Add targeted regressions proving that the visibility check uses the declared owner/repo URL without credentials, accepts public metadata, and refuses private, non-OK, malformed, or failed lookups without falling through to the verify-file path. +4. Run the focused test first, then the repository static and web gates. + +## Verification + +```bash +. "$HOME/.nvm/nvm.sh" --no-use && { nvm use --silent || nvm install; } +npm test --workspace @agentvouch/web -- __tests__/lib/mirror-connectedRepos.test.ts --maxWorkers=1 --no-fileParallelism +npm run format:check +npm run lint:web +npm run typecheck +npm test --workspace @agentvouch/web -- --maxWorkers=1 --no-fileParallelism +npm exec --workspace @agentvouch/web -- next build --webpack +git diff --check +``` + +Acceptance criteria: linked-login registration succeeds only after unauthenticated GitHub metadata says `private: false`; private, unavailable, malformed, and failed responses return an ownership refusal before a connected-repository row or sync is reached; existing verify-file behavior remains unchanged. + +## Rollout + +Merge as a focused server-side validation change. It requires no migration, environment change, GitHub action, wallet action, or chain action. New invalid registrations are refused; existing rows are untouched. + +## Rollback + +Revert the focused commit. No persisted state is changed by the visibility lookup and no existing connection is altered. + +## Blockers + +- Do not send `GITHUB_TOKEN`, `GH_TOKEN`, `GITHUB_PAT`, or a user OAuth token to the visibility lookup; authenticated success would incorrectly make private repositories appear eligible. +- Do not add a dependency or a database migration. If GitHub’s public metadata response cannot reliably expose the `private` field, defer rather than infer visibility. + +## Execution Notes + +- **2026-10-12:** Added the focused regressions first. Before the implementation, the linked-login path returned `{ verified: true, method: "linked-login" }` for private, unavailable, malformed, and failed GitHub metadata responses and never made the visibility request (5 focused-test failures). +- **2026-10-12:** The implemented lookup sends only GitHub’s public media type and `agentvouch-connect` user agent—never the mirror service token—and accepts only a successful response with `private: false`. Focused test now passes (31 tests). +- **2026-10-12:** Under the cron shell’s available Node v22.23.1, `npm run format:check`, `npm run lint:web`, `npm run typecheck`, `git diff --check`, and the full web suite passed (152 files, 1,316 tests). `next build --webpack` compiled successfully and completed TypeScript, then failed only while prerendering `/sitemap.xml` because `web/.env.local` has no `DATABASE_URL`; no environment configuration was changed. The known `ox`/`viem` dynamic-import warning remained. diff --git a/web/__tests__/lib/mirror-connectedRepos.test.ts b/web/__tests__/lib/mirror-connectedRepos.test.ts index 815bd877..a18dfd7a 100644 --- a/web/__tests__/lib/mirror-connectedRepos.test.ts +++ b/web/__tests__/lib/mirror-connectedRepos.test.ts @@ -1,7 +1,17 @@ -import { describe, expect, it, vi, beforeEach } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const { mockResolveAgentIdentityByWallet } = vi.hoisted(() => ({ + mockResolveAgentIdentityByWallet: vi.fn(), +})); + +vi.mock("@/lib/agentIdentity", () => ({ + resolveAgentIdentityByWallet: mockResolveAgentIdentityByWallet, +})); + import { sanitizeSyncedRepoUrl, validateRepoCoords, + verifyRepoOwnership, } from "@/lib/mirror/connectedRepos"; describe("sanitizeSyncedRepoUrl", () => { @@ -91,6 +101,84 @@ describe("validateRepoCoords", () => { ); }); +describe("verifyRepoOwnership — linked-login public visibility", () => { + const repo = { + walletPubkey: "WalletA111111111111111111111111111111111111", + githubOwner: "someorg", + githubRepo: "somerepo", + branch: "main", + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockResolveAgentIdentityByWallet.mockResolvedValue({ + githubProfile: { login: "SomeOrg" }, + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("accepts a linked owner only after unauthenticated GitHub metadata confirms the repo is public", async () => { + const fetchMock = vi + .fn() + .mockResolvedValue( + new Response(JSON.stringify({ private: false }), { status: 200 }) + ); + vi.stubGlobal("fetch", fetchMock); + + await expect(verifyRepoOwnership(repo)).resolves.toEqual({ + verified: true, + method: "linked-login", + }); + expect(fetchMock).toHaveBeenCalledWith( + "https://api.github.com/repos/someorg/somerepo", + expect.objectContaining({ + headers: expect.objectContaining({ + Accept: "application/vnd.github+json", + "User-Agent": "agentvouch-connect", + }), + }) + ); + expect(fetchMock.mock.calls[0][1]?.headers).not.toHaveProperty( + "Authorization" + ); + }); + + it.each([ + [ + "private", + new Response(JSON.stringify({ private: true }), { status: 200 }), + ], + ["unavailable", new Response(null, { status: 404 })], + ["malformed", new Response(JSON.stringify({}), { status: 200 })], + ])("rejects a %s linked repository", async (_kind, response) => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(response)); + + await expect(verifyRepoOwnership(repo)).resolves.toEqual( + expect.objectContaining({ + verified: false, + reason: expect.stringContaining("must be public"), + }) + ); + }); + + it("rejects a linked repository when GitHub visibility lookup fails", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockRejectedValue(new Error("network down")) + ); + + await expect(verifyRepoOwnership(repo)).resolves.toEqual( + expect.objectContaining({ + verified: false, + reason: expect.stringContaining("must be public"), + }) + ); + }); +}); + // ---- createConnectedRepo conflict path (INSERT ... ON CONFLICT) ---- const { mockSqlFn } = vi.hoisted(() => ({ mockSqlFn: vi.fn() })); diff --git a/web/lib/mirror/connectedRepos.ts b/web/lib/mirror/connectedRepos.ts index d94a1950..1b828b40 100644 --- a/web/lib/mirror/connectedRepos.ts +++ b/web/lib/mirror/connectedRepos.ts @@ -115,6 +115,31 @@ async function linkedGithubLogin(walletPubkey: string): Promise { return summary?.githubProfile?.login ?? null; } +async function isPublicGithubRepo( + githubOwner: string, + githubRepo: string +): Promise { + try { + // Deliberately do not reuse the mirror transport headers here: its optional + // server token can read private repositories, while connected-repo content + // must be reachable from unauthenticated raw.githubusercontent.com. + const res = await fetch( + `https://api.github.com/repos/${githubOwner}/${githubRepo}`, + { + headers: { + Accept: "application/vnd.github+json", + "User-Agent": "agentvouch-connect", + }, + } + ); + if (!res.ok) return false; + const body = (await res.json()) as { private?: unknown }; + return body.private === false; + } catch { + return false; + } +} + async function repoVerificationFileMatches( githubOwner: string, githubRepo: string, @@ -146,7 +171,13 @@ export async function verifyRepoOwnership(input: { }): Promise { const login = await linkedGithubLogin(input.walletPubkey); if (login && login.toLowerCase() === input.githubOwner.toLowerCase()) { - return { verified: true, method: "linked-login" }; + if (await isPublicGithubRepo(input.githubOwner, input.githubRepo)) { + return { verified: true, method: "linked-login" }; + } + return { + verified: false, + reason: `${input.githubOwner}/${input.githubRepo} must be public and available from GitHub before it can be connected.`, + }; } if ( From 65b9ec39049e58899b4578d124adc582c5669973 Mon Sep 17 00:00:00 2001 From: dirtybits Date: Wed, 4 Nov 2026 12:35:48 -0800 Subject: [PATCH 2/2] fix(web): distinguish transient GitHub visibility failures --- .../reject-private-connected-repos.plan.md | 1 + .../lib/mirror-connectedRepos.test.ts | 26 +++++++++++++++++-- web/lib/mirror/connectedRepos.ts | 24 ++++++++++++----- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/.agents/plans/reject-private-connected-repos.plan.md b/.agents/plans/reject-private-connected-repos.plan.md index c2abc6ff..fb46ad4b 100644 --- a/.agents/plans/reject-private-connected-repos.plan.md +++ b/.agents/plans/reject-private-connected-repos.plan.md @@ -77,3 +77,4 @@ Revert the focused commit. No persisted state is changed by the visibility looku - **2026-10-12:** Added the focused regressions first. Before the implementation, the linked-login path returned `{ verified: true, method: "linked-login" }` for private, unavailable, malformed, and failed GitHub metadata responses and never made the visibility request (5 focused-test failures). - **2026-10-12:** The implemented lookup sends only GitHub’s public media type and `agentvouch-connect` user agent—never the mirror service token—and accepts only a successful response with `private: false`. Focused test now passes (31 tests). - **2026-10-12:** Under the cron shell’s available Node v22.23.1, `npm run format:check`, `npm run lint:web`, `npm run typecheck`, `git diff --check`, and the full web suite passed (152 files, 1,316 tests). `next build --webpack` compiled successfully and completed TypeScript, then failed only while prerendering `/sitemap.xml` because `web/.env.local` has no `DATABASE_URL`; no environment configuration was changed. The known `ox`/`viem` dynamic-import warning remained. +- **2026-11-04 review fix:** Keep private metadata and 404 responses in the public-repository refusal path. Rate limits (403/429), 5xx, malformed JSON/metadata, and network failures now ask the user to retry without accepting the repository or falling through to verification-file lookup. The focused `mirror-connectedRepos` suite passes all 42 tests; Prettier and `git diff --check` pass. Broader combined checks are tracked in the PR review sweep. diff --git a/web/__tests__/lib/mirror-connectedRepos.test.ts b/web/__tests__/lib/mirror-connectedRepos.test.ts index 16196ab4..f911950c 100644 --- a/web/__tests__/lib/mirror-connectedRepos.test.ts +++ b/web/__tests__/lib/mirror-connectedRepos.test.ts @@ -172,7 +172,6 @@ describe("verifyRepoOwnership — linked-login public visibility", () => { new Response(JSON.stringify({ private: true }), { status: 200 }), ], ["unavailable", new Response(null, { status: 404 })], - ["malformed", new Response(JSON.stringify({}), { status: 200 })], ])("rejects a %s linked repository", async (_kind, response) => { vi.stubGlobal("fetch", vi.fn().mockResolvedValue(response)); @@ -184,6 +183,29 @@ describe("verifyRepoOwnership — linked-login public visibility", () => { ); }); + it.each([ + ["rate-limited", new Response(null, { status: 403 })], + ["too many requests", new Response(null, { status: 429 })], + ["server error", new Response(null, { status: 503 })], + ["malformed metadata", new Response(JSON.stringify({}), { status: 200 })], + ["null metadata", new Response("null", { status: 200 })], + ["invalid JSON", new Response("invalid", { status: 200 })], + ])( + "asks to retry a %s lookup without accepting the repo", + async (_kind, response) => { + const fetchMock = vi.fn().mockResolvedValue(response); + vi.stubGlobal("fetch", fetchMock); + + await expect(verifyRepoOwnership(repo)).resolves.toEqual( + expect.objectContaining({ + verified: false, + reason: expect.stringContaining("try again"), + }) + ); + expect(fetchMock).toHaveBeenCalledOnce(); + } + ); + it("rejects a linked repository when GitHub visibility lookup fails", async () => { vi.stubGlobal( "fetch", @@ -193,7 +215,7 @@ describe("verifyRepoOwnership — linked-login public visibility", () => { await expect(verifyRepoOwnership(repo)).resolves.toEqual( expect.objectContaining({ verified: false, - reason: expect.stringContaining("must be public"), + reason: expect.stringContaining("try again"), }) ); }); diff --git a/web/lib/mirror/connectedRepos.ts b/web/lib/mirror/connectedRepos.ts index a4b6108a..b6bb818a 100644 --- a/web/lib/mirror/connectedRepos.ts +++ b/web/lib/mirror/connectedRepos.ts @@ -118,10 +118,10 @@ async function linkedGithubLogin(walletPubkey: string): Promise { return summary?.githubProfile?.login ?? null; } -async function isPublicGithubRepo( +async function getGithubRepoVisibility( githubOwner: string, githubRepo: string -): Promise { +): Promise<"public" | "not-public" | "unavailable"> { try { // Deliberately do not reuse the mirror transport headers here: its optional // server token can read private repositories, while connected-repo content @@ -135,11 +135,14 @@ async function isPublicGithubRepo( }, } ); - if (!res.ok) return false; + if (res.status === 404) return "not-public"; + if (!res.ok) return "unavailable"; const body = (await res.json()) as { private?: unknown }; - return body.private === false; + if (body.private === false) return "public"; + if (body.private === true) return "not-public"; + return "unavailable"; } catch { - return false; + return "unavailable"; } } @@ -174,12 +177,19 @@ export async function verifyRepoOwnership(input: { }): Promise { const login = await linkedGithubLogin(input.walletPubkey); if (login && login.toLowerCase() === input.githubOwner.toLowerCase()) { - if (await isPublicGithubRepo(input.githubOwner, input.githubRepo)) { + const visibility = await getGithubRepoVisibility( + input.githubOwner, + input.githubRepo + ); + if (visibility === "public") { return { verified: true, method: "linked-login" }; } return { verified: false, - reason: `${input.githubOwner}/${input.githubRepo} must be public and available from GitHub before it can be connected.`, + reason: + visibility === "not-public" + ? `${input.githubOwner}/${input.githubRepo} must be public and available from GitHub before it can be connected.` + : `Cannot verify ${input.githubOwner}/${input.githubRepo} on GitHub now. Please try again later.`, }; }