Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/codex/auth-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1791,7 +1791,7 @@ export async function handleCodexAuthAPI(
const loginOwner: CodexLoginStateRow = { status: "starting", startedAt: Date.now() };
codexAuthLoginState.set(flowId, loginOwner);
try {
const { startLoginFlow, getLoginStatus } = await import("../oauth");
const { startLoginFlow, getLoginStatus, publicOAuthAuthenticationErrorMessage } = await import("../oauth");
const result = await startLoginFlow("chatgpt", { forceLogin: true });

// Open the browser server-side (same pattern as /api/oauth/login in management-api.ts).
Expand Down Expand Up @@ -1993,7 +1993,13 @@ export async function handleCodexAuthAPI(
break;
}
if (st.done && st.error) {
setCodexLoginState(flowId, { status: "error", error: st.error, doneAt: Date.now() });
setCodexLoginState(flowId, {
status: "error",
// startLoginFlow projects background failures before storing login status, so
// fixed actionable OAuth messages retain their type-derived remediation here.
error: st.error,
doneAt: Date.now(),
});
completed = true;
break;
}
Expand All @@ -2011,7 +2017,7 @@ export async function handleCodexAuthAPI(
? "Configuration is busy; retry login shortly."
: error instanceof CodexCredentialRefreshBusyError || error instanceof CodexCredentialRefreshStaleError
? "Credential refresh is busy; retry login shortly."
: error instanceof Error ? error.message : String(error);
: publicOAuthAuthenticationErrorMessage(error);
setCodexLoginState(flowId, {
status: "error",
error: message,
Expand All @@ -2028,15 +2034,16 @@ export async function handleCodexAuthAPI(
} catch (e) {
if (codexAuthLoginState.get(flowId) === loginOwner) codexAuthLoginState.delete(flowId);
const msg = e instanceof Error ? e.message : String(e);
if (msg.includes("already in progress")) {
if (msg === "A login for chatgpt is already in progress") {
return jsonResponse({ error: msg, status: "pending" }, 409);
}
if (e instanceof CodexCredentialRefreshBusyError || e instanceof CodexCredentialRefreshStaleError) {
const response = jsonResponse({ error: "server_busy", code: "server_busy" }, 503);
response.headers.set("Retry-After", "1");
return response;
}
return jsonResponse({ error: msg }, 500);
const { publicOAuthAuthenticationErrorMessage } = await import("../oauth");
return jsonResponse({ error: publicOAuthAuthenticationErrorMessage(e) }, 500);
}
}

Expand Down
46 changes: 40 additions & 6 deletions src/oauth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,38 @@ export class UnsupportedOAuthProviderError extends Error {
}

export class OAuthLoginRequiredError extends Error {
readonly provider: string;

constructor(provider: string) {
super(`Not logged in to ${provider}. Run: ocx login ${provider}`);
this.name = "OAuthLoginRequiredError";
this.provider = provider;
}
}

export class OAuthProviderPublicationError extends Error {
constructor() {
super("OAuth credential was saved, but the provider entry was not written. Resolve the account namespace collision, then retry login.");
this.name = "OAuthProviderPublicationError";
}
}

/** Project arbitrary OAuth failures onto the small, stable public error vocabulary. */
export function publicOAuthAuthenticationErrorMessage(error: unknown): string {
if (error instanceof OAuthMutationBusyError) {
return error.message === "OAuth mutation queue wait timed out"
? "OAuth mutation queue wait timed out"
: "OAuth mutation queue is busy";
}
if (
(error instanceof OAuthLoginRequiredError && isOAuthProvider(error.provider))
|| error instanceof OAuthProviderPublicationError
|| error instanceof OAuthTokenRefreshBusyError
|| error instanceof OAuthTokenRefreshStaleError
Comment thread
luvs01 marked this conversation as resolved.
) return error.message;
return "OAuth authentication failed. Check the OpenCodex account status and retry.";
Comment thread
luvs01 marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve actionable reauthentication identity errors

When reauthenticating an existing account succeeds with a different identity—or when a legacy account lacks verifiable identity—runLogin throws fixed, safe remediation messages, but this fallback replaces them with the generic authentication failure. The dashboard processes s.error before its needsReauth fallback in gui/src/pages/use-providers-oauth.ts, so users are no longer told to sign in with the selected account and cannot diagnose repeated failures. Represent these outcomes with bounded typed errors and preserve their fixed messages, with focused status-polling coverage.

AGENTS.md reference: src/AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

}

function accessSnapshot(provider: string, accountId: string, cred: OAuthCredentials): OAuthAccessSnapshot {
const storedKiroRouting = {
...(cred.kiro?.profileArn ? { profileArn: cred.kiro.profileArn } : {}),
Expand Down Expand Up @@ -1136,10 +1162,7 @@ export async function runLogin(
provider,
);
if (lateCollision) {
throw new Error(
`${lateCollision}. The credential for "${provider}" was saved, but the provider entry was not written. `
+ "Rename the account selector, then re-run the login.",
);
throw new OAuthProviderPublicationError();
}
upsertOAuthProvider(latestConfig, provider);
saveLatestConfig(latestConfig);
Expand Down Expand Up @@ -1381,7 +1404,16 @@ export async function startLoginFlow(
onManualCodeInput: (expectedState?: string) => waitForManualLoginCode(provider, abort.signal, expectedState),
signal: abort.signal,
};
const abandonIfNotOwner = (error?: unknown): boolean => {
if (loginAbort.get(provider) === abort) return false;
if (!urlResolved) reject(error ?? new Error("OAuth login was superseded"));
return true;
};
const settle = async (error?: unknown): Promise<void> => {
// Cancellation deletes this controller and records its own terminal result. A late provider
// rejection (or an older flow settling after a replacement starts) must not overwrite that
// state or delete the replacement flow's controller/manual-code slot.
if (abandonIfNotOwner(error)) return;
let finalError = error;
try {
await lifecycle?.onSettled?.();
Expand All @@ -1390,6 +1422,7 @@ export async function startLoginFlow(
// runtime config. For an already-failed login, keep the original recovery error.
if (finalError === undefined) finalError = settleError;
}
if (abandonIfNotOwner(finalError)) return;
if (finalError === undefined) {
loginAbort.delete(provider);
clearManualCodeSlot(provider);
Expand All @@ -1403,7 +1436,7 @@ export async function startLoginFlow(
const e = finalError;
loginAbort.delete(provider);
clearManualCodeSlot(provider);
const msg = e instanceof Error ? e.message : String(e);
const msg = publicOAuthAuthenticationErrorMessage(e);
Comment thread
luvs01 marked this conversation as resolved.
loginState.set(provider, { done: true, error: msg });
if (!urlResolved) reject(e);
};
Expand All @@ -1414,9 +1447,10 @@ export async function startLoginFlow(
(e: unknown) => settle(e),
).catch((e: unknown) => {
// settle catches lifecycle failures, so this is only a defensive promise-boundary guard.
if (abandonIfNotOwner(e)) return;
loginAbort.delete(provider);
clearManualCodeSlot(provider);
const msg = e instanceof Error ? e.message : String(e);
const msg = publicOAuthAuthenticationErrorMessage(e);
loginState.set(provider, { done: true, error: msg });
if (!urlResolved) reject(e);
});
Expand Down
12 changes: 10 additions & 2 deletions src/server/management/oauth-account-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
getLoginStatus,
isPublicOAuthProvider,
listOAuthProviders,
publicOAuthAuthenticationErrorMessage,
startLoginFlow,
submitManualLoginCode,
} from "../../oauth";
Expand Down Expand Up @@ -175,7 +176,13 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise<
return jsonResponse({ url: authUrl, instructions, deviceCode });
} catch (err) {
if (err instanceof OAuthMutationBusyError) throw err;
return jsonResponse({ error: err instanceof Error ? err.message : String(err) }, 409);
const message = err instanceof Error ? err.message : String(err);
const duplicateLoginMessage = `A login for ${provider} is already in progress`;
return jsonResponse({
error: message === duplicateLoginMessage
? duplicateLoginMessage
: publicOAuthAuthenticationErrorMessage(err),
}, 409);
}
}

Expand Down Expand Up @@ -208,7 +215,8 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise<
if (url.pathname === "/api/oauth/status" && req.method === "GET") {
const provider = (url.searchParams.get("provider") ?? "").trim().toLowerCase();
if (!isPublicOAuthProvider(provider)) return jsonResponse({ error: "unknown oauth provider" }, 400);
return jsonResponse(getLoginStatus(provider));
const status = getLoginStatus(provider);
return jsonResponse(status);
}

if (url.pathname === "/api/oauth/logout" && req.method === "POST") {
Expand Down
11 changes: 5 additions & 6 deletions src/server/responses/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { checkInputAdmission } from "./input-admission";
import { nativeContextLimits } from "../../codex/catalog";
import { describeUpstreamConnectFailure } from "./upstream-error";
import {
getConfigPath,
multiAgentGuidanceEnabled,
resolveEnvValue,
} from "../../config";
Expand Down Expand Up @@ -62,6 +61,7 @@ import {
getOAuthCredentialApiBaseUrl,
getValidAccessTokenForAccount,
getValidAccessTokenSnapshot,
publicOAuthAuthenticationErrorMessage,
type OAuthAccessSnapshot,
UnsupportedOAuthProviderError,
} from "../../oauth";
Expand Down Expand Up @@ -349,8 +349,6 @@ function isFixedCodexAccount(authCtx: CodexAuthContext): boolean {
&& authCtx.fixedAccount === true;
}



export function usesCodexForwardPoolAuth(
authCtx: CodexAuthContext,
provider: OcxProviderConfig,
Expand Down Expand Up @@ -2105,13 +2103,14 @@ async function handleResponsesInner(
}
} catch (err) {
if (err instanceof UnsupportedOAuthProviderError) {
const safeProviderName = redactSecretString(route.providerName);
return formatErrorResponse(
400,
"invalid_request_error",
`${err.message}. Remove or reconfigure provider '${route.providerName}' in ${getConfigPath()}.`,
`${redactSecretString(err.message)}. Remove or reconfigure provider '${safeProviderName}' in the OpenCodex configuration.`,
);
}
return formatErrorResponse(401, "authentication_error", err instanceof Error ? err.message : String(err));
return formatErrorResponse(401, "authentication_error", publicOAuthAuthenticationErrorMessage(err));
}
}
route.provider = resolveProviderTransport(
Expand Down Expand Up @@ -3707,7 +3706,7 @@ async function handleResponsesInner(
refreshed = await forceRefreshOAuthAccessSnapshot(sentOAuthSnapshot);
} catch (err) {
cleanupUpstreamAbort();
return formatErrorResponse(401, "authentication_error", err instanceof Error ? err.message : String(err));
return formatErrorResponse(401, "authentication_error", publicOAuthAuthenticationErrorMessage(err));
}
sentOAuthSnapshot = refreshed;
replayOAuthCredentialSnapshot = {
Expand Down
16 changes: 10 additions & 6 deletions src/vision/anthropic-describe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { CLAUDE_CODE_HEADERS, claudeCodeSessionId } from "../adapters/client-fin
import { signalWithTimeout, cancelBodyOnAbort } from "../lib/abort";
import { sidecarEnter } from "../lib/sidecar-tracker";
import { fetchWithResetRetry } from "../lib/upstream-retry";
import { getValidAccessToken } from "../oauth";
import { getValidAccessToken, publicOAuthAuthenticationErrorMessage } from "../oauth";
import { ANTHROPIC_OAUTH_BETA, CLAUDE_CODE_SYSTEM_INSTRUCTION } from "../oauth/anthropic";
import type { DescribeOutcome, VisionSettings } from "./describe";

Expand Down Expand Up @@ -67,8 +67,8 @@ export async function parseAnthropicVisionSSE(res: Response): Promise<DescribeOu
const delta = isRecord(data.delta) ? data.delta : {};
if (delta.type === "text_delta" && typeof delta.text === "string") text += delta.text;
} else if (data.type === "error") {
const error = isRecord(data.error) ? data.error : {};
terminalError = typeof error.message === "string" ? error.message : "anthropic vision sidecar stream error";
// Provider-authored stream errors can contain credentials, paths, or response bodies.
terminalError = "anthropic vision sidecar stream error";
}
};

Expand Down Expand Up @@ -116,7 +116,7 @@ export async function describeImageAnthropic(
try {
token = await getValidAccessToken(providerName);
} catch (error) {
return { text: "", error: `anthropic vision sidecar auth failed: ${error instanceof Error ? error.message : String(error)}` };
return { text: "", error: `anthropic vision sidecar auth failed: ${publicOAuthAuthenticationErrorMessage(error)}` };
}

const headers: Record<string, string> = {
Expand Down Expand Up @@ -166,7 +166,11 @@ export async function describeImageAnthropic(
if (!res.ok) {
const responseText = await res.text().catch(() => "");
console.warn(`[vision] anthropic sidecar HTTP ${res.status} (${Date.now() - startedAt}ms)`);
return { text: "", error: `anthropic vision sidecar HTTP ${res.status}: ${responseText.slice(0, 200)}` };
if (res.status === 401) {
return { text: "", error: `anthropic vision sidecar auth failed: ${publicOAuthAuthenticationErrorMessage(new Error(responseText))}` };
}
// Upstream bodies are untrusted and may contain credentials, paths, or provider diagnostics.
return { text: "", error: `anthropic vision sidecar HTTP ${res.status}` };
}
const detachBodyGuard = cancelBodyOnAbort(res.body, linkedSignal.signal);
try {
Expand All @@ -177,7 +181,7 @@ export async function describeImageAnthropic(
} catch (error) {
const kind = error instanceof Error && error.name === "TimeoutError" ? "timeout" : "connect_error";
console.warn(`[vision] anthropic sidecar ${kind} (${Date.now() - startedAt}ms)`);
return { text: "", error: error instanceof Error ? error.message : String(error) };
return { text: "", error: `anthropic vision sidecar ${kind}` };
} finally {
sidecarExit();
linkedSignal.cleanup();
Expand Down
14 changes: 8 additions & 6 deletions src/web-search/anthropic-executor.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import type { OcxProviderConfig } from "../types";
import { getValidAccessToken } from "../oauth";
import { getValidAccessToken, publicOAuthAuthenticationErrorMessage } from "../oauth";
import { ANTHROPIC_OAUTH_BETA, CLAUDE_CODE_SYSTEM_INSTRUCTION } from "../oauth/anthropic";
import { CLAUDE_CODE_HEADERS, claudeCodeSessionId } from "../adapters/client-fingerprint";
import { signalWithTimeout, cancelBodyOnAbort } from "../lib/abort";
import { redactSecretString } from "../lib/redact";
import { sidecarEnter } from "../lib/sidecar-tracker";
import { fetchWithResetRetry } from "../lib/upstream-retry";
import type { WebSearchSource } from "./parse";
Expand Down Expand Up @@ -127,7 +126,7 @@ export async function runAnthropicWebSearch(
try {
token = await getValidAccessToken(providerName);
} catch (e) {
return { text: "", sources: [], error: `anthropic sidecar auth failed: ${e instanceof Error ? e.message : String(e)}` };
return { text: "", sources: [], error: `anthropic sidecar auth failed: ${publicOAuthAuthenticationErrorMessage(e)}` };
}
const headers: Record<string, string> = {
"Content-Type": "application/json",
Expand Down Expand Up @@ -174,8 +173,11 @@ export async function runAnthropicWebSearch(
const t = await res.text().catch(() => "");
detachBodyGuard();
console.warn(`[web-search] anthropic sidecar HTTP ${res.status} for query "${query.slice(0, 80)}" (${Date.now() - t0}ms)`);
// Redact before surfacing: the body can echo auth headers/tokens (#398 review).
return { text: "", sources: [], error: `sidecar HTTP ${res.status}: ${redactSecretString(t.slice(0, 200))}` };
if (res.status === 401) {
return { text: "", sources: [], error: `anthropic sidecar auth failed: ${publicOAuthAuthenticationErrorMessage(new Error(t))}` };
}
// Upstream bodies are untrusted and may contain credentials, paths, or provider diagnostics.
return { text: "", sources: [], error: `sidecar HTTP ${res.status}` };
}
try {
return await parseAnthropicSidecarSSE(res);
Expand All @@ -185,7 +187,7 @@ export async function runAnthropicWebSearch(
} catch (e) {
const kind = e instanceof Error && e.name === "TimeoutError" ? "timeout" : "connect_error";
console.warn(`[web-search] anthropic sidecar ${kind} for query "${query.slice(0, 80)}" (${Date.now() - t0}ms)`);
return { text: "", sources: [], error: e instanceof Error ? e.message : String(e) };
return { text: "", sources: [], error: `anthropic sidecar ${kind}` };
} finally {
sidecarExit();
linkedSignal.cleanup();
Expand Down
Loading
Loading