backend: require the API key on all signal endpoints - #6
Open
cocopollo wants to merge 1 commit into
Open
Conversation
Only POST /ingest and POST /agents/heartbeat verified X-API-Key. These did
not:
GET /signals
GET /stats
GET /agents
PATCH /signals/{signal_id}/status
backend.py binds 0.0.0.0:8443 when run directly, so anyone able to reach it
could read every detection and, via PATCH, mark signals resolved. Because
GET /signals defaults to status=open, which is what the console shows, a
resolved signal disappears from the operator's view.
Extract the existing constant-time check into a require_api_key dependency
and apply it to all six data endpoints, including the two that already had
it inline so there is a single auth path. /health stays public for
monitoring.
The web UI now prompts for the key on first load, keeps it in
sessionStorage for the lifetime of the tab, and sends it as X-API-Key on
every request; a 401 clears the stored key so the next load re-prompts.
updateSignalStatus also checks the response, which it previously ignored,
so a rejected update no longer looks like it succeeded.
Also fixes the test fixture: it set an API key of "test-api-key" (12
chars), below the 16-character minimum enforced at import, so the backend
suite could not run at all. CI has no Python job, so this went unnoticed.
Tests: 6 passed (5 new covering missing key, wrong key, the resolve-without-key
case, the authenticated happy path, and /health staying public).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
backend.pychecksX-API-KeyonPOST /ingestandPOST /agents/heartbeat. These four are not checked at all:GET /signalsGET /statsGET /agentsPATCH /signals/{signal_id}/statusRun directly, the server binds
0.0.0.0:8443(L520). So anyone who can reach it reads every detection: host IDs, actor and target paths, hashes, argv, process trees.The
PATCHis the worse one.GET /signalsdefaults tostatus=open, and that's the view the console renders, so resolving a signal removes it from the operator's screen. Someone on a monitored endpoint can look up which of their actions tripped a rule and then clear those alerts, without the key.That cuts against what SECURITY.md sets out as the goal:
Repro on current main
With this patch the
PATCHreturns 401 and the signal stays instatus=open.Changes
backend.py: pulled the existing constant-time check out into arequire_api_keydependency and applied it to all six data endpoints./ingestand/agents/heartbeatare switched over too, so there's one auth path instead of an inline check in some handlers and nothing in others./healthstays open for monitoring.static/index.html: the UI needs to send the key now. It prompts on first load, keeps it insessionStoragefor the life of the tab, and attaches it via a smallapiFetchwrapper. A 401 clears the stored key so a reload re-prompts.updateSignalStatusnow checks the response too, which it wasn't doing, so a rejected update no longer looks like it worked.test_backend.py: five tests covering missing key, wrong key, the resolve-without-key case above, the authenticated path, and/healthstaying public.This also fixes the fixture. It set
SANTAMON_API_KEYto"test-api-key", 12 chars, under the 16 char minimum enforced at import, so the module raisedRuntimeErroron collection and the backend suite couldn't run at all. There's no Python job in CI, so nothing caught it.README.md: marked the newly gated endpoints, noted/healthis intentionally public, described the UI prompt.Testing
Same command on main fails with
RuntimeError: SANTAMON_API_KEY must be at least 16 characters long.A couple of notes
sessionStorageplus aprompt()is the smallest thing that keeps the console working once the API is gated. If you'd rather it sat behind a reverse proxy or a real login, I'll drop the UI part and just gate the API.Unrelated but nearby: run without
cert.pemand it falls back to plain HTTP while still binding0.0.0.0(L516-520). It does warn, but the key goes over the wire in clear. Can open that separately if useful.