mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
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>
This commit is contained in:
@@ -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<string, string> {
|
||||
const headers: Record<string, string> = {
|
||||
Accept: "application/vnd.github+json",
|
||||
|
||||
@@ -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<typeof fetch>().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([
|
||||
{
|
||||
|
||||
@@ -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<BranchPullRequestsSnapshot> {
|
||||
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<BranchPullRequestsSnapshot> {
|
||||
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);
|
||||
|
||||
@@ -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<typeof fetch>().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<typeof fetch>().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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<ProjectsSearchRemoteResult> {
|
||||
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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user