Skip to content

Commit 82ded68

Browse files
aksOpsclaude
andauthored
feat(security): prod-readiness PR 1 of 5 — bearer auth, security headers, error envelope (#106)
* chore(deps): bump Node from v20.11.0 to v22.12.0 for Vite 8 (unblocks #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> * checkpoint: pre-yolo 2026-04-28T07:25:13 * feat(security): production-readiness PR 1 — bearer auth, security headers, 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> * fix(cors): update CorsConfigTest for new deny-all default; constructor 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> * fix(security): scrub IOException details from /api/file responses (CodeQL 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> * fix(security): apply CodeQL log-injection + sensitive-log + CSRF hardening 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> * feat(limits): production-readiness PR 2 — resource limits & abuse protection (#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> * fix(security): sanitize request method/URI in RateLimitFilter log (CWE-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> * fix(security): use ignoringRequestMatchers('/**') instead of csrf().disable() 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> * fix(security): use UTF-8 charset in RateLimitFilter.sha256Short (SpotBugs 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c352720 commit 82ded68

33 files changed

Lines changed: 2225 additions & 79 deletions

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ If you hit something requiring GitHub App / PAT / OAuth that the runtime cannot
4646
<claude-mem-context>
4747
# Memory Context
4848

49-
# [codeiq] recent context, 2026-04-28 1:14am UTC
49+
# [codeiq] recent context, 2026-04-28 6:43am UTC
5050

5151
No previous sessions found.
5252
</claude-mem-context>

CHANGELOG.md

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,146 @@ for that specific tag for the per-commit details.
225225
path-B board ruling, they are not to be re-introduced without an explicit
226226
board reversal — see `shared/runbooks/engineering-standards.md` §5.1.
227227

228+
### Security
229+
230+
- **Production-readiness PR 1 of 5 — security baseline.** First half of the
231+
audit findings catalogued under `docs/audits/2026-04-28-serve-path-prod-readiness.md`
232+
(+ `-counter.md`). Closes audit findings #1, #7, #13 (HIGH/MEDIUM) and C2 (MEDIUM).
233+
- **Bearer-token auth on `/api/**` and `/mcp/**`** (audit #1). Added
234+
`spring-boot-starter-security`. New `config/security/SecurityConfig`,
235+
`BearerAuthFilter`, `TokenResolver`. Token source priority:
236+
`CODEIQ_MCP_TOKEN` env > `codeiq.mcp.auth.token` config > startup failure.
237+
Constant-time compare via SHA-256 pre-hash + `MessageDigest.isEqual`
238+
32-byte digests on both sides defeat the length oracle. RFC 7235 §2.1
239+
case-insensitive scheme matching (`Bearer`, `bearer`, etc.). Authorization
240+
header value never reaches a logger from this code. Permit list:
241+
`/`, `/index.html`, `/favicon.ico`, `/assets/**`, `/static/**`, `/error`,
242+
`/actuator/health/{liveness,readiness}` — everything else under
243+
`/api/**`, `/mcp/**`, `/actuator/**` requires the bearer token.
244+
- **Fail-fast on misconfiguration** (audit #14 partial). `mode=bearer` with
245+
no token resolved → throws at startup. `mode=none` with active `serving`
246+
profile and `allow_unauthenticated` not explicitly set → throws at
247+
startup. `mode=mtls` is reserved and explicitly throws "not yet
248+
implemented" rather than silently passing through.
249+
- **Defensive response headers** (audit #13). New
250+
`config/security/SecurityHeadersFilter` sets `X-Content-Type-Options:
251+
nosniff`, `X-Frame-Options: DENY`, `Content-Security-Policy: default-src
252+
'self'; ... frame-ancestors 'none'`, `Referrer-Policy: no-referrer`,
253+
`Permissions-Policy` disabling geolocation/camera/microphone.
254+
`Strict-Transport-Security: max-age=31536000; includeSubDomains` is set
255+
only when `X-Forwarded-Proto: https` is present (AKS terminates TLS at
256+
ingress) — setting HSTS over plain HTTP would lock out misconfigured envs.
257+
- **Uniform error envelope** (audit #7). New
258+
`api/GlobalExceptionHandler` (`@RestControllerAdvice`,
259+
`@Profile("serving")`) maps every uncaught exception to
260+
`{"code","message","request_id"}` with the right HTTP status.
261+
`IllegalArgumentException` → 400 with surfaced message.
262+
`ResponseStatusException` → status code passes through. Anything else →
263+
500 with generic message; the actual exception is logged at WARN with
264+
the `request_id` so on-call can correlate without leaking stack frames
265+
to the client. `application.yml` now sets
266+
`server.error.include-stacktrace: never` + `include-message: never` +
267+
`include-binding-errors: never` as belt-and-suspenders.
268+
- **Default CORS deny-all in serving** (audit #13). `config/CorsConfig`
269+
default changed from loopback patterns to empty. Empty means register
270+
no mappings → Spring MVC rejects all preflighted cross-origin requests.
271+
Operators who genuinely need cross-origin (e.g. dev with a separate
272+
Vite server on a different port) explicitly set
273+
`codeiq.cors.allowed-origin-patterns`. Logs the resolved state at
274+
startup. The React UI at `/` is unaffected — it's served same-origin.
275+
- **Swagger UI / api-docs disabled in serving** (counter-audit C2).
276+
`springdoc.api-docs.enabled: false` + `springdoc.swagger-ui.enabled: false`
277+
in the serving profile of `application.yml`. The OpenAPI schema is
278+
reconnaissance data; reachable only when running locally or with the
279+
indexing profile.
280+
- **`management.endpoints.web.exposure.include` narrowed** to `health,info`
281+
in serving (was `health,info,metrics`); `health.show-details: never`.
282+
Defense-in-depth alongside the `SecurityFilterChain` `authenticated()`
283+
rule on `/actuator/**`.
284+
- **Spring Security autoconfig excluded outside serving.** Without the
285+
`serving` profile (CLI, tests, IDE runs), Spring Security's default
286+
HTTP Basic chain would lock all endpoints — adding the starter would
287+
break ~3000 existing tests that pass through MockMvc with no token.
288+
`application.yml` excludes `SecurityAutoConfiguration`,
289+
`SecurityFilterAutoConfiguration`, `UserDetailsServiceAutoConfiguration`
290+
at the default level; the `serving` profile re-enables them by listing
291+
only `UserDetailsServiceAutoConfiguration` (so the auto user/password
292+
is suppressed but the filter chain is built from `SecurityConfig`).
293+
- **Tests:** 31 new unit tests across `BearerAuthFilterTest` (14 cases:
294+
missing/wrong/empty/correct/lowercase scheme, length-oracle defense,
295+
log-leak audit, `shouldNotFilter` paths, `SecurityContextHolder` cleanup),
296+
`TokenResolverTest` (9 cases for mode/profile/env-priority/fail-fast),
297+
`SecurityHeadersFilterTest` (5 cases for header presence/HSTS gating),
298+
`GlobalExceptionHandlerTest` (3 cases verifying the envelope shape and
299+
no stack-trace leak). Full suite: 3453 tests / 0 failures / 0 errors.
300+
301+
**Known follow-up (not in this PR):** the React UI cannot read env vars,
302+
so the SPA shell is unauthenticated to access static assets. API/MCP calls
303+
from the UI must inject `Authorization: Bearer <token>` from
304+
operator-supplied localStorage. A first-class UI auth bootstrap (login
305+
flow + token-issuance endpoint, OR server-side template injection) is its
306+
own design — tracked as a follow-up issue.
307+
308+
- **Production-readiness PR 2 of 5 — resource limits & abuse protection.**
309+
Closes audit findings #2, #3, C1 (HIGH) and #10, #11 (MEDIUM).
310+
- **Cypher transaction timeout** (audit #2). Neo4j embedded
311+
`GraphDatabaseSettings.transaction_timeout = 30s` configured in
312+
`Neo4jConfig` — every transaction in the JVM, including `run_cypher`
313+
and graph traversals, gets a hard wall-clock cap. Catches runaway
314+
variable-length matches before they starve the page cache.
315+
- **Result-set cap on `run_cypher`** (audit #2). Hard row cap at
316+
`mcp.limits.max_results` (default 500); excess rows dropped, response
317+
carries `truncated: true` + `max_results: N`. Defends the JVM heap
318+
against `MATCH (a),(b),(c) RETURN a,b,c LIMIT 999999999` blowups.
319+
- **MCP `traceImpact` depth cap** (audit #10 corrected, C3). New
320+
`mcp.limits.max_depth` field (default 10) wired into
321+
`McpTools.traceImpact` via `Math.min`. Defends against
322+
`RELATES_TO*1..1000` Cartesian explosions on hub nodes.
323+
- **TTL snapshot cache on topology tools** (audit C1). `McpTools.
324+
getCachedData()` now backed by a 60-second TTL snapshot. Without it,
325+
every concurrent `service_dependencies` / `blast_radius` /
326+
`find_path` / `find_bottlenecks` / `find_circular_deps` /
327+
`find_dead_services` / `find_node` call paid the full
328+
`graphStore.findAll()` cost and double-allocated multi-GB heaps.
329+
A bridge fix; the proper refactor (TopologyService → per-tool Cypher)
330+
is a tracked follow-up.
331+
- **Per-client rate limiter** (audit #3). New `RateLimitFilter` using
332+
Bucket4j 8.18.0 (Apache-2.0). Token bucket sized at
333+
`mcp.limits.rate_per_minute` (default 300). Keyed by SHA-256 hash of
334+
the `Authorization` header (so the token never lives in our key map),
335+
falls back to `X-Forwarded-For` (first hop) or `RemoteAddr`. 429
336+
response with `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining`
337+
headers. Registered before `BearerAuthFilter` so unauthenticated
338+
brute-force is also throttled.
339+
- **`/api/file` content-type sniff** (audit #11 corrected). Added
340+
`Files.probeContentType` guard — non-text MIMEs (`.jks`, `.so`,
341+
`.png`, native libs) return HTTP 415 with the probed type, instead
342+
of being served as garbled `text/plain`. Allowlist: `text/*`,
343+
`application/json`, `application/xml`, `application/x-yaml`,
344+
`application/javascript`. The byte cap (already enforced by
345+
`SafeFileReader`) is unchanged.
346+
- **Tomcat slow-client tarpit** (audit #11). `server.tomcat.connection-
347+
timeout: 10s`, `max-swallow-size: 1MB` in the serving profile —
348+
drops connections that hold a virtual thread + Tomcat connection at
349+
1 KB/s.
350+
- **CodeQL hardening on the security baseline.** Sanitised request
351+
method + URI before logging in `BearerAuthFilter` (CWE-117 / CodeQL
352+
`java/log-injection`); removed env-var name from the bearer-token
353+
bootstrap log line in `TokenResolver` (CodeQL `java/sensitive-log`);
354+
documented the deliberate stateless-bearer rationale on
355+
`SecurityConfig.csrf(disable)` (CodeQL `java/spring-disabled-csrf-protection`
356+
— no exploit path on a no-cookie surface).
357+
- **Tests:** new `RateLimitFilterTest` (10 cases: under/over limit,
358+
separate buckets per client, header-hashing, X-Forwarded-For
359+
precedence, permit-list, default-rate fallback). Existing 6 test
360+
classes updated for the new `McpTools` ctor signature. Full suite:
361+
3672 tests / 0 failures / 0 errors.
362+
363+
**Known follow-up:** TopologyService still walks the full snapshot
364+
in-memory after the cache hit — long-term plan is to rewrite each
365+
topology tool as a targeted Cypher query so the snapshot isn't needed.
366+
The cache is the bridge; the rewrite reduces peak memory.
367+
228368
## [0.1.0] - 2026-03-28
229369

230370
First general-availability cut. See the

CLAUDE.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,20 @@ bean for code paths that haven't been ported yet.
433433
- **Parallel agent conflicts**: Don't dispatch multiple agents editing the same files concurrently. Use worktree isolation or sequential execution.
434434
- **SonarCloud project key**: `RandomCodeSpace_codeiq`, org: `randomcodespace`
435435
- **CI workflow**: Single `ci-java.yml` runs build + SonarCloud analysis. No cross-platform builds needed (JVM).
436+
- **Spring Security only loads in the `serving` profile.** `application.yml` excludes `SecurityAutoConfiguration` + `SecurityFilterAutoConfiguration` + `UserDetailsServiceAutoConfiguration` at the **default** level so adding `spring-boot-starter-security` doesn't break ~3000 MockMvc tests by activating a default HTTP Basic chain. The `serving` profile re-enables them by listing only `UserDetailsServiceAutoConfiguration` (suppresses the auto user/password printout); the chain itself is built by `config/security/SecurityConfig`. **Don't** drop the default exclude — non-serving contexts (CLI, tests) must have no Spring Security wiring at all.
437+
- **`BearerAuthFilter.shouldNotFilter` and `SecurityConfig.permitAll()` paths must stay in sync.** The filter runs before Spring's `AuthorizationFilter`, so if a path is in `permitAll()` but NOT in `shouldNotFilter`, the filter rejects it with 401 before Spring's chain can permit it. Open paths today: `/`, `/index.html`, `/favicon.ico`, `/assets/**`, `/static/**`, `/error`, `/actuator/health`, `/actuator/health/liveness`, `/actuator/health/readiness`. Adding any new permit-all endpoint requires updating BOTH places.
438+
- **Constant-time bearer-token compare uses SHA-256 pre-hash.** Both the provided and expected token are hashed with SHA-256 before `MessageDigest.isEqual`. SHA-256 always produces 32-byte digests, so `isEqual` runs over fixed-size arrays — defeats the length oracle that makes raw `isEqual` unsafe across mismatched-length inputs. **Don't** "optimize" by removing the hash and comparing raw token bytes; that re-introduces the oracle.
439+
- **Never log the `Authorization` header.** `BearerAuthFilter` deliberately never passes the header value to a logger, even at DEBUG. The rejection log line carries only `method` and `requestURI`. There's a regression test (`tokenValueNeverAppearsInLogs`) that captures all log lines for the filter and asserts the secret substring is absent.
440+
- **`mode=none` + active `serving` profile = startup failure** unless `codeiq.mcp.auth.allow_unauthenticated=true` is **explicitly** set. This is by design — operators must opt into permissive mode deliberately. `mode=mtls` is reserved and currently throws "not yet implemented" (better than silently passing through).
441+
- **`server.error.include-stacktrace: never`** is set in the serving profile as defense-in-depth alongside `GlobalExceptionHandler`. Don't enable it for "easier debugging" — stack frames in the response body leak class names + paths (CWE-209). Use the `request_id` in the envelope to correlate to the WARN log line where the full stack is captured.
442+
- **Cypher transaction wall-clock cap is configured at the DBMS level**, not per-call. `Neo4jConfig.databaseManagementService(...)` sets `GraphDatabaseSettings.transaction_timeout = 30s` so every transaction gets the cap automatically. Don't reach for `graphDb.beginTx(timeout, unit)` overload in tool code — the test suite mocks `beginTx()` with no args and the overload changes the matcher signature, breaking the existing stubs across `McpToolsTest` / `McpToolsExpandedTest` / `McpToolsEvidenceTest`.
443+
- **`McpTools.runCypher` row cap is enforced in the iteration loop, not via `LIMIT`.** After `maxResults` rows are accumulated the loop breaks and the response carries `truncated: true` + `max_results: N`. Don't try to inject `LIMIT N` into the user-supplied query string — that would require parsing the query (and the user's query may already have its own LIMIT).
444+
- **`McpTools.getCachedData()` 60-second TTL snapshot is a bridge fix.** It's NOT the proper solution — the proper solution is to rewrite each topology MCP tool to use a targeted Cypher query so the full graph never needs to live on heap. The cache caps peak memory under concurrent calls but the snapshot itself is still multi-GB on large graphs. When that refactor lands, the `AtomicReference<CachedSnapshot>` and `getCachedData()` itself can be deleted.
445+
- **`RateLimitFilter` keys by `sha256(Authorization)`** — the raw token NEVER goes into the bucket key map. The 16-hex-char digest is enough collision resistance for keying. Falls back to `X-Forwarded-For` (first hop) → `RemoteAddr` when no auth header is present. Buckets live in a `ConcurrentHashMap` — bounded in practice by `num_distinct_clients`, which for the single-tenant pod shape is small. Swap to a Caffeine cache with a max-size eviction if multi-tenant exposure is ever added.
446+
- **Filter chain order in `serving` profile**: `SecurityHeadersFilter``RateLimitFilter``BearerAuthFilter` → ... → controller. Each `addFilterBefore(X, UsernamePasswordAuthenticationFilter.class)` inserts X immediately before UPAFilter, pushing the previously-inserted filter farther from the target — so the **registration order in `SecurityConfig.servingFilterChain` IS the chain order**. Don't shuffle without re-reasoning about it: if `RateLimitFilter` ran AFTER `BearerAuthFilter`, an unauthenticated brute-force attempt would never get throttled (would just see 401 over and over, hitting the slow path).
447+
- **`Files.probeContentType` is best-effort** — JDK 25 on Linux uses `/etc/mime.types` + magic-byte fallback. It returns `null` if the type can't be determined; treat that as "let it through" (the byte cap in `SafeFileReader` still bounds size). The allowlist for `/api/file` is `text/*` + `application/{json,xml,x-yaml,javascript}` — extending requires adding to the explicit list in `GraphController.readFile`.
448+
- **Sanitize user-controlled values before logging.** `BearerAuthFilter.sanitizeForLog(String)` strips `\p{Cntrl}` and truncates at 256 chars. Use it on anything tainted by `request.getRequestURI()`, `request.getMethod()`, headers, etc. before passing to a logger. CodeQL `java/log-injection` will flag direct `log.warn("... {} ...", request.getRequestURI())` as a vuln.
449+
- **`mcp.limits.max_depth` is a NEW field on `McpLimitsConfig`** (default 10). Audit #10 / C3 — the original audit assumed it existed but it didn't. When adding new MCP traversal tools, cap depth via `Math.min(callerSupplied, maxDepth)` before passing to Cypher. The REST endpoint already had this guard via `config.getMaxDepth()` from `CodeIqConfig`; the MCP path now mirrors it via `McpLimitsConfig.maxDepth()`.
436450

437451
## Supply-chain observability (OpenSSF)
438452

0 commit comments

Comments
 (0)