Skip to content

Verify sensitive requests actually came through Traefik (#125)#179

Open
ClaydeCode wants to merge 3 commits into
mainfrom
fix/clayde/verify-traefik-secret
Open

Verify sensitive requests actually came through Traefik (#125)#179
ClaydeCode wants to merge 3 commits into
mainfrom
fix/clayde/verify-traefik-secret

Conversation

@ClaydeCode

Copy link
Copy Markdown
Contributor

What & why

Closes the short-term half of #125.

Auth on a shard is enforced entirely by Traefik forwardAuth + docker network topology; FastAPI routes carry no auth dependency. Every installed app joins the same portal docker network as shard_core, which serves plain HTTP with no in-app auth — so any app container can curl http://shard_core/protected/... or /management/... directly and bypass all auth.

This adds a shared secret that only Traefik injects:

  • service/traefik_secret.py generates a secret once (stored in kv_store, matching the terminal_jwt_secret pattern) and exposes a verify_traefik_secret FastAPI dependency that constant-time-compares the X-Ptl-Traefik-Secret request header.
  • The Traefik dynamic config gains a verify-traefik middleware (customRequestHeaders) that injects the secret, wired onto the shard_core_private and shard_core_management routers.
  • The /protected and /management routers now depend on verify_traefik_secret. A request that did not traverse Traefik lacks the header and is rejected with 403.

/internal is deliberately left ungated: its forwardAuth endpoints (authenticate_terminal, authenticate_management, auth) are called by Traefik directly (they never pass through the middleware chain, so they never carry the header), and call_backend/call_peer are called by apps directly by design.

⚠️ Scope limitation — please read (needs your call, @max-tet)

The review panel (below) found this mitigation is intentionally partial and only raises the bar. Because both Postgres (kv_store, default creds) and the Traefik dashboard (api.insecure: true) sit on the same portal network as the apps, an app that actively targets shard_core can still read the secret (psql ... SELECT value FROM kv_store or curl http://traefik:8080/api/rawdata) and forge the header. So this stops accidental/naive cross-app calls and any app not specifically hunting the secret — but not a determined one.

Fully sealing it needs the network-isolation work that #125 itself defers to its long-term half: isolate Postgres from portal (+ non-default creds) and disable the unauthenticated Traefik API. I kept this PR surgical (no topology changes) and did not fold that in, since #125 explicitly scopes it separately and it carries real deployment blast radius. Decision for you: merge this as the documented short-term step and track the network-hardening follow-up, or you'd prefer I expand scope here. The agents.md note is written to not overstate the protection.

Acceptance criteria

  • ✅ Test that a /protected/... request from inside portal without the injected secret is rejected (test_protected_without_secret_is_rejected).
  • ✅ All existing tests pass; app→internal flows unaffected. Local run: 236 passed, 1 failed. The single failure is test_app_lifecycle.py::test_app_starts_and_stops, a pre-existing environmental collision — the grind VM's own clayde-traefik-1 permanently holds host port 80, which the quick_stop mock app publishes; the test fails at docker compose up -d, a path this change never touches. It is green on CI (port 80 free).

Review panel

Three adversarial reviewers ran (each given only the diff + issue, instructed to refute):

Adversarial correctness — 1 blocking, 1 advisory.

  • BLOCKING: secret leaks via the unauthenticated Traefik dashboard (api.insecure: true on traefik:8080, reachable on portal) → an app reads customRequestHeaders and forges the header. Resolved: confirmed real; it is a facet of the co-location limitation now documented in agents.md and in the scope section above, and escalated to you rather than silently fixed (dashboard hardening belongs with the deferred network-isolation half). Not fixed in-PR by design.
  • Advisory: websocket route /protected/ws/updates gated via HTTPException (works today, unpinned). Fixed in e60f242 — added a behavioral handshake test (with an accept-with-secret positive control).
  • Cleared as non-issues: header duplicate/parse (Traefik overwrites), startup race, /internal correctly ungated, controller /management calls unaffected, no dead code.

Security — 2 blocking, 1 advisory, all the same co-location root cause.

  • BLOCKING: secret readable from Postgres on portal (default creds) via SELECT value FROM kv_store. BLOCKING: secret readable from the Traefik dashboard (same as above). Advisory: readable from the dyn-config file via a path-traversal volume mount. Resolved: these are the co-location limitation — documented honestly and escalated as the network-isolation follow-up; not fixable inside this PR's scope (reviewer agreed: "cannot be fixed inside traefik_secret.py … requires network segmentation"). Cleared as sound: compare_digest usage, 512-bit entropy, no spoof-through-Traefik, no leak via authResponseHeadersRegex ^X-Ptl-.* (that copies response headers on the app routers only; verify-traefik is a request header on the shard_core_* routers), no secret in logs, fail-closed persistence.

Test adversary — 0 blocking, 2 advisory. Verified the committed negative/positive tests have real teeth by stripping the dependency and watching them fail.

  • Advisory: ws route untested → fixed (e60f242). Advisory: 500-on-missing-secret branch untested → fixed (e60f242, test_missing_secret_fails_closed_with_500). Confirmed the conftest header injection masks nothing (routes had no gate before).

Recommended reading order

  1. shard_core/service/traefik_secret.py — the secret + verify dependency (new)
  2. shard_core/service/traefik_dynamic_config.pyverify-traefik middleware, wired onto the two routers
  3. shard_core/service/app_installation/util.py — fetch/ensure secret when compiling the config
  4. shard_core/web/protected/__init__.py, shard_core/web/management/__init__.py — the router gate
  5. tests/conftest.py — fixtures carry the secret header (simulate Traefik traversal)
  6. tests/test_traefik_secret.py, tests/test_traefik_dyn_spec.py — coverage
  7. agents.md — auth-model note incl. the scope limitation

🤖 Generated with Claude Code

ClaydeCode and others added 3 commits July 22, 2026 05:22
Every installed app shares the `portal` docker network with shard_core, so
an app can reach `/protected` and `/management` directly and bypass the
Traefik forwardAuth gate. As the first half of closing that hole, introduce a
shared secret that only Traefik knows.

`service/traefik_secret.py` generates the secret once (stored in kv_store,
matching the terminal_jwt_secret pattern) and exposes a FastAPI dependency to
verify it. `write_traefik_dyn_config` now compiles a `verify-traefik`
middleware that injects the secret as the `X-Ptl-Traefik-Secret` request
header, wired onto the `shard_core_private` and `shard_core_management`
routers. Enforcement is added in the next commit; on its own this change only
adds an ignored header, so behaviour is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Gate the `/protected` and `/management` routers on the shared secret injected
by the `verify-traefik` middleware: a request arriving directly from an app on
the `portal` network lacks the `X-Ptl-Traefik-Secret` header and is now
rejected with 403. `/internal` stays ungated by design — its forwardAuth
endpoints are called by Traefik directly (never through the middleware chain)
and `call_backend`/`call_peer` are called by apps directly.

Test fixtures (`app_client`, `api_client`) now carry the secret header to
represent requests that arrived through Traefik, so existing tests are
unaffected. New `tests/test_traefik_secret.py` proves the bypass is rejected,
the correct/absent/wrong-secret cases, that public and forwardAuth routes stay
reachable, and that the middleware is attached with the stored secret.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Review panel flagged that the shared secret only raises the bar: because
Postgres (default creds) and the unauthenticated Traefik dashboard both sit on
the `portal` network, an app that actively targets shard_core can still read
the secret. Reword the agents.md note so it no longer overstates the
protection and points at the network-isolation follow-up (the long-term half
of #125) as what actually seals the gap.

Add two tests closing advisory coverage gaps: the `/protected/ws/updates`
websocket handshake is denied without the secret (with an accept-with-secret
positive control so the check has teeth), and a missing kv_store secret fails
closed with 500 rather than opening the gate.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant