ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-27 12:56:01 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(gateway): isolate mutable GitHub identity checks

Browse Source 
Co-authored-by: Jesse Merhi <79823012+jesse-merhi@users.noreply.github.com>
This commit is contained in:
roboclaw-bot
2026-08-27 04:40:05 +00:00
parent a329cfac7a
commit 32d984e507
4 changed files with 68 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/concepts/user-model.md
+1 -1
View File
@@ -20,7 +20,7 @@ GitHub-backed sign-in is supported through Cloudflare Access and Tailscale Serve
The **GitHub account** row is read-only. Generic trusted proxies, token, password, and unauthenticated connections cannot claim a GitHub account, and agent or tool GitHub credentials are never used for this identity. The forwarded Cloudflare Access assertion is connection-scoped: OpenClaw does not persist, export, log, or expose it to the UI or model.
Identity lookup runs after WebSocket sign-in, so connection status and other identity-independent reads remain available. Profile and session work waits for the lookup; a Cloudflare or GitHub rate limit or network failure returns retryable unavailability without exposing a mutable alias or erasing a previously verified account. Concurrent checks for the same GitHub login share only the active request; a later connection verifies the mutable login again before attaching a profile. Immutable numeric account-id results can be reused during a rate-limit backoff. A later request, connection, or Profile refresh retries the lookup. GitHub login renames are reconciled by numeric account id so profile history and preferences stay attached to one person.
Identity lookup runs after WebSocket sign-in, so connection status and other identity-independent reads remain available. Profile and session work waits for the lookup; a Cloudflare or GitHub rate limit or network failure returns retryable unavailability without exposing a mutable alias or erasing a previously verified account. GitHub lookups are serialized against the shared credential quota and independently verified for every connection before profile attachment. A later request, connection, or Profile refresh retries the lookup. GitHub login renames are reconciled by numeric account id so profile history and preferences stay attached to one person.
Public commit metadata is a separate choice. **Git co-author credit** defaults off. Enabling it adds the verified account's public GitHub noreply address to commits created from shared sessions; OpenClaw never requests or stores a private GitHub email for this feature. Signing in as a different numeric GitHub account resets the choice, so one account cannot inherit another account's consent.
src/gateway/github-user-identity-coordinator.ts
+24 -91
View File
@@ -1,107 +1,59 @@
import { KeyedAsyncQueue } from "../plugin-sdk/keyed-async-queue.js";
import { ControlUiGitHubError } from "./control-ui-github-api.js";
const SUCCESS_CACHE_MS = 5 * 60_000;
const RATE_LIMIT_FALLBACK_MS = 60_000;
const CAPACITY_RETRY_MS = 1_000;
const CACHE_LIMIT = 200;
export type ResolvedGitHubUserIdentity = { accountId: number; login: string };
type LookupEntry = {
freshUntil: number;
inFlight?: Promise<ResolvedGitHubUserIdentity>;
value?: ResolvedGitHubUserIdentity;
};
class GitHubUserIdentityCoordinator {
private readonly backoffs = new Map<string, number>();
private readonly lookups = new Map<string, LookupEntry>();
private pendingLookups = 0;
private queue = new KeyedAsyncQueue();
lookup(params: {
credentialScope: string;
identityKind: "account-id" | "login";
lookupKey: string;
request: () => Promise<ResolvedGitHubUserIdentity>;
}): Promise<ResolvedGitHubUserIdentity> {
const cacheKey = `${params.credentialScope}:${params.lookupKey}`;
const cacheCompleted = params.identityKind === "account-id";
const cached = this.touchLookup(cacheKey);
if (cacheCompleted && cached?.value && cached.freshUntil > Date.now()) {
return Promise.resolve(cached.value);
}
if (cached?.inFlight) {
return cached.inFlight;
}
const backoffRemaining = this.backoffRemaining(params.credentialScope);
if (backoffRemaining > 0) {
// Numeric account IDs are immutable. Login aliases are not, so they must
// never use stale verification when GitHub cannot confirm current ownership.
if (cacheCompleted && cached?.value) {
return Promise.resolve(cached.value);
}
return Promise.reject(this.rateLimitError(backoffRemaining));
}
if (!cached) {
this.pruneLookups(CACHE_LIMIT - 1);
if (this.lookups.size >= CACHE_LIMIT) {
return Promise.reject(this.capacityError());
}
if (this.pendingLookups >= CACHE_LIMIT) {
return Promise.reject(this.capacityError());
}
const entry = cached ?? { freshUntil: 0 };
const current = this.queue.enqueue(params.credentialScope, async () => {
const queuedBackoffRemaining = this.backoffRemaining(params.credentialScope);
if (queuedBackoffRemaining > 0) {
if (cacheCompleted && entry.value) {
return entry.value;
return this.queue.enqueue(
params.credentialScope,
async () => {
const queuedBackoffRemaining = this.backoffRemaining(params.credentialScope);
if (queuedBackoffRemaining > 0) {
throw this.rateLimitError(queuedBackoffRemaining);
}
throw this.rateLimitError(queuedBackoffRemaining);
}
try {
const identity = await params.request();
if (!cacheCompleted) {
return identity;
}
entry.value = identity;
entry.freshUntil = Date.now() + SUCCESS_CACHE_MS;
return identity;
} catch (error) {
if (error instanceof ControlUiGitHubError && error.statusCode === 429) {
const retryAfterMs = error.retryAfterMs ?? RATE_LIMIT_FALLBACK_MS;
this.setBackoff(params.credentialScope, retryAfterMs);
if (cacheCompleted && entry.value) {
return entry.value;
try {
return await params.request();
} catch (error) {
if (error instanceof ControlUiGitHubError && error.statusCode === 429) {
const retryAfterMs = error.retryAfterMs ?? RATE_LIMIT_FALLBACK_MS;
this.setBackoff(params.credentialScope, retryAfterMs);
}
}
throw error;
}
});
entry.inFlight = current;
this.lookups.delete(cacheKey);
this.lookups.set(cacheKey, entry);
void current.then(
() => {
entry.inFlight = undefined;
if (!cacheCompleted) {
this.lookups.delete(cacheKey);
throw error;
}
},
() => {
entry.inFlight = undefined;
if (!entry.value) {
this.lookups.delete(cacheKey);
}
{
onEnqueue: () => {
this.pendingLookups += 1;
},
onSettle: () => {
this.pendingLookups -= 1;
},
},
);
return current;
}
reset(): void {
this.backoffs.clear();
this.lookups.clear();
this.pendingLookups = 0;
this.queue = new KeyedAsyncQueue();
}
@@ -114,16 +66,6 @@ class GitHubUserIdentityCoordinator {
return remaining;
}
private pruneLookups(maxSize: number): void {
while (this.lookups.size > maxSize) {
const oldestIdle = [...this.lookups].find(([, entry]) => !entry.inFlight);
if (!oldestIdle) {
return;
}
this.lookups.delete(oldestIdle[0]);
}
}
private capacityError(): ControlUiGitHubError {
return new ControlUiGitHubError(
429,
@@ -153,15 +95,6 @@ class GitHubUserIdentityCoordinator {
this.backoffs.delete(oldestKey);
}
}
private touchLookup(cacheKey: string): LookupEntry | undefined {
const cached = this.lookups.get(cacheKey);
if (cached) {
this.lookups.delete(cacheKey);
this.lookups.set(cacheKey, cached);
}
return cached;
}
}
export const githubUserIdentityCoordinator = new GitHubUserIdentityCoordinator();
src/gateway/github-user-identity.test.ts
+17 -50
View File
@@ -304,49 +304,17 @@ describe("authenticated GitHub identity sync", () => {
});
});
it("deduplicates concurrent lookups across Gateway connections", async () => {
await withOpenClawTestState({ scenario: "minimal" }, async () => {
const profile = ensureProfileForTailscaleIdentity({ login: "ada@github" });
let resolveLookup: ((response: Response) => void) | undefined;
const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementationOnce(
async () =>
await new Promise<Response>((resolve) => {
resolveLookup = resolve;
}),
);
const connections = Array.from({ length: 10 }, () =>
createAuthenticatedGitHubIdentitySync({
authResult: {
ok: true,
method: "tailscale",
user: "ada@github",
tailscaleIdentity: { login: "ada@github", name: "Ada" },
},
}),
);
const requests = connections.map((sync) => {
if (!sync) {
throw new Error("GitHub test identity did not produce a sync function");
}
return sync();
});
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce());
resolveLookup?.(githubResponse({ id: 583231, login: "Ada" }));
await expect(Promise.all(requests)).resolves.toEqual(
Array.from({ length: 10 }, () => expect.objectContaining({ profileId: profile.id })),
);
expect(getUserProfileListItem(profile.id).githubIdentity).toMatchObject({ login: "Ada" });
expect(fetchMock).toHaveBeenCalledOnce();
});
});
it("freshly verifies a mutable login before attaching a later connection", async () => {
it("independently verifies a mutable login before attaching a concurrent connection", async () => {
await withOpenClawTestState({ scenario: "minimal" }, async () => {
let resolveFirst: ((response: Response) => void) | undefined;
const fetchMock = vi
.spyOn(globalThis, "fetch")
.mockResolvedValueOnce(githubResponse({ id: 1, login: "ada" }))
.mockImplementationOnce(
async () =>
await new Promise<Response>((resolve) => {
resolveFirst = resolve;
}),
)
.mockResolvedValueOnce(githubResponse({ id: 2, login: "ada" }));
const createSync = () =>
createAuthenticatedGitHubIdentitySync({
@@ -358,8 +326,15 @@ describe("authenticated GitHub identity sync", () => {
},
});
const first = await createSync()?.();
const reassigned = await createSync()?.();
const firstRequest = createSync()?.();
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce());
const reassignedRequest = createSync()?.();
expect(fetchMock).toHaveBeenCalledOnce();
resolveFirst?.(githubResponse({ id: 1, login: "ada" }));
const first = await firstRequest;
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2));
const reassigned = await reassignedRequest;
expect(fetchMock).toHaveBeenCalledTimes(2);
expect(reassigned?.profileId).not.toBe(first?.profileId);
@@ -377,8 +352,6 @@ describe("authenticated GitHub identity sync", () => {
const inFlight = Array.from({ length: 200 }, (_, index) =>
githubUserIdentityCoordinator.lookup({
credentialScope: "capacity-test",
identityKind: "login",
lookupKey: `login:user-${index}`,
request: async () => {
await requestGate;
return { accountId: index + 1, login: `user-${index}` };
@@ -391,8 +364,6 @@ describe("authenticated GitHub identity sync", () => {
await expect(
githubUserIdentityCoordinator.lookup({
credentialScope: "capacity-test",
identityKind: "login",
lookupKey: "login:overflow",
request: overflowRequest,
}),
).rejects.toMatchObject({ statusCode: 429, retryAfterMs: 1_000 });
@@ -415,14 +386,10 @@ describe("authenticated GitHub identity sync", () => {
const first = githubUserIdentityCoordinator.lookup({
credentialScope: "shared-quota",
identityKind: "login",
lookupKey: "login:ada",
request: firstRequest,
});
const second = githubUserIdentityCoordinator.lookup({
credentialScope: "shared-quota",
identityKind: "login",
lookupKey: "login:grace",
request: secondRequest,
});
await vi.waitFor(() => expect(firstRequest).toHaveBeenCalledOnce());
src/gateway/github-user-identity.ts
+26 -9
View File
@@ -33,6 +33,19 @@ type AuthenticatedGitHubIdentitySyncResult = { profileId: string; updatedAt: num
export type AuthenticatedGitHubIdentitySync = () => Promise<AuthenticatedGitHubIdentitySyncResult>;
type GitHubApiCredentialScope = ReturnType<typeof resolveGitHubApiCredentialScope>;
class GitHubIdentityLookupError extends ControlUiGitHubError {
constructor(
error: ControlUiGitHubError,
readonly retryableForCachedIdentity: boolean,
) {
super(
error.statusCode,
error.message,
error.retryAfterMs === undefined ? undefined : { retryAfterMs: error.retryAfterMs },
);
}
}
function headerValue(value: string | string[] | undefined): string | undefined {
return Array.isArray(value) ? value[0] : value;
}
@@ -116,13 +129,21 @@ async function resolveCloudflareAccessIdentity(
}
async function fetchGitHubIdentityPayload(rawUrl: string, token: string | undefined) {
let response: Response | undefined;
try {
return await readGitHubJsonResponse(await fetchGitHubApi(rawUrl, fetch, token));
response = await fetchGitHubApi(rawUrl, fetch, token);
return await readGitHubJsonResponse(response);
} catch (error) {
if (error instanceof ControlUiGitHubError) {
throw error;
throw new GitHubIdentityLookupError(
error,
response === undefined || error.statusCode === 429 || response.status >= 500,
);
}
throw new ControlUiGitHubError(502, "GitHub request failed");
throw new GitHubIdentityLookupError(
new ControlUiGitHubError(502, "GitHub request failed"),
response === undefined,
);
}
}
@@ -136,8 +157,6 @@ async function resolveGitHubUserIdentityByLogin(
}
return githubUserIdentityCoordinator.lookup({
credentialScope: credential.cacheScope,
identityKind: "login",
lookupKey: `login:${requestedLogin}`,
request: async () => {
const payload = await fetchGitHubIdentityPayload(
`${GITHUB_API_ORIGIN}/users/${encodeURIComponent(requestedLogin)}`,
@@ -166,8 +185,6 @@ function resolveGitHubUserIdentityById(
): Promise<ResolvedGitHubUserIdentity> {
return githubUserIdentityCoordinator.lookup({
credentialScope: credential.cacheScope,
identityKind: "account-id",
lookupKey: `id:${accountId}`,
request: async () => {
const payload = await fetchGitHubIdentityPayload(
`${GITHUB_API_ORIGIN}/user/${accountId}`,
@@ -275,8 +292,8 @@ export function createAuthenticatedGitHubIdentitySync(params: {
identity = await resolveGitHubUserIdentityById(accessIdentity.accountId, credential);
} catch (error) {
const retryable =
error instanceof ControlUiGitHubError &&
(error.statusCode === 429 || error.statusCode >= 500);
(error instanceof GitHubIdentityLookupError && error.retryableForCachedIdentity) ||
(error instanceof ControlUiGitHubError && error.statusCode === 429);
if (retryable) {
// Retry failures may reuse only the exact verified email + immutable-account binding.
const cached = resolveCachedGitHubIdentity({