fix(server): safe-by-default HTTP boundary + tests (CRITICAL)#28
Merged
Conversation
Previously a fresh `npx augur-server` or `docker compose up` shipped:
- HOST=0.0.0.0 (listens on every interface)
- AUGUR_API_KEY unset (no auth required)
- cors:true (Access-Control-Allow-Origin reflects any origin)
- /admin/clear, /admin/stats, DELETE /traces open to anyone who
could reach the port
In combination, any operator who started the server before deciding
on an auth posture exposed destructive endpoints to their LAN. This
patch makes "I didn't configure anything" a safe state instead.
Library changes (`buildServer`):
- cors default → `false` (was `true`). Opt in explicitly.
- Destructive endpoints (/admin/*, DELETE /traces) return 503
"admin endpoints disabled" until apiKey is configured. They
never run unauthenticated, even from loopback.
- When apiKey is set, every non-public endpoint requires the header
(admin endpoints included). Auth rejection now uses `return reply`
so the intent is unambiguous.
CLI changes (`augur-server`):
- HOST default → 127.0.0.1 (was 0.0.0.0). Loopback-only out of
the box. Set HOST=0.0.0.0 explicitly to expose on the LAN.
- New AUGUR_CORS env var: comma-separated allowlist or `*` for any.
- Loud warning at startup when binding non-loopback without an
API key, and when CORS reflects every origin.
Docker:
- docker-compose.yml binds the published port to 127.0.0.1:3001
so a default `docker compose up` doesn't publish on the LAN.
- HOST=0.0.0.0 inside the container (single process, no exposure
surface there).
- Commented AUGUR_API_KEY placeholder.
Tests (`packages/server/src/server.test.ts`, 22 new):
- Public routes (/health, /openapi.json, /docs) reachable without
auth, even when apiKey is set.
- /search /index /traces require apiKey when configured;
correct / wrong / missing headers all covered.
- /admin/* and DELETE /traces return 503 when apiKey is unset.
- /admin/* and DELETE /traces still require the header when set.
- Query-string variants don't bypass the destructive check.
- CORS: default false, explicit string origin allowed, other
origins rejected.
- Input validation: /search missing query → 400, /index non-array
documents → 400.
The server now has its first test file (was zero coverage before).
prepublishOnly runs tests; tests run on every CI build.
Total: 192 core tests + 22 server tests, all passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Address the critical server-defaults finding from the code review. Before this patch, a fresh
npx augur-serverordocker compose upexposed destructive admin endpoints to whoever could reach the port. After: doing nothing yields a safe state, and only explicit choices open the door.What changed
Library (
buildServer)corsdefaults tofalse(wastrue= reflect any origin)./admin/*andDELETE /tracesreturn 503 untilapiKeyis configured. They never run unauthenticated, even on loopback.apiKeyis set, every non-public endpoint requiresx-api-key(admin included). Auth rejection usesreturn replyfor clarity.CLI (
augur-server)HOSTdefaults to127.0.0.1(was0.0.0.0). Bind LAN-wide only by explicit opt-in.AUGUR_CORSenv var: comma-separated allowlist or*.Docker
docker-compose.ymlpublishes the host port to127.0.0.1:3001only.HOST=0.0.0.0inside the container (correct for a single-process container; the published-port restriction limits exposure).AUGUR_API_KEYcommented in for easy enable.Tests —
packages/server/src/server.test.ts, 22 new tests, the first tests this package has ever had:/admin/*+DELETE /traces→ 503 when key unset./admin/*+DELETE /traces→ 401 when key set but missing/wrong header.Test plan
pnpm build→ cleanpnpm typecheck→ cleanpnpm test→ 192 core + 22 server, all passingnpx augur-server→ server listens on127.0.0.1:3001,/admin/clearreturns 503.AUGUR_API_KEY=k npx augur-server→/admin/clearreturns 401 without header, 200 withx-api-key: k.HOST=0.0.0.0 npx augur-server(no key) → loud warning logged.Notes
AUGUR_API_KEYand an explicitHOST. Users who relied on the implicitHOST=0.0.0.0need to setHOST=0.0.0.0(now an explicit choice).🤖 Generated with Claude Code