fix(verifier): fail-fast on missing/weak JWT secret (token-forgery P0) - #582
fix(verifier): fail-fast on missing/weak JWT secret (token-forgery P0)#582NeOMakinG wants to merge 2 commits into
Conversation
NewAuthService stored []byte(secret) with ZERO validation, and server.jwt_secret is omitempty in config — so an unset secret meant HS256 tokens were signed AND validated with an empty key, letting anyone forge a token for any public_key. Validate the secret (>= 32 chars, mirroring the agent-backend JWT_SECRET floor) and fail server startup if it's missing/weak, instead of silently shipping a forgeable verifier. Adds a rejection test + a test helper so the existing suite's short test secrets are padded to the new minimum without losing their identity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… dev/CI secrets
Codex review of the first commit caught two things:
1. internal/portal/auth.go NewPortalAuthService had the IDENTICAL forgery gap
(stored []byte(secret) with no validation). Apply the same >=32-char floor;
fatal at startup (the portal NewServer is a struct-literal ctor with no error
return, so internal fatal is the minimal correct fix).
2. The new validation would BREAK dev/CI startup — verifier.example.json
('mysecret'), CI workflow ('test-secret'), Makefile ('devsecret'/
'test-portal-secret'), and the portal test fixture all used <32-char secrets.
Bumped each to a clearly-non-production >=32-char value, and added
SERVER_JWT_SECRET to both docker-compose verifier services (env overrides the
mounted config via AutomaticEnv).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Closing — out of scope for our agent work. The finding (verifier JWT secret unvalidated → token-forgery if jwt_secret is unset) is real and documented in the spike notes for the verifier team to pick up, but it's not part of the agent stack we're driving. |
Fail-fast on a missing/weak verifier JWT secret (token-forgery P0)
The bug
NewAuthService(internal/service/auth.go) andNewPortalAuthService(internal/portal/auth.go) both stored[]byte(secret)with zero validation, andjwt_secretisomitemptyin the config. So an unset (or trivially short) secret meant HS256 tokens were signed AND validated with a zero-length key — letting anyone forge a token for anypublic_keyand bypass vault-ownership auth on every protected route.The agent-backend already guards this (
JWT_SECRET required:"true"+ a>= 32length check in its configValidate()); the verifier and portal did not — an asymmetry surfaced during a stack security audit.The fix
NewAuthServicenow returns(*AuthService, error)and fails if the secret is empty or< 32chars (mirroring the backend floor);server.gologrus.Fatalfs on that error.NewPortalAuthServicegets the same>= 32floor (fatal internally, since itsNewServeris a struct-literal constructor with no error return — a forgeable auth service must never boot).verifier.example.jsonmysecret, the CI workflowtest-secret,Makefiledevsecret/test-portal-secret, the portal test fixture) bumped to clearly-non-production>= 32-char values;SERVER_JWT_SECRETadded to bothdocker-compose.yamlverifier services.Verification (codex-reviewed)
A codex review of the first commit caught (and this PR fixes) two things that would otherwise have shipped broken: the identical gap in the portal auth, and the dev/CI startup breakage from the new validation rejecting the old short secrets.
receipts
This is a backend security fix — no UI. Receipt = the new rejection test + build/lint:
TestNewAuthService_RejectsWeakSecret(new) pins the validation.newAuthServiceForTesthelper that pads short test secrets to the new minimum while preserving the right-vs-wrong-secret distinctness in theValidateTokentests.gofmt/parse clean on all changed Go files; YAML + JSON configs validated.Out of scope (tracked separately)
The
auth.enabled=falseproduction-disable gap (internal/api/middleware.go:42) and the cosmos-signing verifier bypass are separate findings from the same audit, not addressed here.🤖 Generated with Claude Code