From 2ba33d54203de3854fbd6c4d30f736372e53920c Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:49:56 -0300 Subject: [PATCH] fix(security): keep the Kiro auto-import route out of the manage-scope bypass /api/oauth/kiro/auto-import reads host-local Kiro credential files (kiro-cli data.sqlite3, ~/.aws/sso/cache, the Kiro profile.json). It is LOCAL_ONLY, but it was reviewed as bypass-eligible, so an operator-set LOCAL_ONLY manage-scope bypass list could expose it to a remote manage-scoped API key. Add it to the never-bypassable set in spawnCapablePrefixes.ts, next to the other non-spawn entries (/api/discovery/, /api/issue-agent/), so the shared isSpawnCapableBypassPrefix() predicate refuses it at all three layers: the settings PATCH refine (BYPASS_PREFIX_NOT_ALLOWED), the runtime snapshot normalization (also dropping covering parents such as /api/oauth/kiro/), and the request-time isLocalOnlyBypassableByManageScope check. The authz inventory and the dashboard pick it up as "cannot be made bypassable" through the same list. Tests cover the PATCH rejection (exact path and /api/oauth/kiro/), the stored-list normalization, and a manage-scoped key from a non-loopback address getting 403 LOCAL_ONLY both through the settings pipeline and with a raw snapshot that skips normalization. The drift inventory moves the route from reviewed-eligible to the deny-list. --- .../0000-kiro-auto-import-no-manage-bypass.md | 1 + docs/security/ROUTE_GUARD_TIERS.md | 5 +- src/shared/constants/spawnCapablePrefixes.ts | 5 + .../local-only-bypass-spawn-denylist.test.ts | 13 ++- ...spawn-capable-prefixes-client-safe.test.ts | 4 +- tests/unit/settings/authz-bypass.test.ts | 104 ++++++++++++++++++ 6 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 changelog.d/fixes/0000-kiro-auto-import-no-manage-bypass.md diff --git a/changelog.d/fixes/0000-kiro-auto-import-no-manage-bypass.md b/changelog.d/fixes/0000-kiro-auto-import-no-manage-bypass.md new file mode 100644 index 000000000000..275137a42b9a --- /dev/null +++ b/changelog.d/fixes/0000-kiro-auto-import-no-manage-bypass.md @@ -0,0 +1 @@ +- **fix(security):** `/api/oauth/kiro/auto-import` can no longer be opened to remote callers through the LOCAL_ONLY manage-scope bypass list. The route reads host-local Kiro credential files (kiro-cli data, `~/.aws/sso/cache`), so it now sits in the same never-bypassable set as the spawn-capable routes: `PATCH /api/settings` rejects it (and any broader prefix such as `/api/oauth/kiro/`) with `BYPASS_PREFIX_NOT_ALLOWED`, the runtime snapshot drops it from a stored bypass list, and the request-time check refuses it for a manage-scoped key from a non-loopback address. diff --git a/docs/security/ROUTE_GUARD_TIERS.md b/docs/security/ROUTE_GUARD_TIERS.md index c6958c96020d..414013ea7861 100644 --- a/docs/security/ROUTE_GUARD_TIERS.md +++ b/docs/security/ROUTE_GUARD_TIERS.md @@ -132,7 +132,10 @@ operator responsibilities remain: only with a `manage`-scoped API key. The `SPAWN_CAPABLE_PREFIXES` can never be added to the bypass list — the zod schema rejects them and `isLocalOnlyBypassableByManageScope` denies them at runtime (defence-in-depth), - which is what the dashboard means by "cannot be made bypassable". The runtime + which is what the dashboard means by "cannot be made bypassable". The same + deny-list also holds a few LOCAL_ONLY routes that do not spawn but must never be + remotely reachable either — `/api/oauth/kiro/auto-import` (reads host-local Kiro + credential files), `/api/discovery/` and `/api/issue-agent/`. The runtime snapshot loader also drops such entries from a stored settings row, and the legacy JSON import (`/api/settings/import-json`) never writes the bypass keys. Every `LOCAL_ONLY_API_PREFIXES` entry must be either in the deny-list or diff --git a/src/shared/constants/spawnCapablePrefixes.ts b/src/shared/constants/spawnCapablePrefixes.ts index 6cd5b4bd401c..6de51992436c 100644 --- a/src/shared/constants/spawnCapablePrefixes.ts +++ b/src/shared/constants/spawnCapablePrefixes.ts @@ -82,6 +82,10 @@ export const SPAWN_CAPABLE_PREFIXES: ReadonlyArray = [ "/dashboard/providers/services/", // reverse proxy into embedded service UIs — Hard Rule #17 pairs it with /api/services/ "/api/discovery/", // not a spawn, but declared strict-loopback with no manage-scope bypass (outbound provider probes, SSRF-adjacent) "/api/issue-agent/", // local triage executor surface, held loopback/LAN "until sandbox + audit hardening is complete" — not opt-in remote until then + // Host-credential reads: no spawn, but the handler reads credential files from the host's + // home directory and imports them, so a manage-scoped key must never reach it remotely + // either. Held to the same never-bypassable bar as the spawn routes above. + "/api/oauth/kiro/auto-import", // reads kiro-cli data.sqlite3, ~/.aws/sso/cache and the Kiro profile.json ]; /** @@ -120,6 +124,7 @@ export const SPAWN_CAPABLE_PATTERN_ANCESTORS: ReadonlyArray = ["/api/pro /** * Single source of truth for "may this candidate bypass prefix ever be honoured?". + * (Covers the few non-spawn entries above too, e.g. the Kiro host-credential import.) * Returns true when `prefix` overlaps a spawn-capable route in EITHER direction: * - it equals or sits under a spawn-capable prefix (`/api/middleware/hooks`), or * - it is a parent that would cover one (`/api/`, `/api/cli-tools/`, `/api/middleware`), diff --git a/tests/unit/authz/local-only-bypass-spawn-denylist.test.ts b/tests/unit/authz/local-only-bypass-spawn-denylist.test.ts index 8d9c7115c9a1..58a5a26f3067 100644 --- a/tests/unit/authz/local-only-bypass-spawn-denylist.test.ts +++ b/tests/unit/authz/local-only-bypass-spawn-denylist.test.ts @@ -65,6 +65,9 @@ const CODE_EXEC_ROUTES: ReadonlyArray<{ prefix: string; sample: string }> = [ { prefix: "/api/providers/validate", sample: "/api/providers/validate" }, { prefix: "/api/providers/import", sample: "/api/providers/import" }, { prefix: "/api/providers/bulk", sample: "/api/providers/bulk" }, + // Not a spawn: reads host-local Kiro credential files (kiro-cli SQLite, ~/.aws/sso/cache). + // Held to the same never-bypassable bar so a manage-scoped key cannot read them remotely. + { prefix: "/api/oauth/kiro/auto-import", sample: "/api/oauth/kiro/auto-import" }, ]; function schemaRejectsBypassPrefix(prefix: string): boolean { @@ -95,7 +98,14 @@ for (const { prefix, sample } of CODE_EXEC_ROUTES) { } test("schema refuses a parent prefix that would cover a spawn-capable route", () => { - for (const parent of ["/api/", "/api/cli-tools/", "/api/acp/", "/api/middleware", "/"]) { + for (const parent of [ + "/api/", + "/api/cli-tools/", + "/api/acp/", + "/api/middleware", + "/api/oauth/kiro/", + "/", + ]) { assert.equal(schemaRejectsBypassPrefix(parent), true, parent); } }); @@ -185,7 +195,6 @@ const REVIEWED_BYPASS_ELIGIBLE = new Map([ ["/api/copilot/", "in-process LLM driver; documented opt-in remote access"], ["/api/jobs", "JobRegistry admin; runs only built-in job handlers"], ["/api/jobs/", "JobRegistry admin children"], - ["/api/oauth/kiro/auto-import", "reads host-local Kiro credential files; no spawn"], ["/api/resilience/connections", "per-account resilience state"], ]); diff --git a/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts b/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts index 0eb9f75316b6..025e09e6c75d 100644 --- a/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts +++ b/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts @@ -128,6 +128,7 @@ test("SPAWN_CAPABLE_PREFIXES is defined in the server-free constants leaf with t "/dashboard/providers/services/", "/api/discovery/", "/api/issue-agent/", + "/api/oauth/kiro/auto-import", ]) { assert.ok( SPAWN_CAPABLE_PREFIXES.includes(prefix), @@ -136,6 +137,7 @@ test("SPAWN_CAPABLE_PREFIXES is defined in the server-free constants leaf with t } // 20 at extraction time + 14 (GHSA-35fw-cv32-2373) + 2 (GHSA-jx89-f37j-pq89) + 1 (version-manager) // + 14 LOCAL_ONLY spawn / code-exec routes that had never been mirrored here. + // + 1 host-credential read (/api/oauth/kiro/auto-import) that must never be bypassable. // qwen-settings is the one pre-existing entry not enumerated above. - assert.equal(SPAWN_CAPABLE_PREFIXES.length, 51); + assert.equal(SPAWN_CAPABLE_PREFIXES.length, 52); }); diff --git a/tests/unit/settings/authz-bypass.test.ts b/tests/unit/settings/authz-bypass.test.ts index ef974fb9e349..79c4272fbfc1 100644 --- a/tests/unit/settings/authz-bypass.test.ts +++ b/tests/unit/settings/authz-bypass.test.ts @@ -353,3 +353,107 @@ test("Defence-in-depth: even if a malformed snapshot lists /api/cli-tools/runtim assert.equal(routeGuard.isLocalOnlyBypassableByManageScope("/api/cli-tools/runtime/foo"), false); }); + +// ─── Host-credential read: /api/oauth/kiro/auto-import is never bypassable ─ +// +// The Kiro auto-import route reads host-local credential files (kiro-cli +// SQLite, ~/.aws/sso/cache, Kiro profile.json). It is LOCAL_ONLY, but until it +// sat in the non-bypassable set an operator-configured manage-scope bypass list +// could expose it to a remote manage-scoped key. All three layers must refuse +// it — the exact path AND any broader entry that would cover it. + +const KIRO_AUTO_IMPORT = "/api/oauth/kiro/auto-import"; + +for (const candidate of [KIRO_AUTO_IMPORT, "/api/oauth/kiro/"]) { + test(`PATCH with ${candidate} in bypass list → 400 BYPASS_PREFIX_NOT_ALLOWED + snapshot unchanged`, async () => { + process.env.JWT_SECRET = "test-jwt-secret-authz-bypass"; + process.env.INITIAL_PASSWORD = "initial-pass-kiro"; + await settingsDb.updateSettings({ requireLogin: true }); + const { ensurePersistentManagementPasswordHash } = + await import("../../../src/lib/auth/managementPassword.ts"); + await ensurePersistentManagementPasswordHash({ source: "test.bootstrap" }); + const seeded = await settingsDb.getSettings(); + await runtime.applyRuntimeSettings(seeded); + const snapshotBefore = runtime.getAuthzBypassSnapshot(); + + const response = await settingsRoute.PATCH( + await makeManagementSessionRequest("http://localhost/api/settings", { + method: "PATCH", + body: { + localOnlyManageScopeBypassPrefixes: ["/api/mcp/", candidate], + currentPassword: "initial-pass-kiro", + }, + }) + ); + + assert.equal(response.status, 400); + const body = (await response.json()) as { + error: { details?: Array<{ field: string; message: string }> }; + }; + const offending = body.error.details?.find((d) => + d.message.includes("BYPASS_PREFIX_NOT_ALLOWED") + ); + assert.ok(offending, `expected BYPASS_PREFIX_NOT_ALLOWED in details: ${JSON.stringify(body)}`); + + const settings = await settingsDb.getSettings(); + assert.deepEqual(settings.localOnlyManageScopeBypassPrefixes, ["/api/mcp/"]); + assert.deepEqual(runtime.getAuthzBypassSnapshot().prefixes, snapshotBefore.prefixes); + }); +} + +test("stored bypass list naming the Kiro auto-import route (or a parent of it) is neutralized by normalization", async () => { + await runtime.applyRuntimeSettings({ + localOnlyManageScopeBypassEnabled: true, + localOnlyManageScopeBypassPrefixes: ["/api/mcp/", KIRO_AUTO_IMPORT, "/api/oauth/kiro/"], + }); + assert.deepEqual(runtime.getAuthzBypassSnapshot().prefixes, ["/api/mcp/"]); + assert.equal(routeGuard.isLocalOnlyBypassableByManageScope(KIRO_AUTO_IMPORT), false); +}); + +test("manage-scoped key from non-loopback cannot reach the Kiro auto-import route via the bypass list → 403 LOCAL_ONLY", async () => { + process.env.JWT_SECRET = "test-jwt-secret-authz-bypass"; + process.env.INITIAL_PASSWORD = "initial-pass-kiro-req"; + await mockSettings({ requireLogin: true }); + const managePolicy = await import("../../../src/server/authz/policies/management.ts"); + const created = await apiKeysDb.createApiKey("kiro-mgmt", "machine-kiro", ["manage"]); + const headers = new Headers({ authorization: `Bearer ${created.key}` }); + + // Control: the same key IS granted the default /api/mcp/ bypass, so the + // refusal below is the deny-list's doing, not a broken key/fixture. + const control = await managePolicy.managementPolicy.evaluate(nonLoopbackCtx(headers)); + assert.equal(control.allow, true); + + // Through the production settings pipeline (normalization layer). + await runtime.applyRuntimeSettings( + { + localOnlyManageScopeBypassEnabled: true, + localOnlyManageScopeBypassPrefixes: ["/api/mcp/", KIRO_AUTO_IMPORT], + }, + { force: true } + ); + const viaSettings = await managePolicy.managementPolicy.evaluate( + nonLoopbackCtx(headers, KIRO_AUTO_IMPORT) + ); + assert.equal(viaSettings.allow, false); + if (!viaSettings.allow) { + assert.equal(viaSettings.status, 403); + assert.equal(viaSettings.code, "LOCAL_ONLY"); + } + + // A snapshot writer that skips normalization: the request-time predicate + // must still refuse, for the exact path and for a covering parent entry. + const store = globalThis as unknown as Record; + store.__omniroute_authzBypass_config__ = { + enabled: true, + prefixes: ["/api/mcp/", KIRO_AUTO_IMPORT, "/api/oauth/kiro/"], + }; + assert.equal(routeGuard.isLocalOnlyBypassableByManageScope(KIRO_AUTO_IMPORT), false); + const viaRawSnapshot = await managePolicy.managementPolicy.evaluate( + nonLoopbackCtx(headers, KIRO_AUTO_IMPORT) + ); + assert.equal(viaRawSnapshot.allow, false); + if (!viaRawSnapshot.allow) { + assert.equal(viaRawSnapshot.status, 403); + assert.equal(viaRawSnapshot.code, "LOCAL_ONLY"); + } +});