diff --git a/src/adapters/kiro-errors.ts b/src/adapters/kiro-errors.ts index 7c475f321e..1595f1b1f4 100644 --- a/src/adapters/kiro-errors.ts +++ b/src/adapters/kiro-errors.ts @@ -109,6 +109,17 @@ function classifyKiroFailure( retryable: false, }; } + // #993: a gated model demanding a profileArn gets a stable, actionable code + // instead of the generic validation bucket. Non-retryable by definition. + if (evidence.includes("profilearn") && evidence.includes("required")) { + return { + message: "kiro_profile_required: Kiro requires a CodeWhisperer profileArn for this account and model. Re-login or re-import the matching Kiro account (ocx account login kiro --reauth) so the profile is captured, then retry.", + status: 400, + errorType: "invalid_request_error", + code: "kiro_profile_required", + retryable: false, + }; + } if ( evidence.includes("insufficient_quota") || evidence.includes("quota exhausted") diff --git a/src/oauth/kiro.ts b/src/oauth/kiro.ts index ebbbaa9aa5..716e6cbfa3 100644 --- a/src/oauth/kiro.ts +++ b/src/oauth/kiro.ts @@ -172,13 +172,37 @@ async function defaultKiroCliRunner(args: string[], signal?: AbortSignal): Promi } } -async function readKiroCliIdentity(runner: KiroCliRunner, signal?: AbortSignal): Promise<{ email?: string }> { +/** Kiro profile ARN structure: arn::codewhisperer:::profile/ */ +const KIRO_PROFILE_ARN_PATTERN = /^arn:[a-z0-9-]+:codewhisperer:[a-z0-9-]+:\d{12}:profile\/[A-Za-z0-9-]+$/; +const KIRO_PROFILE_ARN_MAX_LENGTH = 256; + +function parseKiroProfileArn(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const trimmed = value.trim(); + if (trimmed.length === 0 || trimmed.length > KIRO_PROFILE_ARN_MAX_LENGTH) return undefined; + return KIRO_PROFILE_ARN_PATTERN.test(trimmed) ? trimmed : undefined; +} + +function profileArnFromWhoami(parsed: Record): string | undefined { + // Only narrowly-named documented-ish shapes; never invent an ARN (#993). + return parseKiroProfileArn(parsed.profileArn) + ?? parseKiroProfileArn(parsed.profile_arn) + ?? (parsed.profile && typeof parsed.profile === "object" && !Array.isArray(parsed.profile) + ? parseKiroProfileArn((parsed.profile as Record).arn) + : undefined); +} + +async function readKiroCliIdentity(runner: KiroCliRunner, signal?: AbortSignal): Promise<{ email?: string; profileArn?: string }> { try { const result = await runner(["whoami", "--format", "json"], signal); if (result.exitCode !== 0) return {}; - const parsed = JSON.parse(result.stdout) as { email?: unknown }; + const parsed = JSON.parse(result.stdout) as Record; const email = typeof parsed.email === "string" ? parsed.email.trim().toLowerCase() : ""; - return email && email.length <= 320 ? { email } : {}; + const profileArn = profileArnFromWhoami(parsed); + return { + ...(email && email.length <= 320 ? { email } : {}), + ...(profileArn ? { profileArn } : {}), + }; } catch { return {}; } @@ -251,14 +275,34 @@ async function oauthCredentialFromImported( runner: KiroCliRunner, signal?: AbortSignal, ): Promise { - const identity = imported.source === "sqlite" ? await readKiroCliIdentity(runner, signal) : {}; - const metadata = metadataFromImported(imported); + let identity: { email?: string; profileArn?: string } = {}; + if (imported.source === "sqlite") { + identity = await readKiroCliIdentity(runner, signal); + // Session-switch race (#993 review): another process may have switched the + // active Kiro CLI session between the SQLite read and whoami. Accept + // whoami's identity only when the session token STILL matches the import — + // refresh token, or access token when refresh is absent. + if (identity.profileArn !== undefined) { + const current = readKiroCliSqliteCredential(); + const importedKey = imported.refresh || imported.access; + const currentKey = current ? current.refresh || current.access : ""; + if (!current || currentKey !== importedKey) identity = {}; + } + } + // Builder ID imports often lack a profileArn in SQLite; whoami against the + // SAME active CLI session can supply it (#993). Imported stays authoritative. + const resolvedProfileArn = imported.profileArn ?? identity.profileArn; + const metadata: KiroOAuthMetadata | undefined = (() => { + const base = metadataFromImported(imported) ?? {}; + if (resolvedProfileArn && !base.profileArn) base.profileArn = resolvedProfileArn; + return Object.keys(base).length > 0 ? base : undefined; + })(); return { access: imported.access, refresh: imported.refresh, expires: imported.expires, source: imported.source === "json" ? "credential-file" : "local-cli", - ...(imported.profileArn ? { accountId: imported.profileArn } : {}), + ...(resolvedProfileArn ? { accountId: resolvedProfileArn } : {}), ...(identity.email ? { email: identity.email } : {}), ...(metadata ? { kiro: metadata } : {}), }; diff --git a/tests/kiro-oauth.test.ts b/tests/kiro-oauth.test.ts index 348eb0207c..d9487386ce 100644 --- a/tests/kiro-oauth.test.ts +++ b/tests/kiro-oauth.test.ts @@ -425,6 +425,131 @@ describe("kiro oauth — import-first", () => { expect(existsSync(kiroCliRecoveryPath())).toBe(false); }); + test("whoami supplies a valid profileArn when the SQLite import lacks one (#993)", async () => { + const arn = "arn:aws:codewhisperer:us-east-1:123456789012:profile/ABCD1234"; + seedKiroCliDb({ access_token: "aoa-builder", refresh_token: "rt-builder" }); + const runner = async (args: string[]) => { + if (args[0] === "whoami") { + return { exitCode: 0, stdout: JSON.stringify({ email: "builder@example.com", profileArn: arn }) }; + } + throw new Error(`unexpected Kiro CLI command: ${args[0]}`); + }; + + const cred = await loginKiro({}, { cliRunner: runner }); + + expect(cred).toMatchObject({ + access: "aoa-builder", + accountId: arn, + kiro: { profileArn: arn }, + }); + }); + + test("a nested profile.arn shape is accepted (#993)", async () => { + const arn = "arn:aws:codewhisperer:eu-west-1:123456789012:profile/XY-99"; + seedKiroCliDb({ access_token: "aoa-nested", refresh_token: "rt-nested" }); + const nestedRunner = async (args: string[]) => { + if (args[0] === "whoami") return { exitCode: 0, stdout: JSON.stringify({ profile: { arn } }) }; + throw new Error("unexpected"); + }; + const nested = await loginKiro({}, { cliRunner: nestedRunner }); + expect(nested.accountId).toBe(arn); + }); + + test("a malformed whoami ARN is ignored without breaking the login (#993)", async () => { + seedKiroCliDb({ access_token: "aoa-bad", refresh_token: "rt-bad" }); + const badRunner = async (args: string[]) => { + if (args[0] === "whoami") { + return { exitCode: 0, stdout: JSON.stringify({ profileArn: "not-an-arn", email: "bad@example.com" }) }; + } + throw new Error("unexpected"); + }; + const bad = await loginKiro({}, { cliRunner: badRunner }); + // Malformed ARN: login still succeeds for ungated models, but no ARN is borrowed. + expect(bad.accountId).toBeUndefined(); + expect(bad.kiro?.profileArn).toBeUndefined(); + }); + + test("an imported SQLite profileArn stays authoritative over whoami (#993)", async () => { + const sqliteArn = "arn:aws:codewhisperer:us-east-1:123456789012:profile/SQLITE"; + const whoamiArn = "arn:aws:codewhisperer:us-east-1:123456789012:profile/WHOAMI"; + seedKiroCliDb({ access_token: "aoa-both", refresh_token: "rt-both", profile_arn: sqliteArn }); + const runner = async (args: string[]) => { + if (args[0] === "whoami") return { exitCode: 0, stdout: JSON.stringify({ profileArn: whoamiArn }) }; + throw new Error("unexpected"); + }; + const cred = await loginKiro({}, { cliRunner: runner }); + expect(cred.accountId).toBe(sqliteArn); + }); + + test("a session switch between the SQLite read and whoami discards whoami's ARN (#993)", async () => { + const arnB = "arn:aws:codewhisperer:us-east-1:123456789012:profile/OTHER-ACCOUNT"; + seedKiroCliDb({ access_token: "aoa-accountA", refresh_token: "rt-accountA" }); + const runner = async (args: string[]) => { + if (args[0] === "whoami") { + // Another process switches the active CLI session mid-flight. + removeKiroCliDb(); + seedKiroCliDb({ access_token: "aoa-accountB", refresh_token: "rt-accountB" }); + return { exitCode: 0, stdout: JSON.stringify({ email: "b@example.com", profileArn: arnB }) }; + } + throw new Error("unexpected"); + }; + const cred = await loginKiro({}, { cliRunner: runner }); + // Account A's token must never carry account B's ARN. + expect(cred.accountId).toBeUndefined(); + expect(cred.kiro?.profileArn).toBeUndefined(); + }); + + test("the discarded whoami identity takes the email with it, not just the ARN (#993)", async () => { + // Review finding: asserting only that the ARN is absent also passes against + // pre-fix code, which ignored whoami's ARN entirely. The email is the part + // that pre-fix code WOULD have kept, so it is the assertion that actually + // proves the mismatch path clears the whole identity. + const arnB = "arn:aws:codewhisperer:us-east-1:123456789012:profile/OTHER-ACCOUNT"; + seedKiroCliDb({ access_token: "aoa-accountA", refresh_token: "rt-accountA" }); + const runner = async (args: string[]) => { + if (args[0] === "whoami") { + removeKiroCliDb(); + seedKiroCliDb({ access_token: "aoa-accountB", refresh_token: "rt-accountB" }); + return { exitCode: 0, stdout: JSON.stringify({ email: "b@example.com", profileArn: arnB }) }; + } + throw new Error("unexpected"); + }; + const cred = await loginKiro({}, { cliRunner: runner }); + expect(cred.email).toBeUndefined(); + expect(cred.kiro?.profileArn).toBeUndefined(); + }); + + test("with no refresh token the access token is the revalidation key (#993)", async () => { + // The implementation falls back to the access token when refresh is absent. + // Without this case that branch is unexercised in either direction. + const arn = "arn:aws:codewhisperer:us-east-1:123456789012:profile/BUILDER"; + seedKiroCliDb({ access_token: "aoa-only" }); + const runner = async (args: string[]) => { + if (args[0] === "whoami") { + return { exitCode: 0, stdout: JSON.stringify({ email: "a@example.com", profileArn: arn }) }; + } + throw new Error("unexpected"); + }; + const cred = await loginKiro({}, { cliRunner: runner }); + expect(cred.kiro?.profileArn).toBe(arn); + }); + + test("an access-token-only session that changes under whoami is rejected (#993)", async () => { + const arnB = "arn:aws:codewhisperer:us-east-1:123456789012:profile/OTHER-ACCOUNT"; + seedKiroCliDb({ access_token: "aoa-accountA" }); + const runner = async (args: string[]) => { + if (args[0] === "whoami") { + removeKiroCliDb(); + seedKiroCliDb({ access_token: "aoa-accountB" }); + return { exitCode: 0, stdout: JSON.stringify({ email: "b@example.com", profileArn: arnB }) }; + } + throw new Error("unexpected"); + }; + const cred = await loginKiro({}, { cliRunner: runner }); + expect(cred.kiro?.profileArn).toBeUndefined(); + expect(cred.email).toBeUndefined(); + }); + test("invalid recovery data names the file the operator must remove", async () => { seedKiroCliDb({ access_token: "aoa-prior", refresh_token: "rt-prior" }); writeFileSync(kiroCliRecoveryPath(), "not a recovery database", { mode: 0o600 }); diff --git a/tests/kiro-retry.test.ts b/tests/kiro-retry.test.ts index 113e6c900c..df4285546b 100644 --- a/tests/kiro-retry.test.ts +++ b/tests/kiro-retry.test.ts @@ -330,6 +330,22 @@ describe("kiro retry fetch", () => { expect(mock.calls).toHaveLength(1); }); + test("a profileArn-required 400 classifies as kiro_profile_required with actionable copy (#993)", async () => { + const mock = mockFetch([ + new Response(JSON.stringify({ + __type: "ValidationException", + message: "profileArn is required for this account", + }), { status: 400 }), + ]); + const res = await fetchKiroWithRetry(request, { timeoutMs: 5_000 }); + const text = await res.text(); + expect(res.status).toBe(400); + expect(text).toContain("kiro_profile_required"); + expect(text).toContain("ocx account login kiro --reauth"); + // Non-retryable: exactly one upstream call. + expect(mock.calls).toHaveLength(1); + }); + test("normalizes final transient 429 after bounded adapter retries", async () => { const mock = mockFetch([ new Response("rate limited", { status: 429, headers: { "Retry-After": "0" } }), diff --git a/tests/kiro-stream.test.ts b/tests/kiro-stream.test.ts index b2a92bdf17..744fa8ab1f 100644 --- a/tests/kiro-stream.test.ts +++ b/tests/kiro-stream.test.ts @@ -1096,6 +1096,21 @@ describe("kiro adapter — parseStream", () => { expect(errors[0]).not.toContain("{"); }); + test("an event-stream profileArn-required exception classifies as kiro_profile_required (#993)", async () => { + const payload = JSON.stringify({ + __type: "ValidationException", + message: "profileArn is required for this account", + }); + const frame = encodeMessage({ ":message-type": "exception", ":exception-type": "ValidationException" }, enc.encode(payload)); + const errors: string[] = []; + for await (const e of createKiroAdapter(provider).parseStream(new Response(streamOf(frame)))) { + if (e.type === "error") errors.push(e.message); + } + expect(errors).toHaveLength(1); + expect(errors[0]).toContain("kiro_profile_required"); + expect(errors[0]).toContain("ocx account login kiro --reauth"); + }); + test("auth and model exceptions become actionable Kiro errors", async () => { const authFrame = encodeMessage( { ":message-type": "exception", ":exception-type": "AccessDeniedException" },