feat(tls): single-source and activate neutral TLS + management surface - #138
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe gateway now defines TLS policy in ChangesTLS and management configuration
WebSocket log catalogue
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍(Review updated until commit dd502ba)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.java (1)
107-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis test duplicates
enabledRequiresClientAuthWithTrustAnchorand cannot observe the property it names.The act and the two assertions at lines 118-123 are identical to
enabledRequiresClientAuthWithTrustAnchor(lines 48-61). The only addition is the pre-assertion at lines 114-115. That pre-assertion reads a bare Vert.xHttpServerOptions, which never carries a Quarkus property in a plain unit test. So it asserts the Vert.x library default, not thatquarkus.http.ssl.client-authwas deleted.The stated behaviour is "no raw
quarkus.http.ssl.client-authdefault is needed". The mechanism that would prove it is the content ofapi-sheriff/src/main/resources/application.propertiesandapi-sheriff/src/test/resources/application.properties. Neither is reachable from here.Assert the absence where it is observable. A contract test that scans those two property files for
quarkus.http.ssl.client-authproves the single-source claim; this test does not. The PR already hasSingleSourceTlsContractTest, which looks like the correct home.As per path instructions: "Report a stated behaviour whose mechanism is absent, and name the file that would have to contain it. Prose, a code comment, or a summary table is not a mechanism."
Source: Path instructions
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSource.java (1)
88-94: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDerive the plain-management bucket reference from a single definition.
PLAIN_MANAGEMENT_BUCKET,quarkus.tls.plain-management.reload-period,QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME=plain-management, and the integration test string are each written independently. The opt-out contract only needs one change to name a missing bucket if they drift; source them from the Java constant or the application.properties bucket key instead.Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d230750c-030a-4dad-8c54-c9aa59adfc84
📒 Files selected for processing (37)
.plan/project-architecture/api-sheriff/enriched.jsonapi-sheriff/pom.xmlapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/ConfigLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/load/ConfigLoader.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/ManagementConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/TlsConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/ManagementPlainHttpAudit.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.javaapi-sheriff/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProviderapi-sheriff/src/main/resources/application.propertiesapi-sheriff/src/main/resources/schema/gateway.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSourceTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/load/ConfigLoaderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/ManagementPlainHttpAuditTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizerTest.javaapi-sheriff/src/test/resources/application.propertiesapi-sheriff/src/test/resources/config/testboot/gateway.yamlapi-sheriff/src/test/resources/config/valid/gateway.yamldoc/LogMessages.adocdoc/adr/0025-The_whole_server-TLS_surface_is_neutral_in_gatewayyaml_and_bound_by_exactly_two_seams.adocdoc/architecture.adocdoc/configuration.adocdoc/user/README.adocdoc/user/environment-variable-overrides.adocdoc/user/tls-edge.adocintegration-tests/docker-compose.ymlintegration-tests/pom.xmlintegration-tests/scripts/start-integration-container.shintegration-tests/src/main/docker/sheriff-config/gateway.yamlintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpOptOutIT.java
Triage dispositionsIn reply to comment_id:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
doc/user/environment-variable-overrides.adoc (1)
374-379: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd an automated TLS-key drift check.
doc/user/environment-variable-overrides.adocsays reused bare TLS keys now fail the build, but the build is./mvnw clean install/verify, and no CI workflow or test runs an assertion that those keys must be absent or covered ingateway.yaml. Keep the text accurate by adding that check or rewriting it as a manual requirement. Apply the same check or wording change to the parallel conclusion indoc/configuration.adoc.Source: Path instructions
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java (1)
123-126: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject bare Compose environment entries.
The predicate misses a bare
QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAMElist item. Docker Compose resolves a bare key from the host shell, so a set host value can activate plain management while this negative test still passes. Also reject bare entries before checking forKEY=value.Proposed matcher
- assertFalse(environment.stream().anyMatch(e -> e.startsWith("QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME=")), + assertFalse(environment.stream().anyMatch(e -> + e.equals("QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME") + || e.startsWith("QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME=")),Sources: Path instructions, MCP tools
🧹 Nitpick comments (1)
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java (1)
211-216: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueExtend the cipher-suite error path to supported-but-not-default-enabled suites.
applyCipherSuites()accepts suite names that this JDK supports, then rejects configs where no enabled protocol can negotiate any declared suite. This can happen for TLSv1.2 on JDK 17 whentls.cipher_suitescontains only suites supported-but-not-default-enabled byTLSv1.2defaults, such asTLS_AES_256_GCM_SHA384orTLS_CHACHA20_POLY1305_SHA256. The current fix text only suggeststls.min_versionor adding another negotiable suite, which omitstls.cipher_suitesfallbacks like switching to a default-enabled suite or tightening to TLSv1.3 only if the allowlist should be TLS 1.3 only.Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c083a1e5-db7a-46f3-b520-0ccd8e18c703
📒 Files selected for processing (11)
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizerTest.javadoc/architecture.adocdoc/configuration.adocdoc/user/environment-variable-overrides.adocdoc/user/tls-edge.adocintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/CipherSuiteFixtureWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpOptOutIT.java
🚧 Files skipped from review as they are similar to previous changes (4)
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.java
- doc/architecture.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSource.java
- integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpOptOutIT.java
|
/review |
Triage dispositionsIn reply to comment_id:
|
… negative control The negative control asserted no non-opt-out gateway instance activates plain management via an `=`-anchored prefix match on QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME. Docker Compose also accepts a BARE list entry, whose value resolves from the host shell at `compose up` time — such an entry on an HTTPS-management instance would activate plain-HTTP management while the control still passed green. Match the bare variable name as well as the KEY=value form, and add a matched positive/negative control over the predicate itself so a regression that drops the bare form cannot pass silently. Addresses CodeRabbit review_body finding 7642e4 on PR #138. Co-Authored-By: Claude <noreply@anthropic.com>
|
/review |
…tener Add TlsServerCustomizer mapping tls.min_version, tls.cipher_suites and tls.alpn from gateway.yaml onto the terminated HTTPS listener, making gateway.yaml the single effective source of terminated-listener TLS policy. Delete the inert raw quarkus.tls.protocols/.cipher-suites/.alpn keys, which never reached the listener because the TLS registry is consulted only when the certificate comes from quarkus.tls.key-store.*, while this gateway supplies it via quarkus.http.ssl.certificate.*. Relocate the build-time quarkus.ssl.native knob to the pom native profile. Add the cipherSuites component to TlsConfig and align the test profile onto quarkus.http.ssl.certificate.* so tests exercise the production listener path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
…gement.port in-schema gateway.yaml gains a first-class neutral `management:` policy block, bound through a new single-ordinal ConfigSource seam, and the deliberate absence of a port key is made discoverable instead of silent. - ManagementConfig: the neutral management policy record (tls.enabled), declaring no port component. The management port stays deployment-bound at quarkus.management.port (default 9000, QUARKUS_MANAGEMENT_PORT). - NeutralTlsConfigSource + its ConfigSourceProvider registration: the seam that reaches the management listener, which never enumerates HttpServerOptionsCustomizer beans. Its ordinal sits above the environment source's 300 so a policy named in gateway.yaml is not silently overridden; that is safe only because it projects policy keys only — never a port, never trust material, and never key material into the default TLS registry bucket, which would make management inherit HTTPS invisibly through registry.getDefault() (upstream quarkus issue 43380). - gateway.schema.json: the management object declares `port` solely in order to refuse it, via an always-failing subschema plus the networknt errorMessage extension, so the actionable sentence naming quarkus.management.port / QUARKUS_MANAGEMENT_PORT and ADR-0025 lands at the /management/port pointer rather than surfacing as a generic unknown-key error against the parent object. ConfigLoader enables the extension, which is off unless the keyword is named on the registry configuration. No ConfigValidator rule is added: loadGateway returns on the first schema failure and never reaches validation, so a validator rule for a schema-rejected key would be unreachable dead code. The errorMessage adoption is backed by an executed test asserting the exact sentence, written before the mechanism was relied on.
…a sixth instance Running the management interface on plain HTTP is a legitimate deployment behind a trusted boundary, so it stays possible — but it stops being silent. The downgrade route is Quarkus-native rather than bespoke: quarkus.management.tls- configuration-name selects the `plain-management` TLS bucket that application.properties now declares with no key material, the recorder finds no key/cert, and the management listener starts plain. It is runtime configuration, so the same native image serves both postures with no rebuild — which is why the new compose instance needs neither a variant image nor a config overlay, only an environment variable. - ManagementPlainHttpAudit observes the real StartupEvent and warns (ApiSheriff-115) on the OBSERVED EFFECTIVE STATE — the TLS material the listener actually resolved, the same question the recorder asks. An audit keyed on a configuration key would report a comfortable fiction as soon as that key was renamed or superseded. - The observer actively invokes the audit rather than holding a collaborator, and the accompanying test fires the event through the container's own bus. A normal-scoped observer whose proxy is never touched silently never runs while a directly-invoking test stays green (lesson 2026-07-20-18-002). - api-sheriff-plain-mgmt publishes management on 19005 with its own http:// readiness gate, deliberately outside the https-hard-coded loop that gates 19000-19004. The lockstep comments on the other five instances now describe the exception instead of quietly becoming false. - ManagementPlainHttpActivationWiringTest asserts the activation is really wired, with the five HTTPS instances as the matched negative control, and pins the two constraints that make the mechanism work: the selected bucket carries no key material, and no DEFAULT quarkus.tls.key-store.* bucket exists — the management interface falls back to the default registry bucket and would otherwise inherit HTTPS through a path no config file mentions. Also repairs the testboot fixture, which declared no token_validation and so could not complete a container boot at all — TokenValidatorProducer forces the validator eagerly regardless of the route table's auth requirements.
…g seams ADR-0025 restates ADR-0011's neutral-name ruling as a general rule and extends it from the JWKS trust surface to the whole server-TLS surface: every knob is policy (neutral in gateway.yaml), deployment-bound (ports and material, supplied by the deployment), or build-time (never in the runtime surface at all). ADR-0011 stays Accepted and is extended, not superseded. The classification is not filing. It is what makes the single-ordinal projecting ConfigSource safe: because the source projects policy keys only, no deployment-bound knob is in the projected set, so nothing an operator sets in the environment can be outranked by surprise — and the second, lower-ordinal source that would otherwise be needed is unnecessary rather than merely forbidden. Also recorded: why the two seams are each the only one available on their path (the customizer is enumerated after createSslOptions on the main path; the management interface never enumerates customizers); why the neutral management block declares no port; why management HTTPS stays the default with a runtime-native, audited opt-out rather than a bespoke key or a boot refusal; and the reversal on quarkus.management.tls-configuration-name — both original objections are plausible readings of its name and both fail against the mechanism, so the reversal is recorded to stop a future reader re-deriving them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
…onsumer The neutral key was bound but never read: NeutralTlsConfigSource.project() returned an empty map for every document, so gateway.yaml's management.tls.enabled reached nothing. A neutral name that binds nothing is the exact defect this plan exists to eliminate — it reads as enforcement while enforcing nothing — so the seam now projects. management.tls.enabled=false projects exactly one key, quarkus.management.tls-configuration-name=plain-management, selecting the key-less bucket application.properties already declares. Absent and true project NOTHING, deliberately: the secure default needs no key to express it, and the silence is what keeps QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME a working route onto the same Quarkus key. The neutral door and the deployment door are two routes onto one lever, not two mechanisms with a precedence puzzle between them. Every projection assertion goes through the assembled SmallRyeConfig rather than the source's own accessor, because the question Quarkus asks is what the resolved Config returns after ordinal arbitration — and each no-projection case runs against a competing value at the environment source's ordinal, so 'the deployment value survives' is an observation rather than the absence of one. The projected key set's cardinality is pinned so a second key must arrive as a visible diff. The top-level tls block is no longer parsed here. It governs the terminated listener, which has a live CDI seam able to observe the SSL options Quarkus actually built; reading it in the config source was a parse with no key to emit for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
…WARN ApiSheriff-115 observes the effective state and cannot tell which door produced it, so a remedy naming only the deployment environment variable is now incomplete: gateway.yaml's management.tls.enabled=false reaches the same Quarkus key. An operator who took the neutral door would be told to remove an override they never set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
The configuration reference gains cipher_suites, the now-effective min_version/alpn semantics with their terminated-listener scope limit, a management field reference explaining why the block declares no port and why a downgrade takes the whole surface, and a per-key verdict table recording the disposition of every raw quarkus.* server-TLS key with its class and reason — including both named exceptions, since an undocumented exception would be a silent trim. The architecture document records the two binding seams and, for each, why it is the only one available on its path: the customizer is enumerated after the SSL options are built, and the management recorder never enumerates customizers at all. It also records the ordering constraint that follows — the projecting ConfigSource is constructed before CDI exists, so it cannot read the bound model and re-reads gateway.yaml itself, which is why a malformed document degrades silently there and an unexpected shape reads as 'not explicitly disabled'. The operator guide gains the TLS policy keys and the management interface: HTTPS by default, the single-port constraint that makes any downgrade total, the two doors onto the opt-out and why they do not compete, why the escape hatch is runtime-native rather than bespoke, and an explicit warning that quarkus.management.enabled=false is the opposite of a fix — build-time fixed, and it moves probes onto the TLS-terminated router. New operator page environment-variable-overrides.adoc states the three-way classification in operator language and enumerates what stays deployment-bound. LogMessages registers ApiSheriff-115 and the previously-undocumented ApiSheriff-106, and repairs three rows that had drifted from their templates — including a phantom INFO-5 row for an event the code emits as WARN-106. The primary IT fixture now declares min_version and a cipher allowlist so the integration stack exercises a gateway.yaml-sourced protocol and cipher setting. The floor is 1.2 and the allowlist spans both enabled protocols: under a 1.2 floor a 1.3-only allowlist would leave TLS 1.2 with nothing to negotiate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
The keys this plan deleted were inert, not merely redundant, and nothing structural stops that defect growing back: a contributor reaching for a familiar quarkus.tls.* name would reintroduce a surface that reads as enforcement while reaching no listener, and every existing test would stay green. SingleSourceTlsContractTest turns that reintroduction into a failing build by classifying every default-profile key in the shipped application.properties and refusing any server-TLS POLICY key outside a named allowlist. The R1 relocation is asserted in BOTH halves — quarkus.ssl.native absent from application.properties AND present, enabled, inside the pom's native profile. The absence assertion alone would pass just as happily if the knob had been dropped outright, and a native image without SSL support fails at runtime rather than at build. One assertion guards the guard: every allowlist entry must be present in the file AND reachable by the detector. Without it the allowlist could quietly fill with decorative entries — a stale key that no longer exists, or one the namespace matcher never reaches — leaving the bar weaker than it reads. That check is what caught quarkus.http.ssl-port falling outside a dot-terminated quarkus.http.ssl. prefix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
…oute pairs The management-TLS pair is one instance of a general shape, so the operator page now answers the same four questions for the rest of the surface: default value, security posture of that default, override route, and whether the route is deployment-bound by necessity (a port, material, a build-time knob) or merely by accident (a policy knob that never got a neutral name). Swept: terminated-listener TLS policy, mTLS, the SNI split and its ports, JWKS trust and egress, token validation and its eager boot assembly, the inbound filter modes, request limits and admission budgets, and response headers/CORS. Two answers are worth naming. The transport framing bounds have NO override route at all, deliberately — an operator hunting for the variable should be told it does not exist rather than left searching. And the inbound filter mode has no environment route on purpose: a validation posture relaxable by an environment variable would be a weakening with no trace in the document under review. Two areas are recorded as findings rather than fixed, since both sit outside this plan's footprint. The interaction between the gateway's own validator and the extension's parallel sheriff.token.issuers.* readiness surface could not be established by reading, so the page says so explicitly instead of asserting a verdict it did not earn. And QUARKUS_HTTP_SSL_PROTOCOLS on the mtls IT instance works today only because that overlay declares no tls.min_version — adding one would silently shadow the env pin, which is the precedence rule behaving correctly and surprisingly at the same time. No inert key was found in the shipped properties after this change, and the page says that is a statement about this revision rather than a general guarantee. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
api-sheriff-plain-mgmt could never boot. It is the only gateway instance that mounts the shared sheriff-config rather than overlaying its own gateway.yaml -- deliberately, so the management plain-HTTP opt-out is the single variable under test. But that shared gateway.yaml declares a non-empty passthrough_sni, which starts the SNI front listener on the public port 8443, and the instance was never given QUARKUS_HTTP_SSL_PORT=8444 to move the terminated HTTPS listener to the internal port. The two collided and the container crash-looped with "Port 8443 seems to be in use by another process". The four other gateway instances escape this only because each overlays gateway.yaml with a variant declaring no passthrough_sni, so the invariant -- documented at the passthrough_sni block in sheriff-config/gateway.yaml -- had never bitten before. Add the port move, and delete the "no passthrough front on this instance" comment, which asserted the opposite of what the mounted config does. Found by running the containerised suite: a green test-compile showed nothing. After the fix the suite is 89 completed, 0 failures, 0 errors, 0 skipped, 0 flakes, with ManagementPlainHttpOptOutIT 3/3 and WARN ApiSheriff-115 observed firing in the native container through the real StartupEvent path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
…uards NeutralTlsConfigSource carried an unresolved OpenRewrite marker before a deliberately broad `catch (Exception e)` guarding the YAML pre-parse of gateway.yaml. CLAUDE.md forbids committing code with a marker present. The catch itself is required, not speculative -- it guards a real external-input parse boundary -- so the fix is to place the suppression comment in its correct position ahead of `catch`, matching the convention already used at JwksTrustProfileResolver.java:115, and drop the stray TODO. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
TlsServerCustomizer forwarded every declared cipher suite straight to HttpServerOptions with no validation. A probe against a real Vert.x HTTPS listener showed the bind SUCCEEDS with a bogus suite name -- the gateway boots green and then resets every TLS handshake with SSLHandshakeException. A partially mistyped list is worse: the valid entries still negotiate, silently narrowing the declared cipher policy to an unaudited subset. Validate each declared suite against the JDK's supported set and abort the boot with a message naming the offending suite and its closest supported matches, mirroring the existing applyMinVersion/applyAlpn fail-closed shape. The supported set is resolved per boot, not in a static initializer, so a native image cannot bake in the build host's cipher set. Adds symmetric boot-failure coverage for all three TLS knobs plus a guard test pinning the integration fixture's four-suite allowlist. Found by the finalize security-audit sweep (finding 1421ff). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
Tier-1 re-enrichment scoped to api-sheriff, the only module this plan added components to. Records TlsServerCustomizer and ManagementPlainHttpAudit under gateway.tls, NeutralTlsConfigSource under gateway.config, and ManagementConfig under gateway.config.model, plus the ADR-0025 three-way key classification (POLICY / DEPLOYMENT-BOUND / BUILD-TIME) as a durable hint. Tier-0 found no structural change: the new components landed inside existing modules, so the discovered descriptor is byte-unchanged and is not recommitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TFEs53xjkecC5nX6XRdEMo
Clears SonarCloud java:S6485 on NeutralTlsConfigSource.policyBlocksOf. new HashMap<>(n) takes an initial CAPACITY, not a mapping count, so it under-sizes the table and forces a rehash; HashMap.newHashMap(n) sizes for the intended number of mappings, which is what the call site means. Behaviour-preserving one-line change.
…drift guard The cipher allowlist guard only rejected suite NAMES the JDK does not know. It could not fire for the operator error with the identical outcome: a list that no enabled protocol can negotiate. A 1.2 floor enables TLSv1.2 AND TLSv1.3, so an allowlist of TLS 1.3-only suites passed every name check while every TLS 1.2 client failed the handshake against a listener that bound happily — the fail-open shape this plan exists to eliminate. applyCipherSuites now also asserts that every protocol on the listener's enabled set retains a negotiable suite, aborting with a message naming the stranded protocol and both knobs that resolve it. The per-protocol negotiable sets are derived from the platform's own protocol-named SSLContexts (a context reports the suites applicable to itself and every protocol below it, and names those lower protocols, so subtracting isolates each one) rather than from a hardcoded suite classification. A protocol whose set cannot be derived is skipped, not treated as unreachable. Note the JDK does not filter getEnabledCipherSuites() by setEnabledProtocols(), so that derivation was verified empirically before being relied on. supportedCipherSuites() now uses Set.copyOf over Set.of: the array comes from a pluggable JSSE provider that may report a name twice, which Set.of would turn into a context-free IllegalArgumentException at boot. The integration fixture's allowlist is no longer mirrored by hand in the api-sheriff test. That drift guard moves to CipherSuiteFixtureWiringTest in the module owning the fixture, where it parses gateway.yaml directly and cannot fall out of step with it. Also drops clientAuthIsOwnedByTheMtlsCustomizer, a duplicate of enabledRequiresClientAuthWithTrustAnchor whose only addition asserted the Vert.x library default rather than the property-file claim it stated; SingleSourceTlsContractTest already owns that guarantee.
Both guards stated a guarantee their mechanism could not deliver. The negative control in ManagementPlainHttpActivationWiringTest iterated a hardcoded five-name list, so the case it exists for — a NEWLY added gateway instance that sets QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME by accident — was outside the enumeration and could never fail it. The set is now derived from the same docker-compose.yml parse the class already performs, keeping every api-sheriff* instance except the opt-out one. Because a derived set can silently become empty, the control now asserts its own non-emptiness first; without that, a parse change would turn it into a vacuous pass. managementRefusesHttps asserted IOException, which ConnectException, HttpConnectTimeoutException and DNS failures all satisfy — so it also passed when nothing was listening on the port at all, reporting success for the wrong reason. Narrowed to SSLException, which is what a TLS handshake against a plain listener actually raises. managementServesHealthOverPlainHttp remains the matched positive control proving the port does answer.
No behaviour changes; each edit removes a promise the implementation does not keep. (1) architecture.adoc still asserted that 'no plain-HTTP management surface remains: every scraper and probe must speak TLS', and described health as served over the same HTTPS surface — both stale since management.tls.enabled landed. They now state HTTPS as the default and shipped posture, name the opt-out and its deployment twin, and keep the single-port consequence: a downgrade takes health AND metrics together, announced by WARN ApiSheriff-115. (2) 'These keys are the single effective source' was unconditional, but the customizer is a no-op for any key gateway.yaml omits — so a deployment-supplied QUARKUS_HTTP_SSL_PROTOCOLS stays effective until tls.min_version is declared, which is how the api-sheriff-mtls instance pins TLS 1.2 today. The claim is now conditional in the four places that overstated it, while keeping the true statement that no raw quarkus.tls.* policy key ships. Also corrects the mechanism: for the terminated listener the neutral key wins by being the last writer, not by ordinal — ordinal governs the management key only. (3) The gateway.yaml example promised 'neutral names only ... trust material never appears here' ten lines above tls.mtls.client_ca, a path to trust material. client_ca is NOT deleted from the examples — it is real, schema-declared and consumed by MtlsServerCustomizer, so an mTLS example without it is unusable. The contradiction is fixed instead by naming the carve-out: gateway.yaml carries a reference to deployment-mounted material, never the material. ADR-0025's three-way rule does not distinguish a path reference from material, so this is recorded as an explicitly named exception rather than a rule the ADR carries; amending the ADR is out of scope and is raised in decision.log. Boy Scout, same paragraphs: the cipher-allowlist guidance now states that an allowlist stranding an enabled protocol is refused at boot, which the sibling implementation change made true.
… negative control The negative control asserted no non-opt-out gateway instance activates plain management via an `=`-anchored prefix match on QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME. Docker Compose also accepts a BARE list entry, whose value resolves from the host shell at `compose up` time — such an entry on an HTTPS-management instance would activate plain-HTTP management while the control still passed green. Match the bare variable name as well as the KEY=value form, and add a matched positive/negative control over the predicate itself so a regression that drops the bare form cannot pass silently. Addresses CodeRabbit review_body finding 7642e4 on PR #138. Co-Authored-By: Claude <noreply@anthropic.com>
21ff759 to
be0271a
Compare
|
/review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 03ef266b-1128-4ac5-bfb3-ddf507bb835b
📒 Files selected for processing (38)
.plan/project-architecture/api-sheriff/enriched.jsonapi-sheriff/pom.xmlapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/ConfigLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/load/ConfigLoader.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/ManagementConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/TlsConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/ManagementPlainHttpAudit.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.javaapi-sheriff/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProviderapi-sheriff/src/main/resources/application.propertiesapi-sheriff/src/main/resources/schema/gateway.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSourceTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/load/ConfigLoaderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/ManagementPlainHttpAuditTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizerTest.javaapi-sheriff/src/test/resources/application.propertiesapi-sheriff/src/test/resources/config/testboot/gateway.yamlapi-sheriff/src/test/resources/config/valid/gateway.yamldoc/LogMessages.adocdoc/adr/0025-The_whole_server-TLS_surface_is_neutral_in_gatewayyaml_and_bound_by_exactly_two_seams.adocdoc/architecture.adocdoc/configuration.adocdoc/user/README.adocdoc/user/environment-variable-overrides.adocdoc/user/tls-edge.adocintegration-tests/docker-compose.ymlintegration-tests/pom.xmlintegration-tests/scripts/start-integration-container.shintegration-tests/src/main/docker/sheriff-config/gateway.yamlintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/CipherSuiteFixtureWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpOptOutIT.java
🚧 Files skipped from review as they are similar to previous changes (32)
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/ManagementPlainHttpAuditTest.java
- api-sheriff/pom.xml
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.java
- integration-tests/src/main/docker/sheriff-config/gateway.yaml
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/ConfigLogMessages.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.java
- api-sheriff/src/test/resources/application.properties
- integration-tests/pom.xml
- api-sheriff/src/test/resources/config/testboot/gateway.yaml
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/load/ConfigLoaderTest.java
- api-sheriff/src/main/resources/schema/gateway.schema.json
- doc/architecture.adoc
- api-sheriff/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProvider
- api-sheriff/src/test/resources/config/valid/gateway.yaml
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/ManagementConfig.java
- doc/user/README.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/ManagementPlainHttpAudit.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSourceTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/load/ConfigLoader.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizerTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/TlsConfig.java
- api-sheriff/src/main/resources/application.properties
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSource.java
- doc/user/tls-edge.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java
- doc/LogMessages.adoc
- doc/configuration.adoc
- .plan/project-architecture/api-sheriff/enriched.json
- doc/user/environment-variable-overrides.adoc
Triage dispositionsIn reply to comment_id:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- affected files ---'
fd -a -t f \
'TlsServerCustomizer.java' \
'TlsServerCustomizerTest.java' \
'MtlsServerCustomizerTest.java' \
'SingleSourceTlsContractTest.java' \
'ManagementPlainHttpActivationWiringTest.java' \
'application.properties' \
'docker-compose.yml' \
| sort
printf '%s\n' '--- outlines ---'
for f in \
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.java \
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java
do
if [ -f "$f" ]; then
echo "### $f"
ast-grep outline "$f" --items all || true
fi
done
printf '%s\n' '--- cipher-suite collection and mTLS test references ---'
rg -n -C 8 \
-e 'getSupportedSSLParameters\(\)\.getCipherSuites' \
-e 'Set\.of\(' \
-e 'enabledRequiresClientAuthWithTrustAnchor' \
-e 'quarkus\.http\.ssl\.client-auth' \
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.java || true
printf '%s\n' '--- Compose environment predicate and wiring coverage ---'
rg -n -C 10 \
-e 'QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME' \
-e 'plain-management' \
-e 'environment' \
-e 'matches\(' \
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java \
integration-tests/docker-compose.yml \
api-sheriff/src/main/resources/application.properties || trueLength of output: 2657 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- affected files ---'
fd -a -t f '(TlsServerCustomizer|MtlsServerCustomizerTest|SingleSourceTlsContractTest|ManagementPlainHttpActivationWiringTest|application\.properties|docker-compose\.yml)$' . | sort
printf '%s\n' '--- outlines ---'
for f in \
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.java \
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java
do
echo "### $f"
ast-grep outline "$f" --items all || true
done
printf '%s\n' '--- cipher-suite collection and mTLS test references ---'
rg -n -C 8 \
-e 'getSupportedSSLParameters\(\)\.getCipherSuites' \
-e 'Set\.of\(' \
-e 'enabledRequiresClientAuthWithTrustAnchor' \
-e 'quarkus\.http\.ssl\.client-auth' \
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/tls/MtlsServerCustomizerTest.java \
api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.java || true
printf '%s\n' '--- Compose environment predicate and wiring coverage ---'
rg -n -C 10 \
-e 'QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME' \
-e 'plain-management' \
-e 'environment' \
-e 'matches\(' \
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/ManagementPlainHttpActivationWiringTest.java \
integration-tests/docker-compose.yml \
api-sheriff/src/main/resources/application.properties || trueLength of output: 72771
I accept the declined dispositions. The cited controls make the related review concerns inapplicable. ✏️ Learnings added
You are interacting with an AI system. |
…nt-bound keys The 'projects policy keys only' section claimed nothing an operator supplies from the environment is in the projected set. That is over-broad: the one projected key, quarkus.management.tls-configuration-name, is reachable from the environment as QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME — as the ADR's own 'deployment door' passage already says. Narrow the claim to the property the single ordinal actually depends on (no deployment-bound key, port or trust/key material is ever projected) and state the deliberate shared-key case in the same place, cross-referenced to the 'two doors onto one lever' subsection. Align the NeutralTlsConfigSource class Javadoc's matching clause so code and ADR cannot drift. Prose-only; no behaviour change.
…wn method everyDeclaredSuiteIsSupportedByThisJdk() filtered a freshly-parsed list and asserted the result empty, so an empty tls.cipher_suites fixture passed it without checking anything. The non-emptiness guarantee lived in a sibling test and evaporated if that sibling were renamed, moved or deleted. Capture declaredCipherSuites() once into a local, assert non-emptiness inside the method that depends on it, and filter that same local — which also removes the method's second parse of the fixture. theFixtureDeclaresACipherAllowlist() stays as the named statement of the fixture contract; its message no longer claims to protect the support assertion, since it no longer does. Test-only; the fixture is still parsed, never mirrored.
…y probe Two defects in the host-side readiness gate, both from the same PR review round. Time limits (CodeRabbit e877a8): every readiness and diagnostic curl ran with no --connect-timeout and no --max-time, so a listener that completed the TCP handshake but never answered blocked curl indefinitely — the loop hung on attempt 1, never reached its failure branch, and the CI job died on its outer timeout with no diagnostics. Every curl in the file now carries --connect-timeout 2 --max-time 5. The failure messages said 'within N seconds' for loops that count attempts, not seconds; they now say attempts. The loop counter comparisons are quoted (SC2086). Derived targets (CodeRabbit d15d2e): the instance list, management ports and probe scheme were hand-maintained in two places that had to mirror docker-compose.yml, under a comment telling the reader to keep them in lockstep. Each api-sheriff* service now carries a de.cuioss.sheriff.management-scheme label, and one loop discovers every instance, its host port published against container port 9000, and its scheme from the resolved Compose model. The plain-management instance is no longer a separate block — its http:// probe comes from its label, not from a branch on its name, and still uses no -k. No management port number and no gateway service name is left anywhere in the readiness path, so adding, removing or renumbering an instance needs no edit to the script. Discovery fails loudly and specifically on zero api-sheriff* services, a missing or invalid scheme label, a missing 9000 mapping, or a Compose version without 'config --format json' — a silently empty target list would make the readiness gate a no-op that passes green, which is worse than the list it replaces. Verified by running the readiness path against the live stack (all six instances discovered and probed, plain-mgmt over http) and by a matched negative control against a gateway-free Compose model, which aborts with exit 1.
The Python discovery block already exits non-zero on every path that would yield an empty target set: an empty api-sheriff* service set and any service missing its scheme label or its 9000 mapping each sys.exit with a named message. The bash-level re-check of READINESS_TARGETS could therefore never fire, so it was dead code rather than a boundary guard. TASK-22's non-vacuity guarantee is unchanged and stays enforced inside the discovery block, where each failure names the offending service. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TEnYPWdaeHMg1GLVWsTC1H
|
/review |
1 participant
Summary
The gateway's declared TLS floor and cipher-suite allowlist were inert in production — the
neutral
gateway.yamltlskeys had zero production consumers, so neither side of the apparent"double source" was actually in force. This PR makes the neutral surface the single effective
source:
gateway.yaml's TLS policy is now bound onto the terminated listener through theHttpServerOptionsCustomizerseam (TlsServerCustomizer, sibling of the existingMtlsServerCustomizer), and the rawquarkus.*duplicates that were never doing anything aredeleted outright. The management interface gets a first-class neutral
management:policy blockprojected onto
quarkus.management.*by a single-ordinal (>300) SmallRyeConfigSource(
NeutralTlsConfigSource) — the seam the management path actually enumerates, sinceVertxHttpRecorder.initializeManagementInterfacenever consultsHttpServerOptionsCustomizerbeans. The plain-HTTP management opt-out is a loud WARN, never a boot refusal, and is proven on a
dedicated sixth compose gateway instance (
api-sheriff-plain-mgmt). ADR-0025 extends ADR-0011'sneutral-name ruling from the JWKS trust surface to the whole server-TLS surface, with a three-way
policy / deployment-bound / build-time classification.
Changes
D1 — Bind the neutral
gateway.yamlTLS block to the terminated listener, delete the inert raw keys, relocate the build-time knobapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java(new): mapstls.min_version,tls.cipher_suites,tls.alpnonto the realHttpServerOptionsfor the terminated main listener; every mapping is a no-op when its key is absent.api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/TlsConfig.java: adds thecipherSuitescomponent.api-sheriff/src/main/resources/schema/gateway.schema.json: addstls.cipher_suites.api-sheriff/src/main/resources/application.properties: deletesquarkus.tls.protocols,quarkus.tls.cipher-suites,quarkus.tls.alpn,quarkus.http.ssl.client-auth(all proven inert or redundant against the shippedquarkus-vertx-http-3.37.4bytecode); relocatesquarkus.ssl.nativetoapi-sheriff/pom.xml'snativeprofile (it is a build-time GraalVM knob); retainsquarkus.http.ssl-portandquarkus.http.insecure-requestsas deliberately deployment-bound.TlsServerCustomizerTest(new),MtlsServerCustomizerTest,ConfigModelContractTest,ConfigLoaderTestupdated for the new arity/coverage;config/valid/gateway.yamland the testapplication.propertiesfixture reconciled so tests and production take the same branch throughHttpServerOptionsUtils.getTlsConfiguration's registry-default guard.D2 — Neutral management policy block and the projecting
ConfigSourceapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/ManagementConfig.java(new): neutralmanagement:block, TLS policy only — no port component (see Scope Deviation Accepted below).api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/NeutralTlsConfigSource.java(new) +META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProvider(new): single-ordinal (>300) SmallRyeConfigSourceparsinggateway.yaml'stls:/management:blocks directly (it runs before CDI).api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.java: adds themanagementcomponent.api-sheriff/src/main/resources/schema/gateway.schema.json: adds themanagementobject;portis declared solely to be refused ("not": {}plus a siblingerrorMessage), landing the actionable rejection at the exact/management/portJSON Pointer.api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/load/ConfigLoader.java: enables the validator'serrorMessageextension.ConfigLoaderTestgains the R7 gate (rejectsManagementPortNamingTheDeploymentKnobAtItsOwnPointerplus its matched positive control);NeutralTlsConfigSourceTest(new);ConfigModelContractTest,SheriffMetricsTestupdated for the new arity.D3 — Audited plain-HTTP management opt-out, proven on a sixth compose instance
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/ConfigLogMessages.java: addsWARN.MANAGEMENT_PLAIN_HTTP(ApiSheriff-115).api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/ManagementPlainHttpAudit.java(new):@ApplicationScopedStartupEventobserver emitting the WARN on the observed effective state, eagerly activated and actively invoking its collaborator to defeat the lazy-CDI-proxy trap (lesson2026-07-20-18-002).api-sheriff/src/main/resources/application.properties: declares the key-less named TLS bucket the opt-out selects, unconditionally (not%it-scoped).integration-tests/docker-compose.yml: adds the sixth gateway serviceapi-sheriff-plain-mgmt(publishing19005:9000, settingQUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME, omitting the management cert env pair); updates the "all five instances" lockstep comments to describe the deliberate exception.integration-tests/scripts/start-integration-container.sh: adds a dedicatedhttp://readiness gate block (not folded into the existinghttps://-hard-coded loop).ManagementPlainHttpAuditTest(new, real startup-event path),ManagementPlainHttpOptOutIT(new),ManagementPlainHttpActivationWiringTest(new, no-Docker descriptor-parsing test with the five HTTPS instances as the matched negative control).api-sheriff/src/main/java/de/cuioss/sheriff/gateway/tls/TlsServerCustomizer.java(commit50abc37, post-outline finalize finding):tls.cipher_suitesnow validates every declared suite againstSSLContext.getDefault().getSupportedSSLParameters().getCipherSuites()and aborts boot on an unsupported/mistyped entry, symmetric with the existingmin_version/alpnfail-closed checks.D4 — ADR-0025
doc/adr/0025-The_whole_server-TLS_surface_is_neutral_in_gatewayyaml_and_bound_by_exactly_two_seams.adoc(new): extends ADR-0011's neutral-name ruling to the whole server-TLS surface; records the policy / deployment-bound / build-time three-way classification, both binding seams (HttpServerOptionsCustomizerfor the terminated listener, the projectingConfigSourcefor management), and the rejectedManagementConfig.tlsConfigurationName()-as-fallback candidate.D5 — Documentation and the machine-asserted completion bar
doc/configuration.adoc,doc/architecture.adoc,doc/LogMessages.adoc(addsApiSheriff-115and the previously-undocumentedApiSheriff-106),doc/user/tls-edge.adoc,doc/user/README.adoc,doc/user/environment-variable-overrides.adoc(new): the three-layer documentation of the neutral TLS surface, the management opt-out, and the secure-default-vs-override-route audit of the config surface.api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/SingleSourceTlsContractTest.java(new): parses the shippedapplication.propertiesand mechanically asserts no raw server-TLSquarkus.*policy key remains, with an explicit allowlist for the two named exceptions, plus thequarkus.ssl.nativeabsent-here/present-in-pom.xml-native-profile pair.integration-tests/src/main/docker/sheriff-config/gateway.yaml: the primary IT instance now declaresmin_versionandcipher_suitesso the stack exercises agateway.yaml-sourced protocol/cipher setting end-to-end.Scope Deviation Accepted
Two scope deviations from the original outline, both recorded in this plan's
decision.logunderthe
(scope-deviation:accept)marker and reflected in the code as shipped:hard-rejection of
management.portAND aConfigValidatorrule with an actionable message.ConfigLoaderaborts on schema failure before binding ever runs, so the validator rule wouldhave been unreachable dead code. Research against the
quarkusio/quarkus3.37 branchestablished that (a)
quarkus.management.tls-configuration-namepointed at a key-less namedTLS bucket is a genuine, runtime-overridable, native-safe off-switch for management TLS with a
legible
ConfigurationExceptionon misuse, and (b) the actionable rejection message belongs inthe JSON schema at its exact JSON Pointer via the networknt
errorMessageextension.Resolution: management HTTPS stays the default on deployment-bound port 9000; the
originally-specified bespoke
gateway.yamlacknowledgement key was dropped in favour of theQuarkus-native
QUARKUS_MANAGEMENT_TLS_CONFIGURATION_NAME; WARN115is retained but triggerson observed effective state at startup, not on a declared key.
managementblock declares noport. Its only route into Quarkus would be thesame single-ordinal (>300) config source, where it would silently outrank a deployment-set
QUARKUS_MANAGEMENT_PORT.ConfigValidator/schema instead refuses amanagement.portkey witha message naming the real knob (
quarkus.management.port/QUARKUS_MANAGEMENT_PORT).Two Corrections to the Original Spec, Established by Evidence
gateway.yamlandquarkus.*bothconfigured the same concepts. In fact the neutral
tls:keys had zero production consumers— parsed, schema-validated, then never applied. The declared TLS 1.2 floor and cipher allowlist
were not in force in production. This was a live security defect, not a
configuration-tidiness problem. It hid because the test fixture supplied its certificate through
quarkus.tls.key-store.pem.*(selecting the TLS-registry listener path), while productionsupplies it through
quarkus.http.ssl.certificate.*— so tests and production took oppositebranches through
HttpServerOptionsUtils.getTlsConfiguration's registry-default guard.HttpServerOptionsCustomizeras the binding seam for the management interface.Verified against
quarkus-vertx-http-3.37.4bytecode:VertxHttpRecorderenumerates those beansonly on the main-server path, never for the management interface. ADR-0025 records the correct
seam (the projecting
ConfigSource).Verification
verify -Ppre-commitandverify -Pcoverageare green.failures, 0 errors, 0 skipped. It was run twice more after later commits, green each time.
api-sheriff-plain-mgmtinstance crash-looped with "Port 8443 seems to be in use" because itmounts the shared
sheriff-config(which declarespassthrough_sni, starting the SNI frontlistener on 8443) without the
QUARKUS_HTTP_SSL_PORT=8444move. Fixed in commitbc3d622.tls.cipher_suiteshad no fail-closed validation. A probeagainst a real Vert.x HTTPS listener established that a bogus suite name lets
server.listen()succeed — the gateway boots green and then resets every TLS handshake. Fixed in commit
50abc37with symmetric boot-failure coverage for all three TLS knobs.
Known-Open Findings (out of this plan's footprint, not fixed here)
Both are recorded as findings and documented in
doc/user/environment-variable-overrides.adocrather than fixed, because both are outside this plan's server-TLS-configuration footprint:
4b0102—QUARKUS_HTTP_SSL_PROTOCOLSon theapi-sheriff-mtlsIT instance works onlywhile that overlay's
gateway.yamldeclares notls.min_version; adding one there wouldsilently shadow the environment pin.
df7bd1— what thetoken-sheriffextension's readiness check reports in a deployment thatconfigures
gateway.yamlissuers but nosheriff.token.issuers.*was not established by readingand is flagged as explicitly inconclusive.
Related Issues
None.
Generated by plan-finalize skill
Summary by CodeRabbit
New Features
Documentation
Bug Fixes