fix: repoint config rules at property keys Spring actually reads - #42
Merged
Conversation
Nothing validated that a rule's property keys correspond to properties Spring actually reads. Two rules shipped with keys that exist nowhere in Spring Boot: SPR-CONFIG-004 matches spring.web.cors.* / spring.mvc.cors.* and SPR-CONFIG-005 matches spring.security.debug. Spring ignores unknown properties silently, so nothing at runtime says otherwise, and each rule's fixtures encode the same invented keys, so the suite stayed green while the rules could not fire on a real project. Adds Rule.configKeys(), a vendored index of every property name Spring Boot declares across Boot 2.0 through 3.5, and ConfigKeyMetadataTest, which checks the two against each other. The index is regenerated by tools/generate-spring-property-index.py; Map-typed properties are marked so logging.level.<logger> resolves correctly. The test fails as of this commit, naming the three offenders (including SPR-CONFIG-001's dead secondary key management.endpoints.exposure.include). The rule fixes follow; committing the check first so the fix is what turns it green. Refs #39, #40, #41
Turns ConfigKeyMetadataTest green. SPR-CONFIG-004 matched spring.web.cors.* and spring.mvc.cors.*. Neither has ever existed. It now matches the two surfaces Spring Boot really does expose as configuration properties: management.endpoints.web.cors.* (Boot 2.0+) and spring.graphql.cors.* (Boot 2.7+). Origins and credentials are now paired within a namespace rather than across the whole key list, so an Actuator wildcard is never combined with a GraphQL allow-credentials; cors-config-cross-namespace pins that down. SPR-CONFIG-005 matched spring.security.debug, which Spring does not read. It now matches logging.level.org.springframework.security, and loggers nested under it, set to DEBUG or TRACE. That is how verbose security logging is actually turned on, verified against a running Boot 3.5.16 app: 6 security DEBUG lines at startup and 6 per request, against none at all for the old property. SPR-CONFIG-001 keeps its real key and drops the dead secondary management.endpoints.exposure.include. Detection is unchanged. Scanning the probe projects from the issue reports now gives the opposite verdicts to before, which is the point: the invented namespace is clean and the real one is flagged. Rule docs rewritten. SPR-CONFIG-004's remediation previously told users to write a property Spring ignores; it now documents that below Framework 5.3 the wildcard leaks at runtime while from 5.3 onward it fails the context at startup, both measured. Neither doc now recommends allowed-origin-patterns as a fix, since that bypasses the validation rather than satisfying it. Goldens regenerated: same 11 findings and the same rule ids, with updated messages, property paths and lines. Fixes #39 Fixes #40 Refs #41
5 tasks
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
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.
What
Repoints three CONFIG rules at property keys Spring Boot actually reads, and adds a test that
makes the class of defect impossible to reintroduce.
Two commits, in order:
test:addsRule.configKeys(), a vendored index of every property name Spring Bootdeclares across Boot 2.0 through 3.5, and
ConfigKeyMetadataTest. This commit is red onpurpose, naming all three offending rules.
fix:repoints the rules, rewrites the fixtures and rule docs, and turns it green.Why
Closes #39
Closes #40
Partially addresses #41: the Actuator CORS namespace is now covered, so the HIGH case in that
report is handled. The MEDIUM tier proposed there (wildcard without credentials) is not in
this PR and #41 stays open for it.
spring.web.cors.*,spring.mvc.cors.*andspring.security.debugdo not exist in any SpringBoot release. Spring ignores unknown properties silently, so nothing at runtime ever said
otherwise, and each rule's fixtures encoded the same invented keys. The suite was green while
two of ten rules could not fire on a real project, and SPR-CONFIG-004's documented remediation
told users to write a property Spring throws away.
What changed per rule
spring.web.cors.*,spring.mvc.cors.*management.endpoints.web.cors.*(Boot 2.0+),spring.graphql.cors.*(Boot 2.7+)spring.security.debuglogging.level.org.springframework.securityand loggers below it, atDEBUGorTRACEmanagement.endpoints.exposure.includeWhile rewriting SPR-CONFIG-004 I found a second defect that is not in any issue. The old
firstPresent()logic searched for origins across the whole key list, then credentials acrossthe whole key list, without requiring them to come from the same namespace. With two real
namespaces that would pair an Actuator wildcard with a GraphQL
allow-credentialsand report afinding that is not there. Pairing is now per namespace, and
fixtures/cors-config-cross-namespacefails if that regresses.How was this verified?
Tests: 59 tests, 0 failures, spotless clean.
mvn verifyrun twice andmvn clean verifyonce,
BUILD SUCCESSeach time. Local JDK is 17; CI builds on 21, which I could not exerciselocally (
maven.compiler.releaseis 17, so this should be uneventful).The first commit is genuinely red. Checked out at
78e62b7in a separate worktree, the testfails naming exactly six keys and nothing else:
Scanning the probe projects from the issue reports gives the opposite verdicts to before,
which is the point of the change. Both columns are measured, the before column with a jar built
from
c26a0e7:spring.web.cors.*wildcard + credentialsmanagement.endpoints.web.cors.*wildcard + credentialsspring.security.debug: truelogging.level.org.springframework.security: DEBUGlogging.level.org.springframework.security.web: TRACERuntime behaviour behind the doc rewrite was measured, not assumed. SPR-CONFIG-004's doc now
states two different outcomes depending on the Spring Framework version, and both were observed
against running apps:
Access-Control-Allow-Credentials: trueand the endpoint body is readable cross-origin.BeanCreationException ... When allowCredentials is true, allowedOrigins cannot contain the special value "*". The application never starts.For SPR-CONFIG-005, a Boot 3.5.16 app with
spring-boot-starter-securityemits 6 Spring SecurityDEBUG lines at startup and 6 more per request under the new key, and none at all under the old
one, while still returning 401 to prove security was active either way.
Dogfood scan (the same command CI runs) reports one finding, the pre-existing
NoOpPasswordEncoderRule.java:91self-match. No new findings, and the vendored index is notpicked up as a config file.
Goldens regenerated with
-DupdateGoldens=trueand reviewed as a diff: the same 11 findingswith the same rule ids, differing only in message, property path and line.
Notes for review
tools/generate-spring-property-index.py. The generator aborts rather than writing a file ifsentinel properties are missing, so a failed download cannot produce an index that passes
vacuously.
ConfigKeyMetadataTestcarries the same guard plus a check that the matcheractually rejects invented keys.
Map-typed properties are marked in the index, which is what makeslogging.level.<logger>resolve. Without that, the SPR-CONFIG-005 fix would fail its own test.key shape, not a fixed list.
allowed-origin-patternsas a remedy. It is the documented way tobypass
validateAllowCredentials(), so suggesting it to silence a startup failure wouldrestore the exposure the check exists to prevent.
Checklist
mvn verifypasses locallyREADME.md,docs/rules/*.md) if user-facing behavior changeddocs/rules/