chore(deps): true up token-sheriff floor and audit the 0.9.3 delta - #128
Conversation
version.token-sheriff is asserted (not edited) at 0.9.3, the confirmed latest
upstream release. The build metadata that went stale around it is corrected.
Re-derived the smallrye-config convergence as a real resolved-dependency check
(dependency:tree -Dincludes=io.smallrye.config:*, -Dverbose, and dependency:list
across the reactor) rather than trusting the existing comment:
- io.smallrye.config converges on a single 3.17.2 across the whole reactor at
compile scope. No split, so no structural pom change was warranted.
- token-sheriff-bom 0.9.3 declares its Quarkus artifacts (quarkus-config-yaml,
quarkus-hibernate-validator) at 3.38.0; the quarkus-bom-first import pins
them back to 3.37.4. The ordering seam still does its job.
The pom.xml:83-87 comment now names the versions that actually resolve, replacing
the stale "Quarkus 3.37.2's native 3.17.2 / cui-java-parent bumps to 3.18.0"
wording. CLAUDE.md's Project Overview is corrected from Quarkus 3.37.2 to the
3.37.4 that pom.xml:53 pins.
Retrospective whole-delta audit of the 16 PRs that landed unreviewed via the bot
bump; full verdict record in the plan's work/delta-audit-verdicts.md:
- #581 flipped IssuerConfig.azpAudienceFallbackEnabled from true to false. The
audience-less path (TokenValidatorProducer:128-130) is unaffected because
audience validation is disabled wholesale; the audience-configured path
(:129) IS affected at runtime -- a token with a matching azp but no aud was
accepted under 0.9.2 and is rejected under 0.9.3. Documented on the
issuers[].audience row in doc/configuration.adoc.
- #607 (step-up fail-closed on blank constraints) has no gateway impact: the
gateway never builds a StepUpChallenge, detectChallenge short-circuits blank
headers before the parser, and no path relies on an empty ACR being a no-op.
- #593 (upstream Quarkus 3.38.0) is absorbed by the import ordering, verified
above rather than assumed.
- The remaining PRs are verdicted individually on their diffs, not their
titles.
The request's third named change ("HTTP client caching is now scheme-keyed") is
recorded as OPEN rather than closed: settling it needs the diff hunks of #575 and
#577, and only their changed-file lists were available. Filed as a triage finding.
No file under api-sheriff/src is modified.
Co-Authored-By: Claude <noreply@anthropic.com>
![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: 7 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 (2)
📝 WalkthroughWalkthroughDocumentation updates clarify audience validation behavior and update Quarkus 3.37.4 references in project and dependency-management guidance. No dependency coordinates, plugin settings, or public entities changed. ChangesDocumentation and build guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 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 🔍
|
Triage dispositionsIn reply to comment_id:
|
…sions ADR-0021 settles which BOM owns the shared Quarkus / smallrye-config version line: quarkus-bom is imported first and is the authority, every later BOM import is a subordinate. Reordering them splits smallrye-config at compile scope and breaks Quarkus config-mapping augmentation. ADR-0022 settles what the gateway does when its security dependency tightens a default: adopt it and document the narrowed operator contract, never restore the permissive behaviour locally. A tightened default the config model cannot accommodate is escalated as a finding, not patched around; a floor move is audited on the whole delta, not on the feature that motivated it. Both were surfaced by the phase-6 adr-propose gate on PLAN-31A and confirmed by the operator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XC1JSf6dxTVa4S9RqvNJuY
1 participant
Summary
pom.xml:54 already readsversion.token-sheriff0.9.3— the latest release. That bump landedon
mainvia an unreviewed one-line bot PR (#117) and was never audited. This PR does two things:smallrye-configconvergence against the Quarkus version actually in the POM (
3.37.4, not the3.37.2the oldcomment named) and refresh the stale comment +
CLAUDE.mdreference.0.9.2 → 0.9.3delta — all 16 PRs — for behavioural changein authentication, since none of them were reviewed before landing.
Operator-relevant findings, surfaced up front:
token-sheriff0.9.3floor was already onmainbefore this plan started (via chore: update token-sheriff-validation from 0.9.2 to 0.9.3 #117's unreviewed bot bump). This PR is a true-up plus the
retrospective audit that never happened, not a new version bump.
IssuerConfig.azpAudienceFallbackEnablednow defaults tofalse. Anazp-only token (noaudclaim) that previously validated on an issuer withaudienceconfigured is now rejected. The audience-less path is unaffected. This is a config-model
finding reported here, not patched around — documented in
doc/configuration.adoc.Changes
pom.xml— re-derived thesmallrye-configconvergence (single3.17.2across the reactor,no split) against
version.quarkus3.37.4; refreshed the:83-87BOM-ordering comment tostate the versions actually resolved. No version edit —
0.9.3was already the pinned floor.CLAUDE.md— corrected the stale "Quarkus 3.37.2" statement to3.37.4, matchingpom.xml:53.doc/configuration.adoc— documented theIssuerConfig.azpAudienceFallbackEnableddefault flip:audis now required onceaudienceis configured on an issuer; a matchingazpno longersubstitutes; an
azp-only IdP must either emitaudor omit theaudiencekey.Retrospective delta audit — verdict for all 16 PRs (
token-sheriff0.9.2 → 0.9.3)Verdicts are given on diffs, not titles. Full evidence trail lives in
.plan/plans/plan-31a-token-sheriff-dependency-floor/work/delta-audit-verdicts.md.Resolved-dependency baseline (D1)
Re-derived across the reactor with
dependency:tree(-Dincludes=io.smallrye.config:*,-Dverbose) anddependency:list:io.smallrye.configconverges on a single3.17.2at compilescope across
smallrye-config,-core,-common,-source-yaml,-validator— no split.token-sheriff-bom0.9.3 declares its Quarkus artifacts at3.38.0; the quarkus-bom-first importstill pins them back to
3.37.4. No structural POM change was warranted.Behaviour-bearing PRs
IssuerConfig.azpAudienceFallbackEnabledflipped defaulttrue→false. Audience-less path (TokenValidatorProducer.toValidationIssuer:128-130) is unaffected — audience validation is switched off wholesale (buildsFromHttpIssuerWithoutAudiencegreen). Audience-configured path (:129) is affected at runtime — anazp-matching,aud-less token that validated under 0.9.2 is rejected under 0.9.3. Both tripwires are build-time tests (assertNotNull(validator)) and do not exercise the runtime token path, so the affected-path verdict rests on the default flip plus the code path. Documented indoc/configuration.adoc.SSLContext; step-up fail-closed; scheme-keyed HTTP client cachingStepUpCoordinator) short-circuitsnull/blank ACR headers toOptional.empty()before the parser is reached;StepUpCoordinatorTestalways builds a non-empty ACR (2/2 green). Per-clientSSLContextis an added optional capability, not adopted here (chain note for PLAN-31D). Also the carrier PR for scheme-keyedBackChannelHttpclient caching (a third named behavioural change the request called out, initially unmatched to any title — located from the actual diff hunks): the shared-client cache is now keyed by URI scheme instead of one process-wide singleton, closing a cross-scheme TLS-trust bleed. Gateway verdict: no impact, and strictly safer —sharedClient(HttpHandler)'s signature is unchanged.version.quarkus3.37.2 → 3.38.0smallrye-configstays single at3.17.2.BackChannelHttp.java,StepUpHandler.java,DiscoveryResolver.java,TokenEndpointClient.java, others); none is on the two gateway surfaces this audit verified. ItsBackChannelHttp.javahunk is a Javadoc accessor rename only, no behavioural code change..plan/CI-heavy pre-commit-profile change and no gateway-consumed artifact in its file list carries a behavioural hunk.cui-java-parent1.5.2 → 1.5.4smallrye-configdecision regardless of what the parent supplies.All 16 PRs carry an explicit verdict. The request's three named behavioural changes are each
located: #581 (audience/
azpfallback default flip), #607 (blankacr_valuesfail-closed andscheme-keyed client caching and per-client
SSLContext).Test Plan
verify -Ppre-commit)verify)dependency:tree/dependency:listre-derivation confirms singlesmallrye-config3.17.2convergence (evidence captured in the work artifact, not re-run by CI)
Related Issues
None.
Generated by plan-finalize skill
Summary by CodeRabbit
audvalidation and rejection of tokens relying solely onazp.