fix(auth): reject self-asserted principal headers from external callers (ARN-167) - #452
fix(auth): reject self-asserted principal headers from external callers (ARN-167)#452rita-aga wants to merge 2 commits into
Conversation
…rs (ARN-167) TemperPaw trusted any request carrying x-temper-principal-kind/-id as a PreAuthenticatedRequest, so any external client could self-assert admin. Add an ingress edge that strips all client-asserted identity headers (x-temper-*, x-agent-id, x-tenant-id) unless the request originates from a genuine loopback peer (checked via ConnectInfo, never a forwarding header). Only a loopback-origin internal call may self-assert its principal; external callers have those headers removed before Cedar/bearer can trust them. TemperPaw side of systemic Class A; kernel counterpart is ARN-170 (temper #343). ADR-0066. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@greptile review |
ARN-165 / ARN-167 principle audit — request changesThis blocks the simplest direct non-loopback header spoof, but it preserves the underlying identity vulnerability by treating loopback as authentication. Blocking findings
Required directionRemove header-only loopback preauthentication. Trusted Rust callers need a scoped internal service credential, Unix-domain/in-process capability, or trusted request extension. Temper must strip guest authority headers and inject identity only from verified credentials or Both unresolved IPv4-mapped-IPv6 threads are duplicates. Normalize mapped addresses and test them, but note that this is currently an internal availability regression, not the root privilege bypass. The branch is clean and targeted tests pass, but it uses a stale Temper revision. Rebase and run cross-repo/live verification only after the trust redesign. |
|
@greptile-apps please review the latest ARN-167 head 152936c. Focus areas: loopback no longer grants pre-auth, all client identity/tenant headers are stripped before credential resolution, PawApiClient no longer synthesizes admin headers, and internal calls use bearer/session-derived identity only. |
|
ARN-167 current-head validation for 152936c:\n\n- Re-reviewed the prior Class A blocker: loopback is no longer a trust root. Header-only loopback identity is removed; TemperPaw strips client-assertable identity/tenant headers for every peer, and internal transport calls now use bearer credentials instead of raw admin principal headers.\n- Local validation from isolated worktree:\n - cargo fmt --check\n - git diff --check origin/main...HEAD\n - cargo test -p temperpaw auth::tests -- --nocapture (11/11 auth tests passed)\n - cargo test -p paw-transport tests::paw_api_client_ -- --nocapture (3/3 request-construction tests passed)\n - cargo clippy -p temperpaw -p paw-transport --all-targets -- -D warnings\n\n@greptile-apps please review the current head 152936c with focus on: no self-asserted identity headers from any peer, loopback not used as auth, PawApiClient bearer behavior, and whether any public path can still forward x-temper-* / x-agent-id / x-tenant-id into the kernel. |
Closes ARN-167 (CRITICAL) — the TemperPaw side of systemic Class A, mirroring the kernel fix in temper PR #343 (ARN-170). Draft for review — do not merge.
Problem
crates/temperpaw/src/auth.rsmarked ANY request carryingx-temper-principal-kind+x-temper-principal-idasPreAuthenticatedRequestand trusted the client-supplied principal — no cookie, no bearer, no origin check. The server binds0.0.0.0, so any external caller could self-assert any principal, including admin.Fix
An ingress edge that strips all client-asserted identity headers (
x-temper-*,x-agent-id,x-tenant-id) unless the request originates from a genuine loopback peer — checked via the actual TCP peer address (ConnectInfo), never a client-supplied forwarding header. Only a loopback-origin internal call (the WASM-agent/transport path) may self-assert its principal; external callers have those headers removed before Cedar or the kernel bearer check can trust them. MissingConnectInfo→ treated as non-loopback (strip) — the safe default.Red→green
external_self_asserted_principal_headers_are_rejected— an external request withx-temper-principal-kind: adminis no longer treated as authenticated/admin (was: PreAuthenticated). Loopback-origin internal calls still self-assert as before.Notes
🤖 Generated with Claude Code
Greptile Summary
This PR closes the Class A self-asserted-identity bypass (ARN-167) by replacing the previous loopback-gated pre-authentication branch with a credential-only boundary: the outer middleware now strips the entire
x-temper-*,x-agent-id, andx-tenant-idheader family from every request before any auth decision, and the removed pre-auth code path is not replaced with another network-location check.auth.rsingress strip:strip_client_identity_headersruns unconditionally before public-path bypass or credential checks; session cookies re-inject identity server-side viainject_auth_headers, bearer tokens pass to the kernel middleware for resolution, and requests with neither credential get 401.paw-transport:PawApiClient.build_requestno longer synthesizes admin identity headers for loopback URLs; theuses_internal_loopbackhelper is deleted and theapi_key-based bearer path is now the sole auth mechanism.setup.rs/startup.rs: All five call sites that manually setx-temper-principal-kind: adminhave been cleaned up; internal HTTP helpers now authenticate throughauth.apply()or theapi_keybearer credential exclusively.Confidence Score: 5/5
Safe to merge after the noted deploy-topology verification (internal WASM-agent/transport calls traverse loopback and carry bearer credentials in the deployed environment).
The credential-only boundary is implemented correctly end-to-end: the strip runs before every auth branch (including public-path bypass), session and bearer paths re-inject identity from server state, and the pre-auth header branch is fully deleted rather than patched. All four reviewer focus areas are verified by dedicated tests.
No files require special attention. The only open item is the operational verification called out in the PR description: confirming that internal WASM-agent/transport calls in the deployed topology carry a bearer token before merging to production.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant C as External/Loopback Client participant M as auth::middleware participant S as Session Handler participant B as bearer_auth_check participant H as Route Handler C->>M: HTTP request (any headers) Note over M: strip_client_identity_headers()<br/>removes x-temper-*, x-agent-id, x-tenant-id alt Public path M->>H: pass through (no identity injected) else Valid session cookie M->>S: claims_from_headers() S-->>M: SessionClaims Note over M: inject_auth_headers()<br/>x-temper-principal-kind: admin<br/>x-temper-principal-id: email<br/>x-tenant-id: state.tenant() M->>H: PreAuthenticatedRequest extension set else Bearer token present Note over M: ensure_tenant_header(state.tenant()) M->>B: forward with bearer Note over B: resolves principal from token<br/>injects kernel identity headers B->>H: authenticated request else No credential M-->>C: 401 UNAUTHORIZED end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant C as External/Loopback Client participant M as auth::middleware participant S as Session Handler participant B as bearer_auth_check participant H as Route Handler C->>M: HTTP request (any headers) Note over M: strip_client_identity_headers()<br/>removes x-temper-*, x-agent-id, x-tenant-id alt Public path M->>H: pass through (no identity injected) else Valid session cookie M->>S: claims_from_headers() S-->>M: SessionClaims Note over M: inject_auth_headers()<br/>x-temper-principal-kind: admin<br/>x-temper-principal-id: email<br/>x-tenant-id: state.tenant() M->>H: PreAuthenticatedRequest extension set else Bearer token present Note over M: ensure_tenant_header(state.tenant()) M->>B: forward with bearer Note over B: resolves principal from token<br/>injects kernel identity headers B->>H: authenticated request else No credential M-->>C: 401 UNAUTHORIZED endComments Outside Diff (1)
crates/temperpaw/src/auth.rs, line 174-177 (link)Authorization: Bearer …andx-temper-principal-kind: adminwill have the identity header stripped and the kernel will resolve the principal from the token alone. This is the correct behaviour, but there is no test that proves it. Adding a test that presents a bearer token alongside forgedx-temper-*headers and verifies that those headers do not survive to downstream would complete coverage for the bearer path.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (4): Last reviewed commit: "fix(auth): reject loopback self-asserted..." | Re-trigger Greptile