From b17946619381fe1203cfb3a2c396b59d5ce89355 Mon Sep 17 00:00:00 2001 From: Pal Lakatos-Toth Date: Mon, 29 Jun 2026 22:04:36 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(egress):=20repair=20learn/enforce=20flo?= =?UTF-8?q?w=20=E2=80=94=20operator=20toggle=20+=20CLI=20approve/deny/enfo?= =?UTF-8?q?rce?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both bugs stem from CLI/operator code still calling router endpoints that Slice 5c.1 removed (/egress/approve, /egress/deny, /egress/enforce, /egress/pending). The authoritative model is now the KarsSandbox CRD (allowedEndpoints → controller-published, cosign-verified bundle), the EgressApproval CRD (TTL grants), and egressMode (Learn|Strict) with a live POST /egress/learn toggle. 1. Operator (TUI) could not move Strict → Learn. learnEgress called the runtime /egress/learn probe FIRST, uncaught, so a probe failure skipped the authoritative CRD patch entirely. It also sent no body (defaulting the router to enabled:false, i.e. DISABLING learn). Fix: patch the CRD egressMode first (mirrors enforceEgress), then a best-effort {enabled:true} probe wrapped in its own .catch. Added a symmetric best-effort {enabled:false} toggle to enforceEgress. 2. kars egress --approve/--deny/--enforce/--pending + the default status view hit the removed endpoints (the reported exit code 1). Re-pointed to the real mechanisms: - --approve : add host:port (default :443) to baseline allowedEndpoints + re-sign (sign-by-default). - --deny : remove host + re-sign. --deny is now in the signing context so a revocation is authoritative (was fail-open). - --enforce: patch egressMode=Strict + sign baseline; best-effort live learn-off toggle. - --pending / status: show learned-but-not-allowlisted domains. - --learn/--no-learn: durable CRD patch in k8s mode, runtime-only in Docker. Robustness: port-less baseline entries are normalized to :443 before signing (the signer requires a port and would otherwise silently drop them); approve/ deny self-heal a prior failed sign by always re-signing in a signing context; runSignFlow stays fail-CLOSED (allowlistRef patched only after a successful cosign) and now returns status so callers warn "not yet authoritative" on failure. Docker local-dev paths preserved (--approve/--deny/--enforce refuse clearly; --learn keeps the runtime toggle). Also rewrote tests/e2e-manual/scenarios/egress_lifecycle.sh to the CRD model (egressMode patch + EgressApproval create/delete) and fixed its stale spec.egress.mode field → spec.networkPolicy.egressMode. Tests: egress.test.ts +16 (parseDomainPort, unionEndpoint/removeHost incl. port-less preservation, updated signing-context error text). CLI typecheck + oxlint (0 errors) + build clean; vitest 903 pass / 2 skipped. All 7 ci-gates pass. Two rubber-duck reviews (k8s + architecture) addressed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/src/commands/egress.test.ts | 78 +++- cli/src/commands/egress.ts | 416 ++++++++++++++---- cli/src/commands/operator/actions.ts | 31 +- ...-06-29-egress-learn-enforce-flow-repair.md | 95 ++++ .../e2e-manual/scenarios/egress_lifecycle.sh | 85 ++-- 5 files changed, 581 insertions(+), 124 deletions(-) create mode 100644 docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md diff --git a/cli/src/commands/egress.test.ts b/cli/src/commands/egress.test.ts index 93e6c1794..c81f1385b 100644 --- a/cli/src/commands/egress.test.ts +++ b/cli/src/commands/egress.test.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { describe, it, expect } from "vitest"; -import { egressCommand } from "./egress.js"; +import { egressCommand, parseDomainPort, unionEndpoint, removeHost } from "./egress.js"; /** * Lightweight CLI-shape tests for the S12.c `--sign` family. The full @@ -89,7 +89,7 @@ describe("egressCommand — --sign requires --enforce or --approve", () => { } const all = logged.join("\n"); - expect(all).toMatch(/--sign requires --enforce or --approve/); + expect(all).toMatch(/--sign requires --enforce, --approve, or --deny/); expect(process.exitCode).toBe(1); process.exitCode = prevExit; }); @@ -126,7 +126,7 @@ describe("egressCommand — S12.g default-on sign + emit-manifest guards", () => } finally { cap.restore(); } - expect(cap.logs.join("\n")).toMatch(/--emit-manifest requires --enforce or --approve/); + expect(cap.logs.join("\n")).toMatch(/--emit-manifest requires --enforce, --approve, or --deny/); expect(process.exitCode).toBe(1); process.exitCode = prev; }); @@ -152,3 +152,75 @@ describe("egressCommand — S12.g default-on sign + emit-manifest guards", () => process.exitCode = prev; }); }); + +describe("parseDomainPort", () => { + it("defaults the port to 443 when omitted", () => { + expect(parseDomainPort("api.telegram.org")).toEqual({ host: "api.telegram.org", port: 443 }); + }); + it("parses an explicit host:port", () => { + expect(parseDomainPort("example.com:8443")).toEqual({ host: "example.com", port: 8443 }); + }); + it("lowercases the host", () => { + expect(parseDomainPort("API.Telegram.ORG")).toEqual({ host: "api.telegram.org", port: 443 }); + }); + it("honours a custom default port", () => { + expect(parseDomainPort("redis.internal", 6379)).toEqual({ host: "redis.internal", port: 6379 }); + }); + it("rejects an empty value", () => { + expect(() => parseDomainPort(" ")).toThrow(/must not be empty/); + }); + it("rejects a URL (scheme/path)", () => { + expect(() => parseDomainPort("https://api.telegram.org")).toThrow(/bare host/); + }); + it("rejects an out-of-range port", () => { + expect(() => parseDomainPort("example.com:70000")).toThrow(/out of range/); + }); + it("treats a non-numeric suffix after ':' as part of the host (IPv6-ish guard)", () => { + // No digits after the colon → not a port; whole string is the host. + expect(parseDomainPort("weird:name")).toEqual({ host: "weird:name", port: 443 }); + }); +}); + +describe("unionEndpoint / removeHost", () => { + const base = [ + { host: "a.example.com", port: 443 }, + { host: "b.example.com", port: 443 }, + ]; + it("adds a new endpoint and keeps the list sorted", () => { + const out = unionEndpoint(base, { host: "0.example.com", port: 443 }); + expect(out.map((e) => e.host)).toEqual(["0.example.com", "a.example.com", "b.example.com"]); + }); + it("is idempotent for an existing host+port", () => { + const out = unionEndpoint(base, { host: "a.example.com", port: 443 }); + expect(out).toHaveLength(2); + }); + it("treats a different port on the same host as a distinct endpoint", () => { + const out = unionEndpoint(base, { host: "a.example.com", port: 8443 }); + expect(out).toHaveLength(3); + expect(out.filter((e) => e.host === "a.example.com")).toHaveLength(2); + }); + it("removeHost drops every port for the host", () => { + const withTwoPorts = [...base, { host: "a.example.com", port: 8443 }]; + const out = removeHost(withTwoPorts, "a.example.com"); + expect(out.map((e) => e.host)).toEqual(["b.example.com"]); + }); + it("removeHost is case-insensitive and a no-op when absent", () => { + expect(removeHost(base, "A.EXAMPLE.COM").map((e) => e.host)).toEqual(["b.example.com"]); + expect(removeHost(base, "z.example.com")).toHaveLength(2); + }); +}); + +describe("unionEndpoint — port-less existing entries", () => { + it("treats a port-less existing host as :443 for the dedupe check", () => { + const raw = [{ host: "api.telegram.org" }]; // no port + const out = unionEndpoint(raw, { host: "api.telegram.org", port: 443 }); + // Already considered present (port-less ⇒ 443), so no duplicate is added. + expect(out).toHaveLength(1); + }); + it("preserves a port-less entry when adding a different host", () => { + const raw = [{ host: "keep.example.com" }]; + const out = unionEndpoint(raw, { host: "new.example.com", port: 443 }); + expect(out).toHaveLength(2); + expect(out.find((e) => e.host === "keep.example.com")).toEqual({ host: "keep.example.com" }); + }); +}); diff --git a/cli/src/commands/egress.ts b/cli/src/commands/egress.ts index d3f0ab066..11ec15e38 100644 --- a/cli/src/commands/egress.ts +++ b/cli/src/commands/egress.ts @@ -23,6 +23,80 @@ import { writeEmitManifest, } from "./egress/sign.js"; +/** A baseline allowlist endpoint we ADD: host plus an explicit port (the signed + * allowlist requires a concrete port — the canonical builder rejects 0). */ +export interface BaselineEndpoint { + host: string; + port: number; +} + +/** An endpoint as it appears in the live CR: the port may be absent (the CRD + * permits a port-less host even though the signer later requires one). Read + * and patched verbatim so we never silently drop an existing entry. */ +export interface RawEndpoint { + host: string; + port?: number; +} + +/** Parse a `--approve`/`--deny` domain argument of the form `host` or + * `host:port`. Defaults the port to 443 (HTTPS) when omitted — the common + * case for agent egress, and the value the signed baseline requires. Throws + * on an empty host or an out-of-range port. */ +export function parseDomainPort(raw: string, defaultPort = 443): BaselineEndpoint { + const trimmed = (raw ?? "").trim(); + if (!trimmed) throw new Error("domain must not be empty"); + // Reject a scheme (https://…) — callers pass a bare host[:port]. + if (trimmed.includes("/")) { + throw new Error(`domain must be a bare host[:port], not a URL: ${raw}`); + } + const lastColon = trimmed.lastIndexOf(":"); + let host = trimmed; + let port = defaultPort; + if (lastColon > 0) { + const maybePort = trimmed.slice(lastColon + 1); + if (/^\d+$/.test(maybePort)) { + host = trimmed.slice(0, lastColon); + port = Number(maybePort); + } + } + host = host.trim().toLowerCase(); + if (!host) throw new Error(`domain must not be empty: ${raw}`); + if (!Number.isInteger(port) || port < 1 || port > 65535) { + throw new Error(`port out of range [1,65535]: ${port}`); + } + return { host, port }; +} + +/** Add `ep` to `endpoints` if not already present (host+port match), returning + * a new sorted array that preserves every existing entry verbatim (including + * port-less ones). Pure — used to build the merge-patch payload. */ +export function unionEndpoint( + endpoints: RawEndpoint[], + ep: BaselineEndpoint, +): RawEndpoint[] { + const exists = endpoints.some( + (e) => e.host.toLowerCase() === ep.host && (e.port ?? 443) === ep.port, + ); + const next = exists ? endpoints.slice() : [...endpoints, ep]; + return sortEndpoints(next); +} + +/** Remove every entry whose host matches `host` (any port), returning a new + * sorted array. Pure. */ +export function removeHost( + endpoints: RawEndpoint[], + host: string, +): RawEndpoint[] { + const h = host.trim().toLowerCase(); + return sortEndpoints(endpoints.filter((e) => e.host.toLowerCase() !== h)); +} + +function sortEndpoints(eps: RawEndpoint[]): RawEndpoint[] { + return eps + .slice() + .sort((a, b) => (a.host < b.host ? -1 : a.host > b.host ? 1 : (a.port ?? 0) - (b.port ?? 0))); +} + export function egressCommand(): Command { const cmd = new Command("egress"); // Slice 5a — `kars egress blocked ` subcommand surfaces @@ -45,11 +119,11 @@ export function egressCommand(): Command { .option("--learn", "Enable learn mode (log all accessed domains)") .option("--no-learn", "Disable learn mode") .option("--learned", "Show domains discovered during learn mode") - .option("--pending", "Show domains pending operator approval") - .option("--approve ", "Approve a domain for egress") - .option("--deny ", "Deny and remove a pending domain request") + .option("--pending", "Show learned domains not yet in the allowlist (candidates to approve)") + .option("--approve ", "Add a domain to the sealed baseline allowlist (default port 443) and re-sign") + .option("--deny ", "Remove a domain from the baseline allowlist and re-sign") .option("--allowlist", "Show currently approved domains") - .option("--enforce", "Graduate: promote all learned domains to allowlist, switch to enforcement mode") + .option("--enforce", "Seal: switch the sandbox to Strict egress mode and sign the current baseline allowlist") .option("--status", "Show blocklist and learn mode status") .option("--sign", "Build canonical allowlist artifact, push to OCI registry, sign with cosign, patch allowlistRef. **Default-on** when combined with --enforce or --approve. Pass --no-sign to opt out.") .option("--no-sign", "Skip signing. The controller will refuse to use the artifact in authoritative mode (SignerPolicyMissing). Use only for local dev.") @@ -62,21 +136,24 @@ export function egressCommand(): Command { .action(async (name: string, options) => { const { execa } = await import("execa"); - // S12.g — sign-by-default. When the operator runs --enforce or - // --approve, signing happens automatically unless --no-sign is - // passed. options.sign is: + // S12.g — sign-by-default. When the operator mutates the baseline + // allowlist (--approve / --deny) or seals the sandbox (--enforce), + // signing happens automatically unless --no-sign is passed. options.sign: // - undefined → not specified → default to true in signing context // - true → user passed --sign explicitly // - false → user passed --no-sign - const inSigningContext = Boolean(options.enforce || options.approve); + // --deny MUST be in the signing context: a removal that is not re-signed + // leaves the old signed bundle authoritative, so the "denied" host would + // still be served (a fail-open revocation). + const inSigningContext = Boolean(options.enforce || options.approve || options.deny); const signRequested = options.sign === false ? false : (options.sign === true || inSigningContext); - // --emit-manifest implies a signing context; require --enforce or --approve. + // --emit-manifest implies a signing context; require --enforce/--approve/--deny. if (options.emitManifest && !inSigningContext) { console.log( chalk.red( - `\n --emit-manifest requires --enforce or --approve (the artifact is built from the live allowlist).\n`, + `\n --emit-manifest requires --enforce, --approve, or --deny (the artifact is built from the live allowlist).\n`, ), ); process.exitCode = 1; @@ -100,7 +177,7 @@ export function egressCommand(): Command { // Legacy guard: --sign without --enforce/--approve is still a hard // error (sign-by-default only applies inside a signing context). if (options.sign === true && !inSigningContext) { - console.log(chalk.red(`\n --sign requires --enforce or --approve.\n`)); + console.log(chalk.red(`\n --sign requires --enforce, --approve, or --deny.\n`)); process.exitCode = 1; return; } @@ -169,87 +246,179 @@ export function egressCommand(): Command { return JSON.parse(stdout); } - // Approve a domain + // The baseline-mutating + sealing operations require the KarsSandbox CRD + // and the controller's signing pipeline, neither of which exists for a + // local Docker sandbox. Refuse clearly rather than failing deep in a + // kubectl call. + if (mode === "docker" && (options.approve || options.deny || options.enforce)) { + console.log(chalk.red( + `\n --approve / --deny / --enforce operate on the KarsSandbox CRD and the signed allowlist,\n` + + ` which only exist on a Kubernetes-deployed sandbox (kars up / kars add).\n` + + ` For local Docker dev, use --learn / --learned to observe egress.\n`, + )); + return; + } + + // Approve a domain — add it to the sealed baseline allowlist and re-sign. + // Slice 5c.1 removed the in-process /egress/approve side door; the + // allowlist is now the controller-published, cosign-verified bundle built + // from KarsSandbox.spec.networkPolicy.allowedEndpoints. So "approve" means + // "add to that baseline (host+port) and re-sign". For a temporary, + // TTL-scoped grant instead, use `kars egress allow-extra`. if (options.approve) { + let ep: BaselineEndpoint; try { - const result = await routerPost("/egress/approve", { domain: options.approve }); - console.log(chalk.green(`\n ✅ Approved: ${result.domain}`)); - console.log(chalk.dim(` Domain added to egress allowlist. The agent can now reach it.\n`)); + ep = parseDomainPort(options.approve); + } catch (e: any) { + console.log(chalk.red(`\n Invalid --approve value: ${e.message}\n`)); + return; + } + try { + const crNs = await discoverKarsSandboxNamespace(name, ns); + const endpoints = await readRawAllowedEndpoints(crNs, name); + const already = endpoints.some((e) => e.host.toLowerCase() === ep.host && (e.port ?? 443) === ep.port); + const hasPortless = endpoints.some((e) => e.port === undefined); + const next = unionEndpoint(endpoints, ep); + // Patch when we add a new endpoint OR when there are port-less entries + // to normalize (so the about-to-run sign step doesn't silently drop + // them). If neither, the baseline is already correct and we fall + // straight through to re-signing (self-healing a prior failed sign). + if (!already || hasPortless) { + const { normalized } = await patchBaselineEndpoints(crNs, name, next); + console.log(already + ? chalk.dim(`\n ${ep.host}:${ep.port} already approved for '${name}'.`) + : chalk.green(`\n ✅ Approved: ${ep.host}:${ep.port}`)); + if (!already) console.log(chalk.dim(` Added to the baseline allowlist (${next.length} endpoint(s) total).`)); + if (normalized > 0) { + console.log(chalk.dim(` (${normalized} existing port-less entr${normalized === 1 ? "y" : "ies"} defaulted to :443 so the signed baseline stays valid.)`)); + } + } else { + console.log(chalk.dim(`\n ${ep.host}:${ep.port} is already in the baseline allowlist for '${name}' — re-signing to confirm.`)); + } } catch (e: any) { console.log(chalk.red(`\n Failed to approve: ${e.message}\n`)); return; } if (signRequested) { - await runSignFlow(name, ns, options); + const ok = await runSignFlow(name, ns, options); + if (!ok) { + console.log(chalk.yellow(` ⚠ The baseline was updated but signing did NOT complete — the previously-signed allowlist is still authoritative. Re-run \`kars egress ${name} --approve ${options.approve}\` to finish.\n`)); + } + } else { + console.log(chalk.yellow(`\n ⚠ --no-sign: the controller will not serve this change in authoritative mode until a signed bundle is published.\n`)); } return; } - // Deny a domain + // Deny a domain — remove it from the baseline allowlist and re-sign. if (options.deny) { + let host: string; + try { + host = parseDomainPort(options.deny).host; + } catch (e: any) { + console.log(chalk.red(`\n Invalid --deny value: ${e.message}\n`)); + return; + } try { - const result = await routerPost("/egress/deny", { domain: options.deny }); - console.log(chalk.yellow(`\n ❌ Denied: ${result.domain}\n`)); + const crNs = await discoverKarsSandboxNamespace(name, ns); + const endpoints = await readRawAllowedEndpoints(crNs, name); + const next = removeHost(endpoints, host); + const changed = next.length !== endpoints.length; + const hasPortless = endpoints.some((e) => e.port === undefined); + if (changed || hasPortless) { + await patchBaselineEndpoints(crNs, name, next); + } + if (changed) { + console.log(chalk.yellow(`\n ❌ Denied: ${host}`)); + console.log(chalk.dim(` Removed from the baseline allowlist (${next.length} endpoint(s) remain).`)); + } else { + // Not in the inline baseline. Still re-sign so a previously-removed + // host whose revocation never got signed is reconciled now (the + // signed bundle is the authoritative artifact, not the inline list). + console.log(chalk.dim(`\n ${host} is not in the baseline allowlist for '${name}' — re-signing the current baseline to ensure the revocation is authoritative.`)); + } } catch (e: any) { console.log(chalk.red(`\n Failed to deny: ${e.message}\n`)); + return; + } + if (signRequested) { + const ok = await runSignFlow(name, ns, options); + if (!ok) { + console.log(chalk.yellow(` ⚠ The host was removed from the baseline but signing did NOT complete — the previously-signed bundle may still allow '${host}'. Re-run \`kars egress ${name} --deny ${host}\` to finish the revocation.\n`)); + } + } else { + console.log(chalk.yellow(`\n ⚠ --no-sign: the removal is NOT yet authoritative — the previously-signed bundle still allows '${host}'. Re-run with --sign (default) to revoke it.\n`)); } return; } - // Enforce: graduate from learn mode to enforcement + // Enforce: seal the sandbox — switch egress to Strict mode and sign the + // current baseline. Slice 5c.1 removed /egress/enforce; the authoritative + // path is the CRD `egressMode` field plus the signed allowlist bundle. if (options.enforce) { try { - const result = await routerPost("/egress/enforce", {}); - if (result.status === "already_enforcing") { - console.log(chalk.yellow(`\n Already in enforcement mode.`)); - console.log(chalk.dim(` Allowlist: ${result.allowlist_count} domain(s)\n`)); - } else { - console.log(chalk.green(`\n 🔒 Enforcement mode activated for '${name}'`)); - console.log(chalk.dim(` ${result.promoted} learned domain(s) promoted to allowlist`)); - console.log(chalk.dim(` ${result.allowlist_count} total domain(s) in allowlist\n`)); - if (result.allowlist && result.allowlist.length > 0) { - for (const domain of result.allowlist) { - console.log(` ${chalk.green("✓")} ${domain}`); - } - console.log(); + const crNs = await discoverKarsSandboxNamespace(name, ns); + await patchEgressMode(crNs, name, "Strict"); + // Normalize any port-less baseline entries to :443 BEFORE signing so + // the signer (which requires a port) doesn't drop them. + const endpoints = await readRawAllowedEndpoints(crNs, name); + if (endpoints.some((e) => e.port === undefined)) { + const { normalized } = await patchBaselineEndpoints(crNs, name, endpoints); + if (normalized > 0) { + console.log(chalk.dim(` (${normalized} port-less baseline entr${normalized === 1 ? "y" : "ies"} defaulted to :443 for signing.)`)); } - console.log(chalk.dim(` Learn mode is now OFF. Only allowlisted domains will pass.`)); - console.log(chalk.dim(` New domains will go to pending approval.\n`)); - console.log(chalk.dim(` Commands:`)); - console.log(chalk.dim(` kars egress ${name} --pending Show pending requests`)); - console.log(chalk.dim(` kars egress ${name} --approve Approve a new domain`)); - console.log(chalk.dim(` kars egress ${name} --learn Re-enable learn mode\n`)); } + // Best-effort live toggle so Strict takes effect without waiting for a + // pod roll; never let a probe failure block the authoritative patch. + if (mode === "k8s" && pod) { + await routerPost("/egress/learn", { enabled: false }).catch(() => {}); + } + console.log(chalk.green(`\n 🔒 Enforcement (Strict) mode set for '${name}'`)); + console.log(chalk.dim(` Only allowlisted host:port pairs will pass; the blocklist still applies.`)); + console.log(chalk.dim(` (The controller may roll the sandbox pod to apply the new mode.)\n`)); } catch (e: any) { console.log(chalk.red(`\n Failed to enforce: ${e.message}\n`)); return; } if (signRequested) { - await runSignFlow(name, ns, options); + const ok = await runSignFlow(name, ns, options); + if (!ok) { + console.log(chalk.yellow(` ⚠ Strict mode is set but signing did NOT complete — publish a signed bundle (re-run \`kars egress ${name} --enforce\`) or the controller will refuse the allowlist in authoritative mode.\n`)); + } + } else { + console.log(chalk.yellow(` ⚠ --no-sign: publish a signed bundle (re-run with --sign) before the controller will serve the allowlist in authoritative mode.\n`)); } + console.log(chalk.dim(` Next:`)); + console.log(chalk.dim(` kars egress ${name} --approve Add a domain to the baseline + re-sign`)); + console.log(chalk.dim(` kars egress allow-extra ${name} --host --ttl PT4H --reason "" Temporary grant`)); + console.log(chalk.dim(` kars egress ${name} --learn Re-open learn mode\n`)); return; } - // Show pending approvals + // Show learned domains not yet in the allowlist — the closest analogue to + // the removed in-memory "pending" queue: domains the agent has tried to + // reach in learn mode that aren't approved yet. if (options.pending) { try { - const data = await routerGet("/egress/pending"); - console.log(chalk.hex("#0078D4")(`\n Pending Egress Approvals for '${name}'`)); - if (data.pending && data.pending.length > 0) { - console.log(); - for (const p of data.pending) { - console.log(` ${chalk.yellow("⏳")} ${chalk.white(p.domain)}`); - console.log(chalk.dim(` URL: ${p.url}`)); - console.log(chalk.dim(` Time: ${p.timestamp}`)); - console.log(chalk.dim(` Approve: kars egress ${name} --approve ${p.domain}`)); - console.log(); + const [allow, learned] = await Promise.all([ + routerGet("/egress/allowlist").catch(() => ({ domains: [] as string[] })), + routerGet("/egress/learned").catch(() => ({ domains: [] as string[], learn_mode: false })), + ]); + const approved = new Set((allow.domains ?? []).map((d: string) => d.split(":")[0].toLowerCase())); + const candidates = (learned.domains ?? []).filter((d: string) => !approved.has(d.split(":")[0].toLowerCase())); + console.log(chalk.hex("#0078D4")(`\n Egress candidates for '${name}' (learned, not yet approved)`)); + console.log(chalk.dim(` Learn mode: ${learned.learn_mode ? "ON" : "off"}\n`)); + if (candidates.length > 0) { + for (const d of candidates.sort()) { + console.log(` ${chalk.yellow("⏳")} ${d}`); + console.log(chalk.dim(` Approve: kars egress ${name} --approve ${d.split(":")[0]}`)); } - console.log(chalk.dim(` ${data.count} domain(s) pending approval.\n`)); + console.log(chalk.dim(`\n ${candidates.length} candidate(s).\n`)); } else { - console.log(chalk.dim(`\n No pending requests.\n`)); + console.log(chalk.dim(` None. Enable learn mode and exercise the agent to discover endpoints.\n`)); } } catch (e: any) { - console.log(chalk.red(`\n Failed to query pending: ${e.message}\n`)); + console.log(chalk.red(`\n Failed to query candidates: ${e.message}\n`)); } return; } @@ -274,10 +443,19 @@ export function egressCommand(): Command { return; } - // Enable learn mode + // Enable learn mode. In Kubernetes this is durable: patch the CRD + // egressMode (authoritative, survives a pod roll) then best-effort live + // toggle. In local Docker dev there is no KarsSandbox CR, so fall back to + // the runtime-only toggle (original behaviour). if (options.learn === true) { try { - await routerPost("/egress/learn", { enabled: true }); + if (mode === "k8s") { + const crNs = await discoverKarsSandboxNamespace(name, ns); + await patchEgressMode(crNs, name, "Learn"); + await routerPost("/egress/learn", { enabled: true }).catch(() => {}); + } else { + await routerPost("/egress/learn", { enabled: true }); + } console.log(chalk.green(`\n ✅ Learn mode enabled for '${name}'.`)); console.log(chalk.dim(` All accessed domains will be logged (blocklist still enforced).`)); console.log(chalk.dim(` Run ${chalk.white(`kars egress ${name} --learned`)} to see discovered domains.\n`)); @@ -287,11 +465,20 @@ export function egressCommand(): Command { return; } - // Disable learn mode + // Disable learn mode. In Kubernetes: switch the CRD to Strict (the only + // non-learn mode) then best-effort live toggle. In Docker: runtime-only. if (options.learn === false && process.argv.includes("--no-learn")) { try { - await routerPost("/egress/learn", { enabled: false }); - console.log(chalk.yellow(`\n Learn mode disabled for '${name}'.\n`)); + if (mode === "k8s") { + const crNs = await discoverKarsSandboxNamespace(name, ns); + await patchEgressMode(crNs, name, "Strict"); + await routerPost("/egress/learn", { enabled: false }).catch(() => {}); + console.log(chalk.yellow(`\n Learn mode disabled for '${name}' (egress mode is now Strict).`)); + console.log(chalk.dim(` Only allowlisted host:port pairs will pass. Seal the baseline with ${chalk.white(`kars egress ${name} --enforce`)} to (re)sign it.\n`)); + } else { + await routerPost("/egress/learn", { enabled: false }); + console.log(chalk.yellow(`\n Learn mode disabled for '${name}'.\n`)); + } } catch (e: any) { console.log(chalk.red(`\n Failed to disable learn mode: ${e.message}\n`)); } @@ -320,43 +507,41 @@ export function egressCommand(): Command { // Default: show status try { - const [blStatus, allowlist, pending, learned] = await Promise.all([ + const [blStatus, allowlist, learned] = await Promise.all([ routerGet("/blocklist/status"), routerGet("/egress/allowlist"), - routerGet("/egress/pending"), - routerGet("/egress/learned").catch(() => ({ count: 0, domains: [] })), + routerGet("/egress/learned").catch(() => ({ count: 0, domains: [], learn_mode: false })), ]); + // "Pending" no longer exists as an in-router queue (Slice 5c.1). The + // useful analogue is learned domains not yet in the allowlist. + const approved = new Set((allowlist.domains ?? []).map((d: string) => d.split(":")[0].toLowerCase())); + const candidates = (learned.domains ?? []).filter((d: string) => !approved.has(d.split(":")[0].toLowerCase())); console.log(chalk.hex("#0078D4")(`\n Egress Security — '${name}'`)); console.log(` Blocklist: ${blStatus.enabled ? chalk.green("enabled") : chalk.red("disabled")} (${blStatus.domain_count.toLocaleString()} domains)`); console.log(` Learn mode: ${blStatus.learn_mode ? chalk.green("ON") : chalk.dim("off")}`); console.log(` Allowlist: ${chalk.white(allowlist.count)} domain(s) approved`); - console.log(` Pending: ${pending.count > 0 ? chalk.yellow(pending.count + " awaiting approval") : chalk.dim("none")}`); + console.log(` Candidates: ${candidates.length > 0 ? chalk.yellow(candidates.length + " learned, not yet approved") : chalk.dim("none")}`); if (learned.count > 0) { console.log(` Learned: ${chalk.cyan(learned.count)} domain(s) discovered`); } console.log(); - if (pending.count > 0) { - for (const p of pending.pending) { - console.log(` ${chalk.yellow("⏳")} ${p.domain}`); - } - console.log(); - } - if (learned.count > 0 && blStatus.learn_mode) { - console.log(chalk.dim(` Discovered domains (learn mode):`)); - for (const d of learned.domains) { + if (candidates.length > 0 && blStatus.learn_mode) { + console.log(chalk.dim(` Discovered domains not yet approved (learn mode):`)); + for (const d of candidates.sort()) { console.log(` ${chalk.cyan("◉")} ${d}`); } console.log(); - console.log(chalk.hex("#0078D4")(` → Ready to enforce? Run: ${chalk.white(`kars egress ${name} --enforce`)}`)); - console.log(chalk.dim(` This promotes ${learned.count} learned domain(s) to allowlist and activates enforcement.\n`)); + console.log(chalk.hex("#0078D4")(` → Approve them, then seal: ${chalk.white(`kars egress ${name} --approve `)} … then ${chalk.white(`kars egress ${name} --enforce`)}`)); + console.log(); } console.log(chalk.dim(` Commands:`)); - console.log(chalk.dim(` kars egress ${name} --enforce Promote learned → allowlist, enforce`)); - console.log(chalk.dim(` kars egress ${name} --pending Show pending requests`)); - console.log(chalk.dim(` kars egress ${name} --approve Approve a domain`)); - console.log(chalk.dim(` kars egress ${name} --deny Deny a domain`)); - console.log(chalk.dim(` kars egress ${name} --allowlist Show approved domains`)); - console.log(chalk.dim(` kars egress ${name} --learned Show discovered domains`)); + console.log(chalk.dim(` kars egress ${name} --approve Add to baseline allowlist + re-sign (default :443)`)); + console.log(chalk.dim(` kars egress ${name} --deny Remove from baseline + re-sign`)); + console.log(chalk.dim(` kars egress ${name} --enforce Seal: Strict mode + sign baseline`)); + console.log(chalk.dim(` kars egress ${name} --pending Show learned, not-yet-approved domains`)); + console.log(chalk.dim(` kars egress ${name} --allowlist Show approved domains`)); + console.log(chalk.dim(` kars egress ${name} --learned Show discovered domains`)); + console.log(chalk.dim(` kars egress allow-extra ${name} --host --ttl PT4H --reason "" Temporary TTL grant`)); console.log(); } catch (e: any) { console.log(chalk.red(`\n Failed to query status: ${e.message}\n`)); @@ -375,7 +560,7 @@ async function runSignFlow( name: string, ns: string, options: any, -): Promise { +): Promise { const headerSlice = options.emitManifest ? "GitOps mode" : "sign-by-default"; console.log(chalk.hex("#0078D4")(`\n Signing egress allowlist artifact for '${name}' (${headerSlice})`)); try { @@ -507,7 +692,7 @@ async function runSignFlow( ), ); console.log(); - return; + return true; } await patchKarsSandbox({ @@ -521,9 +706,11 @@ async function runSignFlow( }); console.log(chalk.green(` ✅ Patched spec.networkPolicy.allowlistRef`)); console.log(chalk.dim(`\n The controller will verify the artifact and program NetworkPolicy egress on next reconcile (authoritative mode).\n`)); + return true; } catch (e: any) { console.log(chalk.red(`\n Signing aborted: ${e.message}\n`)); process.exitCode = 1; + return false; } } @@ -589,6 +776,71 @@ async function discoverRegistry(): Promise { * for unusual setups). Surfaces a clear error rather than letting * a downstream kubectl call fail with a confusing 'not found'. */ +async function readRawAllowedEndpoints( + crNamespace: string, + name: string, +): Promise { + const { execa } = await import("execa"); + const { stdout } = await execa("kubectl", [ + "get", `karssandbox/${name}`, "-n", crNamespace, "-o", "json", + ], { stdio: "pipe" }); + const obj = JSON.parse(stdout || "{}"); + const raw: unknown = obj?.spec?.networkPolicy?.allowedEndpoints; + const out: RawEndpoint[] = []; + if (Array.isArray(raw)) { + for (const ep of raw) { + const e = ep as { host?: unknown; port?: unknown }; + if (typeof e.host === "string" && e.host) { + out.push(typeof e.port === "number" ? { host: e.host, port: e.port } : { host: e.host }); + } + } + } + return out; +} + +async function patchBaselineEndpoints( + crNamespace: string, + name: string, + endpoints: RawEndpoint[], +): Promise<{ normalized: number }> { + const { execa } = await import("execa"); + // The signed allowlist requires an explicit port on every entry (the + // canonical builder rejects port-less), and `readKarsSandboxState` (used by + // the sign flow) silently drops port-less entries. To keep the persisted + // baseline sign-compatible AND avoid silently losing an existing host on the + // next re-sign, normalize any port-less entry to :443 here. + let normalized = 0; + const withPorts = endpoints.map((e) => { + if (e.port === undefined) { + normalized += 1; + return { host: e.host, port: 443 }; + } + return e; + }); + // JSON merge patch: a list value replaces the existing list wholesale, so we + // pass the full desired union/remainder. Other networkPolicy fields + // (egressMode, allowlistRef) are preserved by the merge. + const patch = { spec: { networkPolicy: { allowedEndpoints: withPorts } } }; + await execa("kubectl", [ + "patch", `karssandbox/${name}`, "-n", crNamespace, + "--type", "merge", "-p", JSON.stringify(patch), + ], { stdio: "pipe" }); + return { normalized }; +} + +async function patchEgressMode( + crNamespace: string, + name: string, + mode: "Strict" | "Learn", +): Promise { + const { execa } = await import("execa"); + const patch = { spec: { networkPolicy: { egressMode: mode } } }; + await execa("kubectl", [ + "patch", `karssandbox/${name}`, "-n", crNamespace, + "--type", "merge", "-p", JSON.stringify(patch), + ], { stdio: "pipe" }); +} + async function discoverKarsSandboxNamespace(name: string, podNs: string): Promise { const { execa } = await import("execa"); // 1) Operator default — covers >99% of installs. diff --git a/cli/src/commands/operator/actions.ts b/cli/src/commands/operator/actions.ts index 2f10a1578..ab13043f2 100644 --- a/cli/src/commands/operator/actions.ts +++ b/cli/src/commands/operator/actions.ts @@ -42,6 +42,16 @@ export function createActions(ctx: ActionContext): OperatorActions { "--type", "merge", "-p", JSON.stringify({ spec: { networkPolicy: { egressMode: "Strict" } } }), ], kubeContext), { stdio: "pipe" }); + // Best-effort live toggle so Strict takes effect without waiting for the + // controller to roll the pod. Never let a probe failure (missing admin + // token, older router) surface as an "enforce failed" — the CRD patch + // above is the authoritative source of truth. + await execa("kubectl", kctl([ + "exec", "-n", sb.namespace, sb.podName, + "-c", "inference-router", "--", + "/usr/local/bin/kars-inference-router", "probe", "POST", "/egress/learn", + JSON.stringify({ enabled: false }), + ], kubeContext), { stdio: "pipe" }).catch(() => {}); activityLog.log(`{green-fg}🔒 Enforced{/} ${sb.name}`); activityLog.log(`{gray-fg} ↳ saved to CRD — may trigger pod restart{/}`); } catch (e: any) { @@ -52,15 +62,26 @@ export function createActions(ctx: ActionContext): OperatorActions { async function learnEgress(sb: SandboxInfo): Promise { if (!sb.podName) return; try { - await execa("kubectl", kctl([ - "exec", "-n", sb.namespace, sb.podName, - "-c", "inference-router", "--", - "/usr/local/bin/kars-inference-router", "probe", "POST", "/egress/learn", - ], kubeContext), { stdio: "pipe" }); + // Authoritative: the CRD `egressMode` drives the router's EGRESS_MODE on + // the next reconcile. Patch it FIRST (mirrors enforceEgress, which only + // patches the CRD) so the mode change is durable even if the live toggle + // below can't reach the router. await execa("kubectl", kctl([ "patch", "karssandbox", sb.name, "-n", "kars-system", "--type", "merge", "-p", JSON.stringify({ spec: { networkPolicy: { egressMode: "Learn" } } }), + ], kubeContext), { stdio: "pipe" }); + // Best-effort live toggle so learn mode takes effect immediately without + // waiting for a pod restart. MUST send {enabled:true} (an empty body + // defaults the router to enabled:false, i.e. it would DISABLE learn). + // Wrapped in its own catch so a probe failure can never block or fail the + // authoritative CRD patch above — the root cause of the prior + // "cannot move Strict → Learn" error. + await execa("kubectl", kctl([ + "exec", "-n", sb.namespace, sb.podName, + "-c", "inference-router", "--", + "/usr/local/bin/kars-inference-router", "probe", "POST", "/egress/learn", + JSON.stringify({ enabled: true }), ], kubeContext), { stdio: "pipe" }).catch(() => {}); activityLog.log(`{yellow-fg}📖 Learning{/} ${sb.name}`); activityLog.log(`{gray-fg} ↳ saved to CRD — may trigger pod restart{/}`); diff --git a/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md b/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md new file mode 100644 index 000000000..800e9850d --- /dev/null +++ b/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md @@ -0,0 +1,95 @@ +# Security Audit — Egress learn/enforce flow repair (operator toggle + CLI approve/deny/enforce) + +Date: 2026-06-29 +Scope: `cli/src/commands/egress.ts`, `cli/src/commands/operator/actions.ts`, `cli/src/commands/egress.test.ts`, `tests/e2e-manual/scenarios/egress_lifecycle.sh`. +Gated paths: `cli/src/commands/egress.ts`, `cli/src/commands/operator/actions.ts`. + +## Summary + +Two operator-reported bugs in the egress learning/enforcement flow, both caused +by CLI/operator code still calling router endpoints that **Slice 5c.1 removed** +(`/egress/approve`, `/egress/deny`, `/egress/enforce`, `/egress/pending`). The +authoritative model is now: baseline allowlist = `KarsSandbox.spec.networkPolicy. +allowedEndpoints` compiled into a controller-published, cosign-verified bundle +(`allowlistRef`); temporary grants = `EgressApproval` CRs; `egressMode` +(Learn|Strict) drives the router via the `EGRESS_MODE` env var with a live +`POST /egress/learn {enabled}` toggle. + +1. **Operator (TUI) could not move Strict → Learn.** `learnEgress` called the + runtime `/egress/learn` probe FIRST, uncaught, so if it threw the authoritative + CRD patch was skipped. Fix: patch the CRD `egressMode` first (mirrors the + working `enforceEgress`), then a best-effort `{enabled:true}` probe in its own + `.catch`. (The old call also sent no body, defaulting the router to + `enabled:false`, i.e. it DISABLED learn even when it ran.) Added a symmetric + best-effort `{enabled:false}` toggle to `enforceEgress`. + +2. **`kars egress --approve/--deny/--enforce/--pending` + the default status view + hit removed endpoints** (the reported `exit code 1`). Re-pointed to the real + mechanisms: + - `--approve ` adds `host:port` (default **:443**) to the + baseline `allowedEndpoints` and re-signs (sign-by-default). + - `--deny ` removes the host and re-signs. **`--deny` is now in the + signing context** so a revocation actually updates the authoritative bundle + (otherwise the old signed allowlist would keep serving the host — a fail-open + revocation). + - `--enforce` patches `egressMode=Strict` and signs the baseline. + - `--pending` and the status view show learned-but-not-allowlisted domains. + +## Security analysis + +### T1: New capability / attack surface? (NO) +- No new endpoint/route/privilege. All mutations go through `kubectl patch`/`apply` + against existing CRDs (`KarsSandbox`, `EgressApproval`) and the existing + cosign-signing pipeline (`runSignFlow`). Reads are `kubectl get` + the existing + in-pod router probe. Agents hold no new capability; this is operator tooling. + +### T2: Security-control change? (STRENGTHENED / NEUTRAL) +- The signed-allowlist control is unchanged and now correctly driven from the CLI: + - `--deny` re-signs by default → revocation is authoritative (was fail-open). + - `runSignFlow` remains fail-CLOSED: `allowlistRef` is patched ONLY after a + successful cosign sign; on any failure the previous signed bundle stays + authoritative and the CLI warns + exits non-zero. + - Port-less baseline entries are normalized to `:443` before signing so the + signer (which requires a concrete port) cannot silently drop an existing + allowed host from the re-signed bundle (data-loss / accidental-deny guard). +- `egressMode` is patched on the CRD (durable, authoritative); the live + `/egress/learn` probe is now strictly best-effort and can never block or revert + the CRD source of truth. + +### T3: Availability / fail-open risk? (REDUCED) +- Operator learn toggle no longer wedges on a probe error. CLI approve/deny/enforce + are self-healing: in a signing context they always re-sign the current baseline, + so a prior run that patched the inline list but failed to sign is reconciled on + re-run (the failure is surfaced with an explicit "not yet authoritative" warning). +- Local Docker dev is preserved: `--approve/--deny/--enforce` (which need the CRD + + signer) refuse clearly in Docker mode; `--learn/--learned` keep the runtime-only + path. The merge-patch replaces only `allowedEndpoints`, preserving sibling + `networkPolicy` fields (`egressMode`, `allowlistRef`). + +### Anti-affordances preserved +- The router's removal of the in-process approval side door is respected — the CLI + never re-introduces a runtime "approve this host" mutation; approval is a signed + CRD change. No call to a removed endpoint remains in the CLI/operator paths. + +## Verification +- CLI typecheck (`tsc --noEmit`) + `oxlint` (0 errors; 29 pre-existing warnings, + none in the changed files) + `npm run build` clean. +- `vitest`: 903 pass / 2 skipped (49 files). `egress.test.ts` gains coverage for + `parseDomainPort` (default :443, host:port, URL/port-range rejection), + `unionEndpoint`/`removeHost` (idempotency, distinct-port, port-less preservation + + :443-equivalence), and the updated signing-context error text. +- `tests/e2e-manual/scenarios/egress_lifecycle.sh` rewritten to the CRD model + (egressMode patch + EgressApproval create/delete); `bash -n` + `shellcheck -S + error` clean. +- Two independent rubber-duck reviews (k8s + kars-architecture lens); all + blocking findings (deny-not-signed fail-open, enforce not live-disabling learn, + port-less drop, Docker-mode regression, manual-E2E dead endpoints) addressed. + +## Verdict +Accept. Repairs a broken security workflow by routing CLI/operator actions through +the authoritative CRD + signed-allowlist pipeline; strengthens revocation +(deny re-signs), keeps signing fail-closed, and removes the last callers of the +deleted in-router approval endpoints. No security control weakened. + +Signed-off-by: Pal Lakatos-Toth +Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com> diff --git a/tests/e2e-manual/scenarios/egress_lifecycle.sh b/tests/e2e-manual/scenarios/egress_lifecycle.sh index c46420f13..e73b48372 100755 --- a/tests/e2e-manual/scenarios/egress_lifecycle.sh +++ b/tests/e2e-manual/scenarios/egress_lifecycle.sh @@ -9,18 +9,20 @@ # [1/4] learn — sandbox in learn mode records outbound domains # when the agent (here: an exec-driven curl) # touches them. Probe via /egress/learned. -# [2/4] enforce — switch to enforce mode via /egress/enforce. -# Probe a previously-unseen domain → expect -# block (NetworkPolicy denies, curl times out). -# [3/4] approve — POST /egress/approve to allowlist a new domain, -# re-probe → expect success. -# [4/4] deny — POST /egress/deny to block a learned domain, -# re-probe → expect block. +# [2/4] enforce — switch to Strict via the KarsSandbox CRD +# (spec.networkPolicy.egressMode). Probe a +# previously-unseen domain → expect block +# (NetworkPolicy denies, curl times out). +# [3/4] approve — grant example.org via an EgressApproval CR +# (the runtime widening mechanism), re-probe → +# expect success. +# [4/4] deny — delete the EgressApproval CR, re-probe → expect +# block. # -# We never invoke `kars egress … --sign` here — the cosign+ACR -# signing path has its own integration coverage in the controller crate -# and would require a writable ACR + cosign keys for E2E. This -# scenario validates the in-cluster control plane only. +# Slice 5c.1 removed the in-router /egress/approve|deny|enforce|pending +# endpoints; the allowlist is now driven by the KarsSandbox CRD (baseline +# + signed bundle) and EgressApproval CRs (TTL-scoped grants). This +# scenario exercises the CRD control plane only (no cosign/ACR signing). set -euo pipefail @@ -46,7 +48,7 @@ metric_start "admit_${name}" cr_dispatch openclaw "$name" "$ns" \ | yq eval ' select(.kind == "KarsSandbox") - | .spec.egress.mode = "learn" + | .spec.networkPolicy.egressMode = "Learn" , select(.kind != "KarsSandbox") ' - \ @@ -129,51 +131,66 @@ else fi # ── [2/4] enforce ───────────────────────────────────────────────────── -log_step "[2/4] enforce: graduate to enforce, probe an unseen domain → expect block" +log_step "[2/4] enforce: switch the KarsSandbox to Strict, probe an unseen domain → expect block" metric_start "egress_enforce_switch" -code=$(router_curl POST "/egress/enforce") -metric_finish "egress_enforce_switch" egress_lifecycle enforceSwitchLatency -if [[ "$code" != "200" && "$code" != "204" ]]; then - log_skip "/egress/enforce returned ${code} — endpoint may not be wired; remaining steps depend on it" +if ! kubectl patch karssandbox "$name" -n "$ns" --type merge \ + -p '{"spec":{"networkPolicy":{"egressMode":"Strict"}}}' >/dev/null 2>&1; then + log_skip "could not patch egressMode=Strict on KarsSandbox/${name} — remaining steps depend on it" scenario_summary "Egress allowlist lifecycle" exit 0 fi -sleep 2 +# Best-effort live toggle so Strict applies without waiting for a pod roll. +_=$(router_curl POST "/egress/learn" '{"enabled":false}') +metric_finish "egress_enforce_switch" egress_lifecycle enforceSwitchLatency +sleep 3 # example.org should not have been seen during learn — should now be blocked. http=$(agent_curl "https://example.org" 6) if [[ "$http" == "000" || -z "$http" ]]; then - log_pass "previously-unseen domain (example.org) blocked under enforce" + log_pass "previously-unseen domain (example.org) blocked under Strict" else log_skip "example.org returned HTTP ${http} — egress NetworkPolicy may not have refreshed yet" fi # ── [3/4] approve ───────────────────────────────────────────────────── -log_step "[3/4] approve: allowlist example.org via /egress/approve, re-probe" -code=$(router_curl POST "/egress/approve" '{"domain":"example.org"}') -if [[ "$code" != "200" && "$code" != "204" ]]; then - log_skip "/egress/approve returned ${code} — endpoint may not be wired" +log_step "[3/4] approve: grant example.org via an EgressApproval CR, re-probe" +appr="${name}-allow-exampleorg" +if ! kubectl apply -f - >/dev/null 2>&1 </dev/null 2>&1; then + log_skip "could not delete EgressApproval/${appr} — may not have been created" else - sleep 5 - http=$(agent_curl "https://example.com" 6) + sleep 6 + http=$(agent_curl "https://example.org" 6) if [[ "$http" == "000" || -z "$http" ]]; then - log_pass "deny revoked example.com (probe blocked)" + log_pass "revoking the EgressApproval blocked example.org again" else - log_skip "deny returned 200 but probe got HTTP=${http} — propagation lag" + log_skip "EgressApproval deleted but probe got HTTP=${http} — propagation lag" fi fi From 8f9ad129ddcfe0b54722bff34fd45e7926be31b0 Mon Sep 17 00:00:00 2001 From: Pal Lakatos-Toth Date: Mon, 29 Jun 2026 23:07:35 +0200 Subject: [PATCH 2/3] fix(egress): clean up remaining CRD-move leftovers (router 403 guidance + docs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up sweep after the learn/enforce flow repair found more references to the removed Slice 5c.1 endpoints — none were live callers, but several gave users stale instructions: - inference-router/src/routes/egress.rs: the /egress/fetch 403 response told agents to run the REMOVED `kars egress --pending`/`--approve` workflow. Now points to the real remediation (operator `--approve` re-sign, or a temporary EgressApproval via `allow-extra`). Also fixed the handler's doc comment ("create pending approval request" → deny in Strict / log in Learn; no queue). - inference-router/src/routes/internal.rs: corrected a stale comment about the `/egress/learned/blocked` "old shape" / --pending workflow. - cli/src/commands/operator/dialogs/egress.ts: legend "learned (pending approval)" → "learned (not yet approved)"; status label `pending=` → `learned=` (the `signed-pending` cosign-verification state is unrelated and unchanged). - docs/egress-proxy.md: rewrote the lifecycle diagram, operator workflow, CLI table, and API-endpoints table (removed /egress/approve|deny|pending|enforce, documented the CRD + signed-bundle + EgressApproval model and :443 default). - docs/cli-reference.md: fixed the egress section — `--enforce` is "Strict + sign", NOT "promote all learned"; corrected --approve/--deny/--pending descriptions; added allow-extra. - docs/operations/gitops.md: error-text now lists --deny too. Verified `learn_egress` (spawn/handoff) is CURRENT, not stale — spawn reads networkPolicy.egressMode ↔ maps it back, so it's the internal bool form of the CRD field. check_egress already returns the correct new-model deny message. Router compiles; 43 egress router tests pass; CLI typecheck + oxlint (0 errors) + 903 vitest pass; all 7 ci-gates pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/src/commands/operator/dialogs/egress.ts | 4 +- docs/cli-reference.md | 37 ++++--- docs/egress-proxy.md | 99 ++++++++++++------- docs/operations/gitops.md | 2 +- ...-06-29-egress-learn-enforce-flow-repair.md | 15 ++- inference-router/src/routes/egress.rs | 8 +- inference-router/src/routes/internal.rs | 4 +- 7 files changed, 107 insertions(+), 62 deletions(-) diff --git a/cli/src/commands/operator/dialogs/egress.ts b/cli/src/commands/operator/dialogs/egress.ts index 81c8469d2..ff59fe40c 100644 --- a/cli/src/commands/operator/dialogs/egress.ts +++ b/cli/src/commands/operator/dialogs/egress.ts @@ -164,7 +164,7 @@ export function openEgressDrawer(ctx: EgressDrawerContext): void { tags: true, style: { fg: "white", bg: "black" }, content: ` Fleet egress at a glance. Each row is one sandbox.\n` + - ` L = learned (pending approval) A = allowlist (approved) sign = live spec.networkPolicy.allowlistRef + AllowlistAuthoritative`, + ` L = learned (not yet approved) A = allowlist (approved) sign = live spec.networkPolicy.allowlistRef + AllowlistAuthoritative`, }); blessed.box({ @@ -208,7 +208,7 @@ export function openEgressDrawer(ctx: EgressDrawerContext): void { cur.signed === "unsigned" ? `{gray-fg}unsigned{/} (inline allowedEndpoints — press [s] to sign)` : `{gray-fg}—{/}`; detail.setContent( - ` Selected: {bold}${cur.name}{/bold} pending=${cur.learned} allowlist=${cur.allowlist} ` + + ` Selected: {bold}${cur.name}{/bold} learned=${cur.learned} allowlist=${cur.allowlist} ` + `mode=${cur.mode} ${sigDetail}\n` + (sec ? ` blocklist=${sec.blocklistDomains ?? 0} blocklist-learn=${sec.blocklistLearnMode ? "yes" : "no"}` : ""), ); diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 384d17f81..f35a59eec 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -1241,13 +1241,15 @@ kars policy sign \ ### `kars egress` -Full egress lifecycle management: learn mode (observe domains without -blocking), pending approvals, allowlist management, and signed OCI artifact -generation + cosign signing. With `--enforce`, all learned domains are -promoted to the allowlist and enforcement mode is activated. Signing is -on by default when combined with `--enforce` or `--approve`; the controller -will refuse to use unsigned artifacts in authoritative mode -(`SignerPolicyMissing`). +Egress lifecycle management: learn mode (observe domains without blocking), +baseline allowlist management on the `KarsSandbox` CRD, and signed OCI artifact +generation + cosign signing. `--enforce` switches the sandbox to **Strict** +egress mode and signs the current baseline (`allowedEndpoints`); it does **not** +auto-promote learned domains — approve the ones you want with `--approve` first. +Signing is on by default when combined with `--approve`, `--deny`, or +`--enforce`; the controller refuses unsigned artifacts in authoritative mode +(`SignerPolicyMissing`). For temporary, TTL-scoped grants use the +`allow-extra` subcommand (an `EgressApproval` CR). **Usage:** ``` @@ -1263,16 +1265,16 @@ kars egress [name] [options] | Flag | Default | Description | |---|---|---| | `--namespace ` | — | Kubernetes namespace | -| `--learn` | — | Enable learn mode (log all accessed domains) | -| `--no-learn` | — | Disable learn mode | +| `--learn` | — | Enable learn mode (durable: patches `egressMode=Learn` in k8s) | +| `--no-learn` | — | Disable learn mode (switches `egressMode` to Strict) | | `--learned` | — | Show domains discovered during learn mode | -| `--pending` | — | Show domains pending operator approval | -| `--approve ` | — | Approve a domain for egress | -| `--deny ` | — | Deny and remove a pending domain request | +| `--pending` | — | Show learned domains not yet in the allowlist | +| `--approve ` | — | Add a domain to the baseline allowlist (default port 443) and re-sign | +| `--deny ` | — | Remove a domain from the baseline allowlist and re-sign | | `--allowlist` | — | Show currently approved domains | -| `--enforce` | — | Graduate: promote all learned domains to allowlist, switch to enforcement mode | +| `--enforce` | — | Seal: switch to Strict egress mode and sign the current baseline | | `--status` | — | Show blocklist and learn mode status | -| `--sign` | *(on with `--enforce`/`--approve`)* | Build canonical allowlist artifact, push to OCI registry, sign with cosign, patch `allowlistRef` | +| `--sign` | *(on with `--enforce`/`--approve`/`--deny`)* | Build canonical allowlist artifact, push to OCI registry, sign with cosign, patch `allowlistRef` | | `--no-sign` | — | Skip signing (controller refuses the artifact in authoritative mode) | | `--sign-mode ` | *(auto-detect)* | Cosign mode: `keyless`, `identity-token`, `keyed` | | `--sign-key ` | — | Cosign key reference (path or KMS URI like `azurekms://...`) — required for `--sign-mode keyed` | @@ -1281,6 +1283,9 @@ kars egress [name] [options] | `--emit-manifest ` | — | GitOps mode: write the KarsSandbox patch to `` instead of running `kubectl patch` | | `--force` | `false` | With `--emit-manifest`, overwrite an existing file | +See also the `kars egress allow-extra`, `kars egress approvals`, and +`kars egress revoke` subcommands for TTL-scoped `EgressApproval` grants. + **Examples:** ```bash # Enable learn mode @@ -1289,10 +1294,10 @@ kars egress my-agent --learn # Review discovered domains kars egress my-agent --learned -# Approve a domain (signs the updated allowlist automatically) +# Approve a domain into the baseline (default :443; signs automatically) kars egress my-agent --approve api.github.com -# Graduate to enforcement mode (signs + patches) +# Seal: Strict mode + sign the baseline kars egress my-agent --enforce # GitOps mode: emit patch file instead of applying diff --git a/docs/egress-proxy.md b/docs/egress-proxy.md index ff26f09f2..246dd4d09 100644 --- a/docs/egress-proxy.md +++ b/docs/egress-proxy.md @@ -145,11 +145,11 @@ flowchart TD TLD -->|no| PRIV{"DNS resolves to
private/loopback IP?
(rebinding guard)"} PRIV -->|yes| DENY PRIV -->|no| MODE{"Egress mode"} - MODE -->|learn| LOG["Append to learned set
→ operator review queue"] + MODE -->|learn| LOG["Append to learned set
→ operator review (kars egress --learned)"] LOG --> ALLOW - MODE -->|enforce| AL{"Domain on allowlist?
(parent-domain match)"} + MODE -->|strict| AL{"Domain on signed allowlist?
(parent-domain match)"} AL -->|yes| ALLOW["Tunnel / forward request"] - AL -->|no| PEND["Append to PendingApproval
(deduped) → 403 'awaiting approval'"] + AL -->|no| PEND["403 denied
recorded in BlockedBuffer (/egress/learned/blocked)"] classDef bad fill:#fde2e1,stroke:#c0392b,color:#0b1220 classDef good fill:#dff5e1,stroke:#27ae60,color:#0b1220 @@ -161,8 +161,9 @@ flowchart TD Verified against `inference-router/src/forward_proxy.rs` and `inference-router/src/blocklist.rs` (`check_egress`, the high-risk TLD list, -the parent-domain allowlist match, the DNS rebinding guard, and -`PendingApproval` deduplication). +the parent-domain allowlist match, and the DNS rebinding guard). Denied attempts +under Strict mode are recorded in the slice-5a `BlockedBuffer` +(`GET /egress/learned/blocked`) — there is no in-process pending-approval queue. ## Learn → enforce lifecycle @@ -179,67 +180,86 @@ stateDiagram-v2 end note note right of Review kars egress --learned - operator approves / denies per domain + operator approves into the baseline: + kars egress --approve end note note right of Enforce - kars egress --no-learn - Only allowlisted domains pass - New domains → PendingApproval + kars egress --enforce + (egressMode=Strict + signed bundle) + Only allowlisted host:port pass + New domains → denied (not queued) end note Learn --> Review: operator inspects learned set - Review --> Review: approve / deny - Review --> Enforce: --enforce / --no-learn + Review --> Review: --approve / --deny (baseline + re-sign) + Review --> Enforce: --enforce (seal: Strict + sign) Enforce --> Learn: --learn (rare; debugging only) ``` ## Operator Workflow +Egress is driven by the `KarsSandbox` CRD and a cosign-signed allowlist bundle — +there is **no in-router approval queue**. "Approve" means *add to the baseline +`allowedEndpoints` and re-sign*; "enforce" means *switch `egressMode` to Strict +and sign the baseline*. For a temporary, time-boxed grant, use an +`EgressApproval` CR via `kars egress allow-extra` (see below). + ### 1. Deploy with Learn Mode (default) -Learn mode is enabled by default (`network_policy.learn_egress = true` in the -sandbox spec). The blocklist is still enforced — learn mode only affects -unknown, non-malicious domains. +Learn mode is the default (`spec.networkPolicy.egressMode: Learn`). The blocklist +is still enforced — learn mode only affects unknown, non-malicious domains. ### 2. Review Discovered Domains ```bash kars egress # Show status + summary kars egress --learned # Detailed list of discovered domains +kars egress --pending # Learned domains not yet in the allowlist ``` -### 3. Approve Trusted Domains +### 3. Approve Trusted Domains (into the signed baseline) ```bash -kars egress --approve api.telegram.org +kars egress --approve api.telegram.org # defaults to :443; re-signs +kars egress --approve example.com:8443 # explicit port ``` -Approving a parent domain (e.g., `telegram.org`) covers all subdomains -(e.g., `api.telegram.org`). +`--approve` adds the host (default port `443`) to `spec.networkPolicy.allowedEndpoints` +and re-signs the bundle by default (pass `--no-sign` only for local dev). Approving +a parent domain (e.g. `telegram.org`) covers its subdomains. + +For a **temporary** grant that expires on its own (no re-sign needed): + +```bash +kars egress allow-extra --host api.telegram.org --ttl PT4H --reason "incident INC-123" +``` -### 4. Lock Down +### 4. Lock Down (seal) ```bash -kars egress --no-learn +kars egress --enforce # egressMode=Strict + sign the baseline ``` -After disabling learn mode, only explicitly allowlisted domains are reachable. -Any new domain the agent tries to reach will be denied and added to the pending -approval queue. +After sealing, only allowlisted `host:port` pairs are reachable. Any new domain +the agent tries to reach is **denied** (it is still recorded in the learn buffer +for review via `--pending`, but there is no pending-approval queue that widens +egress automatically). ## CLI Reference | Command | Description | |---------|-------------| | `kars egress ` | Show egress status (blocklist, learn mode, counts) | -| `kars egress --pending` | Show domains pending operator approval | -| `kars egress --approve ` | Approve a domain for egress | -| `kars egress --deny ` | Deny and remove a pending domain request | +| `kars egress --pending` | Show learned domains not yet in the allowlist | +| `kars egress --approve ` | Add a domain to the baseline allowlist (default port 443) and re-sign | +| `kars egress --deny ` | Remove a domain from the baseline allowlist and re-sign | | `kars egress --allowlist` | Show currently approved domains | | `kars egress --learned` | Show domains discovered during learn mode | -| `kars egress --learn` | Enable learn mode | -| `kars egress --no-learn` | Disable learn mode | +| `kars egress --enforce` | Seal: switch to Strict egress mode and sign the baseline | +| `kars egress --learn` | Enable learn mode (durable: patches `egressMode`) | +| `kars egress --no-learn` | Disable learn mode (switches to Strict) | | `kars egress --status` | Show blocklist and learn mode status | +| `kars egress allow-extra --host --ttl --reason ` | Grant a temporary, TTL-scoped egress host via an `EgressApproval` CR | | `kars egress --namespace ` | Target a specific Kubernetes namespace | The CLI discovers the running pod via `kubectl get pods` and executes `curl` @@ -251,16 +271,23 @@ All endpoints are served by the inference router on `127.0.0.1:8443`. | Endpoint | Method | Description | |----------|--------|-------------| -| `/egress/fetch` | POST | Proxy an HTTP request (blocklist → allowlist → learn/pending) | -| `/egress/pending` | GET | List pending approval requests | -| `/egress/approve` | POST | Approve a domain (`{"domain": "..."}`) | -| `/egress/deny` | POST | Deny and remove a pending request (`{"domain": "..."}`) | -| `/egress/allowlist` | GET | List approved domains | +| `/egress/fetch` | POST | Proxy an HTTP request (blocklist → allowlist → learn) | +| `/egress/allowlist` | GET | List approved domains (from the compiled bundle) | | `/egress/learned` | GET | List domains discovered in learn mode | | `/egress/learned/clear` | POST | Clear learned domains (after export/review) | +| `/egress/learned/blocked` | GET | List egress attempts blocked under Strict mode | +| `/egress/learn` | POST | Toggle learn mode at runtime (`{"enabled": true|false}`) | | `/blocklist/status` | GET | Blocklist status (enabled, domain count, learn mode) | | `/blocklist/check` | POST | Check if a domain is blocklisted | +> **Removed (Slice 5c.1):** the in-process approval endpoints `/egress/approve`, +> `/egress/deny`, `/egress/pending`, and `/egress/enforce` no longer exist. The +> allowlist is now sourced exclusively from the controller-published, +> cosign-verified bundle (`spec.networkPolicy.allowlistRef`), built from +> `allowedEndpoints`; runtime widening is an `EgressApproval` CR. The CLI flags +> `--approve`/`--deny`/`--enforce` operate on the CRD + signing pipeline, not +> these routes. + ### `/egress/fetch` Request/Response **Request:** @@ -285,9 +312,9 @@ All endpoints are served by the inference router on `127.0.0.1:8443`. **Denied (403):** ```json { - "error": "Domain 'evil.com' not on allowlist — pending operator approval", + "error": "Domain 'evil.com' not on allowlist — denied under Strict egress", "url": "https://evil.com/exfil", - "action": "Run 'kars egress --pending' to see pending requests, then 'kars egress --approve ' to allow." + "action": "If legitimate, add it to the baseline: 'kars egress --approve evil.com' (re-signs), or grant temporarily with 'kars egress allow-extra --host evil.com --ttl PT1H --reason ...'." } ``` @@ -343,7 +370,7 @@ decisions are recorded in the governance audit log. | File | Description | |------|-------------| -| `inference-router/src/blocklist.rs` | Blocklist engine, allowlist, pending approvals, learn mode | +| `inference-router/src/blocklist.rs` | Blocklist engine, allowlist, learn mode | | `inference-router/src/routes/egress.rs` | HTTP handlers for `/egress/*` endpoints | | `cli/src/commands/egress.ts` | CLI `kars egress` command | | `controller/src/reconciler/mod.rs` | iptables init container + NetworkPolicy generation | diff --git a/docs/operations/gitops.md b/docs/operations/gitops.md index 4dbe360fe..1d927a476 100644 --- a/docs/operations/gitops.md +++ b/docs/operations/gitops.md @@ -200,7 +200,7 @@ flow needed in CI. | Symptom | Cause | Fix | | -------------------------------------------------------- | ------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------- | | `--emit-manifest cannot be combined with --no-sign` | GitOps mode requires signed artifacts | Drop `--no-sign` | -| `--emit-manifest requires --enforce or --approve` | Need a signing context to derive the allowlist | Add `--enforce` or `--approve ` | +| `--emit-manifest requires --enforce, --approve, or --deny` | Need a signing context to derive the allowlist | Add `--enforce`, `--approve `, or `--deny ` | | `refusing to overwrite existing file …` | Target file already exists | Add `--force` (typical in CI) | | Controller emits `AllowlistVerified=False/SignerPolicyMissing` | No `SignerPolicy` ConfigMap on the cluster | Install one (see Helm value `controller.signerPolicy.enabled`) | | Controller emits `AllowlistVerified=False/IdentityMismatch` | Signer identity not allowlisted in `SignerPolicy` | Update the `SignerPolicy` SAN/issuer allowlist | diff --git a/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md b/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md index 800e9850d..6183d062a 100644 --- a/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md +++ b/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md @@ -1,8 +1,8 @@ # Security Audit — Egress learn/enforce flow repair (operator toggle + CLI approve/deny/enforce) Date: 2026-06-29 -Scope: `cli/src/commands/egress.ts`, `cli/src/commands/operator/actions.ts`, `cli/src/commands/egress.test.ts`, `tests/e2e-manual/scenarios/egress_lifecycle.sh`. -Gated paths: `cli/src/commands/egress.ts`, `cli/src/commands/operator/actions.ts`. +Scope: `cli/src/commands/egress.ts`, `cli/src/commands/operator/actions.ts`, `cli/src/commands/operator/dialogs/egress.ts`, `cli/src/commands/egress.test.ts`, `inference-router/src/routes/egress.rs`, `inference-router/src/routes/internal.rs`, `tests/e2e-manual/scenarios/egress_lifecycle.sh`, plus docs (`docs/egress-proxy.md`, `docs/cli-reference.md`, `docs/operations/gitops.md`). +Gated paths: `cli/src/commands/egress.ts`, `cli/src/commands/operator/actions.ts`, `inference-router/src/routes/egress.rs`, `inference-router/src/routes/internal.rs`. ## Summary @@ -35,6 +35,17 @@ allowedEndpoints` compiled into a controller-published, cosign-verified bundle - `--enforce` patches `egressMode=Strict` and signs the baseline. - `--pending` and the status view show learned-but-not-allowlisted domains. +3. **Stale runtime guidance + docs (CRD-move cleanup).** The router's + `/egress/fetch` 403 response (`inference-router/src/routes/egress.rs`) still + told agents to run the **removed** `kars egress --pending`/`--approve` + workflow; its action string + doc comment now describe the real remediation + (operator `--approve` re-sign, or a temporary `EgressApproval` via + `allow-extra`). A stale comment in `inference-router/src/routes/internal.rs` + and the operator drawer legend/label (`dialogs/egress.ts`) were corrected. + `docs/egress-proxy.md`, `docs/cli-reference.md`, and `docs/operations/gitops.md` + were updated to remove the deleted endpoints and the "enforce promotes all + learned" claim (enforce = Strict + sign; it does not auto-promote). + ## Security analysis ### T1: New capability / attack surface? (NO) diff --git a/inference-router/src/routes/egress.rs b/inference-router/src/routes/egress.rs index 93c63a469..cf5964ca5 100644 --- a/inference-router/src/routes/egress.rs +++ b/inference-router/src/routes/egress.rs @@ -99,7 +99,9 @@ async fn egress_learned_clear(State(state): State) -> impl IntoRespons /// Security model: /// 1. Blocklist → hard deny (threat intelligence) /// 2. Allowlist → approved domains pass through -/// 3. Unknown domain → deny + create pending approval request +/// 3. Unknown domain → deny (Strict) or log + allow (Learn). There is no +/// in-process pending-approval queue — widening egress is an operator action +/// on the signed allowlist bundle (Slice 5c.1). /// 4. Learn mode → log + allow (discovery phase only) /// 5. Private/internal IP → always deny (SSRF protection) /// 6. Redirects → returned as-is (never followed) @@ -147,13 +149,13 @@ async fn egress_fetch( let sandbox: &str = &state.sandbox_name; - // Check egress access: blocklist → allowlist → pending + // Check egress access: blocklist → allowlist (Strict denies the rest). if let Err(reason) = state.blocklist.check_egress(url, sandbox).await { tracing::warn!(url = %url, reason = %reason, "Egress fetch denied"); return (StatusCode::FORBIDDEN, Json(serde_json::json!({ "error": reason, "url": url, - "action": "Run 'kars egress --pending' to see pending requests, then 'kars egress --approve ' to allow.", + "action": "If legitimate, an operator can add the host to the baseline allowlist ('kars egress --approve ', which re-signs) or grant it temporarily ('kars egress allow-extra --host --ttl --reason ').", }))).into_response(); } diff --git a/inference-router/src/routes/internal.rs b/inference-router/src/routes/internal.rs index 69de62008..94e9f8232 100644 --- a/inference-router/src/routes/internal.rs +++ b/inference-router/src/routes/internal.rs @@ -161,8 +161,8 @@ async fn policy_status(State(state): State) -> impl IntoResponse { // Slice 5a — surfaced egress blocked buffer // // Operator-facing companion to the existing `/egress/learned/blocked` -// endpoint (which keeps its old shape for the -// `kars egress … --pending`/`--approve` workflow). The `/internal` +// endpoint (the observability surface backing `kars egress --pending`, +// i.e. learned domains not yet in the allowlist). The `/internal` // variants are the canonical surface the `kars egress blocked` CLI // and the headlamp plugin consume: // From 74c0c29881d07b9a36a77429ed3229eaaa4d8674 Mon Sep 17 00:00:00 2001 From: Pal Lakatos-Toth Date: Mon, 29 Jun 2026 23:23:01 +0200 Subject: [PATCH 3/3] fix(egress): address rubber-duck round-3 findings (recovery, ambiguity, deny-empty, e2e rigor) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review pass on the egress flow repair. All blocking findings resolved: - operator/actions.ts: learnEgress/enforceEgress no longer early-return when the pod is down. The authoritative CRD egressMode patch now runs regardless — the operator MUST be able to flip the mode to recover a crashing sandbox — and only the best-effort live /egress/learn probe is gated on a running pod. - egress.ts discoverKarsSandboxNamespace: a name that exists in MULTIPLE namespaces is now a hard error ("pass --namespace to disambiguate") instead of silently patching the first match (wrong-sandbox-mutation guard). - egress.ts --deny: removing the LAST endpoint leaves an empty baseline, which the canonical signer refuses. Detect this and tell the operator clearly (the empty inline list already denies all egress under Strict) rather than failing deep inside runSignFlow. - Messaging: "only allowlisted host:port pairs will pass" → "hosts" — the router enforces an L7 host match today; per-endpoint port enforcement is reserved for a later slice (the signed bundle already carries ports). CLI + docs. - manual e2e: the core learn/enforce/approve/revoke assertions now log_fail (not log_skip) after prerequisites pass, using bounded poll_blocked/poll_allowed helpers to tolerate reconcile + NetworkPolicy propagation lag. Prerequisite failures (CRD missing, admission rejected) still skip. Non-blocking, documented: operator TUI actions target the CR in kars-system (the default operator namespace); a non-default release surfaces a clear "not found" rather than a wrong-namespace mutation. The scriptable `kars egress` path resolves the CR namespace + guards ambiguity. egress.test.ts +1 (removeHost to empty). bash -n + shellcheck -S error clean; CLI typecheck + oxlint (0 errors) + 904 vitest pass; all 7 ci-gates pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/src/commands/egress.test.ts | 7 +++ cli/src/commands/egress.ts | 35 +++++++++-- cli/src/commands/operator/actions.ts | 50 ++++++++-------- docs/egress-proxy.md | 2 +- ...-06-29-egress-learn-enforce-flow-repair.md | 24 ++++++++ .../e2e-manual/scenarios/egress_lifecycle.sh | 59 ++++++++++++++----- 6 files changed, 133 insertions(+), 44 deletions(-) diff --git a/cli/src/commands/egress.test.ts b/cli/src/commands/egress.test.ts index c81f1385b..37c8df577 100644 --- a/cli/src/commands/egress.test.ts +++ b/cli/src/commands/egress.test.ts @@ -224,3 +224,10 @@ describe("unionEndpoint — port-less existing entries", () => { expect(out.find((e) => e.host === "keep.example.com")).toEqual({ host: "keep.example.com" }); }); }); + +describe("removeHost — last-endpoint edge", () => { + it("can reduce the baseline to empty (deny of the last host)", () => { + const single = [{ host: "only.example.com", port: 443 }]; + expect(removeHost(single, "only.example.com")).toEqual([]); + }); +}); diff --git a/cli/src/commands/egress.ts b/cli/src/commands/egress.ts index 11ec15e38..93972758d 100644 --- a/cli/src/commands/egress.ts +++ b/cli/src/commands/egress.ts @@ -337,6 +337,19 @@ export function egressCommand(): Command { // signed bundle is the authoritative artifact, not the inline list). console.log(chalk.dim(`\n ${host} is not in the baseline allowlist for '${name}' — re-signing the current baseline to ensure the revocation is authoritative.`)); } + // Removing the LAST endpoint leaves an empty baseline, which the + // canonical signer refuses (`--sign` cannot publish an empty bundle). + // Skip signing and tell the operator how to make the revocation + // authoritative, rather than failing deep in runSignFlow. + if (next.length === 0) { + console.log(chalk.yellow( + `\n ⚠ '${host}' was the last endpoint — the baseline is now empty, which \`--sign\` cannot publish.\n` + + ` The inline allowlist is empty, so under Strict mode the router already denies all egress for '${name}'.\n` + + ` To make this authoritative via a signed bundle, keep at least one endpoint, or seal with\n` + + ` \`kars egress ${name} --enforce\` once an endpoint is re-added. (Use \`kubectl edit karssandbox ${name} -n ${crNs}\` to inspect.)\n`, + )); + return; + } } catch (e: any) { console.log(chalk.red(`\n Failed to deny: ${e.message}\n`)); return; @@ -374,7 +387,7 @@ export function egressCommand(): Command { await routerPost("/egress/learn", { enabled: false }).catch(() => {}); } console.log(chalk.green(`\n 🔒 Enforcement (Strict) mode set for '${name}'`)); - console.log(chalk.dim(` Only allowlisted host:port pairs will pass; the blocklist still applies.`)); + console.log(chalk.dim(` Only allowlisted hosts will pass (L7 host match; port enforcement is reserved for a later slice); the blocklist still applies.`)); console.log(chalk.dim(` (The controller may roll the sandbox pod to apply the new mode.)\n`)); } catch (e: any) { console.log(chalk.red(`\n Failed to enforce: ${e.message}\n`)); @@ -474,7 +487,7 @@ export function egressCommand(): Command { await patchEgressMode(crNs, name, "Strict"); await routerPost("/egress/learn", { enabled: false }).catch(() => {}); console.log(chalk.yellow(`\n Learn mode disabled for '${name}' (egress mode is now Strict).`)); - console.log(chalk.dim(` Only allowlisted host:port pairs will pass. Seal the baseline with ${chalk.white(`kars egress ${name} --enforce`)} to (re)sign it.\n`)); + console.log(chalk.dim(` Only allowlisted hosts will pass. Seal the baseline with ${chalk.white(`kars egress ${name} --enforce`)} to (re)sign it.\n`)); } else { await routerPost("/egress/learn", { enabled: false }); console.log(chalk.yellow(`\n Learn mode disabled for '${name}'.\n`)); @@ -852,15 +865,25 @@ async function discoverKarsSandboxNamespace(name: string, podNs: string): Promis } catch { /* fall through */ } - // 2) Cross-namespace lookup — handles non-default operator releases. + // 2) Cross-namespace lookup — handles non-default operator releases. Treat + // multiple matches as ambiguous: silently patching the first would risk + // mutating the wrong sandbox when a name is reused across namespaces. try { const { stdout } = await execa("kubectl", [ "get", "karssandbox", "-A", "-o", `jsonpath={range .items[?(@.metadata.name=="${name}")]}{.metadata.namespace}{"\\n"}{end}`, ], { stdio: "pipe", timeout: 5_000 }); - const ns = stdout.trim().split("\n").map((s) => s.trim()).filter(Boolean)[0]; - if (ns) return ns; - } catch { + const matches = stdout.trim().split("\n").map((s) => s.trim()).filter(Boolean); + if (matches.length === 1) return matches[0]; + if (matches.length > 1) { + throw new Error( + `KarsSandbox '${name}' exists in multiple namespaces (${matches.join(", ")}). ` + + `Pass --namespace to disambiguate which one to target.`, + ); + } + } catch (e: any) { + // Re-throw the ambiguity error; swallow only the kubectl lookup failure. + if (typeof e?.message === "string" && e.message.includes("multiple namespaces")) throw e; /* fall through */ } // 3) Last-ditch: pod ns. Will likely fail downstream with a clear diff --git a/cli/src/commands/operator/actions.ts b/cli/src/commands/operator/actions.ts index ab13043f2..0262019d1 100644 --- a/cli/src/commands/operator/actions.ts +++ b/cli/src/commands/operator/actions.ts @@ -35,7 +35,6 @@ export function createActions(ctx: ActionContext): OperatorActions { const { activityLog, kubeContext } = ctx; async function enforceEgress(sb: SandboxInfo): Promise { - if (!sb.podName) return; try { await execa("kubectl", kctl([ "patch", "karssandbox", sb.name, "-n", "kars-system", @@ -43,15 +42,18 @@ export function createActions(ctx: ActionContext): OperatorActions { JSON.stringify({ spec: { networkPolicy: { egressMode: "Strict" } } }), ], kubeContext), { stdio: "pipe" }); // Best-effort live toggle so Strict takes effect without waiting for the - // controller to roll the pod. Never let a probe failure (missing admin - // token, older router) surface as an "enforce failed" — the CRD patch - // above is the authoritative source of truth. - await execa("kubectl", kctl([ - "exec", "-n", sb.namespace, sb.podName, - "-c", "inference-router", "--", - "/usr/local/bin/kars-inference-router", "probe", "POST", "/egress/learn", - JSON.stringify({ enabled: false }), - ], kubeContext), { stdio: "pipe" }).catch(() => {}); + // controller to roll the pod. Only attempt it when a pod is running; never + // let a probe failure (missing admin token, older router, pod down) + // surface as an "enforce failed" — the CRD patch above is the + // authoritative source of truth and works even with no pod. + if (sb.podName) { + await execa("kubectl", kctl([ + "exec", "-n", sb.namespace, sb.podName, + "-c", "inference-router", "--", + "/usr/local/bin/kars-inference-router", "probe", "POST", "/egress/learn", + JSON.stringify({ enabled: false }), + ], kubeContext), { stdio: "pipe" }).catch(() => {}); + } activityLog.log(`{green-fg}🔒 Enforced{/} ${sb.name}`); activityLog.log(`{gray-fg} ↳ saved to CRD — may trigger pod restart{/}`); } catch (e: any) { @@ -60,29 +62,31 @@ export function createActions(ctx: ActionContext): OperatorActions { } async function learnEgress(sb: SandboxInfo): Promise { - if (!sb.podName) return; try { // Authoritative: the CRD `egressMode` drives the router's EGRESS_MODE on // the next reconcile. Patch it FIRST (mirrors enforceEgress, which only // patches the CRD) so the mode change is durable even if the live toggle - // below can't reach the router. + // below can't reach the router. This runs even when the pod is down — the + // operator must be able to flip the mode to recover a crashing sandbox. await execa("kubectl", kctl([ "patch", "karssandbox", sb.name, "-n", "kars-system", "--type", "merge", "-p", JSON.stringify({ spec: { networkPolicy: { egressMode: "Learn" } } }), ], kubeContext), { stdio: "pipe" }); // Best-effort live toggle so learn mode takes effect immediately without - // waiting for a pod restart. MUST send {enabled:true} (an empty body - // defaults the router to enabled:false, i.e. it would DISABLE learn). - // Wrapped in its own catch so a probe failure can never block or fail the - // authoritative CRD patch above — the root cause of the prior - // "cannot move Strict → Learn" error. - await execa("kubectl", kctl([ - "exec", "-n", sb.namespace, sb.podName, - "-c", "inference-router", "--", - "/usr/local/bin/kars-inference-router", "probe", "POST", "/egress/learn", - JSON.stringify({ enabled: true }), - ], kubeContext), { stdio: "pipe" }).catch(() => {}); + // waiting for a pod restart. Only when a pod is running. MUST send + // {enabled:true} (an empty body defaults the router to enabled:false, i.e. + // it would DISABLE learn). Wrapped in its own catch so a probe failure can + // never block or fail the authoritative CRD patch above — the root cause + // of the prior "cannot move Strict → Learn" error. + if (sb.podName) { + await execa("kubectl", kctl([ + "exec", "-n", sb.namespace, sb.podName, + "-c", "inference-router", "--", + "/usr/local/bin/kars-inference-router", "probe", "POST", "/egress/learn", + JSON.stringify({ enabled: true }), + ], kubeContext), { stdio: "pipe" }).catch(() => {}); + } activityLog.log(`{yellow-fg}📖 Learning{/} ${sb.name}`); activityLog.log(`{gray-fg} ↳ saved to CRD — may trigger pod restart{/}`); } catch (e: any) { diff --git a/docs/egress-proxy.md b/docs/egress-proxy.md index 246dd4d09..69a384331 100644 --- a/docs/egress-proxy.md +++ b/docs/egress-proxy.md @@ -186,7 +186,7 @@ stateDiagram-v2 note right of Enforce kars egress --enforce (egressMode=Strict + signed bundle) - Only allowlisted host:port pass + Only allowlisted hosts pass (host match) New domains → denied (not queued) end note Learn --> Review: operator inspects learned set diff --git a/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md b/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md index 6183d062a..609db1e5c 100644 --- a/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md +++ b/docs/security-audits/2026-06-29-egress-learn-enforce-flow-repair.md @@ -102,5 +102,29 @@ the authoritative CRD + signed-allowlist pipeline; strengthens revocation (deny re-signs), keeps signing fail-closed, and removes the last callers of the deleted in-router approval endpoints. No security control weakened. +## Review rounds (rubber-duck, k8s + architecture) +Three independent review passes; all blocking findings addressed: +- R1/R2: deny-not-signed fail-open (deny now in signing context), enforce not + live-disabling learn, port-less drop (normalize to :443 before signing), + Docker-mode regression, manual-E2E dead endpoints. +- R3: (a) `--deny` of the **last** endpoint can't sign an empty baseline — now + detected and the operator is told the empty inline list already denies all + egress under Strict (no confusing deep signer error); (b) operator + `learnEgress`/`enforceEgress` no longer early-return when the pod is down — the + authoritative CRD patch runs regardless (you must be able to flip mode to + recover a crashing sandbox), only the live probe is pod-gated; (c) + `discoverKarsSandboxNamespace` now **fails on an ambiguous** cross-namespace + name match instead of silently patching the first; (d) host:port messaging + softened to reflect that the router enforces an **L7 host match** today (per- + endpoint port enforcement is reserved for a later slice; the signed bundle + already carries ports); (e) the manual E2E now **fails** (not skips) the core + learn/enforce/approve/revoke assertions after prerequisites pass, using a + bounded poll to tolerate reconcile/NetworkPolicy propagation lag. + +Known limitation (non-blocking): the operator TUI actions target the CR in +`kars-system` (the default operator release namespace); a non-default release +surfaces a clear "not found" error rather than a wrong-namespace mutation. The +scriptable `kars egress` path resolves the CR namespace and guards ambiguity. + Signed-off-by: Pal Lakatos-Toth Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com> diff --git a/tests/e2e-manual/scenarios/egress_lifecycle.sh b/tests/e2e-manual/scenarios/egress_lifecycle.sh index e73b48372..3e3319907 100755 --- a/tests/e2e-manual/scenarios/egress_lifecycle.sh +++ b/tests/e2e-manual/scenarios/egress_lifecycle.sh @@ -109,6 +109,34 @@ agent_curl() { --connect-timeout "$timeout" "$url" 2>/dev/null || true } +poll_blocked() { + # Poll until the agent's request to $1 is blocked (HTTP 000 / empty), + # tolerating reconcile + NetworkPolicy propagation lag. Returns 0 if it + # becomes blocked within ~${2:-40}s, 1 otherwise. The LAST observed code is + # echoed so the caller can report it on failure. + local url="$1" budget="${2:-40}" http="" + local deadline=$(( SECONDS + budget )) + while (( SECONDS < deadline )); do + http=$(agent_curl "$url" 6) + if [[ "$http" == "000" || -z "$http" ]]; then echo "$http"; return 0; fi + sleep 3 + done + echo "$http"; return 1 +} + +poll_allowed() { + # Poll until the agent's request to $1 succeeds (2xx/3xx), tolerating + # reconcile lag. Returns 0 if allowed within ~${2:-40}s, 1 otherwise. + local url="$1" budget="${2:-40}" http="" + local deadline=$(( SECONDS + budget )) + while (( SECONDS < deadline )); do + http=$(agent_curl "$url" 8) + if [[ "$http" == "200" || "$http" == "301" || "$http" == "302" ]]; then echo "$http"; return 0; fi + sleep 3 + done + echo "$http"; return 1 +} + # ── [1/4] learn ─────────────────────────────────────────────────────── log_step "[1/4] learn: agent touches example.com → expect to see it in /egress/learned" metric_start "egress_learn_touch" @@ -142,13 +170,14 @@ fi # Best-effort live toggle so Strict applies without waiting for a pod roll. _=$(router_curl POST "/egress/learn" '{"enabled":false}') metric_finish "egress_enforce_switch" egress_lifecycle enforceSwitchLatency -sleep 3 # example.org should not have been seen during learn — should now be blocked. -http=$(agent_curl "https://example.org" 6) -if [[ "$http" == "000" || -z "$http" ]]; then +# This is a core assertion: poll (tolerate propagation lag) then FAIL if it +# never blocks. +http=$(poll_blocked "https://example.org" 45) +if [[ $? -eq 0 ]]; then log_pass "previously-unseen domain (example.org) blocked under Strict" else - log_skip "example.org returned HTTP ${http} — egress NetworkPolicy may not have refreshed yet" + log_fail "example.org still reachable under Strict (last HTTP=${http}) — enforcement did not take effect" fi # ── [3/4] approve ───────────────────────────────────────────────────── @@ -169,28 +198,30 @@ spec: ttl: PT15M EOF then + # Prerequisite failure (CRD not installed / admission rejected) → skip, not fail. log_skip "could not create EgressApproval/${appr} — CRD may not be installed in this build" else - sleep 6 - http=$(agent_curl "https://example.org" 8) - if [[ "$http" == "200" || "$http" == "301" || "$http" == "302" ]]; then + http=$(poll_allowed "https://example.org" 45) + if [[ $? -eq 0 ]]; then log_pass "EgressApproval allowlisted example.org (HTTP ${http})" else - log_skip "EgressApproval applied but probe got HTTP=${http} — reconcile/propagation lag or upstream issue" + log_fail "EgressApproval applied but example.org still blocked (last HTTP=${http}) — grant did not take effect" fi fi # ── [4/4] deny ──────────────────────────────────────────────────────── log_step "[4/4] deny: delete the EgressApproval CR, re-probe → expect block" -if ! kubectl delete egressapproval "$appr" -n "$ns" >/dev/null 2>&1; then - log_skip "could not delete EgressApproval/${appr} — may not have been created" +if ! kubectl get egressapproval "$appr" -n "$ns" >/dev/null 2>&1; then + # The approval was never created (step 3 skipped) — nothing to revoke. + log_skip "EgressApproval/${appr} absent — revoke step not applicable" +elif ! kubectl delete egressapproval "$appr" -n "$ns" >/dev/null 2>&1; then + log_skip "could not delete EgressApproval/${appr}" else - sleep 6 - http=$(agent_curl "https://example.org" 6) - if [[ "$http" == "000" || -z "$http" ]]; then + http=$(poll_blocked "https://example.org" 45) + if [[ $? -eq 0 ]]; then log_pass "revoking the EgressApproval blocked example.org again" else - log_skip "EgressApproval deleted but probe got HTTP=${http} — propagation lag" + log_fail "EgressApproval deleted but example.org still reachable (last HTTP=${http}) — revocation did not take effect" fi fi