test: fix oauth rebase conflict

This commit is contained in:
Peter Steinberger
2026-04-18 21:42:01 +01:00
parent 5530cec127
commit f60c3bf6e0
3 changed files with 5 additions and 143 deletions

View File

@@ -6,6 +6,7 @@ import {
OAUTH_REFRESH_LOCK_OPTIONS,
log,
} from "./constants.js";
import { shouldMirrorRefreshedOAuthCredential } from "./oauth-identity.js";
import {
areOAuthCredentialsEquivalent,
hasUsableOAuthCredential,
@@ -18,7 +19,6 @@ import {
shouldReplaceStoredOAuthCredential,
type RuntimeExternalOAuthProfile,
} from "./oauth-shared.js";
import { shouldMirrorRefreshedOAuthCredential } from "./oauth-identity.js";
import { ensureAuthStoreFile, resolveAuthStorePath, resolveOAuthRefreshLockPath } from "./paths.js";
import {
ensureAuthProfileStore,
@@ -317,12 +317,6 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) {
}
return false;
}
if (existing && !isSafeToAdoptMainStoreOAuthIdentity(existing, params.refreshed)) {
log.warn("refused to mirror OAuth credential: identity mismatch or regression", {
profileId: params.profileId,
});
return false;
}
store.profiles[params.profileId] = { ...params.refreshed };
log.debug("mirrored refreshed OAuth credential to main agent store", {
profileId: params.profileId,

View File

@@ -387,7 +387,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
);
});
it("adopts a fresher imported refresh token even when its access token is already expired", async () => {
it("keeps the canonical refresh token when imported Codex CLI state is expired", async () => {
const profileId = "openai-codex:default";
saveAuthProfileStore(
{
@@ -415,8 +415,8 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
refreshProviderOAuthCredentialWithPluginMock.mockImplementationOnce(
async (params?: { context?: unknown }) => {
expect(params?.context).toMatchObject({
access: "newer-but-expired-cli-access-token",
refresh: "fresh-cli-refresh-token",
access: "expired-local-access-token",
refresh: "stale-local-refresh-token",
});
return {
type: "oauth",
@@ -447,7 +447,7 @@ describe("resolveApiKeyForProfile openai-codex refresh fallback", () => {
});
expect(persisted.profiles[profileId]).not.toEqual(
expect.objectContaining({
refresh: "stale-local-refresh-token",
refresh: "fresh-cli-refresh-token",
}),
);
});

View File

@@ -130,138 +130,6 @@ type ResolveApiKeyForProfileParams = {
};
type SecretDefaults = NonNullable<OpenClawConfig["secrets"]>["defaults"];
export function normalizeAuthIdentityToken(value: string | undefined): string | undefined {
const trimmed = value?.trim();
return trimmed ? trimmed : undefined;
}
export function normalizeAuthEmailToken(value: string | undefined): string | undefined {
return normalizeAuthIdentityToken(value)?.toLowerCase();
}
/**
* Returns true if `existing` and `incoming` provably belong to the same
* account. Used to gate cross-agent credential mirroring.
*
* The rule is intentionally strict to satisfy the CWE-284 model:
* 1. If one side carries identity metadata (accountId or email) and the
* other does not, refuse — we have no evidence they match.
* 2. If both sides carry identity, a shared field must match (accountId
* wins over email when both present). If the two sides carry identity
* in non-overlapping fields (one has only accountId, the other only
* email), refuse.
* 3. If neither side carries identity, return true: no evidence of
* mismatch and provider equality is checked separately by the caller.
*
* The previous permissive behaviour (fall back to `true` whenever a strict
* comparison could not be made) was unsafe: a sub-agent whose refreshed
* credential lacked identity metadata could overwrite a known-account main
* credential that had it, allowing cross-account poisoning through the
* mirror path.
*/
export function isSameOAuthIdentity(
existing: Pick<OAuthCredential, "accountId" | "email">,
incoming: Pick<OAuthCredential, "accountId" | "email">,
): boolean {
const aAcct = normalizeAuthIdentityToken(existing.accountId);
const bAcct = normalizeAuthIdentityToken(incoming.accountId);
const aEmail = normalizeAuthEmailToken(existing.email);
const bEmail = normalizeAuthEmailToken(incoming.email);
const aHasIdentity = aAcct !== undefined || aEmail !== undefined;
const bHasIdentity = bAcct !== undefined || bEmail !== undefined;
// Asymmetric identity evidence — refuse. We cannot prove the two
// credentials belong to the same account.
if (aHasIdentity !== bHasIdentity) {
return false;
}
// Both sides carry identity — require a positive match on a shared field.
if (aHasIdentity) {
if (aAcct !== undefined && bAcct !== undefined) {
return aAcct === bAcct;
}
if (aEmail !== undefined && bEmail !== undefined) {
return aEmail === bEmail;
}
// Identity metadata is present on both sides but in non-overlapping
// fields (one has accountId, the other has only email, or vice versa).
// No shared field to compare — refuse rather than guess.
return false;
}
// Neither side carries identity metadata — provider equality is checked
// separately by the caller; no evidence of mismatch here.
return true;
}
/**
* Identity gate used for both directions of credential copy:
* - mirror (sub-agent refresh -> main agent store)
* - adopt (main agent store -> sub-agent store)
*
* Rule: allow the copy iff
* 1. no positive identity mismatch — if both sides expose the same
* identity field (accountId or email), the values must match, AND
* 2. the incoming credential carries at least as much identity
* evidence as the existing one — if existing has accountId/email,
* incoming must carry the same field, AND
* 3. when both sides carry identity but in non-overlapping fields
* (existing has only accountId, incoming has only email, or vice
* versa) we cannot positively prove the same account and the copy
* is refused.
*
* Accepts:
* - matching accountId (positive match on strongest field)
* - matching email when accountId is absent on both sides
* - neither side carries identity (no evidence of mismatch)
* - existing has no identity, incoming has identity (UPGRADE: adds
* the marker without dropping anything)
*
* Refuses:
* - mismatching accountId or email on a shared field (CWE-284 core)
* - incoming drops an identity field present on existing (regression
* that would later let a wrong-account peer pass this gate)
* - non-overlapping fields (no comparable positive match)
*
* Design note: this is a single unified rule for both copy directions.
* The rule is deliberately one-sided because "existing" is whatever is
* about to be overwritten and "incoming" is the new data — the
* constraint is the same regardless of whether existing is main or sub.
*/
export function isSafeToCopyOAuthIdentity(
existing: Pick<OAuthCredential, "accountId" | "email">,
incoming: Pick<OAuthCredential, "accountId" | "email">,
): boolean {
const aAcct = normalizeAuthIdentityToken(existing.accountId);
const bAcct = normalizeAuthIdentityToken(incoming.accountId);
const aEmail = normalizeAuthEmailToken(existing.email);
const bEmail = normalizeAuthEmailToken(incoming.email);
// (1) Positive match on a shared field, if one exists.
if (aAcct !== undefined && bAcct !== undefined) {
return aAcct === bAcct;
}
if (aEmail !== undefined && bEmail !== undefined) {
return aEmail === bEmail;
}
// No shared comparable field beyond this point.
const aHasIdentity = aAcct !== undefined || aEmail !== undefined;
// (2) Refuse if existing has any identity evidence that incoming lacks.
// That covers both the "drop" case (incoming has nothing) and the
// "non-overlapping fields" case (existing has accountId only,
// incoming has email only, or vice versa).
if (aHasIdentity) {
return false;
}
// (3) Existing has no identity. Either incoming has none either
// (allowed: no evidence of mismatch) or incoming adds identity
// (allowed: pure upgrade, no loss).
return true;
}
async function refreshOAuthCredential(
credential: OAuthCredential,