Skip to content

Security hardening: auth, CSRF, CI, and threat model#1

Merged
jameswnl merged 6 commits into
mainfrom
security-fixes
Mar 30, 2026
Merged

Security hardening: auth, CSRF, CI, and threat model#1
jameswnl merged 6 commits into
mainfrom
security-fixes

Conversation

@jameswnl

Copy link
Copy Markdown
Owner

Summary

Addresses all findings from security_analysis.md:

  • Per-launch auth token — random secrets.token_urlsafe(32) generated at startup, required on all API endpoints via Authorization: Bearer header. Token is embedded in the HTML page served to the browser.
  • Host header validation — rejects requests with non-localhost Host headers (DNS rebinding protection)
  • Origin validation on POSTs — CSRF protection for side-effecting endpoints
  • Session validation on /api/resume — verifies session_id and dirname exist in the current dataset before launching a terminal
  • CI hardened — all GitHub Actions pinned to full commit SHAs, default permissions reduced to contents: read, badge generation isolated to a separate job with minimal contents: write
  • Menu bar fix — removed generic lsof/kill fallback that could kill unrelated processes on the same port
  • README — added Security section documenting the local threat model and all measures

Test plan

  • All 110 tests pass
  • New tests: auth rejection (missing token, wrong token), session validation rejection, HTML embeds token
  • Existing tests updated to include auth headers
  • Manual: start server, verify token printed, verify API rejects curl without token
  • Manual: verify dashboard still works end-to-end in browser

🤖 Generated with Claude Code

jameswnl and others added 4 commits March 29, 2026 00:04
…tection

Addresses findings #1 and #2 from security_analysis.md:

- Generate random per-launch auth token (secrets.token_urlsafe)
- All API endpoints require Bearer token in Authorization header
- HTML page embeds token and JS includes it in all fetch() calls
- Host header validation rejects non-localhost requests (DNS rebinding)
- Origin header validation on POST requests (CSRF protection)
- POST /api/resume validates session_id and dirname exist in dataset
- Token printed at startup for debugging

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses finding #3 from security_analysis.md:

- Pin all GitHub Actions to full commit SHAs (with version comments)
- Reduce default workflow permissions to contents: read
- Move badge generation to separate job with minimal contents: write
- Only the badge job (on main push) gets write permissions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses low-severity finding from security_analysis.md:

- Remove generic lsof/kill fallback that could kill unrelated processes
- Only stop the subprocess the menubar app actually started
- Add kill() fallback if terminate() doesn't work within timeout

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses recommendation from security_analysis.md to document the
threat model. Lists all security measures: auth token, host validation,
origin validation, session validation, secret masking, and CI hardening.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jameswnl

Copy link
Copy Markdown
Owner Author

Security review follow-up:

  1. Medium: the new bearer token is still disclosed to any unauthenticated local client because / serves HTML that embeds the live AUTH_TOKEN. That means any local process that can reach 127.0.0.1:8420 can fetch /, extract the token, and then call /api/data or /api/resume. This is a meaningful improvement for hostile web origins, but it does not fully address the broader localhost-client threat model from security_analysis.md.

  2. Low: the new Host and Origin protections are not covered by tests. I could not find regression tests that send an invalid Host header or hostile Origin, so the controls that most directly address the DNS-rebinding / CSRF findings can regress silently.

AI models used for this review:

  • GPT-5.4 for the primary security analysis and PR review
  • a faster helper model for initial repository mapping and context gathering

…in tests

Addresses PR review feedback:

1. HTML page (/) now requires auth — no longer exposes token to
   unauthenticated local clients. Flow: browser opens
   /?token=<secret>, server sets HttpOnly SameSite=Strict cookie
   and redirects to /. Subsequent requests use cookie automatically.

2. Added regression tests for Host and Origin validation:
   - test_rejects_invalid_host_header (DNS rebinding)
   - test_rejects_invalid_origin_on_post (CSRF)
   - test_allows_localhost_origin_on_post
   - test_allows_no_origin_on_post (non-browser clients)
   - test_html_page_requires_auth
   - test_html_page_token_url_sets_cookie

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jameswnl

Copy link
Copy Markdown
Owner Author

Addressed both review items in commit 4796fda:

1. Token disclosure on / (Medium)

  • / now requires authentication — unauthenticated GET / returns 401
  • Auth flow uses cookies: browser opens /?token=<secret>, server sets HttpOnly; SameSite=Strict cookie and redirects to /
  • Subsequent browser requests (including JS fetch) automatically include the cookie
  • Startup URL printed as http://localhost:8420/?token=<token> and opened in browser
  • A local process blindly fetching / can no longer extract the token

2. Missing Host/Origin validation tests (Low)
Added 4 regression tests:

  • test_rejects_invalid_host_header — verifies 403 for non-localhost Host (DNS rebinding)
  • test_rejects_invalid_origin_on_post — verifies 403 for hostile Origin (CSRF)
  • test_allows_localhost_origin_on_post — verifies legitimate Origin passes
  • test_allows_no_origin_on_post — verifies non-browser clients without Origin work

Plus 2 tests for the new auth-on-HTML behavior:

  • test_html_page_requires_auth — verifies unauthenticated / gets 401
  • test_html_page_token_url_sets_cookie — verifies cookie flow (302 + Set-Cookie)

All 115 tests pass.

@jameswnl

Copy link
Copy Markdown
Owner Author

Security review outcome: approved in principle.

The previously identified blocking findings are addressed, including:

  • authenticated HTML bootstrap instead of exposing the token to unauthenticated local clients
  • regression coverage for Host and Origin validation

One non-blocking nit:

  • test_html_page_token_url_sets_cookie would be a little stronger if it validated the query-token bootstrap path without also sending an Authorization header, so the test proves the /?token=... flow on its own.

I couldn't submit a formal GitHub approval because the authenticated account is the PR author, and GitHub does not allow approving your own pull request.

Remove Authorization header from test_html_page_token_url_sets_cookie
so the test proves the /?token=... bootstrap path works on its own,
without relying on a Bearer token as a fallback.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jameswnl jameswnl merged commit 58f76a3 into main Mar 30, 2026
2 checks passed
@jameswnl jameswnl deleted the security-fixes branch March 30, 2026 01:38
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