Security + reliability + docs: whole-repo audit follow-ups#105
Conversation
…tify fleet-key handling Whole-repo audit follow-ups (security lens): - worker_api: capability + host-network validation is now an ALLOWLIST derived live from services/*.yml (only caps/host-mode a real catalog service declares are permitted) instead of a 3-entry denylist that missed SYS_MODULE/DAC_READ_ SEARCH/etc.; expand blocked bind-mount roots (/mnt,/media,/opt,/srv,/data,/tmp); add the missing auth-rejection test for the socket-privileged deploy endpoint - worker_api: only adopt an issued per-worker key AFTER it is durably persisted (a write failure no longer leaves a memory-only key that vanishes on restart) - fleet.html: esc() now escapes quotes too, closing stored DOM-XSS via a worker name breaking out of a double-quoted attribute - database: an undecryptable per-worker key (secret-key rotation) now reports the worker as unenrolled so it re-enrols on the shared key, instead of collapsing to an empty string that bricks fleet auth in both directions - auth: CASHPILOT_SECURE_COOKIE accepts the usual truthy/falsy spellings, not just the exact string 'true'
…me, bound rate-limit memory - main: the stale-worker reaper no longer deletes an ENROLLED worker's row after 1h offline (that dropped its per-worker key and locked it out permanently on the next reboot); enrolled workers are only marked offline, removal is a UI action. Per-worker error boundary so one bad timestamp can't abort the whole sweep - main: health check now records check_down for a known non-external service that vanishes from an online worker's heartbeat, instead of freezing its score green - main: login rate-limit dict releases keys when attempts age out / on success, so it can't grow unbounded per distinct client IP - exchange_rates: per-source freshness — crypto/fiat staleness tracked independently so one source failing can't mislabel the other (additive crypto_stale/fiat_stale keys; existing shape unchanged) - collectors/storj: an unrecognized API response now raises (surfaced via collector alerts) instead of silently recording $0
…release path filter - release.yml: the change-detection filter now includes app/routers, app/deps, app/metrics, app/setup_token — fixes to those (incl. auth/API-key guards) now actually trigger a release + image build instead of silently not shipping - docs: document the first-run setup token; correct CASHPILOT_COLLECT_INTERVAL (name+minutes) and CASHPILOT_PORT (worker-only); fix the 'no port forwarding on the worker' claim + document CASHPILOT_WORKER_URL; update SECURITY.md to the per-worker fleet-key model + proxy/cookie/session env vars; fix architecture.md (python:3.14-alpine/su-exec, workers table, no worker SQLite); add the Prometheus guide to the nav; CLAUDE.md routers/deps/setup_token/metrics + port 8080 + 15 collectors; refresh the docker-compose pin example to 1.0.1
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
==========================================
+ Coverage 92.63% 93.04% +0.41%
==========================================
Files 31 31
Lines 3243 3276 +33
==========================================
+ Hits 3004 3048 +44
+ Misses 239 228 -11
🚀 New features to boost your workflow:
|
1 participant
Follow-ups from a 12-lens whole-repo audit (security-weighted). Every critical/high finding was adversarially verified before inclusion. This PR fixes the bounded, clearly-correct security/correctness/docs findings; the larger refactors and one product decision are tracked as issues (see below) rather than bundled here.
Security (commit 19a39d7)
network_mode: hostare now permitted only if a realservices/*.ymlcatalog entry declares them (derived live from the catalog, fail-closed), replacing a 3-entry capability denylist that missedSYS_MODULE/DAC_READ_SEARCH/etc. Blocked bind-mount roots expanded (/mnt,/media,/opt,/srv,/data,/tmp). Added the missing auth-rejection test on the socket-privileged deploy endpoint. (Verified the worker image shipsservices/+catalog.py, so mysterium's NET_ADMIN + host-network deploy still passes.)fleet.html:esc()now escapes quotes, so a worker name can't break out ofdata-worker-name="…".CASHPILOT_SECURE_COOKIEaccepts the usual truthy/falsy spellings.Reliability (commit 916b357)
check_downfor a known non-external service missing from an online worker's heartbeat, instead of freezing green.Docs + CI (commit 068e007)
release.ymlchange-detection filter was stale — it omittedapp/routers,app/deps,app/metrics,app/setup_token, so fixes to those (including auth/API-key guards) never triggered a release/image build. Now included.CASHPILOT_COLLECT_INTERVAL/CASHPILOT_PORTcorrected; worker reachability +CASHPILOT_WORKER_URL; SECURITY.md per-worker fleet-key model; architecture.md (python:3.14-alpine/su-exec, workers table); Prometheus guide added to nav; CLAUDE.md refreshed; compose pin example → 1.0.1.Verification
ruff check .+ruff format --check .clean; 1142 tests pass (regression tests added for every code fix above).Tracked separately (not in this PR)
Larger refactors + one product call, filed as issues: auth-revocation durability across restart (needs a schema migration — highest-priority follow-up), worker
client_idprotocol, SSRF DNS-rebind TOCTOU, CSPunsafe-inline/inline-onclick refactor, main.py/app.js god-module + duplicate-deploy-path consolidation, DB connection-proxy cleanup, perf batch,:latestpolicy decision for our own images, test-coverage hardening, dead-code removal.Not auto-merging — flagging for review since this touches the just-shipped v1.0.0 fleet-key code.