Skip to content

Commit b47f730

Browse files
aksOpsclaude
andcommitted
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>
1 parent b1872f4 commit b47f730

19 files changed

Lines changed: 1171 additions & 17 deletions

CHANGELOG.md

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,86 @@ 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+
228308
## [0.1.0] - 2026-03-28
229309

230310
First general-availability cut. See the

CLAUDE.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,12 @@ 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.
436442

437443
## Supply-chain observability (OpenSSF)
438444

pom.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@
155155
<groupId>org.springframework.boot</groupId>
156156
<artifactId>spring-boot-starter-actuator</artifactId>
157157
</dependency>
158+
<dependency>
159+
<groupId>org.springframework.boot</groupId>
160+
<artifactId>spring-boot-starter-security</artifactId>
161+
</dependency>
158162

159163
<!-- Neo4j Embedded (Community Edition) -->
160164
<dependency>
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package io.github.randomcodespace.iq.api;
2+
3+
import org.slf4j.Logger;
4+
import org.slf4j.LoggerFactory;
5+
import org.slf4j.MDC;
6+
import org.springframework.context.annotation.Profile;
7+
import org.springframework.http.HttpStatus;
8+
import org.springframework.http.ResponseEntity;
9+
import org.springframework.web.bind.annotation.ExceptionHandler;
10+
import org.springframework.web.bind.annotation.RestControllerAdvice;
11+
import org.springframework.web.server.ResponseStatusException;
12+
13+
import java.util.LinkedHashMap;
14+
import java.util.Map;
15+
import java.util.UUID;
16+
17+
/**
18+
* Uniform error envelope for the REST API: {@code {"code","message","request_id"}}
19+
* with the appropriate HTTP status. Stack traces and class names never reach the
20+
* response body — only logged at WARN with the {@code request_id} so on-call can
21+
* correlate.
22+
*
23+
* <p>Active in the {@code serving} profile only.
24+
*/
25+
@RestControllerAdvice
26+
@Profile("serving")
27+
public class GlobalExceptionHandler {
28+
29+
private static final Logger log = LoggerFactory.getLogger(GlobalExceptionHandler.class);
30+
31+
@ExceptionHandler(ResponseStatusException.class)
32+
public ResponseEntity<Map<String, Object>> handleResponseStatus(ResponseStatusException ex) {
33+
HttpStatus status = HttpStatus.resolve(ex.getStatusCode().value());
34+
String code = status != null ? status.name() : "ERROR";
35+
String message = ex.getReason() != null
36+
? ex.getReason()
37+
: (status != null ? status.getReasonPhrase() : "Error");
38+
return ResponseEntity.status(ex.getStatusCode()).body(envelope(code, message));
39+
}
40+
41+
@ExceptionHandler(IllegalArgumentException.class)
42+
public ResponseEntity<Map<String, Object>> handleBadInput(IllegalArgumentException ex) {
43+
// Validation errors are surfaceable — but never include the class name or
44+
// a stack trace.
45+
return ResponseEntity
46+
.status(HttpStatus.BAD_REQUEST)
47+
.body(envelope("INVALID_INPUT", ex.getMessage()));
48+
}
49+
50+
@ExceptionHandler(Throwable.class)
51+
public ResponseEntity<Map<String, Object>> handleAny(Throwable ex) {
52+
String requestId = currentRequestId();
53+
log.warn("Unhandled exception (request_id={})", requestId, ex);
54+
return ResponseEntity
55+
.status(HttpStatus.INTERNAL_SERVER_ERROR)
56+
.body(envelope("INTERNAL_ERROR", "An internal error occurred."));
57+
}
58+
59+
private static Map<String, Object> envelope(String code, String message) {
60+
Map<String, Object> body = new LinkedHashMap<>();
61+
body.put("code", code);
62+
body.put("message", message);
63+
body.put("request_id", currentRequestId());
64+
return body;
65+
}
66+
67+
private static String currentRequestId() {
68+
String id = MDC.get("request_id");
69+
return id != null ? id : UUID.randomUUID().toString();
70+
}
71+
}

src/main/java/io/github/randomcodespace/iq/config/CorsConfig.java

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package io.github.randomcodespace.iq.config;
22

3+
import org.slf4j.Logger;
4+
import org.slf4j.LoggerFactory;
35
import org.springframework.beans.factory.annotation.Value;
46
import org.springframework.context.annotation.Bean;
57
import org.springframework.context.annotation.Configuration;
68
import org.springframework.context.annotation.Profile;
79
import org.springframework.web.servlet.config.annotation.CorsRegistry;
810
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
911

12+
import jakarta.annotation.PostConstruct;
13+
1014
/**
1115
* CORS configuration for the {@code serving} profile.
1216
*
@@ -16,17 +20,21 @@
1620
* analysis happens locally via the CLI ({@code codeiq index} / {@code codeiq enrich})
1721
* and the server never accepts data manipulation.
1822
*
19-
* <p>Default origin patterns cover the common local-dev cases (loopback on any port).
20-
* Override via {@code codeiq.cors.allowed-origin-patterns} (CSV) when serving over a
21-
* trusted network or behind a reverse proxy.
23+
* <p><b>Default is deny-all in serving.</b> The React UI is served same-origin from the
24+
* same Spring container, so cross-origin access is not required for normal operation.
25+
* Operators who genuinely need cross-origin access (e.g., serving the API behind a
26+
* reverse proxy at a different origin) must explicitly set
27+
* {@code codeiq.cors.allowed-origin-patterns} to a non-empty CSV — when empty, no CORS
28+
* mappings are registered and Spring MVC rejects all preflighted cross-origin requests.
29+
*
30+
* <p>Local development with the Vite dev server (running on a separate port) is the
31+
* usual reason to set this — typical value: {@code http://localhost:[*],http://127.0.0.1:[*]}.
2232
*/
2333
@Configuration
2434
@Profile("serving")
2535
public class CorsConfig {
2636

27-
/** Default allowed origin patterns: loopback on any port (covers local dev / IDE proxies). */
28-
static final String DEFAULT_ALLOWED_ORIGIN_PATTERNS =
29-
"http://localhost:[*],http://127.0.0.1:[*]";
37+
private static final Logger log = LoggerFactory.getLogger(CorsConfig.class);
3038

3139
/** Read-only REST API: only safe / preflight verbs. */
3240
static final String[] API_ALLOWED_METHODS = {"GET", "OPTIONS"};
@@ -37,11 +45,26 @@ public class CorsConfig {
3745
/** Allow all request headers — clients commonly send custom MCP / Auth headers. */
3846
static final String ALLOWED_HEADERS = "*";
3947

40-
@Value("${codeiq.cors.allowed-origin-patterns:" + DEFAULT_ALLOWED_ORIGIN_PATTERNS + "}")
41-
private String allowedOriginPatterns = DEFAULT_ALLOWED_ORIGIN_PATTERNS;
48+
/** Empty default = deny-all (no mappings registered). */
49+
@Value("${codeiq.cors.allowed-origin-patterns:}")
50+
private String allowedOriginPatterns = "";
51+
52+
@PostConstruct
53+
void logCorsState() {
54+
if (allowedOriginPatterns == null || allowedOriginPatterns.isBlank()) {
55+
log.info("CORS: deny-all (no allowed-origin-patterns configured). "
56+
+ "Set codeiq.cors.allowed-origin-patterns to enable cross-origin access.");
57+
} else {
58+
log.info("CORS: allowed-origin-patterns = {}", allowedOriginPatterns);
59+
}
60+
}
4261

4362
@Bean
4463
public WebMvcConfigurer corsConfigurer() {
64+
if (allowedOriginPatterns == null || allowedOriginPatterns.isBlank()) {
65+
// Deny-all: register no mappings. Spring MVC rejects cross-origin requests.
66+
return new WebMvcConfigurer() {};
67+
}
4568
String[] patterns = allowedOriginPatterns.split(",");
4669
return new WebMvcConfigurer() {
4770
@Override

0 commit comments

Comments
 (0)