Skip to content

fix(canopy): SEC-F22/F19 #420 follow-up — CI smoke attest + WS per-session log hygiene#426

Merged
pcalnon merged 2 commits into
mainfrom
fix/pr420-followup-smoke-attest-log-hygiene
Jul 5, 2026
Merged

fix(canopy): SEC-F22/F19 #420 follow-up — CI smoke attest + WS per-session log hygiene#426
pcalnon merged 2 commits into
mainfrom
fix/pr420-followup-smoke-attest-log-hygiene

Conversation

@pcalnon

@pcalnon pcalnon commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to #420 (fix(canopy): SEC-F22/F19 — startup bind-guard + global/per-session WS caps (D2/D4)), addressing findings from its §9 independent security review. This PR lands the two review items still outstanding after #421:

  1. CI Docker smoke — attest past the new bind-guard (Fixed)
  2. SEC-F19 log hygiene — hash the canopy_session cookie before logging (Security)

Note on the review's third item (global-cap slot leak). The review's Medium bug — a global-cap rejection stranding per-IP/per-session reservations — was already fixed on main by #421 (fix: release websocket cap reservations on global reject): WebSocketManager.connect() now returns whether it registered the socket, release_connection_limits() rolls back both counters, all three WS endpoints release on reject/exception, and test_ws_connection_caps.py::TestGlobalConnectionCap::test_global_cap_rejection_releases_reserved_session_slots asserts _per_ip_counts/_per_session_counts return to their pre-reservation values. This PR does not re-implement it (dup-guard) — it is verified present and complete.

1. CI Docker smoke — attest the container past the bind-guard (.github/workflows/ci.yml)

#420's startup loopback bind-guard (enforce_loopback_bind_guard, run in main.lifespan) refuses to serve on a non-loopback interface without JUNIPER_CANOPY_FRONTING_AUTH_ATTESTED=true. The Dockerfile bakes JUNIPER_CANOPY_SERVER__HOST=0.0.0.0 (required for the container to be reachable through the published port), so the docker-build Verify Container Starts smoke step would abort at startup (NonLoopbackBindError) and never reach healthy.

Fix: pass -e JUNIPER_CANOPY_FRONTING_AUTH_ATTESTED=true to docker run — scoped to that ephemeral CI container only (verified the sole container-start site in .github/workflows/; no fronting proxy is present, the container just needs to boot for the /v1/health probe).

Mechanism verified directly:

enforce_loopback_bind_guard('0.0.0.0', attested=False) -> raises NonLoopbackBindError (container aborts startup)
enforce_loopback_bind_guard('0.0.0.0', attested=True)  -> permitted (container boots; smoke reaches health probe)

2. SEC-F19 log hygiene — hash the session cookie (src/communication/websocket_manager.py)

check_per_session_limit logged a raw 8-char prefix of the anonymous canopy_session cookie (session_key[:8]) when the per-session cap tripped. That cookie is a signed Starlette session token, so even a prefix in a log line is an avoidable session-identifier leak. New _hash_session_key_for_log helper emits a short, non-reversible tag instead — keyed HMAC-SHA256 over the raw cookie with a per-process random secret (_LOG_HASH_KEY), truncated to 12 hex chars — so the logged digest is not an offline-computable function of the cookie and does not correlate across process restarts. Mirrors the cascor sibling, which hashes its identity before logging (juniper-cascor src/api/workers/security.py).

Tests

  • src/tests/unit/test_ws_connection_caps.py::TestPerSessionLogHygiene (new): the over-cap warning omits the raw cookie and its [:8] prefix and carries the keyed hash instead; the hash is deterministic-within-process, hex, compact, and not the raw value.
  • Full CI-equivalent scope green: pytest -m "not requires_cascor and not requires_server and not slow" src/tests/unit/ src/tests/regression/ --cov=src --cov-fail-under=80.
  • Full canopy pre-commit passed (Black, isort, Flake8 strict+relaxed, MyPy, Bandit, yamllint, async-route audit, doc-links).

Merge

Owner-gated — do not auto-merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LX5ToumBaABH3QutQC86sM

…ssion log hygiene

Addresses two items from the §9 independent security review of #420. The
review's third item (global-cap rejection leaking per-IP/per-session slots)
already landed on main via #421 and is NOT re-implemented here (dup-guard).

1. CI Docker smoke — attest past the bind-guard (.github/workflows/ci.yml).
   #420's startup loopback bind-guard (enforce_loopback_bind_guard, run in
   main.lifespan) refuses to serve on a non-loopback interface without
   JUNIPER_CANOPY_FRONTING_AUTH_ATTESTED=true. The Dockerfile bakes
   JUNIPER_CANOPY_SERVER__HOST=0.0.0.0, so the docker-build "Verify Container
   Starts" smoke would abort at startup (NonLoopbackBindError) and never reach
   healthy. Pass -e JUNIPER_CANOPY_FRONTING_AUTH_ATTESTED=true to docker run,
   scoped to that ephemeral CI container only (the sole container-start site in
   .github/workflows/).

2. SEC-F19 log hygiene — hash the canopy_session cookie before logging
   (src/communication/websocket_manager.py). check_per_session_limit logged a
   raw 8-char prefix of the signed Starlette session cookie (session_key[:8]).
   New _hash_session_key_for_log emits a keyed HMAC-SHA256 (per-process secret
   _LOG_HASH_KEY) 12-hex-char tag instead, so the raw cookie never reaches a log
   line and the digest is not offline-computable. Mirrors juniper-cascor
   src/api/workers/security.py.

Tests: src/tests/unit/test_ws_connection_caps.py::TestPerSessionLogHygiene.
Verified: full CI unit+regression scope green (coverage 85.85%); canopy
pre-commit green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LX5ToumBaABH3QutQC86sM
@pcalnon pcalnon self-assigned this Jul 5, 2026

@pcalnon pcalnon left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

Signed-off-by: Overtoad <paul.calnon@gmail.com>
@pcalnon pcalnon merged commit 9ad7e60 into main Jul 5, 2026
20 checks passed
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