From b6fbe527570c7b8d249300e7b11c6ac1ae857256 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 10 Jul 2026 03:26:20 -0700 Subject: [PATCH] fix(github): drive public repo-stats allowlist from env instead of a hardcoded repo (#4612) GET /v1/public/github/repos/:owner/:repo/stats hard-rejected every repo except a literal jsonbored/gittensory, even though the route is generically parameterized. Replace the two constants with PUBLIC_REPO_STATS_ALLOWLIST, a comma-separated owner/repo allowlist mirroring GITTENSORY_PUBLIC_STATS_REPOS's shape; empty/unset allows any syntactically-valid repo, so a self-hoster's own fork works without code changes. --- apps/gittensory-ui/public/openapi.json | 2 +- src/env.d.ts | 7 +++ src/github/public.ts | 31 ++++++++++--- src/openapi/spec.ts | 2 +- test/integration/api.test.ts | 60 ++++++++++++++++++++------ 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 9237010d48..fdab8e0cf8 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -13855,7 +13855,7 @@ ], "responses": { "200": { - "description": "Public GitHub repository stars/forks for the website chrome; only JSONbored/gittensory is accepted.", + "description": "Public GitHub repository stars/forks for the website chrome; any syntactically-valid owner/repo is accepted unless the deployment configures PUBLIC_REPO_STATS_ALLOWLIST.", "content": { "application/json": { "schema": { diff --git a/src/env.d.ts b/src/env.d.ts index fb965729e5..aadfe6c225 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -173,6 +173,13 @@ declare global { GITTENSOR_UPSTREAM_REF?: string; GITTENSOR_REGISTRY_URL: string; GITHUB_PUBLIC_TOKEN?: string; + /** Public repo-stats endpoint (#4612): comma-separated allowlist of "owner/repo" pairs (case-insensitive) + * permitted to query GET /v1/public/github/repos/:owner/:repo/stats. Default "" (unset) => allow any + * syntactically-valid owner/repo -- this route proxies the live GitHub API rather than gittensory's own + * installation DB, so (unlike the sibling badge.svg route) there is no "installed" gate to fall back on + * instead. Set this to restrict a deployment to specific repos. Mirrors GITTENSORY_PUBLIC_STATS_REPOS's + * comma-separated shape (src/review/public-stats.ts). See src/github/public.ts. */ + PUBLIC_REPO_STATS_ALLOWLIST?: string; /** #703: owner-gated global to apply upstream sigmoid time-decay in score previews. Default off. */ SCORING_TIME_DECAY_ENABLED?: string; /** #776 agent-layer GLOBAL kill-switch — when truthy, halts ALL agent actions across every repo. */ diff --git a/src/github/public.ts b/src/github/public.ts index 5d2e071871..01d5997c7e 100644 --- a/src/github/public.ts +++ b/src/github/public.ts @@ -51,8 +51,6 @@ type RepoStatsCacheEntry = { staleUntilMs: number; }; -const PUBLIC_REPO_STATS_OWNER = "jsonbored"; -const PUBLIC_REPO_STATS_REPO = "gittensory"; const REPO_STATS_CACHE_TTL_MS = 1000 * 60 * 10; const REPO_STATS_STALE_TTL_MS = 1000 * 60 * 60 * 24; const repoStatsCache = new Map(); @@ -122,8 +120,12 @@ export async function fetchPublicContributorProfile(login: string, env?: Pick, owner: string, repo: string): Promise { - const repoFullName = publicRepoFullName(owner, repo); +export async function fetchPublicRepoStats( + env: Pick, + owner: string, + repo: string, +): Promise { + const repoFullName = publicRepoFullName(env, owner, repo); const cacheKey = repoFullName.toLowerCase(); const nowMs = Date.now(); const cached = repoStatsCache.get(cacheKey); @@ -143,15 +145,30 @@ export function clearPublicRepoStatsCacheForTests(): void { repoStatsCache.clear(); } -function publicRepoFullName(owner: string, repo: string): string { +// Parses PUBLIC_REPO_STATS_ALLOWLIST into a lowercased "owner/repo" set, mirroring publicStatsProjects's +// comma-separated parsing in src/review/public-stats.ts. Empty/unset -> empty set -> no allowlist restriction +// (#4612): a self-hosted fork's own repo works out of the box, with no code change, exactly like the sibling +// badge.svg route already does for any installed repo. +function publicRepoStatsAllowlist(env: Pick): Set { + const allowlist = new Set(); + for (const entry of (env.PUBLIC_REPO_STATS_ALLOWLIST ?? "").split(",")) { + const project = entry.trim().toLowerCase(); + if (project) allowlist.add(project); + } + return allowlist; +} + +function publicRepoFullName(env: Pick, owner: string, repo: string): string { const ownerName = owner.trim(); const repoName = repo.trim(); if (!/^[A-Za-z0-9][A-Za-z0-9-]{0,38}$/.test(ownerName)) throw new Error("invalid_github_repo"); if (!/^[A-Za-z0-9._-]{1,100}$/.test(repoName) || repoName === "." || repoName === "..") throw new Error("invalid_github_repo"); const normalizedOwnerName = ownerName.toLowerCase(); const normalizedRepoName = repoName.toLowerCase(); - if (normalizedOwnerName !== PUBLIC_REPO_STATS_OWNER || normalizedRepoName !== PUBLIC_REPO_STATS_REPO) throw new Error("invalid_github_repo"); - return `${normalizedOwnerName}/${normalizedRepoName}`; + const normalizedFullName = `${normalizedOwnerName}/${normalizedRepoName}`; + const allowlist = publicRepoStatsAllowlist(env); + if (allowlist.size > 0 && !allowlist.has(normalizedFullName)) throw new Error("invalid_github_repo"); + return normalizedFullName; } async function fetchRepoStatsFromGitHub(env: Pick, repoFullName: string, nowMs: number): Promise { diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 0b77fba715..434fcf8aa6 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -185,7 +185,7 @@ export function buildOpenApiSpec() { path: "/v1/public/github/repos/{owner}/{repo}/stats", request: { params: z.object({ owner: z.string(), repo: z.string() }) }, responses: { - 200: { description: "Public GitHub repository stars/forks for the website chrome; only JSONbored/gittensory is accepted.", content: { "application/json": { schema: PublicRepoStatsSchema } } }, + 200: { description: "Public GitHub repository stars/forks for the website chrome; any syntactically-valid owner/repo is accepted unless the deployment configures PUBLIC_REPO_STATS_ALLOWLIST.", content: { "application/json": { schema: PublicRepoStatsSchema } } }, 400: { description: "Invalid or non-allowlisted GitHub repository" }, 503: { description: "GitHub repository stats are unavailable" }, }, diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 30b14070c7..181af6e6dd 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -155,6 +155,54 @@ describe("api routes", () => { expect(calls).toHaveLength(1); }); + it("serves public GitHub repo stats for any repo when PUBLIC_REPO_STATS_ALLOWLIST is unset (#4612)", async () => { + const app = createApp(); + // No PUBLIC_REPO_STATS_ALLOWLIST override: a self-hoster's own fork must work without code changes. + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + const calls: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + calls.push(input.toString()); + return Response.json({ full_name: "acme/widgets", html_url: "https://github.com/acme/widgets", stargazers_count: 5, forks_count: 1 }); + }); + + const response = await app.request("/v1/public/github/repos/acme/widgets/stats", {}, env); + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + repoFullName: "acme/widgets", + htmlUrl: "https://github.com/acme/widgets", + stargazers_count: 5, + forks_count: 1, + source: "github", + stale: false, + }); + expect(calls).toEqual(["https://api.github.com/repos/acme/widgets"]); + }); + + it("rejects public GitHub repo stats for repos outside a configured PUBLIC_REPO_STATS_ALLOWLIST, tolerating whitespace/casing/empty entries (#4612)", async () => { + const app = createApp(); + const env = createTestEnv({ + GITHUB_PUBLIC_TOKEN: "public-token", + PUBLIC_REPO_STATS_ALLOWLIST: " JSONbored/gittensory , Acme/Widgets ,,", + }); + const calls: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + calls.push(input.toString()); + return Response.json({ stargazers_count: 9, forks_count: 2 }); + }); + + // A listed repo (with mismatched casing, exercising the trim + lowercase parsing) is served. + const allowed = await app.request("/v1/public/github/repos/ACME/WIDGETS/stats", {}, env); + expect(allowed.status).toBe(200); + await expect(allowed.json()).resolves.toMatchObject({ stargazers_count: 9, forks_count: 2, source: "github" }); + expect(calls).toEqual(["https://api.github.com/repos/acme/widgets"]); + + // A repo absent from the allowlist is rejected before ever calling GitHub. + const rejected = await app.request("/v1/public/github/repos/Attacker/missing-one/stats", {}, env); + expect(rejected.status).toBe(400); + await expect(rejected.json()).resolves.toMatchObject({ error: "invalid_github_repo" }); + expect(calls).toHaveLength(1); + }); + it("normalizes allowlisted public GitHub repo stats casing before fetching and caching", async () => { const app = createApp(); const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); @@ -408,18 +456,6 @@ describe("api routes", () => { expect(fetchMock).not.toHaveBeenCalled(); }); - it("rejects non-allowlisted public GitHub repo stats paths before calling GitHub", async () => { - const app = createApp(); - const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); - const fetchMock = vi.fn(); - vi.stubGlobal("fetch", fetchMock); - - const response = await app.request("/v1/public/github/repos/Attacker/missing-one/stats", {}, env); - expect(response.status).toBe(400); - await expect(response.json()).resolves.toMatchObject({ error: "invalid_github_repo" }); - expect(fetchMock).not.toHaveBeenCalled(); - }); - it("serves registry drift through the canonical registry change endpoint", async () => { const app = createApp(); const env = createTestEnv();