Skip to content
Merged
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
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 4 additions & 1 deletion docs/security/ROUTE_GUARD_TIERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/shared/constants/spawnCapablePrefixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ export const SPAWN_CAPABLE_PREFIXES: ReadonlyArray<string> = [
"/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
];

/**
Expand Down Expand Up @@ -120,6 +124,7 @@ export const SPAWN_CAPABLE_PATTERN_ANCESTORS: ReadonlyArray<string> = ["/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`),
Expand Down
13 changes: 11 additions & 2 deletions tests/unit/authz/local-only-bypass-spawn-denylist.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
});
Expand Down Expand Up @@ -185,7 +195,6 @@ const REVIEWED_BYPASS_ELIGIBLE = new Map<string, string>([
["/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"],
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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);
});
104 changes: 104 additions & 0 deletions tests/unit/settings/authz-bypass.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
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");
}
});
Loading