From 6cfc05ae2f08f32f15eaa40768f43ad8e054bcfa Mon Sep 17 00:00:00 2001 From: PIYUSH RATHORE <163632958+Piyushrathoree@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:09:30 +0530 Subject: [PATCH] fix(projects): avoid stale GitHub results after token rotation (#122613) * fix(projects): avoid stale GitHub results after token rotation * fix(gateway): scope GitHub caches by credential * style(gateway): format GitHub credential scope --------- Co-authored-by: FullerStackDev <263060202+fuller-stack-dev@users.noreply.github.com> --- src/gateway/control-ui-github-api.ts | 13 ++++++++ src/gateway/control-ui-session-prs.test.ts | 36 +++++++++++++++++++++ src/gateway/control-ui-session-prs.ts | 12 ++++--- src/gateway/project-github-search.test.ts | 37 +++++++++++++++++++++- src/gateway/project-github-search.ts | 18 ++++++----- 5 files changed, 102 insertions(+), 14 deletions(-) diff --git a/src/gateway/control-ui-github-api.ts b/src/gateway/control-ui-github-api.ts index ffbd040c5c7a..0085e848a8a0 100644 --- a/src/gateway/control-ui-github-api.ts +++ b/src/gateway/control-ui-github-api.ts @@ -2,6 +2,7 @@ // previews, session pull request chips): pinned origin, manual redirects, // bounded bodies, and normalized upstream error statuses. export { isRecord } from "@openclaw/normalization-core/record-coerce"; +import { createHash } from "node:crypto"; import { asFiniteNumber } from "@openclaw/normalization-core/number-coercion"; import { readNonBlankString } from "@openclaw/normalization-core/string-coerce"; import { readResponseWithLimit } from "../infra/http-body.js"; @@ -45,6 +46,18 @@ export function githubApiToken(env: NodeJS.ProcessEnv = process.env): string | u return env.GH_TOKEN?.trim() || env.GITHUB_TOKEN?.trim() || undefined; } +/** Captures the effective token and a non-secret cache scope from the same env snapshot. */ +export function resolveGitHubApiCredentialScope(env: NodeJS.ProcessEnv = process.env): { + token: string | undefined; + cacheScope: string; +} { + const token = githubApiToken(env); + return { + token, + cacheScope: token ? createHash("sha256").update(token).digest("hex") : "anonymous", + }; +} + function githubApiHeaders(token?: string): Record { const headers: Record = { Accept: "application/vnd.github+json", diff --git a/src/gateway/control-ui-session-prs.test.ts b/src/gateway/control-ui-session-prs.test.ts index 59e34a857153..e687218636ed 100644 --- a/src/gateway/control-ui-session-prs.test.ts +++ b/src/gateway/control-ui-session-prs.test.ts @@ -118,6 +118,42 @@ describe("loadControlUiSessionPullRequests", () => { expect(fetchImpl.mock.calls[1]?.[1]?.headers).not.toHaveProperty("Authorization"); }); + it("does not reuse cached private PRs after the GitHub token is removed", async () => { + vi.stubEnv("GH_TOKEN", "github-token-a"); + const fetchImpl = vi.fn().mockImplementation(async (_input, init) => { + const authorization = new Headers(init?.headers).get("Authorization"); + return authorization === "Bearer github-token-a" + ? githubJson([ + pullListItem({ + title: "private PR from token A", + merged_at: "2026-08-12T00:00:00Z", + }), + ]) + : githubJson({ message: "Not Found" }, 404); + }); + + const first = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ); + + vi.stubEnv("GH_TOKEN", ""); + await expect( + loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ), + ).rejects.toMatchObject({ statusCode: 404 }); + + expect(first.pullRequests[0]?.title).toBe("private PR from token A"); + expect(fetchImpl).toHaveBeenCalledTimes(2); + expect(fetchImpl.mock.calls[0]?.[1]?.headers).toHaveProperty( + "Authorization", + "Bearer github-token-a", + ); + expect(fetchImpl.mock.calls[1]?.[1]?.headers).not.toHaveProperty("Authorization"); + }); + it("skips diff and check fetches for merged PRs", async () => { const fetchImpl = routedFetch([ { diff --git a/src/gateway/control-ui-session-prs.ts b/src/gateway/control-ui-session-prs.ts index e40a3aeeb665..c7b059972a2d 100644 --- a/src/gateway/control-ui-session-prs.ts +++ b/src/gateway/control-ui-session-prs.ts @@ -16,10 +16,10 @@ import { ControlUiGitHubError, fetchGitHubJson, GITHUB_API_ORIGIN, - githubApiToken, isRecord, optionalNumber, readOptionalGitHubString, + resolveGitHubApiCredentialScope, } from "./control-ui-github-api.js"; import { gitOutput, @@ -569,9 +569,10 @@ async function refreshBranchPullRequests( context: SessionPullRequestGitContext, fetchImpl: typeof fetch, entry: CacheEntry, + token: string | undefined, ): Promise { try { - const result = await fetchBranchPullRequests(context, fetchImpl, githubApiToken()); + const result = await fetchBranchPullRequests(context, fetchImpl, token); // Degraded state-only chips still become lastGood: a later refresh that // rate-limits at the list fetch must serve the proven PRs, not an empty // list that would resurrect the Create PR row mid-outage. The shortened @@ -639,7 +640,8 @@ async function cachedBranchPullRequests( deps: LoadSessionPullRequestDeps, refresh: boolean, ): Promise { - const key = `${context.owner.toLowerCase()}/${context.repo.toLowerCase()}#${context.branch}`; + const { token, cacheScope } = resolveGitHubApiCredentialScope(); + const key = `${context.owner.toLowerCase()}/${context.repo.toLowerCase()}#${context.branch}\0${cacheScope}`; const cached = branchCache.get(key); if (cached && cached.expiresAt > Date.now()) { branchCache.delete(key); @@ -660,7 +662,7 @@ async function cachedBranchPullRequests( } return snapshot; } - return refreshBranchPullRequests(context, deps.fetchImpl ?? fetch, cached); + return refreshBranchPullRequests(context, deps.fetchImpl ?? fetch, cached, token); }); } const entry: CacheEntry = cached ?? { @@ -669,7 +671,7 @@ async function cachedBranchPullRequests( refreshMode: null, }; const promise = trackBranchRefresh(entry, refresh ? "forced" : "normal", () => - refreshBranchPullRequests(context, deps.fetchImpl ?? fetch, entry), + refreshBranchPullRequests(context, deps.fetchImpl ?? fetch, entry, token), ); branchCache.delete(key); branchCache.set(key, entry); diff --git a/src/gateway/project-github-search.test.ts b/src/gateway/project-github-search.test.ts index 2a898629748e..7a04c0e4c7e7 100644 --- a/src/gateway/project-github-search.test.ts +++ b/src/gateway/project-github-search.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { searchRemoteProjects } from "./project-github-search.js"; function repository(fullName: string, updatedAt: string, description?: string) { @@ -23,6 +23,10 @@ function json(value: unknown, status = 200): Response { } describe("project GitHub search", () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + it("returns anonymous public results with a typed missing-credential state", async () => { const fetchImpl = vi.fn().mockResolvedValue( json({ @@ -101,4 +105,35 @@ describe("project GitHub search", () => { expect(refreshed).toEqual(first); expect(fetchImpl).toHaveBeenCalledTimes(2); }); + + it("does not reuse cached results after the GitHub token rotates", async () => { + const fetchImpl = vi.fn().mockImplementation(async (_input, init) => { + const authorization = new Headers(init?.headers).get("Authorization"); + return json({ + items: [ + repository( + authorization === "Bearer github-token-a" + ? "acme/token-rotation-a" + : "acme/token-rotation-b", + "2026-08-10T00:00:00Z", + ), + ], + }); + }); + vi.stubEnv("GH_TOKEN", "github-token-a"); + vi.stubEnv("GITHUB_TOKEN", ""); + + const first = await searchRemoteProjects("token-rotation", { fetchImpl, now: 70_000 }); + + vi.stubEnv("GH_TOKEN", "github-token-b"); + const second = await searchRemoteProjects("token-rotation", { fetchImpl, now: 70_001 }); + + expect(first.projects).toContainEqual( + expect.objectContaining({ fullName: "acme/token-rotation-a" }), + ); + expect(second.projects).toContainEqual( + expect.objectContaining({ fullName: "acme/token-rotation-b" }), + ); + expect(fetchImpl).toHaveBeenCalledTimes(4); + }); }); diff --git a/src/gateway/project-github-search.ts b/src/gateway/project-github-search.ts index a739896160b0..25a16859cb45 100644 --- a/src/gateway/project-github-search.ts +++ b/src/gateway/project-github-search.ts @@ -9,10 +9,10 @@ import { fetchGitHubApi, fetchGitHubJson, GITHUB_API_ORIGIN, - githubApiToken, isRecord, readOptionalGitHubString, readGitHubJsonResponse, + resolveGitHubApiCredentialScope, requiredString, } from "./control-ui-github-api.js"; @@ -170,25 +170,27 @@ export function searchRemoteProjects( options: { env?: NodeJS.ProcessEnv; fetchImpl?: typeof fetch; now?: number } = {}, ): Promise { const normalizedQuery = query.trim().toLowerCase(); + const { token, cacheScope } = resolveGitHubApiCredentialScope(options.env); + // Gateway reloads run in-process, so cache results must stay credential-scoped. + const cacheKey = `${normalizedQuery}\0${cacheScope}`; const now = options.now ?? Date.now(); - const cached = searchCache.get(normalizedQuery); + const cached = searchCache.get(cacheKey); if (cached && cached.expiresAt > now) { - searchCache.delete(normalizedQuery); - searchCache.set(normalizedQuery, cached); + searchCache.delete(cacheKey); + searchCache.set(cacheKey, cached); return cached.promise; } - const token = githubApiToken(options.env); const promise = searchProjectsUncached({ query: query.trim(), fetchImpl: options.fetchImpl ?? fetch, token, }).catch((error: unknown) => { - if (searchCache.get(normalizedQuery)?.promise === promise) { - searchCache.delete(normalizedQuery); + if (searchCache.get(cacheKey)?.promise === promise) { + searchCache.delete(cacheKey); } throw error; }); - searchCache.set(normalizedQuery, { expiresAt: now + SEARCH_CACHE_MS, promise }); + searchCache.set(cacheKey, { expiresAt: now + SEARCH_CACHE_MS, promise }); pruneMapToMaxSize(searchCache, SEARCH_CACHE_LIMIT); return promise; }