Skip to content

Security review fixes (Opus audit)#2

Merged
datariot merged 3 commits into
mainfrom
fix/security-review
Jul 7, 2026
Merged

Security review fixes (Opus audit)#2
datariot merged 3 commits into
mainfrom
fix/security-review

Conversation

@datariot

@datariot datariot commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Addresses all actionable findings from the Opus security review. Split across dependency bumps + four scoped areas; every fix verified with golangci-lint (v2.12.2, matching CI), full -race suite, and govulncheck.

HIGH

  • grpc CVE GO-2026-4762 (authz bypass via missing leading slash in :path) — bumped grpc-go v1.75.1 → v1.79.3. Was the only reachable CVE undermining path/method authz.
  • Streaming RPCs were unauthenticated — the JWT bundle only shipped a unary interceptor; the framework's separate stream chain got nothing, so every server-streaming/bidi method ran with no auth. Added StreamServerInterceptor() sharing one auth helper with the unary path.

MEDIUM

  • otel/sdk RCE GO-2026-4394 (PATH-hijack in resource detection) — bumped otel/sdk v1.38.0 → v1.44.0.
  • /health leaked internal topology — raw driver errors (dial tcp 10.0.1.5:5432: ...) were served unauthenticated. Public endpoint bodies are now redacted (name + status only); full detail still goes to logs. ⚠️ This changes the /health, /health/ready, /health/live response body — the per-check error field is gone.
  • httpclient leaked API-key across cross-host redirects — Go strips Authorization but not custom headers on cross-domain redirects. Added a CheckRedirect that drops Authorization + the API-key header on host change.
  • /metrics unauthenticated by default — added opt-in basic auth (constant-time compare) via HTTPServerConfig; warns when unauthenticated in production. Default behavior unchanged.
  • pprof served in staging — gate was IsProduction()-only, and the staging branch warned but never returned, so pprof actually registered in staging. Now blocked outside development.

LOW

  • gRPC reflection now requires explicit EnableReflection (was implicitly on in dev, and dev is the default env).
  • configloader redacts connection-string values (DATABASE_URL/REDIS_URL) by name and by URL-userinfo parsing.
  • RequireHTTPS no longer trusts X-Forwarded-Proto unless TrustedProxyHeader opt-in is set.
  • Redis rate-limiter uses unique ZSET members (was undercounting at same-nanosecond load).
  • httpclient error-log path now URL-sanitized (matched success path).
  • CORS adds Vary: Origin when reflecting an origin.
  • opt-in AllowedHosts SSRF guard on httpclient request paths.

Verified sound (no change needed)

JWT algorithm confusion (HS256 pinned), 32-byte secret minimum, full claim validation, no-cache of server-side validation, atomic Lua lock unlock, no dynamic SQL, httpclient TLS floor, CORS wildcard+credentials rejection.

Known residual (follow-up, not in this PR)

AllowedHosts validates the initial URL only, not redirect targets — a redirect to a disallowed host isn't blocked (though credentials are stripped on cross-host hops per the redirect fix).

govulncheck now reports 0 vulnerabilities affecting our code (was 2 reachable).

🤖 Generated with Claude Code

datariot and others added 3 commits July 6, 2026 18:38
Closes the two reachable vulnerabilities from the security review:
- GO-2026-4762: grpc-go authorization bypass via missing leading slash in :path
- GO-2026-4394: otel SDK code execution via PATH-resolved binary in resource detection

govulncheck now reports 0 vulnerabilities affecting our code.

Co-Authored-By: Claude Fable 5 <[email protected]>
JWT bundle:
- add StreamServerInterceptor so streaming RPCs are authenticated (shared auth helper with unary); was a silent auth gap (#2)
- RequireHTTPS no longer trusts X-Forwarded-Proto unless TrustedProxyHeader opt-in is set (#10)

httpclient bundle:
- strip Authorization + API-key header on cross-host redirects (#5)
- sanitize URL in error-log path, matching success path (#12)
- opt-in AllowedHosts SSRF guard on request and RawRequest paths (#14)

framework + health:
- redact raw dependency error strings from public health endpoint bodies; full detail stays in logs (#4)
- opt-in basic auth for /metrics with constant-time comparison; warn when unauthenticated in production (#6)
- block pprof outside development (was leaking in staging; old branch warned but never returned) (#7)
- add Vary: Origin when reflecting a CORS origin (#13)

config + configloader + redis:
- gRPC reflection now requires explicit EnableReflection, not implicit in dev (#8)
- redact connection-string values (DATABASE_URL/REDIS_URL) by name and URL-userinfo parsing (#9)
- unique rate-limiter ZSET members to prevent same-nanosecond undercount (#11)

govulncheck: 0 vulnerabilities affecting our code.

Co-Authored-By: Claude Fable 5 <[email protected]>
@datariot datariot merged commit 1b77120 into main Jul 7, 2026
3 checks passed
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