Skip to content

Security hardening: auth, RBAC, CORS, rate limiting, validation#8

Merged
sebastientaggart merged 1 commit intomainfrom
feature/security-hardening
Apr 6, 2026
Merged

Security hardening: auth, RBAC, CORS, rate limiting, validation#8
sebastientaggart merged 1 commit intomainfrom
feature/security-hardening

Conversation

@sebastientaggart
Copy link
Copy Markdown
Member

Implements all 6 security hardening tasks from issue #1:

  1. Required API key — auto-generates a write-scoped key on first run if none configured; removes the no-auth bypass
  2. RBAC with read/write scopes — GET routes require read, POST routes require write; read-only keys get 403 on write endpoints; supports multiple keys via config
  3. CORS middleware — locked to localhost / 127.0.0.1 origins (any port)
  4. Rate limiting — fixed-window per client IP, default 60 req/min, configurable via [rate_limit] rpm or DECKHAND_RATE_LIMIT_RPM
  5. WebSocket first-message auth — token no longer leaked in query params; client sends {type: "auth", token: "..."} after connect, server responds auth_ok or closes with 4001
  6. Input validation — action/signal payloads validated against registered payload_schema before reaching handler code; returns 422 with specific errors

Also fixes pre-existing lint issues (unused vars, stale imports) and updates bridge client + tests.

Closes #1

…handshake, input validation

- Make API key required (auto-generate on first run if unconfigured)
- Add RBAC with read/write scopes (GET=read, POST=write, 403 on insufficient scope)
- Add CORS middleware locked to localhost/127.0.0.1 origins
- Add fixed-window rate limiting per client IP (configurable rpm)
- Move WebSocket auth from query param to first-message handshake
- Validate action/signal payloads against registered schemas before handler execution
- Update OpenDeck bridge client for new WS auth protocol
- Fix pre-existing lint issues (unused vars, missing imports, f-string)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sebastientaggart sebastientaggart linked an issue Apr 6, 2026 that may be closed by this pull request
@sebastientaggart
Copy link
Copy Markdown
Member Author

Review Summary

Verdict: APPROVE

Findings

  • [WARNING] RateLimiter memory growthRateLimiter._windows (defaultdict(list)) never evicts keys for IPs that stop making requests. Each unique client IP creates an entry that persists for the lifetime of the process. In a localhost-only deployment this is benign, but if the service is ever exposed to varied IPs (e.g., behind a proxy logging distinct X-Forwarded-For values), the dict will grow without bound. Consider periodically pruning keys whose hit list is empty after the sliding-window filter. (src/deckhand/security.py, lines 100-116)

  • [WARNING] WebSocket auth does not enforce scope — The WS /events endpoint validates the API key but accepts any scope. A read-only key can subscribe to the full event stream, which may include write-side events (agent status changes, action results, etc.). If the intent is that read-scoped keys should be able to subscribe to events this is fine, but it is worth documenting explicitly since every other endpoint enforces scope. (src/deckhand/main.py, line 510)

  • [NOTE] validate_payload ignores extra fields — The validator only checks declared schema fields; it does not flag unknown keys in the incoming payload. This is a reasonable "open" validation strategy but means typos in field names (e.g., actve instead of active) will be silently accepted. Acceptable as-is, just calling it out.

  • [NOTE] _generated_key accessed across module boundarymain.py reads settings._generated_key (a name-mangled-by-convention private attribute). Consider exposing it via a property or a public attribute to make the API contract clearer. (src/deckhand/main.py, line 69; src/deckhand/config/settings.py)

@sebastientaggart sebastientaggart merged commit 70d6655 into main Apr 6, 2026
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.

Security hardening: auth, RBAC, CORS, rate limiting, validation

1 participant