diff --git a/api-sheriff/src/main/resources/application.properties b/api-sheriff/src/main/resources/application.properties index c77831a4..140ed276 100644 --- a/api-sheriff/src/main/resources/application.properties +++ b/api-sheriff/src/main/resources/application.properties @@ -156,8 +156,9 @@ sheriff.config.dir=config # TokenValidator used by GatewayEdgeRoute -> AuthenticationStage and reported on by # GatewayReadinessCheck. No property in this file participates in it. # - The token-sheriff-validation-quarkus extension carries its OWN sheriff.token.issuers..* -# Quarkus namespace, which feeds only its parallel UNQUALIFIED beans — the two health probes the -# exclusion below removes. The @GatewayValidator qualifier exists precisely to bypass them. +# Quarkus namespace, which feeds only its parallel UNQUALIFIED beans — the two health probes and +# the metrics collector the exclusion below removes. The @GatewayValidator qualifier exists +# precisely to bypass them. # # The shipped artifact configures no issuer on that extension surface and no named trust profile, # in ANY profile. That is the contract, not an omission: a deployment that names a @@ -165,21 +166,35 @@ sheriff.config.dir=config # refuses a name it cannot resolve rather than falling back to default trust), and a deployment that # wants extension-backed health supplies its own issuer. -# Bean-level exclusion of the token-sheriff-validation-quarkus extension's two health probes: -# health.JwksEndpointHealthCheck and health.TokenValidatorHealthCheck. +# Bean-level exclusion of the token-sheriff-validation-quarkus extension's unqualified beans: +# health.JwksEndpointHealthCheck, health.TokenValidatorHealthCheck and metrics.JwtMetricsCollector. # -# Both probes report on the extension's own UNQUALIFIED TokenValidator — the one it builds from its +# All three reach the extension's own UNQUALIFIED TokenValidator — the one it builds from its # sheriff.token.issuers..* Quarkus namespace. This gateway never uses that validator. The # request path injects the @GatewayValidator-qualified TokenValidator built from gateway.yaml's # token_validation block (GatewayEdgeRoute:272 -> AuthenticationStage), and no production class here # imports de.cuioss.sheriff.token.quarkus.* at all. The extension's namespace is consequently empty -# in every profile, so both probes report DOWN with "No issuer configurations found in properties" -# and drag aggregate /q/health/ready down — a false negative about machinery the product does not use. -# -# No JWKS observability is lost by excluding them, because they were observing nothing: both iterate -# the EXTENSION's issuer list, which is empty here, so neither ever reached this gateway's JWKS -# loaders. GatewayReadinessCheck reports `jwks` for the real, @GatewayValidator-qualified validator -# built from gateway.yaml, in that same readiness payload. +# in every profile, so its IssuerConfigResolver raises "No issuer configurations found in properties" +# for every one of them. +# +# One mismatch, two symptoms, because the three beans are driven differently: +# - The two health probes are driven by /q/health/ready and report DOWN. SmallRye composes +# readiness as a conjunction, so they drag the AGGREGATE down — a false negative about machinery +# the product does not use, which leaves a correctly-configured gateway permanently not ready. +# - metrics.JwtMetricsCollector constructor-injects the SecurityEventCounter that same unqualified +# producer produces, and carries @Scheduled(every = "10s"). Bean creation therefore fails on +# every tick, and the scheduler logs roughly sixty lines of stack trace each time. Readiness is +# unaffected and token validation keeps working; what is lost is the log. That noise buries every +# other line an operator needs, which is a real cost during integration work. +# +# No observability is lost by excluding them, because they were observing nothing. Both probes +# iterate the EXTENSION's issuer list, which is empty here, so neither ever reached this gateway's +# JWKS loaders; GatewayReadinessCheck reports `jwks` for the real, @GatewayValidator-qualified +# validator built from gateway.yaml, in that same readiness payload. The metrics collector likewise +# never published a single meter — it failed in its constructor's dependency on every tick — so +# excluding it removes stack traces, not JWT metrics. Publishing the QUALIFIED validator's +# SecurityEventCounter as meters is a real gap and separate work; it belongs next to +# SheriffMetrics.bindSecurityEventCounter, which already does exactly this for the cui-http counter. # # Be precise about what that datum means, though, because it is narrower than it reads. It is a # BOOT-TIME constructibility fact, not a live JWKS signal: TokenValidatorProducer.onStartup forces the @@ -191,16 +206,26 @@ sheriff.config.dir=config # issuer and would have carried it, had it ever been pointed at these issuers. Closing the gap means # giving GatewayReadinessCheck a live loader-status read; it is not something this exclusion removed. # -# The scope is deliberately those two beans in that ONE package and nothing else. This is NOT -# quarkus.health.extensions.enabled=false and NOT any other global switch: blanket-disabling a -# security-relevant readiness check to force a probe green is forbidden (ADR-0022 posture). The -# extension DEPENDENCY likewise stays — its deployment processor registers the GraalVM reflection for -# the de.cuioss.sheriff.token.validation.* classes this gateway does use at runtime, so dropping it -# would break the native image. +# The scope is deliberately those beans in those TWO packages and nothing else. This is NOT +# quarkus.health.extensions.enabled=false, NOT quarkus.scheduler.enabled=false and NOT any other +# global switch: blanket-disabling a security-relevant readiness check to force a probe green is +# forbidden (ADR-0022 posture), and disabling the scheduler wholesale would silently take out any +# future scheduled job this gateway itself wants. The extension DEPENDENCY likewise stays — its +# deployment processor registers the GraalVM reflection for the de.cuioss.sheriff.token.validation.* +# classes this gateway does use at runtime, so dropping it would break the native image. +# +# Named per package rather than as one de.cuioss.sheriff.token.quarkus.* sweep, because that surface +# also carries beans a future change here may legitimately want (the @BearerToken producer, the +# claim-mapper registry). A sweep would remove them silently at the moment someone reached for one. +# +# ExtensionUnqualifiedBeanExclusionTest pins both packages, and its second assertion is the fitness +# function for the next bean of this shape: no scheduled job anywhere may name the extension's +# Quarkus wiring. That catches an eagerly-driven newcomer the moment it lands, which a bean-by-bean +# assertion never would. # # Declared UNCONDITIONALLY and in no profile: a %-profile-scoped exclusion would itself be a profile # branch in the shipped configuration surface, which the standing rule forbids. -quarkus.arc.exclude-types=de.cuioss.sheriff.token.quarkus.health.* +quarkus.arc.exclude-types=de.cuioss.sheriff.token.quarkus.health.*,de.cuioss.sheriff.token.quarkus.metrics.* # Logging quarkus.log.level=INFO diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/ExtensionUnqualifiedBeanExclusionTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/ExtensionUnqualifiedBeanExclusionTest.java new file mode 100644 index 00000000..6b6b0cb9 --- /dev/null +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/ExtensionUnqualifiedBeanExclusionTest.java @@ -0,0 +1,195 @@ +/* + * Copyright © 2026 CUI-OpenSource-Software (info@cuioss.de) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package de.cuioss.sheriff.gateway.quarkus; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; + + +import de.cuioss.sheriff.token.quarkus.metrics.JwtMetricsCollector; + +import io.quarkus.scheduler.Scheduler; +import io.quarkus.scheduler.Trigger; +import io.quarkus.test.junit.QuarkusTest; +import jakarta.enterprise.inject.Instance; +import jakarta.inject.Inject; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Pins the scope of the token-validation extension's bean exclusion: no extension bean that + * reaches the extension's unqualified {@code TokenValidator} survives into the running + * application. + *

+ * {@link DefaultProfileReadinessTest} pins the readiness half of the same decision — the two + * {@code health.*} probes. This class pins the other half, and it exists because the health probes + * were not the only bean in that position. The extension's {@code metrics.JwtMetricsCollector} + * constructor-injects a {@code SecurityEventCounter} produced by the same unqualified + * {@code producer.TokenValidatorProducer}, which resolves issuers from the extension's + * {@code sheriff.token.issuers..*} namespace. This gateway never populates that namespace — the + * request path runs off the {@code @GatewayValidator}-qualified validator built from + * {@code gateway.yaml} — so the producer throws + * {@code IllegalStateException: No issuer configurations found in properties} on every attempt to + * create it. + *

+ * That bean is {@code @Scheduled(every = "10s")}, which turns a latent misfit into a continuous one: + * the collector is re-created on every tick, fails on every tick, and the scheduler logs a full stack + * trace each time. The gateway itself stays healthy and token validation keeps working — the damage + * is roughly sixty lines of stack trace every ten seconds, which buries every other log line the + * operator needs. + *

+ * Two assertions, because either alone would be weak. + *

+ *

+ * Note what the general assertion does not claim. It observes registered scheduled jobs, so + * it catches an eagerly-driven extension bean only when the scheduler is what drives it. An extension + * bean driven by a startup observer or by an HTTP endpoint would pass it. The exclusion remains a + * deliberate, reviewed decision rather than something these assertions can derive. + *

+ * With the exclusion in place nothing in this application schedules anything, so Quarkus does not + * start the scheduler at all and the assertion passes over an empty set. That is the intended resting + * state and it is not vacuous for the purpose: the moment a bean under the extension's Quarkus + * package registers a {@code @Scheduled} method the scheduler starts, the job appears, and the + * assertion fires — which is exactly the regression it exists to catch. Because that resting state + * makes the assertion green over an empty input, the filter it depends on carries its own matched + * positive/negative control, so a filter that had stopped matching anything could not pass for a + * gateway that schedules nothing. + * + * @author API Sheriff Team + * @since 1.0 + */ +@QuarkusTest +@DisplayName("Token-validation extension: unqualified bean exclusion") +class ExtensionUnqualifiedBeanExclusionTest { + + /** + * Package prefix of the extension's Quarkus wiring — the whole surface that reads the + * {@code sheriff.token.issuers.*} namespace this gateway never populates. Matched as a substring + * because a scheduled job is identified by {@code #} with a generated prefix. + */ + private static final String EXTENSION_QUARKUS_PACKAGE = "de.cuioss.sheriff.token.quarkus."; + + @Inject + Instance extensionMetricsCollector; + + @Inject + Instance gatewayMetrics; + + @Inject + Instance scheduler; + + @Test + @DisplayName("Should exclude the extension's JwtMetricsCollector from bean discovery") + void shouldExcludeExtensionMetricsCollectorFromBeanDiscovery() { + // Arrange — the paired guard: the gateway's own metrics bean proves CDI and the metrics + // subsystem genuinely ran, so the absence asserted below is an exclusion rather than a + // container that contributed nothing. + assertTrue(gatewayMetrics.isResolvable(), + "guard: the gateway's own " + SheriffMetrics.class.getSimpleName() + " must resolve — " + + "without it the absence asserted below would prove nothing about the exclusion"); + + // Act + Assert — the exclusion took effect: the extension's collector is not a bean at all. + assertFalse(extensionMetricsCollector.isResolvable(), + "quarkus.arc.exclude-types must remove " + JwtMetricsCollector.class.getName() + + " from bean discovery. It constructor-injects the SecurityEventCounter produced by the " + + "extension's UNQUALIFIED TokenValidatorProducer, which resolves issuers from the " + + "sheriff.token.issuers.* namespace this gateway never populates — so every @Scheduled " + + "tick fails with 'No issuer configurations found in properties'"); + } + + @Test + @DisplayName("Should register no scheduled job from the token-validation extension") + void shouldRegisterNoScheduledJobFromTheExtension() { + // Arrange — without a resolvable Scheduler there is nothing to inspect and the assertion + // below would be vacuously green. + assertTrue(scheduler.isResolvable(), + "guard: the Quarkus Scheduler must resolve — otherwise the registered-job set is " + + "unobservable and the assertion below proves nothing"); + + // Act — a not-started scheduler is the expected state here and is STRONGER than an empty + // filtered set: Quarkus starts the scheduler only once some bean registers a @Scheduled + // method, so "not started" means nothing is scheduled anywhere in the application, extension + // or otherwise. Reading getScheduledJobs() in that state throws rather than returning empty. + Scheduler resolved = scheduler.get(); + List extensionJobs = resolved.isStarted() + ? extensionJobsAmong(resolved.getScheduledJobs().stream() + .map(ExtensionUnqualifiedBeanExclusionTest::describe) + .toList()) + : List.of(); + + // Assert — nothing from the extension's Quarkus wiring runs on a schedule in this gateway. + assertTrue(extensionJobs.isEmpty(), + "no scheduled job may come from " + EXTENSION_QUARKUS_PACKAGE + "* — every bean there reads " + + "the extension's empty sheriff.token.issuers.* namespace, so a scheduled one fails on " + + "every tick and floods the log. Registered extension jobs: " + extensionJobs + + ". Extend quarkus.arc.exclude-types in application.properties rather than relaxing " + + "this assertion"); + } + + /** + * Matched positive/negative control over the filter the assertion above depends on. Without it + * that assertion is unfalsifiable-by-inspection: it is green today because nothing is scheduled, + * and it would be equally green if the filter matched nothing at all — which is precisely the + * state it must detect when a future extension bean lands. + */ + @Test + @DisplayName("The filter flags an extension-owned job and ignores a gateway-owned one") + void filterFlagsExtensionJobsAndIgnoresGatewayJobs() { + // Arrange — the extension-owned entry is the real trigger identity observed on a gateway + // running without the exclusion; the gateway-owned one is the near miss it must not flag. + String extensionJob = "1_de.cuioss.sheriff.token.quarkus.metrics.JwtMetricsCollector#updateCounters " + + "(de.cuioss.sheriff.token.quarkus.metrics.JwtMetricsCollector#updateCounters)"; + String gatewayJob = "2_de.cuioss.sheriff.gateway.quarkus.SheriffMetrics#sweep " + + "(de.cuioss.sheriff.gateway.quarkus.SheriffMetrics#sweep)"; + String validationLibraryJob = "3_de.cuioss.sheriff.token.validation.SomeJob#run " + + "(de.cuioss.sheriff.token.validation.SomeJob#run)"; + + // Act + List flagged = extensionJobsAmong(List.of(gatewayJob, extensionJob, validationLibraryJob)); + + // Assert — the filter selects the extension's Quarkus wiring and only that. The validation + // library entry is the near miss that matters: it shares the de.cuioss.sheriff.token prefix + // but is the library this gateway genuinely uses, so a prefix trimmed one segment too short + // would over-select it. + assertEquals(List.of(extensionJob), flagged, + "the filter must flag a job under " + EXTENSION_QUARKUS_PACKAGE + " and leave both the " + + "gateway's own package and the validation library the gateway does use alone"); + } + + /** @return the entries of {@code jobDescriptions} owned by the extension's Quarkus wiring */ + private static List extensionJobsAmong(List jobDescriptions) { + return jobDescriptions.stream() + .filter(description -> description.contains(EXTENSION_QUARKUS_PACKAGE)) + .toList(); + } + + /** @return an identity for {@code trigger} that names the bean class whichever field carries it */ + private static String describe(Trigger trigger) { + return trigger.getId() + " (" + trigger.getMethodDescription() + ")"; + } +} diff --git a/doc/adr/0027-The_token-validation_extensions_unqualified_beans_are_excluded_not_accommodated.adoc b/doc/adr/0027-The_token-validation_extensions_unqualified_beans_are_excluded_not_accommodated.adoc new file mode 100644 index 00000000..107a89d3 --- /dev/null +++ b/doc/adr/0027-The_token-validation_extensions_unqualified_beans_are_excluded_not_accommodated.adoc @@ -0,0 +1,293 @@ += ADR-0027: The token-validation extension's unqualified beans are excluded, not accommodated +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// Progressive-disclosure metadata block (see manage-adr SKILL.md → "ADR Template +// Structure"). Read by `manage-adr.py scan` so a caller can assess an ADR's +// relevance without reading the full file. List fields are comma-separated. +// summary: The gateway drives token validation from gateway.yaml through the @GatewayValidator qualifier and never uses the token-validation extension's unqualified validator, so every extension bean that reaches it — two health probes and a scheduled metrics collector — is removed from the bean set rather than configured into a working state; the extension dependency itself stays for the GraalVM reflection registration the gateway does use, and the resulting readiness `jwks` datum is a boot-time constructibility fact rather than a live signal. +// tags: health-checks, readiness, metrics, scheduler, cdi, bean-exclusion, observability, extension-boundary, native-image, token-validation +// affects: api-sheriff +// supersedes: +// end-adr-metadata + +// Authoring discipline (see manage-adr SKILL.md → "Authoring Discipline"): +// ADRs are durable architectural statements, not incident write-ups. No PR numbers, +// commit SHAs, dates, or lesson IDs anywhere in the body. CLAUDE.md's "current state +// only" rule applies — describe the architecture, not the chronology. + +== Status + +Proposed + +== Context + +API Sheriff depends on the token-sheriff validation extension for its JWT validation machinery. That +dependency brings two things onto the runtime classpath, and they are separable: + +* *The validation library.* The classes that parse and verify tokens, which the gateway uses + directly. +* *The extension's own opinionated wiring.* A configuration namespace + (`sheriff.token.issuers..*`), a producer that builds an unqualified `TokenValidator` and a + `SecurityEventCounter` from it, two readiness probes that report on that validator, and a metrics + collector that publishes that counter. + +The gateway does not use the second. Its request path injects a `@GatewayValidator`-qualified +`TokenValidator` assembled from `gateway.yaml`'s `token_validation` block, and the qualifier exists +precisely to bypass the extension's unqualified bean. The gateway's own readiness check reads the +same qualified validator. No production class imports the extension's Quarkus wiring package at all. + +This produces a structural mismatch rather than a misconfiguration. Every one of those beans resolves +through the *extension's* issuer list, which this gateway never populates, so every one of them meets +the same "no issuer configurations found" condition. The mismatch is single; how it *presents* +depends on what drives each bean, and the two shapes cost different things: + +* *Driven by the health endpoint.* The two probes report DOWN. SmallRye Health composes readiness as + the conjunction of every `@Readiness` contributor, so probes reporting on machinery the product + does not use take aggregate readiness DOWN for the whole process. A correctly-configured gateway -- + one that drives validation entirely from `gateway.yaml`, as the architecture intends -- is + therefore permanently not ready. +* *Driven by the scheduler.* The metrics collector carries a ten-second `@Scheduled` trigger and + constructor-injects the counter, so bean creation fails on every tick and the scheduler logs a full + stack trace each time. Readiness is unaffected and token validation keeps working; the log is what + is lost, at roughly sixty stack-trace lines every ten seconds, which is enough to bury everything + else an operator needs to read. + +That second shape is the reason this decision is stated over *beans* rather than over *probes*. A +rule scoped to health checks answers only the symptom that happened to be found first, and leaves the +next bean in the same position to be rediscovered from its own symptom. + +The choice point this creates is an ownership question, and it generalises past this one extension. +When a dependency ships observability for a mechanism the consumer deliberately does not use, the +consumer can either *feed* that mechanism enough configuration to make it work, or *remove* it from +the consumer's observability surface. Feeding it means maintaining configuration for machinery with +no consumer, purely to satisfy a check. Removing it means the consumer's observability surface is +composed deliberately rather than inherited. + +A second constraint bounds the available answers. Removing the *dependency* is not equivalent to +removing the *beans*: the extension's deployment processor registers the GraalVM reflection +metadata for the validation classes the gateway genuinely does use at runtime, so the dependency is +load-bearing for the native image regardless of whether any of its beans are wanted. + +== Decision + +*A bean that observes machinery the product does not use is removed from the bean set, not +configured into a working state.* + +The extension's unqualified beans are excluded at the CDI level by type pattern, scoped to exactly +the health and metrics packages of its Quarkus wiring. The exclusion is declared unconditionally and +in no profile, because a profile-scoped exclusion would itself be a branch in the shipped +configuration surface, which +link:0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc[ADR-0032] +forbids. + +=== 1. The scope is those packages, never a global switch + +The exclusion names the health and metrics packages of the extension's Quarkus wiring and nothing +else. It is specifically *not* a blanket disable of extension-contributed health checks, *not* a +disable of the scheduler, and not any other global switch. Blanket-disabling a security-relevant +readiness check to force a probe green is refused under the posture of +link:0022-Upstream_security-hardening_default_changes_are_adopted_and_documented_never_locally_reverted.adoc[ADR-0022]; +disabling the scheduler wholesale would silently take out any scheduled job this gateway itself later +wants. The narrow exclusion removes beans that observe nothing here, whereas a global switch would +also remove beans that observe something, and would keep doing so silently as the dependency set +grows. + +The scope stops short of the whole `de.cuioss.sheriff.token.quarkus.*` surface for the same reason. +That package tree also carries beans a future change here may legitimately want -- the `@BearerToken` +producer, the claim-mapper registry -- and a sweeping pattern would remove them at the moment someone +reached for one, with nothing announcing why. + +=== 2. The scope is enforced by a fitness function, not by vigilance + +The exclusion is a list of packages, and a list is only ever as complete as the last symptom someone +investigated: the health probes were excluded from their symptom, and the metrics collector was found +later from a different one. A test therefore asserts the general form of the rule -- *no scheduled +job may name the extension's Quarkus wiring* -- alongside the specific bean assertions. It fails when +an eagerly-driven newcomer lands rather than when an operator eventually reads the log. + +That fitness function is deliberately partial and is documented as such. It observes registered +scheduled jobs, so it catches the driver that produced the second symptom and not every possible one; +a bean driven by a startup observer or an HTTP endpoint would pass it. The exclusion list remains a +reviewed decision rather than something a test can derive. + +The stronger check -- deriving the required scope from the extension's own CDI registrations rather +than from a hand-maintained list -- was tried and is not available, and the reason is worth recording +so it is not tried again from scratch: + +* *Resolvability of the unqualified `TokenValidator` is not a proxy for reachability.* The obvious + driver-agnostic assertion is that the extension's unqualified validator is unresolvable, on the + reasoning that ARC keeps a producer only while something injects from it. Measured, it is resolvable + even with every reaching bean excluded: the extension's producer is retained regardless. Retained is + not instantiated -- it is `@ApplicationScoped` and nothing calls a method on it, which is why the + ten-second failure is genuinely gone -- but that makes resolvability unable to distinguish the + healthy state from the broken one, so it cannot carry the assertion. +* *Enumerating the container's beans and comparing them to the exclusion list is a membership + snapshot,* which + link:0030-Comment-only_invariants_become_positively-phrased_fitness_functions_shipped_with_a_four-leg_control_set.adoc[ADR-0030] + forbids: a rule asserts a property, never equality with today's members. Such a rule would fail on + every legitimate extension upgrade that adds an unrelated bean, and would be relaxed away. + +What remains derivable is the *symptom*, not the *scope* -- which is what the scheduled-job assertion +checks. The gap is therefore real and is carried in Risks below rather than papered over. + +=== 3. The dependency stays + +Only the beans are excluded. The extension remains a compile dependency because its deployment +processor registers the GraalVM reflection metadata for the validation classes the gateway uses at +runtime. Dropping it would trade three unwanted beans for a new compile dependency on the core +validation library plus a hand-maintained replacement for reflection registration that is currently +generated -- a strictly worse position. + +=== 4. No observability is lost, because none was being produced + +The excluded probes were not observing this gateway's JWKS loaders. Both iterate the *extension's* +issuer list; that list is empty in every profile, so neither probe ever reached the loaders the +gateway actually builds from `gateway.yaml`. Their DOWN status was a report about an unconfigured +parallel mechanism, not a signal about the product. The gateway's own readiness check reports a +`jwks` datum for the real, qualified validator in the same readiness payload. + +The excluded metrics collector had published no meter either. It failed inside its own dependency +resolution on every tick, so it never reached the point of registering anything -- what the exclusion +removes is the stack trace, not a JWT metric. Publishing the *qualified* validator's +`SecurityEventCounter` as meters is genuinely absent and is separate work; the seam for it is the +gateway's own metrics adapter, which already binds the `cui-http` security counter the same way. + +=== 5. That `jwks` datum is boot-time constructibility, not a live signal + +This is stated as part of the decision rather than left to be discovered, because the datum reads +more strongly than it is. The producer forces the application-scoped validator into existence at +boot, and a failure there aborts startup; afterwards the probe's lookup returns the cached instance +and reports ready for the remainder of the process lifetime. An IdP whose JWKS endpoint becomes +unreachable later, a stalled key rotation, or an expired signing key therefore do *not* take +readiness DOWN. + +That is an open gap in the gateway's own probe, and it *pre-dates this exclusion rather than being +caused by it* -- the excluded probe polled per-issuer loader status and would have carried a live +signal, but only for issuers it was pointed at, and it was never pointed at these. Closing the gap +means giving the gateway's own readiness check a live loader-status read. It is not something this +exclusion removed, and restoring the excluded probes would not close it. + +== Consequences + +=== Positive + +* A correctly-configured gateway reports ready. Aggregate readiness reflects the mechanism the + product actually runs on instead of a parallel one it deliberately bypasses. +* The application log is usable. A recurring stack trace on a ten-second cycle is not a cosmetic + defect: it is the difference between logs that can be read during an incident and logs that cannot. +* The observability surface is composed deliberately. Each contributor is there because it observes + something this product does, rather than because a dependency happened to contribute it. +* No configuration is maintained for machinery with no consumer. The alternative -- populating the + extension's issuer namespace to satisfy its beans -- would have created exactly the redundant + parallel declaration ADR-0032 removes. +* The native image is unaffected: the reflection registration the gateway relies on is a + deployment-time concern and is untouched by a runtime bean exclusion. + +=== Negative + +* The gateway's own readiness check is now the *only* JWKS readiness coverage. There is no second + opinion, so a defect in that check is a defect in the whole signal. +* The exclusion is expressed as type patterns over another project's packages. A reorganisation of + the extension's health or metrics package would silently stop matching, and the beans would return + without anything failing to announce it. +* The exclusion list is per-package and grows by discovery. The fitness function narrows that to the + scheduler-driven case; a bean in the same position driven some other way is still found from its + symptom. +* Someone reading the readiness payload sees a `jwks` datum that reports ready and will reasonably + read it as a live signal. It is not one, and only the documentation says so. + +=== Risks + +* *The boot-time-constructibility datum is mistaken for liveness.* This is the principal residual + risk and it is accepted deliberately: the gap is real, it pre-dates this decision, and closing it + is separate work. Until then a JWKS endpoint can become unreachable, a key rotation can stall, or + a signing key can expire, and readiness will continue to report ready throughout. Operational + alerting must not treat this readiness probe as coverage for those conditions. + +* *The exclusion outlives its justification.* It is correct only while the extension's issuer + namespace remains unpopulated in this artifact. A deployment that ever supplies extension-backed + issuers would want those beans back, and nothing detects that combination -- the configuration + would simply be observed by nothing. The exclusion is bound to the architectural stance that + validation is driven from `gateway.yaml`, and it must be revisited if that stance changes. + +* *The list lags the extension.* Each entry was added after a symptom was observed in a running + deployment rather than derived from the extension's bean set. The fitness function closes that + loop for scheduler-driven beans; for any other driver, the loop is still an operator noticing + something. An extension release that adds an eagerly-driven bean outside those two packages will + reproduce this class of defect, and the mitigation is to read the exclusion's scope when the + dependency is upgraded rather than to trust the list. + +== Alternatives Considered + +=== Populate the extension's issuer namespace so its beans work + +Supply the extension with enough issuer configuration to satisfy its own beans, leaving all of them +in the observability surface. + +Rejected because it maintains configuration for machinery with no consumer, purely to satisfy a +check. The probes would then report on a validator the request path never touches, so a green +result would carry no information about the gateway while looking exactly like one that did, and the +metrics collector would publish counters for a validator that counts nothing. It also recreates the +parallel declaration ADR-0032 removes, in the same file, for the same non-reason. + +=== Disable extension-contributed health checks, or the scheduler, globally + +Use a global switch -- suppress health checks contributed by extensions, or turn the scheduler off -- +which is a single key and needs no knowledge of the extension's package structure. + +Rejected on blast radius. It would suppress beans that *do* observe something, both now and as the +dependency set grows, and would do so silently -- the failure mode is an observability surface that +quietly stops covering things nobody decided to stop covering. Forcing a security-relevant readiness +check green with a blanket switch is refused under ADR-0022's posture, and disabling the scheduler +would pre-emptively remove a mechanism this gateway has not yet used but has no reason to forbid. The +narrow exclusion is more brittle to package reorganisation, which is the accepted cost of not +over-reaching. + +=== Drop the extension dependency entirely + +Remove the dependency, taking its beans with it, and depend directly on the core validation +library. + +Rejected on cost and on native-image correctness. The extension's deployment processor generates the +GraalVM reflection registration for the validation classes the gateway uses at runtime; removing it +requires a new compile dependency *plus* a hand-maintained replacement for registration that is +currently derived automatically. That is a larger, more fragile surface adopted to solve a problem +that one line of bean exclusion solves exactly. + +=== Escalate to the extension as a configuration-model finding and wait + +Report upstream that a bean gating on a namespace a consumer may legitimately never populate should +neither force aggregate readiness DOWN nor throw on a schedule, and adopt the fix when it lands. + +Rejected as the sole remedy, on timing rather than on merit -- the observation is sound and is worth +raising, and the scheduled collector makes the stronger version of the case: a metrics collector that +finds no property-based issuers should warn once and go idle rather than fail hard on every tick, for +any embedder in this position. But it puts this gateway's readiness and its log on another project's +release cycle. ADR-0022 directs that a tightened upstream default the gateway cannot accommodate is +escalated rather than patched around; that posture governs *security defaults*, and this is not one. +These are observability beans reporting on a mechanism this consumer deliberately does not use, and +declining to instantiate them narrows nothing the product relies on. Raising it upstream and +excluding here are complementary, not alternatives. + +The unifying principle: the first two alternatives try to make an uninformative bean *work*, and the +last two try to make it *someone else's problem*. An observability surface is composed by the +application that serves it -- a bean belongs in it when it observes something that application does, +and a bean that observes nothing is removed rather than satisfied. + +== References + +* link:0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc[ADR-0032] + -- the rule under which this exclusion is declared unconditionally rather than profile-scoped, and + the masked-defect failure mode a profile-scoped health branch would have reproduced +* link:0022-Upstream_security-hardening_default_changes_are_adopted_and_documented_never_locally_reverted.adoc[ADR-0022] + -- the posture that forbids forcing a security-relevant check green with a global switch, and its + scope boundary against this case +* link:0011-gatewayyaml_exposes_JWKS_trust_and_egress_as_neutral_names_bound_by_the_deployment.adoc[ADR-0011] + -- the `gateway.yaml` token-validation surface that the qualified validator is built from +* link:0005-module-structure.adoc[ADR-0005] -- the framework-bound edge seam that keeps + runtime-specific assembly at the boundary +* link:../configuration.adoc[Configuration reference] -- the `token_validation` surface and the + readiness contract diff --git a/doc/adr/0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc b/doc/adr/0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc deleted file mode 100644 index f83b7f27..00000000 --- a/doc/adr/0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc +++ /dev/null @@ -1,217 +0,0 @@ -= ADR-0027: The token-validation extension's unqualified health probes are excluded, not accommodated -:toc: left -:toclevels: 2 -:sectnums: - -// adr-metadata -// Progressive-disclosure metadata block (see manage-adr SKILL.md → "ADR Template -// Structure"). Read by `manage-adr.py scan` so a caller can assess an ADR's -// relevance without reading the full file. List fields are comma-separated. -// summary: The gateway drives token validation from gateway.yaml through the @GatewayValidator qualifier and never uses the token-validation extension's unqualified validator, so the extension's two health probes are removed from the bean set rather than configured into a green state; the extension dependency itself stays for the GraalVM reflection registration the gateway does use, and the resulting readiness `jwks` datum is a boot-time constructibility fact rather than a live signal. -// tags: health-checks, readiness, cdi, bean-exclusion, observability, extension-boundary, native-image, token-validation -// affects: api-sheriff -// supersedes: -// end-adr-metadata - -// Authoring discipline (see manage-adr SKILL.md → "Authoring Discipline"): -// ADRs are durable architectural statements, not incident write-ups. No PR numbers, -// commit SHAs, dates, or lesson IDs anywhere in the body. CLAUDE.md's "current state -// only" rule applies — describe the architecture, not the chronology. - -== Status - -Proposed - -== Context - -API Sheriff depends on the token-sheriff validation extension for its JWT validation machinery. That -dependency brings two things onto the runtime classpath, and they are separable: - -* *The validation library.* The classes that parse and verify tokens, which the gateway uses - directly. -* *The extension's own opinionated wiring.* A configuration namespace - (`sheriff.token.issuers..*`), a producer that builds an unqualified `TokenValidator` from - it, and two readiness probes that report on that validator. - -The gateway does not use the second. Its request path injects a `@GatewayValidator`-qualified -`TokenValidator` assembled from `gateway.yaml`'s `token_validation` block, and the qualifier exists -precisely to bypass the extension's unqualified bean. The gateway's own readiness check reads the -same qualified validator. No production class imports the extension's Quarkus wiring package at all. - -This produces a structural mismatch rather than a misconfiguration. The extension's two probes -iterate the *extension's* issuer list, which this gateway never populates, so both report DOWN with -a "no issuer configurations found" condition. SmallRye Health composes readiness as the conjunction -of every `@Readiness` contributor, so two probes reporting on machinery the product does not use -take aggregate readiness DOWN for the whole process. A correctly-configured gateway -- one that -drives validation entirely from `gateway.yaml`, as the architecture intends -- is therefore -permanently not ready. - -The choice point this creates is an ownership question, and it generalises past this one extension. -When a dependency ships observability for a mechanism the consumer deliberately does not use, the -consumer can either *feed* that mechanism enough configuration to make its probes green, or *remove* -the probes from its health surface. Feeding it means maintaining configuration for machinery with no -consumer, purely to satisfy a check. Removing it means the consumer's health surface is composed -deliberately rather than inherited. - -A second constraint bounds the available answers. Removing the *dependency* is not equivalent to -removing the *probes*: the extension's deployment processor registers the GraalVM reflection -metadata for the validation classes the gateway genuinely does use at runtime, so the dependency is -load-bearing for the native image regardless of whether any of its beans are wanted. - -== Decision - -*A health probe that reports on machinery the product does not use is removed from the bean set, -not configured into a green state.* - -The two extension probes are excluded at the CDI level by type pattern, scoped to exactly those two -beans in that one package. The exclusion is declared unconditionally and in no profile, because a -profile-scoped exclusion would itself be a branch in the shipped configuration surface, which -link:0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc[ADR-0032] -forbids. - -=== 1. The scope is the two beans, never a global switch - -The exclusion names the health package of the extension's Quarkus wiring and nothing else. It is -specifically *not* a blanket disable of extension-contributed health checks, and not any other -global switch. Blanket-disabling a security-relevant readiness check to force a probe green is -refused under the posture of -link:0022-Upstream_security-hardening_default_changes_are_adopted_and_documented_never_locally_reverted.adoc[ADR-0022]: -the narrow exclusion removes two probes that observe nothing here, whereas a global switch would -also remove probes that observe something, and would keep doing so silently as the dependency set -grows. - -=== 2. The dependency stays - -Only the beans are excluded. The extension remains a compile dependency because its deployment -processor registers the GraalVM reflection metadata for the validation classes the gateway uses at -runtime. Dropping it would trade two unwanted beans for a new compile dependency on the core -validation library plus a hand-maintained replacement for reflection registration that is currently -generated -- a strictly worse position. - -=== 3. No JWKS observability is lost, because none was being produced - -The excluded probes were not observing this gateway's JWKS loaders. Both iterate the *extension's* -issuer list; that list is empty in every profile, so neither probe ever reached the loaders the -gateway actually builds from `gateway.yaml`. Their DOWN status was a report about an unconfigured -parallel mechanism, not a signal about the product. The gateway's own readiness check reports a -`jwks` datum for the real, qualified validator in the same readiness payload. - -=== 4. That `jwks` datum is boot-time constructibility, not a live signal - -This is stated as part of the decision rather than left to be discovered, because the datum reads -more strongly than it is. The producer forces the application-scoped validator into existence at -boot, and a failure there aborts startup; afterwards the probe's lookup returns the cached instance -and reports ready for the remainder of the process lifetime. An IdP whose JWKS endpoint becomes -unreachable later, a stalled key rotation, or an expired signing key therefore do *not* take -readiness DOWN. - -That is an open gap in the gateway's own probe, and it *pre-dates this exclusion rather than being -caused by it* -- the excluded probe polled per-issuer loader status and would have carried a live -signal, but only for issuers it was pointed at, and it was never pointed at these. Closing the gap -means giving the gateway's own readiness check a live loader-status read. It is not something this -exclusion removed, and restoring the excluded probes would not close it. - -== Consequences - -=== Positive - -* A correctly-configured gateway reports ready. Aggregate readiness reflects the mechanism the - product actually runs on instead of a parallel one it deliberately bypasses. -* The health surface is composed deliberately. Each contributor is there because it observes - something this product does, rather than because a dependency happened to contribute it. -* No configuration is maintained for machinery with no consumer. The alternative -- populating the - extension's issuer namespace to satisfy its probes -- would have created exactly the redundant - parallel declaration ADR-0032 removes. -* The native image is unaffected: the reflection registration the gateway relies on is a - deployment-time concern and is untouched by a runtime bean exclusion. - -=== Negative - -* The gateway's own readiness check is now the *only* JWKS readiness coverage. There is no second - opinion, so a defect in that check is a defect in the whole signal. -* The exclusion is expressed as a type pattern over another project's package. A reorganisation of - the extension's health package would silently stop matching, and the probes would return without - anything failing to announce it. -* Someone reading the readiness payload sees a `jwks` datum that reports ready and will reasonably - read it as a live signal. It is not one, and only the documentation says so. - -=== Risks - -* *The boot-time-constructibility datum is mistaken for liveness.* This is the principal residual - risk and it is accepted deliberately: the gap is real, it pre-dates this decision, and closing it - is separate work. Until then a JWKS endpoint can become unreachable, a key rotation can stall, or - a signing key can expire, and readiness will continue to report ready throughout. Operational - alerting must not treat this readiness probe as coverage for those conditions. - -* *The exclusion outlives its justification.* It is correct only while the extension's issuer - namespace remains unpopulated in this artifact. A deployment that ever supplies extension-backed - issuers would want those probes back, and nothing detects that combination -- the configuration - would simply be observed by nothing. The exclusion is bound to the architectural stance that - validation is driven from `gateway.yaml`, and it must be revisited if that stance changes. - -== Alternatives Considered - -=== Populate the extension's issuer namespace so its probes report UP - -Supply the extension with enough issuer configuration to satisfy its own probes, leaving both in the -health surface. - -Rejected because it maintains configuration for machinery with no consumer, purely to satisfy a -check. The probes would then report on a validator the request path never touches, so a green -result would carry no information about the gateway while looking exactly like one that did. It also -recreates the parallel declaration ADR-0032 removes, in the same file, for the same non-reason. - -=== Disable extension-contributed health checks globally - -Use a global switch to suppress health checks contributed by extensions, which is a single key and -needs no knowledge of the extension's package structure. - -Rejected on blast radius. It would suppress probes that *do* observe something, both now and as the -dependency set grows, and would do so silently -- the failure mode is a health surface that quietly -stops covering things nobody decided to stop covering. Forcing a security-relevant readiness check -green with a blanket switch is refused under ADR-0022's posture. The narrow exclusion is more -brittle to package reorganisation, which is the accepted cost of not over-reaching. - -=== Drop the extension dependency entirely - -Remove the dependency, taking its probes with it, and depend directly on the core validation -library. - -Rejected on cost and on native-image correctness. The extension's deployment processor generates the -GraalVM reflection registration for the validation classes the gateway uses at runtime; removing it -requires a new compile dependency *plus* a hand-maintained replacement for registration that is -currently derived automatically. That is a larger, more fragile surface adopted to solve a problem -that two lines of bean exclusion solve exactly. - -=== Escalate to the extension as a configuration-model finding and wait - -Report upstream that probes gating on a namespace a consumer may legitimately never populate should -not force aggregate readiness DOWN, and adopt the fix when it lands. - -Rejected as the sole remedy, on timing rather than on merit -- the observation is sound and is worth -raising. But it puts this gateway's readiness on another project's release cycle, during which a -correctly-configured deployment cannot report ready. ADR-0022 directs that a tightened upstream -default the gateway cannot accommodate is escalated rather than patched around; that posture governs -*security defaults*, and this is not one. It is an observability bean reporting on a mechanism this -consumer deliberately does not use, and declining to instantiate it narrows nothing the product -relies on. - -The unifying principle: the first two alternatives try to make an uninformative probe *green*, and -the last two try to make it *someone else's problem*. A health surface is composed by the -application that serves it -- a probe belongs in it when it observes something that application -does, and a probe that observes nothing is removed rather than satisfied. - -== References - -* link:0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc[ADR-0032] - -- the rule under which this exclusion is declared unconditionally rather than profile-scoped, and - the masked-defect failure mode a profile-scoped health branch would have reproduced -* link:0022-Upstream_security-hardening_default_changes_are_adopted_and_documented_never_locally_reverted.adoc[ADR-0022] - -- the posture that forbids forcing a security-relevant check green with a global switch, and its - scope boundary against this case -* link:0011-gatewayyaml_exposes_JWKS_trust_and_egress_as_neutral_names_bound_by_the_deployment.adoc[ADR-0011] - -- the `gateway.yaml` token-validation surface that the qualified validator is built from -* link:0005-module-structure.adoc[ADR-0005] -- the framework-bound edge seam that keeps - runtime-specific assembly at the boundary -* link:../configuration.adoc[Configuration reference] -- the `token_validation` surface and the - readiness contract diff --git a/doc/adr/0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc b/doc/adr/0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc index c3b681bc..0b97eb35 100644 --- a/doc/adr/0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc +++ b/doc/adr/0032-The_shipped_artifact_declares_nothing_test-shaped_the_deployment_supplies_what_it_names.adoc @@ -145,8 +145,8 @@ A key declared unconditionally *because* a profile-scoped declaration would be a working, not an exception to it. The gateway ships exactly such a declaration: the key-less TLS bucket that marks the plain-HTTP management opt-out (`quarkus.tls.plain-management.reload-period`) is declared in no profile, and the bean-exclusion key -that removes the unused extension health probes (`quarkus.arc.exclude-types`, see -link:0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc[ADR-0027]) +that removes the unused extension health and metrics beans (`quarkus.arc.exclude-types`, see +link:0027-The_token-validation_extensions_unqualified_beans_are_excluded_not_accommodated.adoc[ADR-0027]) likewise. Both carry a comment citing this rule as the reason they are unconditional. Neither is test-shaped: both are product configuration that happens to be inert until a deployment selects the behaviour it enables. @@ -269,7 +269,7 @@ that a passing check is never mistaken for the property itself. * link:0025-The_whole_server-TLS_surface_is_neutral_in_gatewayyaml_and_bound_by_exactly_two_seams.adoc[ADR-0025] -- the policy / deployment-bound / build-time classification of the server-TLS surface, and the boundary rule that keeps ports and trust material deployment-supplied -* link:0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc[ADR-0027] +* link:0027-The_token-validation_extensions_unqualified_beans_are_excluded_not_accommodated.adoc[ADR-0027] -- a bean exclusion declared unconditionally under this rule, and a worked instance of the masked-defect failure mode described above * link:../development/README.adoc[Development guide] -- the contributor-layer statement of this diff --git a/doc/architecture.adoc b/doc/architecture.adoc index fbedc015..db4a052f 100644 --- a/doc/architecture.adoc +++ b/doc/architecture.adoc @@ -726,7 +726,7 @@ exposure. The hardening itself stands regardless. Readiness reports *state, never cause*, consistent with the boot-time-constructibility reading of the `jwks` datum in -link:adr/0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc[ADR-0027]. +link:adr/0027-The_token-validation_extensions_unqualified_beans_are_excluded_not_accommodated.adoc[ADR-0027]. Where the bearer-token validator cannot be resolved the probe marks readiness DOWN with `jwks: unavailable` and a fixed `error: validation-unavailable` token -- it never places the underlying exception message on the payload, which can name issuer URLs, internal hostnames, diff --git a/doc/development/integration-test-topology.adoc b/doc/development/integration-test-topology.adoc index 13838485..94cadac2 100644 --- a/doc/development/integration-test-topology.adoc +++ b/doc/development/integration-test-topology.adoc @@ -163,7 +163,7 @@ a green readiness probe. ==== That narrowness is a known and accepted consequence of the exclusion decision recorded in -link:../adr/0027-The_token-validation_extensions_unqualified_health_probes_are_excluded_not_accommodated.adoc[ADR-0027]: +link:../adr/0027-The_token-validation_extensions_unqualified_beans_are_excluded_not_accommodated.adoc[ADR-0027]: the token-validation extension's two unqualified health probes were removed from the bean set rather than configured into a green state, because they observe an issuer namespace this gateway never populates. The live per-issuer loader signal they would have carried is an open gap in the diff --git a/doc/user/environment-variable-overrides.adoc b/doc/user/environment-variable-overrides.adoc index 824cb92e..d9c9ebbb 100644 --- a/doc/user/environment-variable-overrides.adoc +++ b/doc/user/environment-variable-overrides.adoc @@ -301,12 +301,14 @@ validator from `gateway.yaml` and qualifies it with `@GatewayValidator`, so it c unqualified validator the token-validation extension builds from its own `sheriff.token.issuers.*` property surface. This gateway never uses that second validator and configures no issuer on that surface in *any* profile. The question this note once left open -- what the extension's readiness -check reports when nothing configures it -- no longer arises: both extension probes -(`JwksEndpointHealthCheck` and `TokenValidatorHealthCheck`) are removed from the bean set by -`quarkus.arc.exclude-types`, so they contribute nothing to `/q/health/ready`. Excluding them costs no -observability, because they iterated the extension's own issuer list -- empty here -- and so never -reached this gateway's JWKS loaders. JWKS readiness is reported by the gateway's own -`GatewayReadinessCheck`, against the validator that is actually on the request path. +check reports when nothing configures it -- no longer arises: every extension bean that reads that +surface is removed from the bean set by `quarkus.arc.exclude-types`. That is both probes +(`JwksEndpointHealthCheck` and `TokenValidatorHealthCheck`), which therefore contribute nothing to +`/q/health/ready`, and the `JwtMetricsCollector`, whose ten-second schedule otherwise failed on every +tick and wrote a stack trace to the log each time. Excluding them costs no observability: the probes +iterated the extension's own issuer list -- empty here -- and so never reached this gateway's JWKS +loaders, and the collector failed before publishing a single meter. JWKS readiness is reported by the +gateway's own `GatewayReadinessCheck`, against the validator that is actually on the request path. Read that `jwks` datum for what it is: a *boot-time* constructibility fact, not a live signal. The validator is forced into existence at startup and cached, so the probe reports `ready` for the