feat: verify internal-API session claims via unix-socket peer creds (#302) - #2424
Conversation
Design Review (Fable 5) — 🟡 CONCERNSAdvisory design-level review of Design-Verdict: CONCERNS Kernel attestation only binds callers who volunteer for it — the TCP loopback path still accepts Watch
[DESIGN-REVIEWED] a371505 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsBased on my review of the diff, I traced the security-sensitive paths carefully. Falsification of the key candidates:
No findings. [OPUS-REVIEWED] a371505 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ human override acceptedHuman judgment by @bolichen97 overrides the GPT 5.6 finding for This comment is updated in place on each push. The model was not re-run because an authorized human decision supersedes it. False positive or not applicable? A repository writer can comment: |
ef9297f to
2f0834a
Compare
|
🤖 Kiro Crew Auto-Pipeline [operator: bolichen97] All three GPT 5.6 findings verified legitimate and fixed in 1. BLOCKING — forgeable unsigned mappings (peer_resolve.py). Accepted. The bare 2. BLOCKING — UNVERIFIABLE fails open (token_auth.py). Accepted. The branch now denies unless 3. FINDING — CSRF 403 for AF_UNIX mutating requests. Confirmed real (and blocking in practice): Gates re-run clean: isort/flake8/mypy (857 files), targeted suites 483 passed. Docs updated in the same commit. |
2f0834a to
c3cec63
Compare
|
🤖 Kiro Crew Auto-Pipeline [operator: bolichen97] Round-2 findings on FINDING (peer_resolve.py function-local imports) — accepted; the default-resolution imports are now module-scope ( FINDING (urls.py function-local BLOCKING (unresolvable peer accepted) — rebutted as not-applicable-by-design; this re-litigates the posture the issue itself pre-decided:
Override recorded below for the audit trail; a maintainer can reverse this at merge review if the fail-closed tradeoff should land now. |
Human judgment recorded@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
…302) The dashboard internal HTTP API authenticated callers with loopback + X-Internal-Secret and took the session identity from the fully client-declared X-Session-Key header — a same-uid process could impersonate any session. Close the gap with kernel-attested peer identity, mirroring the SO_PEERCRED + /proc-ancestry mechanism gatewayd already uses for MCP stub registration: - extract gatewayd's ancestry walk into a shared peer_resolve module (gatewayd delegates; behavior-preserving) - serve the internal API additionally on a unix socket next to the TCP site (POSIX only; degrades to TCP-only on any failure) - in token_auth_middleware, kernel-verify the declared X-Session-Key of AF_UNIX peers: deny on mismatch (403 + SEL dashboard.peer-identity-mismatch), mark peer_verified on match, degrade to status quo when unresolvable (warm pool, cron, pooled backends) - teach loopback_urlopen an AF_UNIX transport and prefer it in mcp_core's _API helpers, falling back to TCP only when nothing answered at connect time (never double-sends) Out of scope (deliberate): strict fail-closed denial of unresolvable peers, browser/cookie path changes, Windows named-pipe transport, and migrating the session-directive tools off _resolve_session_key_strict. Closes #302
c3cec63 to
a371505
Compare
|
/ai-review override gpt a371505: same two findings already rebutted on 2f0834a/c3cec63 — unresolvable-peer pass-through is the posture issue #302 explicitly specifies (strictly-monotonic; fail-closed named out-of-scope as a human product decision, and a unix-only deny is ineffective while the TCP internal surface remains), and the urls.py lazy import is the documented stdlib-only-leaf exception under the non-blocking top-level-imports rule. |
Human judgment recorded@bolichen97 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
…irodotdev#302) (kirodotdev#2424) The dashboard internal HTTP API authenticated callers with loopback + X-Internal-Secret and took the session identity from the fully client-declared X-Session-Key header — a same-uid process could impersonate any session. Close the gap with kernel-attested peer identity, mirroring the SO_PEERCRED + /proc-ancestry mechanism gatewayd already uses for MCP stub registration: - extract gatewayd's ancestry walk into a shared peer_resolve module (gatewayd delegates; behavior-preserving) - serve the internal API additionally on a unix socket next to the TCP site (POSIX only; degrades to TCP-only on any failure) - in token_auth_middleware, kernel-verify the declared X-Session-Key of AF_UNIX peers: deny on mismatch (403 + SEL dashboard.peer-identity-mismatch), mark peer_verified on match, degrade to status quo when unresolvable (warm pool, cron, pooled backends) - teach loopback_urlopen an AF_UNIX transport and prefer it in mcp_core's _API helpers, falling back to TCP only when nothing answered at connect time (never double-sends) Out of scope (deliberate): strict fail-closed denial of unresolvable peers, browser/cookie path changes, Windows named-pipe transport, and migrating the session-directive tools off _resolve_session_key_strict. Closes kirodotdev#302
Closes #302
Summary
Follow-up from #300. The dashboard internal HTTP API authenticated callers with loopback TCP +
X-Internal-Secret(a same-uid-readable file) and took the session identity from the fully client-declaredX-Session-Keyheader — so a same-uid process could impersonate any session on every session-scoped internal route. This PR closes the gap with kernel-attested peer identity, reusing the exact mechanism gatewayd already trusts for MCP stub registration (SO_PEERCRED+ /proc ancestry oversocketsec).Posture: verify-when-resolvable, deny-on-mismatch, degrade-to-status-quo when unresolvable. The change is strictly monotonic — never weaker than today's auth, kernel-verified whenever the gateway's own registry can attest the peer.
Changes
peer_resolve.py(new) — gatewayd's_resolve_peer_identityancestry walk extracted into a shared module; gatewayd delegates through its existing_config_dir/_ppid_fntest seams (behavior-preserving: all existing gatewayd/claim tests pass unmodified).dashboard/server.py— both start paths (start_dashboard,start_api_server) additionally bind aweb.UnixSiteon the sameAppRunnerat~/.kiro/crew/dashboard-<port>.sock. POSIX only; any failure logs once and degrades to TCP-only (today's behavior). 0700 dir gate + 0600 socket, stale-socket self-heal at startup, best-effort unlink at shutdown. Port-suffixed name so multi-instance data homes don't collide, and a client resolving its port fromdashboard.urlreaches the same logical endpoint as its TCP fallback.dashboard/token_auth.py— for internal/mixed-internal paths arriving onAF_UNIXand carryingX-Session-Key: confirm peer uid (deny on positive mismatch), resolve the peer's session via the shared ancestry walk (offloaded to the subprocess executor), deny 403 + SELdashboard.peer-identity-mismatchwhen the resolved key differs from the declared header, setrequest["peer_verified"] = Trueon match, and proceed under today's semantics when unresolvable (warm-pool runtimes before claim, cron scripts, pooled MCP backends). The check runs before either auth flavor grants. TCP requests never engage the branch — browser cookies, Windows, and remotelocal_only=Falsedeployments are untouched.loopback_http.py— stdlib-onlyAF_UNIXtransport (_UnixHTTPConnection+ handler);loopback_urlopen(unix_socket_path=...)prefers the socket when the file exists and falls back to TCP only on connect-levelFileNotFoundError/ConnectionRefusedError(cases that provably never delivered the request — cannot double-send). HTTP error statuses and read timeouts propagate unchanged; every caller's error shape is identical.mcp_core.py—_API_UNIX_SOCKETresolved at import next to_API; all 7_APIcall sites route through_api_urlopen.docs/system-specs/features/dashboard-token-auth.mdgains the transport + verification contract.test/test_dashboard_peer_auth.py(28 tests): shared-walk units, middleware deny/allow/status-quo/TCP-never-engages (faked socketsec seams), a real-UnixSiteend-to-end with kernel-populated peer credentials, server-startup bind/degrade/Windows-skip, client transport preference + fallback + no-double-send, and mcp_core wiring.test_identity_topology.pycall-site registry updated for the relocated walk.Deliberately out of scope (per issue #302 spec)
_resolve_session_key_strict(their directive leg is already session-bound).Notes for reviewers
dashboard-<port>.sock(issue sketcheddashboard.sock) so multiple gateway instances sharing a data home cannot collide, and client/server port resolution stays symmetric.KIROCREW_SESSION_KEYviaacp/client.pyand are not descendants of a claimed slot's kiro-cli, so their ancestry resolves to""→ status quo, never a false deny. Pooled MCP backends are children of gatewayd → same. A keepalive from a dead session's process tree after pid recycling could in principle resolve to a different live session and be denied — that denial is the correct outcome.socket.AF_UNIXAttributeErrorthat would have broken the middleware chain on Windows, plus test-collection guards). The in-CI GPT + Opus review gates apply as normal.Testing
isort/flake8/mypyclean (857 files).pytest: 39,689 passed; 90 failures verified pre-existing environment issues (sandbox backend unavailable on this host, root-owned-binary checks,KIROCREW_PORTenv pollution) — identical failure set reproduced on pristineorigin/mainwith the same subset.