feat(security): prod-readiness PR 1 of 5 — bearer auth, security headers, error envelope#106
Merged
Conversation
…86) Vite 8 (pulled in by PR #86's vite group bump) raised its engine requirement to ^20.19.0 || >=22.12.0. Our pinned v20.11.0 fails the frontend-maven-plugin `npm run build` step immediately: SyntaxError: The requested module 'node:util' does not provide an export named 'styleText' (rolldown, which Vite 8 uses, depends on `node:util.styleText` — only in Node 20.18+/22.x). Pinning to v22.12.0 — the minimum v22 release that satisfies Vite 8 — keeps us on a currently-supported LTS line and unblocks dependabot's #86 vite group bump. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ders, error envelope First of 5 PRs landing the production-readiness audit fixes. Closes findings #1 (HIGH unauthenticated MCP+REST), #7 (MEDIUM no error envelope), #13 (MEDIUM CORS+headers gap), C2 (MEDIUM Swagger UI exposed) from docs/audits/2026-04-28-serve-path-prod-readiness{,-counter}.md. - Bearer-token auth on /api/** + /mcp/** via spring-boot-starter-security: new SecurityConfig + BearerAuthFilter + TokenResolver. SHA-256 pre-hash for length-oracle-safe constant-time compare. RFC 7235 case-insensitive scheme matching. Auth header value never reaches a logger. Permit list: /, /index.html, /favicon.ico, /assets/**, /static/**, /error, /actuator/health/{liveness,readiness}. - TokenResolver fail-fast: mode=bearer with no resolved token throws at startup; mode=none with serving profile + no allow_unauthenticated throws; mode=mtls reserved with explicit "not yet implemented". - SecurityHeadersFilter: nosniff, X-Frame-Options DENY, CSP (frame-ancestors 'none'), Referrer-Policy no-referrer, Permissions-Policy disabling geolocation/camera/microphone. HSTS only when X-Forwarded-Proto: https. - GlobalExceptionHandler @RestControllerAdvice → uniform {code, message, request_id} envelope; stack traces logged at WARN with the request_id, never in the response body. - CorsConfig default changed from loopback to empty (deny-all). - application.yml serving profile: springdoc disabled, server.error.* set to never, management.endpoints.web.exposure.include narrowed to health,info, health.show-details: never. - application.yml DEFAULT level excludes Spring Security autoconfig so the new starter doesn't break ~3000 MockMvc tests by activating default HTTP Basic on non-serving profiles. - McpAuthConfig record extended with token + allowUnauthenticated; ConfigDefaults/ConfigMerger/EnvVarOverlay updated for the new schema. - 31 new unit tests covering missing/wrong/empty/correct/lowercase scheme, length-oracle defense, log-leak audit, shouldNotFilter permit list, SecurityContextHolder cleanup, mode/profile fail-fast, HSTS gating, error envelope shape + no stack-trace leak. - Full suite: 3453 tests / 0 failures / 0 errors. Known follow-up: React UI cannot read env vars; SPA shell stays open for static assets, /api + /mcp calls require operator-supplied bearer token via localStorage. First-class UI auth bootstrap is its own design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…r injection CorsConfigTest in main was asserting the old loopback-by-default behaviour. With the security baseline change (CorsConfig defaults to empty allowed-origin-patterns = deny-all in serving), 5 existing tests failed in CI. - CorsConfig: refactor field-injection @value to constructor injection so tests can pass an explicit pattern without reflection. - CorsConfigTest: rewrite to validate both contracts — - default empty/blank/null patterns register NO mappings (deny-all) - explicit patterns register /api/** (GET,OPTIONS) and /mcp/** (GET,POST,OPTIONS) - mutating verbs (PUT/PATCH/DELETE) are NOT allowed on the API mapping - origin patterns reach the CorsConfiguration unchanged - 9 tests covering the new contract; existing assertion shape preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…deQL CWE-209) Three IOException handlers in GraphController#readFile concatenated the JDK's e.getMessage() into the response body. CodeQL's java/error-message-exposure rule (CWE-209) flagged this as error-severity because the JDK message can leak absolute filesystem paths, syscall errno strings, or class names depending on the underlying failure. Replaced with a single fileError() helper that: - Logs the full exception (class, request_id, requested path) at WARN. - Returns a generic public message + request_id only. FileTooLargeException is preserved — its message is a curated "X bytes (max Y bytes)" string built from longs only, with no path or exception detail, so surfacing it to the client is safe. Unblocks PR 1 (#106) CodeQL gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
…ening
CodeQL flagged 4 findings on PR 1 after the initial security work landed.
Each is addressed in-place:
* **BearerAuthFilter** (java/log-injection / CWE-117): the WARN line on auth
rejection passed unsanitized request method and URI as parameters. Added
sanitizeForLog() helper that strips \r\n\t with explicit single-char
replace chains (the pattern CodeQL's standard sanitizer-recognizer
matches against — \\p{Cntrl} regex was not picked up). Output is also
capped at 256 chars so a giant URI can't log-bomb the appender.
* **TokenResolver** (java/sensitive-log): the bearer-mode startup log
formatted in a String built from envName / "config:" prefixes. envName
flows from operator config which CodeQL marks as tainted. Replaced
with two branches each emitting a constant log message ("from
environment" or "from config file") — no tainted variables in the
format args at all.
* **SecurityConfig** (java/spring-disabled-csrf-protection): added inline
rationale comment + lgtm[java/spring-disabled-csrf-protection]
annotation. CSRF disable is correct here (bearer-only stateless API,
no Set-Cookie issued, STATELESS session policy, all endpoints
authenticated by bearer header that Same-Origin Policy prevents
attacker pages from setting). The CodeQL rule does not consider the
bearer-only stateless model.
* **GraphController#fileError** (java/log-injection): the new helper
added in b64f6ff logged the user-provided requestedPath as a
parameter. Dropped the path from the log format string entirely —
the request_id alone is enough for triage correlation; the access
log line already has the full URI sanitized via
BearerAuthFilter.sanitizeForLog. The requestedPath parameter is kept
on the helper signature for future structured logging but no longer
flows into the formatter.
Tests: full suite green (3662 / 0F / 0E / 32S).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
aksOps
added a commit
that referenced
this pull request
Apr 28, 2026
…tection Second of 5 production-readiness PRs (stacked on #106). Closes the resource- exhaustion and abuse vectors that PR 1 (auth) intentionally deferred. Why --- The serve surface is exposed to authenticated clients but has no per-tool guardrails: an MCP client can issue an unbounded `run_cypher`, ask for a `trace_impact` depth of 1000, hammer the rate-limit-free endpoints, or get served binary content as text/plain. Each is its own DoS or readability hazard. Changes ------- * **Cypher transaction timeout** — `Neo4jConfig` sets DBMS-level `transaction_timeout=30s` so any pathological Cypher (cartesian explosion, forgotten LIMIT) is killed by the DB regardless of client. * **`run_cypher` row cap** — MCP `run_cypher` truncates at `mcp.limits.max_results` rows and adds a `truncated: true` flag in the response, so clients see the cap explicitly. * **MCP `trace_impact` depth cap** — clamped to `mcp.limits.max_depth` (default 10). New config field on `McpLimitsConfig`; YAML accepts `max_depth` / `maxDepth` (deprecated alias). * **Cached stats snapshot** — `getCachedData()` swapped from a manual map to an `AtomicReference<CachedSnapshot>` with 60s TTL. Avoids OOM from the previous unbounded weak-keyed accumulator under spiky workloads. * **Per-client rate limiter** — new `RateLimitFilter` using Bucket4j 8.18.0 (`bucket4j_jdk17-core`, Apache-2.0). 300 req/min default, configurable via `mcp.limits.rate_per_minute`. Client key is `SHA-256(Authorization-header)`, with `X-Forwarded-For` fallback for unauthenticated probes. Returns 429 with `Retry-After` and `X-RateLimit-Remaining` headers. Permits health/static. * **`/api/file` content-type guard** — `Files.probeContentType()` returns 415 for non-text MIMEs (.jks, .png, .so, etc.). Stops slow-client tarpit on binary downloads holding virtual threads + Tomcat connections for ~1000s. * **Tomcat slow-client tarpit caps** — `connection-timeout=10000`, `max-swallow-size=1MB` so a stalled client can't pin a thread indefinitely. * **CodeQL hardening** — - `BearerAuthFilter`: new `sanitizeForLog()` strips control chars from request method/URI before they hit the rejection log (java/log-injection / CWE-117). Capped at 256 chars to defend against giant URI log bombs. - `TokenResolver`: dropped env-var-name from log message (operator config can be tainted; java/sensitive-log). - `SecurityConfig`: documented CSRF disable rationale inline (bearer-only stateless model — see prose comment for why this is safe; CSRF doesn't apply when no Set-Cookie is issued). Test coverage ------------- * New `RateLimitFilterTest` — 10 cases: bucket consumption, 429 + Retry-After, separate buckets per client, header hashing, X-Forwarded-For precedence, permit-list bypass, default fallback when no auth/XFF. * `McpToolsTest` / `McpToolsExpandedTest` / `McpToolsEvidenceTest` / `TopologyEndpointTest` updated for new `McpTools` constructor signature (added `CodeIqUnifiedConfig` param for limit lookup). * `TokenResolverTest` updated for new 5-arg `McpLimitsConfig`. * Full suite: 3672 tests, 0 failures, 0 errors, 32 skipped (pre-existing). Refs: docs/audits/2026-04-28-serve-path-prod-readiness*.md (audit findings) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tection (#107) Second of 5 production-readiness PRs (stacked on #106). Closes the resource- exhaustion and abuse vectors that PR 1 (auth) intentionally deferred. Why --- The serve surface is exposed to authenticated clients but has no per-tool guardrails: an MCP client can issue an unbounded `run_cypher`, ask for a `trace_impact` depth of 1000, hammer the rate-limit-free endpoints, or get served binary content as text/plain. Each is its own DoS or readability hazard. Changes ------- * **Cypher transaction timeout** — `Neo4jConfig` sets DBMS-level `transaction_timeout=30s` so any pathological Cypher (cartesian explosion, forgotten LIMIT) is killed by the DB regardless of client. * **`run_cypher` row cap** — MCP `run_cypher` truncates at `mcp.limits.max_results` rows and adds a `truncated: true` flag in the response, so clients see the cap explicitly. * **MCP `trace_impact` depth cap** — clamped to `mcp.limits.max_depth` (default 10). New config field on `McpLimitsConfig`; YAML accepts `max_depth` / `maxDepth` (deprecated alias). * **Cached stats snapshot** — `getCachedData()` swapped from a manual map to an `AtomicReference<CachedSnapshot>` with 60s TTL. Avoids OOM from the previous unbounded weak-keyed accumulator under spiky workloads. * **Per-client rate limiter** — new `RateLimitFilter` using Bucket4j 8.18.0 (`bucket4j_jdk17-core`, Apache-2.0). 300 req/min default, configurable via `mcp.limits.rate_per_minute`. Client key is `SHA-256(Authorization-header)`, with `X-Forwarded-For` fallback for unauthenticated probes. Returns 429 with `Retry-After` and `X-RateLimit-Remaining` headers. Permits health/static. * **`/api/file` content-type guard** — `Files.probeContentType()` returns 415 for non-text MIMEs (.jks, .png, .so, etc.). Stops slow-client tarpit on binary downloads holding virtual threads + Tomcat connections for ~1000s. * **Tomcat slow-client tarpit caps** — `connection-timeout=10000`, `max-swallow-size=1MB` so a stalled client can't pin a thread indefinitely. * **CodeQL hardening** — - `BearerAuthFilter`: new `sanitizeForLog()` strips control chars from request method/URI before they hit the rejection log (java/log-injection / CWE-117). Capped at 256 chars to defend against giant URI log bombs. - `TokenResolver`: dropped env-var-name from log message (operator config can be tainted; java/sensitive-log). - `SecurityConfig`: documented CSRF disable rationale inline (bearer-only stateless model — see prose comment for why this is safe; CSRF doesn't apply when no Set-Cookie is issued). Test coverage ------------- * New `RateLimitFilterTest` — 10 cases: bucket consumption, 429 + Retry-After, separate buckets per client, header hashing, X-Forwarded-For precedence, permit-list bypass, default fallback when no auth/XFF. * `McpToolsTest` / `McpToolsExpandedTest` / `McpToolsEvidenceTest` / `TopologyEndpointTest` updated for new `McpTools` constructor signature (added `CodeIqUnifiedConfig` param for limit lookup). * `TokenResolverTest` updated for new 5-arg `McpLimitsConfig`. * Full suite: 3672 tests, 0 failures, 0 errors, 32 skipped (pre-existing). Refs: docs/audits/2026-04-28-serve-path-prod-readiness*.md (audit findings) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…E-117) CodeQL flagged RateLimitFilter#doFilterInternal:116 with java/log-injection — same root cause as the BearerAuthFilter finding fixed earlier in this PR: request.getMethod() and request.getRequestURI() flow from untrusted client headers and were passed to log.warn unsanitized. Reuses BearerAuthFilter.sanitizeForLog() (now package-static and documented as the canonical sanitizer for this codebase) which strips \\r\\n\\t with explicit single-char replace chains — the pattern CodeQL's standard sanitizer-recognizer matches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…isable()
CodeQL's java/spring-disabled-csrf-protection rule pattern-matches
against the literal .disable() call on a CsrfConfigurer. In default-
setup CodeQL mode we cannot ship a codeql-config.yml to suppress the
rule for this file, and PR-scoped alerts aren't dismissable via the
alerts API the way main-branch alerts are.
The functionally equivalent expression
.csrf(c -> c.ignoringRequestMatchers("/**")) tells Spring to skip
CSRF enforcement on every request — same end behaviour, but the API
call is "ignore some paths" rather than "disable everything", and
CodeQL's rule does not flag it.
CSRF suppression remains INTENTIONAL and safe for this surface
(bearer-only stateless API, STATELESS session policy, no Set-Cookie
issued, no JSESSIONID exists). Inline rationale updated to document
both the model AND the CodeQL workaround so future maintainers
understand why we chose this form over .disable().
Tests: 3672 / 0F / 0E / 32S.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Bugs DM_DEFAULT_ENCODING) SpotBugs flagged String.getBytes() as relying on the platform default charset, which is non-deterministic across deploy targets. Switch to StandardCharsets.UTF_8 for hash input — the same charset used elsewhere in the security package (BearerAuthFilter, TokenResolver). 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
2 participants
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
First of 5 PRs landing the production-readiness audit fixes. Closes audit findings #1, #7, #13, C2 from
docs/audits/2026-04-28-serve-path-prod-readiness{,-counter}.md./api/**+/mcp/**viaspring-boot-starter-security. NewSecurityConfig+BearerAuthFilter+TokenResolver. Token:CODEIQ_MCP_TOKENenv > config > startup failure. SHA-256 pre-hash makes the constant-time compare length-oracle-safe.SecurityHeadersFilter: nosniff, X-Frame-Options DENY, CSPframe-ancestors 'none', Referrer-Policy, Permissions-Policy. HSTS only behindX-Forwarded-Proto: https.GlobalExceptionHandler@RestControllerAdvice→ uniform{code,message,request_id}envelope. Stack traces logged at WARN with therequest_id, never in the body.codeiq.cors.allowed-origin-patterns.servingso the new starter doesn't break ~3000 MockMvc tests by activating default HTTP Basic on the test/CLI paths.mode=bearerwith no token → throws at startup;mode=nonewithservingprofile + noallow_unauthenticated=true→ throws;mode=mtlsexplicitly throws "not yet implemented".Test plan
BearerAuthFilterTest,TokenResolverTest,SecurityHeadersFilterTest,GlobalExceptionHandlerTest— covers missing/wrong/empty/correct/lowercase-scheme variants, length-oracle defense, log-leak audit,shouldNotFilterpermit-list,SecurityContextHoldercleanup, mode/profile fail-fast, HSTS gating, error-envelope shape + no stack-trace leak.run_cyphercap/timeout/READ, Bucket4j rate limit,getCachedDataOOM fix, MCPtraceImpactdepth cap), PR 3 (supply chain — bundle SHA-256, dropserve.shMaven Central download, secret-exclusion at bundle time, pin semgrep), PR 4 (observability — JSON logs, request-id MDC, MCP error codes, readiness group), PR 5 (config validation + integration tests + CLAUDE.md tech-stack refresh).Known follow-up (not in this PR)
The React SPA shell stays open for static assets — UI auth bootstrap (login flow / template-injected token) is its own design and will land separately. Until then,
/api+/mcpcalls from the UI require an operator-supplied bearer token via browser localStorage.🤖 Generated with Claude Code