Skip to content

Commit 331cf0c

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

2 files changed

Lines changed: 68 additions & 32 deletions

File tree

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,11 @@ public class CorsConfig {
4646
static final String ALLOWED_HEADERS = "*";
4747

4848
/** Empty default = deny-all (no mappings registered). */
49-
@Value("${codeiq.cors.allowed-origin-patterns:}")
50-
private String allowedOriginPatterns = "";
49+
private final String allowedOriginPatterns;
50+
51+
public CorsConfig(@Value("${codeiq.cors.allowed-origin-patterns:}") String allowedOriginPatterns) {
52+
this.allowedOriginPatterns = allowedOriginPatterns == null ? "" : allowedOriginPatterns;
53+
}
5154

5255
@PostConstruct
5356
void logCorsState() {

src/test/java/io/github/randomcodespace/iq/config/CorsConfigTest.java

Lines changed: 63 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,59 +18,84 @@ public Map<String, CorsConfiguration> getCorsConfigurations() {
1818
}
1919
}
2020

21-
private CorsConfig createCorsConfig() {
22-
return new CorsConfig();
21+
/** Default empty pattern = deny-all (production default). */
22+
private CorsConfig denyAllByDefault() {
23+
return new CorsConfig("");
24+
}
25+
26+
/** Operator-configured loopback patterns (typical local-dev override). */
27+
private CorsConfig localDevConfig() {
28+
return new CorsConfig("http://localhost:[*],http://127.0.0.1:[*]");
2329
}
2430

2531
@Test
26-
void corsConfigurerReturnsWebMvcConfigurer() {
27-
WebMvcConfigurer configurer = createCorsConfig().corsConfigurer();
28-
assertNotNull(configurer);
32+
void corsConfigurerNeverNull() {
33+
// Both deny-all and explicit configs return a non-null configurer.
34+
assertNotNull(denyAllByDefault().corsConfigurer());
35+
assertNotNull(localDevConfig().corsConfigurer());
36+
}
37+
38+
@Test
39+
void denyAllByDefault_registersNoMappings() {
40+
WebMvcConfigurer configurer = denyAllByDefault().corsConfigurer();
41+
TestableCorsRegistry registry = new TestableCorsRegistry();
42+
configurer.addCorsMappings(registry);
43+
44+
Map<String, CorsConfiguration> configurations = registry.getCorsConfigurations();
45+
assertFalse(configurations.containsKey("/api/**"),
46+
"Empty allowed-origin-patterns must NOT register /api/** CORS — deny-all is the default");
47+
assertFalse(configurations.containsKey("/mcp/**"),
48+
"Empty allowed-origin-patterns must NOT register /mcp/** CORS — deny-all is the default");
2949
}
3050

3151
@Test
32-
void corsConfigurerDoesNotThrowWhenAddingMappings() {
33-
WebMvcConfigurer configurer = createCorsConfig().corsConfigurer();
52+
void blankAllowedOriginPatterns_treatedAsDenyAll() {
53+
// Whitespace-only is the same as empty.
54+
WebMvcConfigurer configurer = new CorsConfig(" ").corsConfigurer();
3455
TestableCorsRegistry registry = new TestableCorsRegistry();
35-
assertDoesNotThrow(() -> configurer.addCorsMappings(registry));
56+
configurer.addCorsMappings(registry);
57+
assertTrue(registry.getCorsConfigurations().isEmpty(),
58+
"Blank allowed-origin-patterns must register no mappings");
3659
}
3760

3861
@Test
39-
void corsRegistryContainsApiAndMcpMappings() {
40-
WebMvcConfigurer configurer = createCorsConfig().corsConfigurer();
62+
void explicitConfig_registersApiAndMcpMappings() {
63+
WebMvcConfigurer configurer = localDevConfig().corsConfigurer();
4164
TestableCorsRegistry registry = new TestableCorsRegistry();
4265
configurer.addCorsMappings(registry);
4366

44-
var configurations = registry.getCorsConfigurations();
67+
Map<String, CorsConfiguration> configurations = registry.getCorsConfigurations();
4568
assertTrue(configurations.containsKey("/api/**"),
46-
"Should register CORS mapping for /api/**");
69+
"Explicit pattern should register CORS mapping for /api/**");
4770
assertTrue(configurations.containsKey("/mcp/**"),
48-
"Should register CORS mapping for /mcp/**");
71+
"Explicit pattern should register CORS mapping for /mcp/**");
4972
}
5073

5174
@Test
52-
void apiMappingAllowsExpectedMethods() {
53-
WebMvcConfigurer configurer = createCorsConfig().corsConfigurer();
75+
void explicitConfig_apiAllowsGetAndOptionsOnly() {
76+
WebMvcConfigurer configurer = localDevConfig().corsConfigurer();
5477
TestableCorsRegistry registry = new TestableCorsRegistry();
5578
configurer.addCorsMappings(registry);
5679

57-
var configurations = registry.getCorsConfigurations();
58-
var apiCors = configurations.get("/api/**");
80+
var apiCors = registry.getCorsConfigurations().get("/api/**");
5981
assertNotNull(apiCors);
6082
var methods = apiCors.getAllowedMethods();
6183
assertNotNull(methods);
6284
assertTrue(methods.contains("GET"));
6385
assertTrue(methods.contains("OPTIONS"));
86+
// Mutating verbs must NOT be allowed — read-only API.
87+
assertFalse(methods.contains("PUT"));
88+
assertFalse(methods.contains("PATCH"));
89+
assertFalse(methods.contains("DELETE"));
6490
}
6591

6692
@Test
67-
void mcpMappingAllowsGetPostOptions() {
68-
WebMvcConfigurer configurer = createCorsConfig().corsConfigurer();
93+
void explicitConfig_mcpAllowsGetPostOptions() {
94+
WebMvcConfigurer configurer = localDevConfig().corsConfigurer();
6995
TestableCorsRegistry registry = new TestableCorsRegistry();
7096
configurer.addCorsMappings(registry);
7197

72-
var configurations = registry.getCorsConfigurations();
73-
var mcpCors = configurations.get("/mcp/**");
98+
var mcpCors = registry.getCorsConfigurations().get("/mcp/**");
7499
assertNotNull(mcpCors);
75100
var methods = mcpCors.getAllowedMethods();
76101
assertNotNull(methods);
@@ -80,31 +105,39 @@ void mcpMappingAllowsGetPostOptions() {
80105
}
81106

82107
@Test
83-
void apiMappingRestrictsToLocalhostOrigins() {
84-
WebMvcConfigurer configurer = createCorsConfig().corsConfigurer();
108+
void explicitConfig_originPatternsPreserved() {
109+
WebMvcConfigurer configurer = localDevConfig().corsConfigurer();
85110
TestableCorsRegistry registry = new TestableCorsRegistry();
86111
configurer.addCorsMappings(registry);
87112

88-
var configurations = registry.getCorsConfigurations();
89-
var apiCors = configurations.get("/api/**");
113+
var apiCors = registry.getCorsConfigurations().get("/api/**");
90114
assertNotNull(apiCors);
91115
var patterns = apiCors.getAllowedOriginPatterns();
92116
assertNotNull(patterns);
93117
assertTrue(patterns.stream().anyMatch(p -> p.contains("localhost")),
94-
"CORS should restrict to localhost origins");
118+
"Configured loopback pattern must reach the CORS configuration");
95119
}
96120

97121
@Test
98-
void apiMappingAllowsAllHeaders() {
99-
WebMvcConfigurer configurer = createCorsConfig().corsConfigurer();
122+
void explicitConfig_allowsAllHeaders() {
123+
WebMvcConfigurer configurer = localDevConfig().corsConfigurer();
100124
TestableCorsRegistry registry = new TestableCorsRegistry();
101125
configurer.addCorsMappings(registry);
102126

103-
var configurations = registry.getCorsConfigurations();
104-
var apiCors = configurations.get("/api/**");
127+
var apiCors = registry.getCorsConfigurations().get("/api/**");
105128
assertNotNull(apiCors);
106129
var headers = apiCors.getAllowedHeaders();
107130
assertNotNull(headers);
108131
assertTrue(headers.contains("*"));
109132
}
133+
134+
@Test
135+
void nullPatterns_treatedAsDenyAll() {
136+
// Defensive — Spring binding can pass null in edge cases.
137+
WebMvcConfigurer configurer = new CorsConfig(null).corsConfigurer();
138+
TestableCorsRegistry registry = new TestableCorsRegistry();
139+
configurer.addCorsMappings(registry);
140+
assertTrue(registry.getCorsConfigurations().isEmpty(),
141+
"Null allowed-origin-patterns must register no mappings");
142+
}
110143
}

0 commit comments

Comments
 (0)