From 0e326a9e9bb74eea7a463dc18cdb93aa8a1668cb Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 07:08:37 +0900 Subject: [PATCH 1/3] fix(kiro): capture profileArn from whoami and classify profileArn-required 400s (#993) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builder ID imports often lack a profileArn in SQLite. The import now reads the documented whoami --format json surface (narrow shapes, structural validation, imported SQLite ARN stays authoritative, fail closed when absent) and persists it account-scoped. A profileArn- required upstream rejection classifies as the stable, actionable kiro_profile_required instead of the generic validation bucket. No live AWS verification — structural tests only. --- src/adapters/kiro-errors.ts | 11 ++++++++ src/oauth/kiro.ts | 41 +++++++++++++++++++++++---- tests/kiro-oauth.test.ts | 56 +++++++++++++++++++++++++++++++++++++ tests/kiro-retry.test.ts | 16 +++++++++++ tests/kiro-stream.test.ts | 15 ++++++++++ 5 files changed, 134 insertions(+), 5 deletions(-) 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..39fae73924 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 {}; } @@ -252,13 +276,20 @@ async function oauthCredentialFromImported( signal?: AbortSignal, ): Promise { const identity = imported.source === "sqlite" ? await readKiroCliIdentity(runner, signal) : {}; - const metadata = metadataFromImported(imported); + // 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..1807a7a5fb 100644 --- a/tests/kiro-oauth.test.ts +++ b/tests/kiro-oauth.test.ts @@ -425,6 +425,62 @@ 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("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" }, From d2dd48990caf2844af47f8bfa8c0c0510496b1f4 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 07:32:31 +0900 Subject: [PATCH 2/3] fix(kiro): bind whoami identity to a revalidated unchanged CLI session (#993) A concurrent external Kiro session switch between the SQLite read and whoami could attach account B's profileArn to account A's token. whoami's identity is now accepted only when the session token still matches the import (refresh, or access when refresh is absent); a mid-flight switch regression pins the behavior. --- src/oauth/kiro.ts | 15 ++++++++++++++- tests/kiro-oauth.test.ts | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/oauth/kiro.ts b/src/oauth/kiro.ts index 39fae73924..716e6cbfa3 100644 --- a/src/oauth/kiro.ts +++ b/src/oauth/kiro.ts @@ -275,7 +275,20 @@ async function oauthCredentialFromImported( runner: KiroCliRunner, signal?: AbortSignal, ): Promise { - const identity = imported.source === "sqlite" ? await readKiroCliIdentity(runner, signal) : {}; + 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; diff --git a/tests/kiro-oauth.test.ts b/tests/kiro-oauth.test.ts index 1807a7a5fb..13fa5a0862 100644 --- a/tests/kiro-oauth.test.ts +++ b/tests/kiro-oauth.test.ts @@ -481,6 +481,24 @@ describe("kiro oauth — import-first", () => { 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("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 }); From 0338d0753239fa29d4f0ab4ce48784b715878a9a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 5 Aug 2026 09:59:35 +0900 Subject: [PATCH 3/3] test(kiro): prove the session-switch guard clears the whole identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1032 review blocker. The existing session-switch test asserted only that accountId and kiro.profileArn were absent — but pre-fix code ignores whoami's ARN entirely, so those assertions passed against the unfixed implementation too. It was a shape assertion wearing an activation proof's clothes. The email is what pre-fix code WOULD have kept, so asserting its absence is what actually exercises the mismatch path. Adds that, plus the two refresh-absent cases the implementation's access-token fallback needs: access-token-only revalidation succeeding, and an access-token-only session that changes under whoami being rejected. Verified by reverting src/oauth/kiro.ts to the dev version: all three new tests fail against pre-fix code and pass against the fix. --- tests/kiro-oauth.test.ts | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/kiro-oauth.test.ts b/tests/kiro-oauth.test.ts index 13fa5a0862..d9487386ce 100644 --- a/tests/kiro-oauth.test.ts +++ b/tests/kiro-oauth.test.ts @@ -499,6 +499,57 @@ describe("kiro oauth — import-first", () => { 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 });