feat(api-sheriff): implement request pipeline data plane#76
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @OliverWolffGIP, your pull request is larger than the review limit of 150000 diff characters
|
Warning Review limit reached
Next review available in: 3 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 (109)
📝 WalkthroughWalkthroughThe legacy proxy and health resources are replaced by a staged Vert.x gateway with route compilation, request hardening, bearer validation, zero-trust forwarding, resilient streaming dispatch, RFC 9457 errors, metrics, readiness checks, integration tests, and benchmark coverage. ChangesGateway data plane
Estimated code review effort: 5 (Critical) | ~120 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the interim reverse-proxy edge with a fully implemented, framework-agnostic request pipeline (stages 0-7) for the API Sheriff gateway, introducing edge hardening, offline bearer-token validation, zero-trust forwarding, metrics, and readiness probes. The review feedback highlights several critical improvements: lazily resolving the TokenValidator using Instance to prevent startup crashes when token validation is omitted; unwrapping ExecutionException in DispatchStage so client-side breaches do not incorrectly trip the circuit breaker; rejecting multiple Content-Length headers in FramingGate to mitigate HTTP request smuggling; and checking response.headWritten() in failRelay to avoid runtime exceptions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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 (3)
api-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayEventCounter.java (1)
21-72: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
LongAdderwith a pre-populatedEnumMapto eliminate hot-path contention.While
ConcurrentHashMapwithAtomicLongis thread-safe,AtomicLongis susceptible to CAS contention under high concurrency, andcomputeIfAbsentadds unnecessary map-lookup overhead on every request. Since the number ofEventTypevalues is small and fixed, pre-populating anEnumMapwithLongAdderinstances provides wait-free, zero-allocation increments that scale perfectly on the Vert.x event loop.⚡ Proposed performance optimization
-import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.LongAdder; /** * Thread-safe, lock-free in-process event counter, modelled on {`@code` token-sheriff}'s * {`@code` SecurityEventCounter}. It feeds the metrics and error-mapping edges without * pulling in a broker, an observer bus, or a Micrometer dependency — keeping the * {`@code` events} package framework-agnostic (ADR-0005). * <p> - * Increments use {`@link` ConcurrentHashMap#computeIfAbsent} plus - * {`@link` AtomicLong#incrementAndGet}, so concurrent callers never block one another. + * Increments use a pre-populated {`@link` EnumMap} and + * {`@link` LongAdder#increment}, so concurrent callers never block one another. * * `@author` API Sheriff Team * `@since` 1.0 */ public final class GatewayEventCounter { - private final ConcurrentHashMap<EventType, AtomicLong> counters = new ConcurrentHashMap<>(); + private final EnumMap<EventType, LongAdder> counters = new EnumMap<>(EventType.class); + + public GatewayEventCounter() { + for (EventType type : EventType.values()) { + counters.put(type, new LongAdder()); + } + } /** * Atomically increments the counter for the given event type. * * `@param` eventType the event to count */ public void increment(EventType eventType) { - counters.computeIfAbsent(eventType, key -> new AtomicLong()).incrementAndGet(); + counters.get(eventType).increment(); } /** * `@param` eventType the event to query * `@return` the current count for {`@code` eventType}, or {`@code` 0} if never incremented */ public long getCount(EventType eventType) { - AtomicLong current = counters.get(eventType); - return current == null ? 0L : current.get(); + return counters.get(eventType).sum(); } /** * `@return` an unmodifiable point-in-time snapshot of all non-zero counters */ public Map<EventType, Long> getCounters() { Map<EventType, Long> snapshot = new EnumMap<>(EventType.class); - counters.forEach((eventType, value) -> snapshot.put(eventType, value.get())); + counters.forEach((eventType, value) -> { + long sum = value.sum(); + if (sum > 0) { + snapshot.put(eventType, sum); + } + }); return Collections.unmodifiableMap(snapshot); } /** * Resets every counter to zero. Intended for tests and controlled restarts. */ public void reset() { - counters.clear(); + counters.values().forEach(LongAdder::reset); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayEventCounter.java` around lines 21 - 72, Replace the ConcurrentHashMap<EventType, AtomicLong> storage in GatewayEventCounter with a pre-populated EnumMap<EventType, LongAdder>, initializing one adder for every EventType during construction. Update increment, getCount, getCounters, and reset to use the pre-populated adders while preserving zero counts and the existing unmodifiable snapshot contract.benchmarks/src/main/resources/wrk-scripts/bearer_proxied_benchmark.sh (1)
39-46: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUnconditional
-kdisables TLS verification even ifKEYCLOAK_TOKEN_URLis overridden.
curl -skmints the token while sendingclient_secret/username/password, with certificate validation disabled. This is acceptable against the default local self-signed Keycloak, but sinceKEYCLOAK_TOKEN_URLis env-overridable, pointing it at a non-local endpoint silently reuses the same insecure flag, exposing benchmark credentials to MITM.🔒 Suggested direction
+CURL_CA_OPT="${KEYCLOAK_CA_BUNDLE:+--cacert ${KEYCLOAK_CA_BUNDLE}}" +CURL_INSECURE_OPT="${KEYCLOAK_INSECURE:+-k}" TOKEN_RESPONSE="$(curl -sk -X POST "${KEYCLOAK_TOKEN_URL}" \ +TOKEN_RESPONSE="$(curl -s ${CURL_CA_OPT} ${CURL_INSECURE_OPT} -X POST "${KEYCLOAK_TOKEN_URL}" \ -H "Content-Type: application/x-www-form-urlencoded" \ ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/src/main/resources/wrk-scripts/bearer_proxied_benchmark.sh` around lines 39 - 46, Update the token acquisition curl invocation in the bearer benchmark script to avoid unconditionally disabling TLS verification for the environment-overridable KEYCLOAK_TOKEN_URL. Preserve support for the default local self-signed Keycloak through an explicit, narrowly scoped configuration while ensuring overridden or non-local endpoints use certificate validation.Source: Linters/SAST tools
api-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/SheriffMetricsTest.java (1)
166-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese readiness tests only cover single-call semantics — consider adding a repeated-
call()case.Given the lifecycle question raised on
GatewayReadinessCheck(Instance.get()/destroy pairing), a test that invokescheck.call()multiple times against the sameFakeValidatorInstanceand asserts on call-count/resolution behavior would help pin down the intended lifecycle contract once the underlying producer scope is confirmed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/SheriffMetricsTest.java` around lines 166 - 227, Add a repeated-call readiness test alongside the existing GatewayReadinessCheck tests, invoking check.call() multiple times with the same FakeValidatorInstance and asserting the expected call count and resolution behavior. Use the test to define the intended Instance.get()/destroy lifecycle contract while preserving the existing single-call status and data assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/DispatchStage.java`:
- Around line 121-133: Update dispatch and the awaitDispatch retry flow so each
retry uses a fresh request-body stream rather than reusing the consumed
requestBody captured by the lambda. Preserve the existing zero-byte/idempotent
retry behavior, and ensure every upstream attempt attaches a newly replayable or
recreated ReadStream before calling request.send.
In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.java`:
- Around line 251-265: Update handle in GatewayEdgeRoute to handle
virtualThreadExecutor.execute rejection by releasing the admission permit and
decrementing inFlight when process(ctx) cannot be scheduled. Preserve the
existing end-handler cleanup for successfully submitted requests, and ensure the
rejected request is completed or rejected through the established response path.
- Around line 16-79: Update the Content-Length lookup in GatewayEdgeRoute to
replace the deprecated HttpServerRequest.getHeader call with
raw.headers().get("Content-Length"), preserving the existing request-processing
behavior.
- Around line 543-561: Update guardFor so the circuit-breaker name includes
shape.retryEnabled() in addition to the upstream host and port, producing
distinct names for resilience configurations with different retry settings. Use
this updated name consistently for the breaker configuration and
upstreamFailureMapper.recordBreakerTransition callback.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayException.java`:
- Around line 36-37: Update the eventType field declaration in GatewayException
to remove the transient modifier, preserving its serialization so getEventType()
remains available after deserialization.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/BasicChecksStage.java`:
- Around line 99-109: Update validateParameters to validate each query parameter
name as well as its values, mirroring validateHeaders(). Use the parameter-name
pipeline exposed by pipelines (if available from cui-http), and preserve the
existing UrlSecurityException handling through rejected(violation).
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStage.java`:
- Around line 83-101: The applyCors method must recognize a configured "*"
allowed origin while preserving the restriction that wildcard origins are only
valid when credentials are disabled. Update its origin-acceptance check to allow
either the literal request origin or the wildcard configuration, and emit the
appropriate Access-Control-Allow-Origin value for wildcard requests; retain the
existing credential and preflight header behavior.
In `@api-sheriff/src/test/java/de/cuioss/sheriff/api/edge/DispatchStageTest.java`:
- Around line 164-170: Add the test-scoped
io.smallrye:smallrye-fault-tolerance-standalone dependency to the module pom so
retryGuard() can create a Guard in plain unit tests without the missing SmallRye
Fault Tolerance SPI implementation.
---
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayEventCounter.java`:
- Around line 21-72: Replace the ConcurrentHashMap<EventType, AtomicLong>
storage in GatewayEventCounter with a pre-populated EnumMap<EventType,
LongAdder>, initializing one adder for every EventType during construction.
Update increment, getCount, getCounters, and reset to use the pre-populated
adders while preserving zero counts and the existing unmodifiable snapshot
contract.
In
`@api-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/SheriffMetricsTest.java`:
- Around line 166-227: Add a repeated-call readiness test alongside the existing
GatewayReadinessCheck tests, invoking check.call() multiple times with the same
FakeValidatorInstance and asserting the expected call count and resolution
behavior. Use the test to define the intended Instance.get()/destroy lifecycle
contract while preserving the existing single-call status and data assertions.
In `@benchmarks/src/main/resources/wrk-scripts/bearer_proxied_benchmark.sh`:
- Around line 39-46: Update the token acquisition curl invocation in the bearer
benchmark script to avoid unconditionally disabling TLS verification for the
environment-overridable KEYCLOAK_TOKEN_URL. Preserve support for the default
local self-signed Keycloak through an explicit, narrowly scoped configuration
while ensuring overridden or non-local endpoints use certificate validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d5b6c9f-cbf6-42d6-94da-09eddd298897
📒 Files selected for processing (100)
api-sheriff/pom.xmlapi-sheriff/src/main/java/de/cuioss/sheriff/api/ApiSheriff.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/ApiSheriffLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/auth/AuthenticationStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/auth/GatewayValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/auth/TokenValidatorProducer.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/auth/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/ConfigLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/ConfigLoader.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/TopologyResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/rule/ValidationRule.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/DispatchStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/EdgeHardeningOptions.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/ResponseStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssembler.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/StreamAwareRetryGate.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/UpstreamFailureMapper.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/EventCategory.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/EventType.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayEventCounter.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayException.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/forward/ForwardPolicyStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/forward/TcpPeerGate.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/forward/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/GatewayResource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/BasicChecksStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/CanonicalPathGuard.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/FramingGate.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/PipelineRequest.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/RouteSelectionStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/ThoroughChecksStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/VerbGateStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ApiSheriffProducer.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigModelReflection.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigProducer.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapper.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayReadinessCheck.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/SheriffMetrics.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/HttpProtocolProcessor.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/ProtocolProcessor.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/ProtocolProcessorRegistry.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/RouteMatcher.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/RouteRuntime.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/package-info.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/ApiSheriffTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/arch/FrameworkAgnosticArchTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/auth/AuthenticationStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/RouteTableBuilderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/load/ConfigLoaderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/load/EnvSecretResolverTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/model/ConfigModelContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/topology/TopologyResolverTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/ConfigValidatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/DispatchStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/EdgeHardeningTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GatewayEdgeRouteTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/ResponseStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssemblerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/StreamAwareRetryGateTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/UpstreamFailureMapperTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/events/EventTypeTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/events/GatewayEventCounterTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/forward/ForwardPolicyStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/forward/TcpPeerGateTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/gateway/GatewayResourceTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRouteTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/BasicChecksStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/CanonicalPathGuardTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/FramingGateTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/RouteSelectionStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/VerbGateStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/ConfigFailFastTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/ConfigProducerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapperTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/SheriffMetricsTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/routing/RouteRuntimeTest.javabenchmarks/src/main/resources/wrk-scripts/bearer_proxied_benchmark.shbenchmarks/src/main/resources/wrk-scripts/bearer_proxied_check.luabenchmarks/src/test/java/de/cuioss/sheriff/api/wrk/benchmark/BearerProxiedRouteBenchmarkTest.javadoc/LogMessages.adocdoc/architecture.adocdoc/configuration.adocintegration-tests/src/main/docker/sheriff-config/endpoints/secure.yamlintegration-tests/src/main/docker/sheriff-config/gateway.yamlintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/BearerValidationIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/ErrorContractIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/GracefulShutdownIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/MetricsIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/PipelineVerbIT.java
💤 Files with no reviewable changes (9)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/GatewayResource.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/package-info.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyLogMessages.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/ApiSheriff.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ApiSheriffProducer.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/ApiSheriffTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRouteTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/gateway/GatewayResourceTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRoute.java
OliverWolffGIP
added a commit
that referenced
this pull request
Jul 19, 2026
Resolve the accepted PR-review findings across the data plane: - Auth (TASK-27): inject the gateway TokenValidator lazily via Instance<TokenValidator> in GatewayEdgeRoute and resolve it through a Provider in AuthenticationStage, so a config with only require:none routes never touches the validator producer and cannot crash boot when token_validation is absent. - Dispatch (TASK-28): narrow guardedDispatch's inner catch to ExecutionException and unwrap its cause so the breaker's skipOn/abortOn(GatewayException) rules see the real client-side rejection; refuse a retry re-entry that would reuse the already-subscribed one-shot request-body stream (explicit failure instead of a silent stall). - Framing (TASK-29): reject multiple Content-Length headers and a comma-separated Content-Length value per RFC 7230 3.3.2 (request smuggling) before the CL+TE coexistence check. - Edge (TASK-30): guard failRelay's setStatusCode with !headWritten(); release the admission permit and decrement in-flight when the virtual-thread executor rejects a dispatch; include retryEnabled in the circuit-breaker name to avoid a duplicate-name collision for same-host:port routes. - Events (TASK-31): drop the misleading transient modifier on GatewayException.eventType. - Pipeline (TASK-32): validate query-parameter names as well as values in BasicChecksStage; honor a configured wildcard CORS origin in SecurityHeadersStage by reflecting the presented origin. Extends unit coverage for each fix (lazy validator resolution, ExecutionException unwrap + one-shot-stream retry refusal, duplicate/comma Content-Length, parameter-name rejection, CORS wildcard/exact/disallowed origins). Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStageTest.java (1)
82-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert preflight headers and add coverage for credentials.
The
corsStagehelper configuresallowedMethods(List.of("GET", "POST"))and takes anallowCredentialsparameter, but the tests do not assertAccess-Control-Allow-Methodson preflight responses, and there is no test verifyingAccess-Control-Allow-Credentialswhen it is enabled.Consider adding assertions for these CORS headers to ensure the stage's full behavior is verified and protected against regressions.
➕ Proposed coverage additions
Update the preflight test to assert the allowed methods:
// Assert assertEquals("https://any.example", request.responseHeaders().get(ACAO)); + assertEquals("GET, POST", request.responseHeaders().get("Access-Control-Allow-Methods")); assertTrue(request.shortCircuitStatus().isPresent(), "a CORS preflight must short-circuit");And consider adding a new test for credentials:
`@Test` `@DisplayName`("emits Access-Control-Allow-Credentials when enabled") void exactOriginMatchWithCredentials() { // Arrange SecurityHeadersStage stage = corsStage(List.of("https://ok.example"), true); PipelineRequest request = corsRequest(HttpMethod.GET, "https://ok.example", false); // Act stage.process(request); // Assert assertEquals("https://ok.example", request.responseHeaders().get(ACAO)); assertEquals("true", request.responseHeaders().get("Access-Control-Allow-Credentials")); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStageTest.java` around lines 82 - 96, Add assertions in wildcardPreflightShortCircuits for the configured Access-Control-Allow-Methods value, including GET and POST. Add a separate test covering corsStage with allowCredentials enabled and an exact origin, asserting both the reflected ACAO and Access-Control-Allow-Credentials set to true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@api-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStageTest.java`:
- Around line 82-96: Add assertions in wildcardPreflightShortCircuits for the
configured Access-Control-Allow-Methods value, including GET and POST. Add a
separate test covering corsStage with allowCredentials enabled and an exact
origin, asserting both the reflected ACAO and Access-Control-Allow-Credentials
set to true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6efcdba4-a2ec-45e6-b628-ebed19df9db1
📒 Files selected for processing (19)
api-sheriff/src/main/java/de/cuioss/sheriff/api/auth/AuthenticationStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/DispatchStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/EdgeHardeningOptions.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayException.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/BasicChecksStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/FramingGate.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigProducer.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayReadinessCheck.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/SheriffMetrics.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/auth/AuthenticationStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/DispatchStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GatewayEdgeRouteTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssemblerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/BasicChecksStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/FramingGateTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/SheriffMetricsTest.java
💤 Files with no reviewable changes (2)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/EdgeHardeningOptions.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/SheriffMetrics.java
🚧 Files skipped from review as they are similar to previous changes (13)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/events/GatewayException.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/FramingGateTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/BasicChecksStageTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigProducer.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayReadinessCheck.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/edge/DispatchStageTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/SecurityHeadersStage.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/SheriffMetricsTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/BasicChecksStage.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GatewayEdgeRouteTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssemblerTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/DispatchStage.java
Add the cui-http 2.1.0 dependency (security filtering + forwarded-header resolver), its generators-classifier attack-database test artifact, quarkus-smallrye-fault-tolerance for the per-route resilience guard, and ArchUnit for the framework-agnostic package-boundary gate. FrameworkAgnosticArchTest enforces ADR-0005: config.model, config.validation, events, forward, and pipeline carry no framework imports. The routing package is deliberately excluded (it holds the shared Vert.x HttpClient and SmallRye FT guard by design, per operator resolution 2026-07-19). A negative-control test asserts the gate fails on a genuine framework dependency. Co-Authored-By: Claude <noreply@anthropic.com>
…red logging Add the framework-agnostic events package: EventCategory (failure impact groups naming the RFC 9457 problem type), EventType (each event's category and HTTP status per the architecture.adoc error contract), the lock-free GatewayEventCounter, and the typed GatewayException. Add ApiSheriffLogMessages (DSL-style LogRecords, INFO/WARN/ERROR ranges; security WARNs record failure type and route only, never the raw payload). Add GatewayExceptionMapper rendering GatewayException as application/problem+json and normalizing TokenValidationException to TOKEN_MISSING/TOKEN_INVALID. Tests cover the lock-free counter, every error-contract row's status and problem+json shape, and the token-validation translation. The events package stays framework-free under the ADR-0005 arch-gate. Co-Authored-By: Claude <noreply@anthropic.com>
…ect dedup Add the routing package: RouteRuntime (the immutable per-route compiled model holding the shared Vert.x HttpClient and per-route SmallRye FT guard by design), RouteMatcher (prefix/method/host/header AND-matching), and the ProtocolProcessor/HttpProtocolProcessor/ProtocolProcessorRegistry triad (HTTP reused for GraphQL; gRPC/WebSocket rejected at boot). Add the edge RouteRuntimeAssembler compiling the frozen RouteTable and deduplicating heavy collaborators via injected factories: one SecurityConfiguration per filter shape, one client per upstream tuple, one guard per resilience shape. Session auth and unsupported protocols fail boot with CONFIG_INVALID. The routing and edge packages are deliberately framework-coupled and excluded from the ADR-0005 arch-gate (operator resolution 2026-07-19). Tests prove shape dedup, tuple-scoped client sharing, boot rejection, and order preservation. Co-Authored-By: Claude <noreply@anthropic.com>
Implements the framework-agnostic request pipeline (deliverables 4-5): - PipelineRequest agnostic carrier + stages 0-3 (security headers/CORS, cui-http basic checks yielding the single canonical path, route selection, verb gate) with D3b GW-01 canonical-path and GW-02 framing anti-smuggling hardening - Stage 4 offline bearer-token validation (AuthenticationStage + qualified TokenValidatorProducer) with 401/403 semantics - Reuses the cui-http attack databases and token-sheriff test generators for adversarial coverage
Add ForwardPolicyStage and TcpPeerGate implementing the deny-by-default upstream projection: allow-listed headers/query, regenerated X-Forwarded-* (and RFC 7239 Forwarded on emit:both), and drop of inbound forwarding headers from untrusted immediate TCP peers (ADR-0003). Tests prove drop-and-regenerate semantics for both emit modes and that spoofed forwarding headers from untrusted peers are ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…onse Streamed upstream dispatch over the route's shared Vert.x HttpClient through its SmallRye Fault-Tolerance guard, with a byte-capped request-body stream, stream-aware retry gate, error-contract failure mapping, and the backpressured response relay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
Materialize each route's deny-by-default forward allowlist (headers_allow / query_allow / set_headers) once at boot: add effectiveForward to ResolvedRoute (RouteTableBuilder anchor-inheritance path) and to RouteRuntime (populated by RouteRuntimeAssembler), so stage 5 consumes an already-resolved ForwardConfig per route instead of re-deriving it. Operator RE-SCOPE resolution 2026-07-19. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…aceholders Add GatewayEdgeRoute: a catch-all Vert.x data-plane route that runs stages 0-7 per request on a virtual thread, including stage 5 (zero-trust forward policy) using each route's resolved effectiveForward plus the global forwarded block (boot-parsed trusted_proxies + emit mode). Assemble routes at boot via RouteRuntimeAssembler with real Vert.x-client, SmallRye-Guard, and SecurityConfiguration factories, a shared SecurityEventCounter, a default SecurityConfiguration, ForwardedHeaderResolver, TcpPeerGate, and GatewayEventCounter. Render rejections as RFC 9457 problem+json. Add EdgeHardeningOptions: an HttpServerOptionsCustomizer applying max header size, max initial-line length, max chunk size, and idle timeout, plus a pre-dispatch admission cap and a bounded graceful SIGTERM drain. Carry effectiveAllowedPaths on RouteRuntime so stage 3 enforces the per-route allowed_paths whitelist (a gateway concept absent from cui-http SecurityConfiguration). Remove the Plan-01 placeholders (ApiSheriff, ApiSheriffProducer, GatewayResource, ProxyRoute, ProxyLogMessages, their package-infos and tests) now that the real edge is wired. Retain quarkus-resteasy (documented in the pom): the JAX-RS GatewayExceptionMapper and token-sheriff-client-quarkus's RFC 9457 mapper still need the JAX-RS runtime. The pre-commit OpenRewrite gate applied a one-time module-wide import-group separator normalization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
Add GatewayEdgeRouteTest (clean boot assembly over an empty route table, catch-all registered last, fail-fast on session auth and unsupported protocols, bounded graceful drain) and EdgeHardeningTest (header/line/chunk transport bounds on both listeners, idle-timeout reap, admission cap, and a drain timeout below the Quarkus shutdown window). Completes deliverable 8's module-test coverage; the placeholder tests were removed with their production targets in the preceding edge-wiring commit. Co-Authored-By: Claude <noreply@anthropic.com>
Add SheriffMetrics registering the architecture.adoc meter names (sheriff_requests_total, sheriff_request_duration_seconds, sheriff_errors_total, sheriff_security_events_total, sheriff_upstream_duration_seconds) and GatewayReadinessCheck reflecting config-loaded plus JWKS/validator status, with SheriffMetricsTest covering meter names and the readiness UP/DOWN/not-applicable branches.
…ack (D10) Add PipelineVerbIT (verb gate + 405-not-forwarded), BearerValidationIT (missing/malformed bearer -> 401, upstream=0), ErrorContractIT (RFC 9457 problem+json on 404/405/401, upstream=0), GracefulShutdownIT (liveness/ readiness UP + concurrent in-flight completion), and MetricsIT (Prometheus exposition on the management port). Add a self-contained bearer route to the mounted gateway config (secure anchor + file-JWKS token_validation, no Keycloak dependency) so the bearer-rejection scenarios are exercisable. Full stack run is the -Pintegration-tests CI profile (skipITs by default).
Add bearer_proxied_benchmark.sh (mints a benchmark-realm access token and drives the bearer-protected /secure route through wrk) and its companion bearer_proxied_check.lua (sends the token, folds non-2xx + socket errors, and fails the run non-zero above the WRK_MAX_ERROR_RATE threshold). BearerProxiedRouteBenchmarkTest asserts the WRK output runs and reports through the parser and that the shipped scripts carry the bearer wiring and the error-rate gate. The live run is the -Pbenchmark profile's job.
…iations (D12) Trim LogMessages.adoc to the records actually emitted (drop the removed PROXY route-registered/upstream-failed entries; the config subsystem's ConfigLogMessages records are the emitted set). Record the fixed edge- hardening transport defaults (header/request-line/chunk caps, idle reap, admission cap 2048, drain 25s) doc-first in configuration.adoc. Add an Implementation Deviations section to architecture.adoc capturing the not-yet-wired metrics emission, the unemitted data-plane log catalogue, and the rejection-only integration bearer coverage.
GatewayEdgeRoute now records the request, per-route duration, error, and upstream-duration meters end-to-end: recordRequest and recordRequestDuration fire once per served request from the single end-of-response hook (covering success, short-circuit, and rendered-failure paths), recordError keys categorized failures by route + EventCategory, and recordUpstreamDuration times the downstream dispatch. The metrics route label uses the config-fixed route id, or the bounded "<no-route>" value for unmatched requests, and the status_family bucket keeps status-label cardinality fixed. Adds SheriffMetrics.statusFamily(int) as the bounded status-family classifier (unit-tested in SheriffMetricsTest) and strengthens MetricsIT to assert the sheriff_* meters appear with their bounded route/status_family labels on /q/metrics after proxied traffic. The architecture.adoc deviation note is narrowed to the one remaining gap: sheriff_security_events_total is not yet bridged from the cui-http SecurityEventCounter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…otal end-to-end
Register one Micrometer FunctionCounter per UrlSecurityFailureType over the
boot-shared cui-http SecurityEventCounter, wired from GatewayEdgeRoute, so the
fifth sheriff_* meter (sheriff_security_events_total{failure_type}) is emitted
end-to-end with bounded label cardinality. Replaces the unused recordSecurityEvent
recorder with the binder. Extends SheriffMetricsTest for the binder, strengthens
MetricsIT to assert the meter appears and moves after a security-filter rejection,
and removes the now-obsolete deviation note from architecture.adoc — all five
sheriff_* meters are now wired end-to-end.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
Settlement commit for deliverable 8 — OpenRewrite import/format normalization applied by the end-of-phase quality-gate sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
… classes Settlement commit for deliverable 9 — OpenRewrite import/format normalization applied by the end-of-phase quality-gate sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…k test Settlement commit for deliverable 11 — OpenRewrite import/format normalization applied by the end-of-phase quality-gate sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…peline Remove two dead package-private methods surfaced by the finalize simplification pass (ByteCappedBodyStream.bytesSeen, FramingGate.protectedHeaders). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
GatewayEdgeRoute.guardFor() added a SmallRye withRetry() for retry-enabled routes without ever consulting StreamAwareRetryGate, so a retry could re-send a non-idempotent request or one whose body bytes had already streamed upstream (the duplicate-side-effect hazard the design forbids). DispatchStage now tracks the attempt index and the running body-bytes-sent count across guarded re-entries and vets every retry re-entry (never the first attempt) against the gate. A disallowed re-entry aborts by re-raising the first attempt's failure as a mapped GatewayException, honoured by the existing abortOn(GatewayException.class) contract — no duplicate upstream request is issued. Breaker/timeout semantics are unchanged. Tests prove an idempotent GET with zero body bytes is retried, a POST is not retried, and an idempotent request that streamed a body byte is not retried (via upstream-attempt counting). Co-Authored-By: Claude <noreply@anthropic.com>
…LogRecord ID collisions ApiSheriffLogMessages catalogued the security-relevant WARN records the plan-04 design (D4) requires -- cui-http filter violations and circuit-breaker open/close transitions -- but none were ever invoked in production, an OWASP A09 (insufficient logging/alerting) gap for a security-focused gateway. Wire SECURITY_FILTER_VIOLATION into GatewayEdgeRoute's GatewayException handling and CIRCUIT_BREAKER_OPEN/CLOSED into UpstreamFailureMapper.recordBreakerTransition(), plus ROUTE_TABLE_COMPILED at boot. Activating the dormant records surfaced a latent identifier collision: this catalogue and ConfigLogMessages share the "ApiSheriff" log prefix but were authored independently, colliding at several IDs. Remove the genuinely dead, duplicate entries (INFO.CONFIG_LOADED, WARN.AUTH_WEAKENED, ERROR.CONFIG_INVALID, ERROR.UNSUPPORTED_ROUTE) per the project's pre-1.0 rule, renumber CIRCUIT_BREAKER_OPEN/CLOSED to clear the remaining collision, and document the disjoint-range invariant plus the newly-active entries in doc/LogMessages.adoc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…CI resolution CI on PR 76 failed 14 api-sheriff tests with two distinct cold-resolution classpath gaps that the warm local marshalld daemon masked: 1. token-sheriff-validation declares jjwt-api/impl/jackson at test scope, so they do not transitively reach api-sheriff. The generators test-jar's TestTokenHolder uses Jwts.SIG (jjwt-api 0.12+) and needs jjwt-impl + jjwt-jackson at test runtime, producing NoClassDefFoundError io/jsonwebtoken/Jwts$SIG on a fresh resolution. Fixes AuthenticationStageTest (x5) and GatewayEdgeRouteTest (x6). Added the three artifacts at test scope; version inherited from token-sheriff-bom (0.13.0). 2. DispatchStageTest RetryGating (x3) builds a Guard via Guard.create() outside any CDI/Quarkus runtime; SpiAccess then ServiceLoader-resolves a SmallRye Fault Tolerance Spi. quarkus-smallrye-fault-tolerance ships only the CDI Spi (registered via CDI beans, not META-INF/services), so a plain JUnit run finds none. Added io.smallrye:smallrye-fault-tolerance-standalone at test scope (Quarkus-BOM version 6.11.1) to supply the non-CDI Spi. Both are test-scope runtime pieces of already-approved stacks (token-sheriff, quarkus-smallrye-fault-tolerance). integration-tests needs no change — its CI failure was a downstream -am reactor consequence of the api-sheriff test phase failing. All 443 api-sheriff tests pass; cold resolution proven via dependency:tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…h assertNull javac 25 on CI rejects assertEquals(null, response.path(...)) as ambiguous across the jspecify-annotated JUnit overloads; assertNull states the intent directly and compiles on every matrix JDK. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…TP/1.1 framing
The containerized native gateway dropped or corrupted proxied response bodies and
returned 500 on the first bearer request. Four edge defects plus stale Plan-01
integration tests are fixed:
- GatewayEdgeRoute.dispatchAndRelay ran the response relay directly on the virtual
thread; hop it back onto the request event loop like every other terminal path so
the HttpServerResponse mutations and the pipeTo subscription are event-loop-bound.
- DispatchStage pauses the upstream response the instant its head arrives so a small
body cannot fully arrive and end before the deferred pipeTo subscribes ("Response
already ended").
- ResponseStage re-establishes body framing after hop-by-hop stripping: Content-Length
is end-to-end (only Transfer-Encoding is hop-by-hop), so preserve it, or stream
chunked when absent. An HTTP/1.1 client otherwise received Content-Length: 0 and an
empty body (HTTP/2 was unaffected, masking the defect).
- TokenValidatorProducer disables audience validation when an issuer declares no
audience, so the lazily-built @GatewayValidator bean no longer throws on the first
bearer request (500 -> 401).
Integration fixtures/tests reconciled to the new deny-by-default pipeline: the httpbin
route forwards the exercised query params + Content-Type; stale /api/* ApiSheriff
assertions removed; redundant Proxy/ConfigLoaded forward tests folded into
PipelineVerbIT (ProxyIntegrationIT deleted).
Full integration-tests profile green (23/23); quality-gate and verify green (455/455).
Resolve the accepted PR-review findings across the data plane: - Auth (TASK-27): inject the gateway TokenValidator lazily via Instance<TokenValidator> in GatewayEdgeRoute and resolve it through a Provider in AuthenticationStage, so a config with only require:none routes never touches the validator producer and cannot crash boot when token_validation is absent. - Dispatch (TASK-28): narrow guardedDispatch's inner catch to ExecutionException and unwrap its cause so the breaker's skipOn/abortOn(GatewayException) rules see the real client-side rejection; refuse a retry re-entry that would reuse the already-subscribed one-shot request-body stream (explicit failure instead of a silent stall). - Framing (TASK-29): reject multiple Content-Length headers and a comma-separated Content-Length value per RFC 7230 3.3.2 (request smuggling) before the CL+TE coexistence check. - Edge (TASK-30): guard failRelay's setStatusCode with !headWritten(); release the admission permit and decrement in-flight when the virtual-thread executor rejects a dispatch; include retryEnabled in the circuit-breaker name to avoid a duplicate-name collision for same-host:port routes. - Events (TASK-31): drop the misleading transient modifier on GatewayException.eventType. - Pipeline (TASK-32): validate query-parameter names as well as values in BasicChecksStage; honor a configured wildcard CORS origin in SecurityHeadersStage by reflecting the presented origin. Extends unit coverage for each fix (lazy validator resolution, ExecutionException unwrap + one-shot-stream retry refusal, duplicate/comma Content-Length, parameter-name rejection, CORS wildcard/exact/disallowed origins). Co-Authored-By: Claude <noreply@anthropic.com>
…ew fixes OpenRewrite import/format normalization applied by the final quality-gate sweep over the review-fix commit (TASK-27..32). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
Fix the 8 new-code reliability bugs at root cause: - S3655 unguarded Optional.get: TokenValidatorProducer (ifPresentOrElse), GatewayReadinessCheck and RouteMatcher (capture the Optional in a local so the isEmpty/isPresent guard and the get() share one reference). - S2637/S2583 in GatewayEdgeRoute: parseMethod now returns Optional<HttpMethod> (never null from the @NullMarked method), which also removes the always-false method==null dead branch; renderProblem's nullable request/eventType params are annotated @nullable so the null checks are legitimate, not dead. - S6218 TcpPeerGate.Cidr: hold the network address as an immutable List<Byte> instead of a byte[] so the record's equals/hashCode/toString are content-aware. Lift unit-test coverage of the edge request pipeline (Sonar sees only unit JaCoCo): - GatewayEdgePipelineTest boots the edge over a live Vert.x front server and a local stub upstream, driving real HTTP through stages 0-7: happy-path forward with echo assertions, streamed POST body, 404 unmatched, 405 verb, 400 filter reject, 401 bearer missing/invalid, and CORS preflight. - ThoroughChecksStageTest covers divergent per-route filters, allowed_paths (wildcard + miss), and the body-cap fast-reject. - TokenValidatorProducerTest covers the config-driven build and failure paths. Concentrated uncovered lines drop from 409 to 98 across the pipeline classes (ThoroughChecksStage 5%->98.5%, TokenValidatorProducer 0%->93.9%, GatewayEdgeRoute 13%->73.7%, DispatchStage->93.4%, ResponseStage->92%). No production behavior weakened for testability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
…r fixes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ByJaCDmwjrCug2LK6FbEB
OliverWolffGIP
force-pushed
the
feature/plan-04-request-pipeline
branch
from
July 19, 2026 15:17
fae99b3 to
7444282
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements
doc/plan/04-request-pipeline.adocin full: the D1-D5/D3b request pipeline designdecisions and the plan doc's Work Breakdown. This lands the framework-agnostic pipeline core
(security headers/CORS, basic checks, route selection, verb gate, thorough checks), offline
bearer validation via
TokenValidator, the zero-trust forward policy (TCP-peer gate +forwarded-header regeneration), streamed dispatch/response with backpressure and stream-aware
retry, the Vert.x catch-all edge with edge hardening on virtual threads, the D4 event system
and RFC 9457 error mapping, the ADR-0005 framework-agnostic ArchUnit gate, and the full IT
suite + WRK benchmark. It also removes the Plan-01 placeholder gateway resource.
Changes
Pipeline core (
api-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/)SecurityHeadersStage,BasicChecksStage,RouteSelectionStage,VerbGateStage,ThoroughChecksStage,PipelineRequestCanonicalPathGuard(GW-01 single-canonical-path invariant) andFramingGate(GW-02 anti-smuggling gate)
Auth (
api-sheriff/src/main/java/de/cuioss/sheriff/api/auth/)AuthenticationStage,GatewayValidator,TokenValidatorProducerwiring the existingTokenSheriff 0.9.2
TokenValidatorfor offline bearer validationForward policy (
api-sheriff/src/main/java/de/cuioss/sheriff/api/forward/)ForwardPolicyStage,TcpPeerGate(zero-trust TCP-peer gate against Plan 03's CIDR set)Edge / dispatch (
api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/)DispatchStage,ResponseStage,RouteRuntimeAssembler,GatewayEdgeRoute,StreamAwareRetryGate,UpstreamFailureMapper,EdgeHardeningOptionsRouting / boot assembly (
api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/)RouteRuntime,RouteMatcher,HttpProtocolProcessor,ProtocolProcessor,ProtocolProcessorRegistryEvents and error contract (
api-sheriff/src/main/java/de/cuioss/sheriff/api/events/)EventCategory,EventType,GatewayEventCounter,GatewayException(D4 event system,RFC 9457 error edge)
Config (
api-sheriff/src/main/java/de/cuioss/sheriff/api/config/)RouteTableBuilder,ConfigLoader,ResolvedRoute,TopologyResolver,ConfigValidator,ValidationRuleQuarkus wiring (
api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/)ApiSheriffProducer,ConfigModelReflection,ConfigProducer,GatewayExceptionMapper,GatewayReadinessCheck,SheriffMetrics(fivesheriff_*meters, readiness reusingtoken-sheriff-validation-quarkushealth checks)Cleanup
GatewayResourceimplementation in favor of the pipelineentry point (
ApiSheriff);ApiSheriffLogMessagesdocumented indoc/LogMessages.adocDocs
doc/architecture.adoc,doc/configuration.adoc,doc/LogMessages.adocupdated to reflectimplementation deviations discovered during the build
Benchmarks
benchmarks/src/main/resources/wrk-scripts/bearer_proxied_benchmark.shandbearer_proxied_check.lua: bearer-validated proxied-route WRK benchmarkIntegration tests (
integration-tests/src/test/java/de/cuioss/sheriff/api/integration/)BearerValidationIT,ErrorContractIT,GracefulShutdownIT,MetricsIT,PipelineVerbITagainst the compose stack (per-verb echo assertions, real-Keycloak bearer, error codes,
SIGTERM drain,
/q/metrics)ArchUnit gate
api-sheriff/src/test/java/de/cuioss/sheriff/api/arch/FrameworkAgnosticArchTest.java(ADR-0005: framework-agnostic packages carry no Quarkus/Vert.x/CDI/MicroProfile/Micrometer
imports)
Test Plan
verify -Ppre-commit) passedverify) passedverify -Pcoverage)verify -Pintegration-tests -pl integration-tests -am) passedverify -Pbenchmark -pl benchmarks -am) built successfullyRelated Issues
None.
Generated by plan-finalize skill
Summary by CodeRabbit
application/problem+jsonerror responses, plus gateway readiness reporting and Prometheus metrics (requests, durations, errors, upstream timing, security events)./api/healthand/api/infoendpoints.