diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ce5ff45..db570f5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,10 +50,17 @@ Before adding a rule, work through: - **Property keys**: a CONFIG rule must return the keys it matches from `Rule.configKeys()`. `ConfigKeyMetadataTest` checks them against Spring Boot's own configuration metadata, because Spring ignores unknown properties - silently and a rule keyed on a property that does not exist can never fire. - Verify the key against the Boot reference documentation before writing the - rule, not against what looks plausible. Two rules shipped with invented keys - before this check existed. + silently: a rule keyed on a property that does not exist fires on nothing + while every fixture stays green. Two rules shipped that way before this + check existed. Verify the key before writing the rule, and against the Boot + source rather than against what looks plausible. + + Read a failure the right way round. The metadata proves presence, never + absence — Boot binds properties it never declares, and + `management.endpoint..access` is one of them. A key missing from the + index is *unconfirmed*, not disproved. Confirm it against the Boot source or + a running app, then either repoint the rule at the real property or add the + key to `ACCEPTED_ABSENCES` with that evidence as its reason. - **False positives**: what looks similar but is safe? Add a fixture for it. - **Severity**: HIGH/CRITICAL only for directly exploitable misconfigurations; MEDIUM/LOW for defense-in-depth or informational findings. diff --git a/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java b/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java index 8803973..41c19ec 100644 --- a/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java +++ b/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java @@ -28,6 +28,16 @@ * the rules could not fire on a real project. This test closes that gap by checking rule keys * against Spring Boot's own configuration metadata rather than against sprig's fixtures. * + *

What absence proves. The metadata is authoritative in one direction only. A + * declared key is a real property. An undeclared key is unconfirmed, not disproved: Boot + * binds properties it never declares, {@code management.endpoint..access} among them. A failure + * here is a prompt to check the key against the Boot source or a running app before shipping it, + * not a verdict that the key is invented. + * + *

That is what happened with the two rules above. Metadata absence is what pointed at them. + * Runtime observation, and a search of the jars for a binding that turned out not to exist, are + * what condemned them. + * *

The index is generated by {@code tools/generate-spring-property-index.py}. */ class ConfigKeyMetadataTest { @@ -64,6 +74,12 @@ class ConfigKeyMetadataTest { * Keys sprig matches on purpose even though Spring's own metadata omits them. Empty is the * healthy state; an entry is a deliberate, explained decision, not a place to silence a * failure. + * + *

An entry needs evidence that Boot binds the property, and the reason string is where that + * evidence goes: either a runtime observation naming the Boot version and the behaviour that + * changed when the property was set, or a pointer to the class that binds it. "The docs mention + * it" and "it looks right next to the declared ones" do not clear that bar. Without evidence in + * the reason, this map mutes the test instead of recording what was checked. */ private static final Map ACCEPTED_ABSENCES = Map.of(); @@ -136,7 +152,7 @@ void configRulesDeclareTheirKeys() { } @Test - @DisplayName("every declared rule key is a property Spring Boot actually reads") + @DisplayName("every declared rule key is confirmed by Spring Boot's metadata, or explained") void everyRuleKeyExistsInSpring() { Set offenders = new TreeSet<>(); for (Rule rule : BuiltInRules.all()) { @@ -148,11 +164,14 @@ void everyRuleKeyExistsInSpring() { } assertThat(offenders) .as( - "rule keys absent from Spring Boot's configuration metadata. Spring ignores" - + " unknown properties silently, so these rules cannot fire on a real" - + " project and any finding they produce is wrong. Repoint the rule at" - + " the real property, or add the key to ACCEPTED_ABSENCES with a" - + " reason.") + "rule keys absent from Spring Boot's configuration metadata. Absence is not" + + " proof the key is wrong: Boot binds some properties it never" + + " declares. It does mean the key is unconfirmed, and Spring ignores" + + " unknown properties silently, so an invented key fires on nothing" + + " while every fixture stays green. Confirm this one against the Boot" + + " source or a running app. Then either repoint the rule at the real" + + " property, or add the key to ACCEPTED_ABSENCES with that evidence" + + " as its reason.") .isEmpty(); }