feat(gateway): add anchor type/access axes and asset-serving action#84
Merged
Conversation
Introduce the operator-facing (doc/user/) and contributor-facing (doc/development/) documentation layers as tree-index documents, distinct from the design docs under doc/. Add the three-layer documentation convention as a fifth bullet to the plan README's Conventions section, and state configuration.adoc's dual reference/design role in the design-documents index without relocating the file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
Add doc/plan/10-anchor-types-assets.adoc, the design spec covering the nine deliverable groups -- the required type/access anchor axes and the passthrough->proxy rename, the fail-closed access->auth boot matrix, the asset terminal action with directory and upstream sources, the gateway-owned response envelope, canonicalize-and-confine path safety, auth-before-source-resolution ordering, the threat-model reopening, the integration tests, and the reference-doc updates. Register plan 10 in the plan-sequence table and the implementation-plans narrative. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
Add ADR-0013 recording the two required, orthogonal anchor classification axes (type/access) with the three-argument rationale (non-OAuth extensibility, fail-closed declaration, linear growth) and the infrastructure-proxy vs application-security prior-art survey; it extends ADR-0007 and covers the passthrough->proxy value rename. Add ADR-0014 recording asset serving as a second terminal action with the two-source model (directory + upstream reusing the ADR-0008 SSRF-controlled data plane, no parallel fetch stack), the gateway-owned response envelope, and canonicalize-and-confine with auth-before-source-resolution ordering. Register both in the Architecture Decisions table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
Introduce the AnchorType (proxy/bff/asset) and AccessLevel (public/authenticated) enums and make both a required, non-null component of AnchorConfig, enforced in the canonical constructor. Add the two axes as required properties on every anchor in gateway.schema.json (case-insensitive lowercase YAML binds to the uppercase constants), and register the new enums for native reflection. The passthrough->proxy rename is an anchor-type value rename only; tls.passthrough_sni is untouched. Sweep every existing AnchorConfig construction site: the model contract, loader, route-table-builder, and validator tests now supply type and access, and the anchored / anchor-squatter fixtures carry the required axes and stay access->auth matrix consistent (the bff anchor gains a bearer floor; the squatter anchor is access: public). Adds binding and mandatory-field coverage for the two new axes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
Add the ADR-0013 access->auth matrix rules to ConfigValidator: a type 'bff' anchor must be access: authenticated; an access: authenticated anchor requires a non-'none' auth floor with its backing block present (bearer needs token_validation issuers, session needs oidc); an access: public anchor must not declare an auth block. Every violation collects into the shared errors list and never fails fast. Cover each matrix rule in ConfigValidatorTest (generator/enum-driven over the enumerable posture space) plus the single-pass aggregation contract, and keep the ADR-0007 anchor fixtures matrix-consistent. Prove enforcement on the eager startup-event path in ConfigProducerTest: a matrix-violating anchor aborts boot through onStartup -> buildOnce, not merely a direct validator call. Co-Authored-By: Claude <noreply@anthropic.com>
…finement Introduce the asset terminal action (ADR-0014). Add AssetConfig (source discriminator directory|upstream plus source-specific fields) and wire an optional, mutually-exclusive-with-upstream asset component into RouteConfig and the endpoint schema. Create the framework-agnostic asset package: PathConfinement canonicalizes an untrusted sub-path through the cui-http url-path pipeline and confines it under a configured root, rejecting the whole traversal/encoding attack class before any source is touched; AssetResponseEnvelope is the gateway-owned response governance (fixed content-type map, nosniff, forced no-store for authenticated access, stripped Set-Cookie, GET/HEAD-only); AssetSource is the sealed source seam carrying the auth-before-source-resolution ordering contract, permitting the directory and upstream sources completed in later deliverables. Cover confinement with an explicit, exhaustive adversarial traversal/encoding corpus proving no vector escapes the root, and assert every envelope governance rule plus the sealed seam. Co-Authored-By: Claude <noreply@anthropic.com>
Implement DirectoryAssetSource: serve regular files from a configured directory root through the shared PathConfinement and AssetResponseEnvelope, with GET/HEAD-only serving, a bounded file-size cap, and a governed Served response. Confinement is applied before the filesystem is touched, so an out-of-root escape reads no byte. Cover in-root serving with the governed envelope, out-of-root denial, content-type resolution from the fixed map, forced no-store for authenticated access, HEAD-without-body, and write-verb rejection. Co-Authored-By: Claude <noreply@anthropic.com>
…envelope Implement UpstreamAssetSource: serve from a fixed-topology upstream reusing the resolved-upstream model and the shared PathConfinement and AssetResponseEnvelope. The untrusted remainder is confined before the upstream is touched; the fetch runs through an injectable seam whose default embodies the SSRF controls — a scheme allowlist, followRedirects(NEVER), a connect/read timeout, and a bounded body — so no parallel async fetch stack is introduced. Every response is governed by the gateway: the content type is taken from the fixed map (overriding a hostile upstream Content-Type), Set-Cookie is stripped, and an authenticated asset is forced to no-store even when the upstream sent Cache-Control: public. Cover the three hostile-response overrides, confinement-before-fetch, scheme allowlist, no-redirect-follow, bounded size, and timeout mapping through the deterministic fetch seam. Co-Authored-By: Claude <noreply@anthropic.com>
Reopen GW-11 to account for the new asset-serving file-path surface (ADR-0014): document that the traversal class is closed for both the local-directory and upstream sources via canonicalize-then-contain (cui-http URLPathValidationPipeline + the shared gateway PathConfinement) proven by the generator-driven traversal/encoding regression corpus, and add an upstream-asset-source clause to GW-05 noting the reuse of the fixed-topology upstream, scheme allowlist, EgressPolicy posture, and followRedirects(NEVER). Update the GW-05 and GW-11 validation-matrix rows and statuses accordingly. Co-Authored-By: Claude <noreply@anthropic.com>
Apply the idempotent OpenRewrite/format normalizations the pre-commit quality gate produced over the asset package (null-safe equalsIgnoreCase ordering and import formatting). Behaviour is unchanged; the full pre-commit gate is green. Co-Authored-By: Claude <noreply@anthropic.com>
ResolvedRoute now carries a terminal-action union (upstream XOR asset) via a new ResolvedAsset model; RouteTableBuilder materializes directory/upstream asset actions, RouteRuntimeAssembler/DispatchStage/GatewayEdgeRoute dispatch type: asset routes through DirectoryAssetSource/UpstreamAssetSource behind PathConfinement and AssetResponseEnvelope with auth-before-source ordering, and ConfigValidator enforces the type/terminal-action consistency rules (ADR-0013/0014). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
…ary origin Adds a secondary asset-origin static server and a mounted /app/assets volume to the compose stack; the IT gateway config gains type/access axes on all anchors plus two asset anchors (public directory+upstream, authenticated directory), with require:none relocated off the now-matrix-constrained public anchors onto the httpbin endpoint. DirectoryAssetServingIT and UpstreamAssetServingIT exercise serving, GET/HEAD-only, envelope governance, and auth-before-source ordering end-to-end (ADR-0013/0014). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
…minal action configuration.adoc gains the mandatory type/access anchor axes, the access->auth matrix rules, the passthrough->proxy value rename, and the asset terminal action with both sources and its envelope/path-safety governance; architecture.adoc documents asset serving as the second terminal action, the auth-before-source-resolution ordering, the shared gateway-owned response envelope, and the upstream source's reuse of the existing data plane (ADR-0013/0014). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
Collapse the three near-identical served-response records (DirectoryAssetSource.Served, UpstreamAssetSource.Served, DispatchStage.AssetDispatch) into a single AssetSource.Served record on the sealed AssetSource interface, and reduce DispatchStage.serveAsset to a thin validating delegate over source.serve(...). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
Key asset cache governance on the route's effective auth posture instead of the anchor's static access axis (an auth-strengthened route under a public anchor now serves no-store), close the directory-source symlink escape by verifying toRealPath() containment under the configured root (fail-closed on I/O error), and replace the upstream asset source's buffer-then-truncate body handler with a streaming subscriber that aborts the fetch the instant maxBytes is crossed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
![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
📝 WalkthroughWalkthroughAdds required anchor ChangesAsset terminal serving
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 implements the asset-serving terminal action (ADR-0014) and anchor type/access classification axes (ADR-0013), allowing the gateway to serve static content directly from local directories or secondary origins under strict security controls. Feedback on the changes focuses on critical performance optimizations: offloading blocking asset-serving operations from the Vert.x event loop to a worker thread, caching the resolved real path of the root directory in DirectoryAssetSource to prevent redundant disk I/O, and adding a fast-return check in CappedByteArrayBodySubscriber.onNext to avoid processing overhead after subscription cancellation.
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: 1
🧹 Nitpick comments (2)
integration-tests/src/test/java/de/cuioss/sheriff/api/integration/DirectoryAssetServingIT.java (1)
104-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDefensively handle a potentially null
contentType.If the server drops the response early or fails to return a
Content-Typeheader (e.g., a bare401without a body during a test failure),response.contentType()will evaluate tonull. This will cause aNullPointerExceptionat.indexOf(';')and hide the root test failure. Returning a fallback fornullallows theassertEqualscaller to fail cleanly with a descriptive message.💡 Proposed refactor
private static String problemType(String contentType) { + if (contentType == null) { + return ""; + } int semicolon = contentType.indexOf(';'); return (semicolon < 0 ? contentType : contentType.substring(0, semicolon)).trim(); }🤖 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 `@integration-tests/src/test/java/de/cuioss/sheriff/api/integration/DirectoryAssetServingIT.java` around lines 104 - 107, Update problemType to handle a null contentType before calling indexOf; return a suitable fallback string for null so the assertEquals caller reports the original test failure instead of throwing a NullPointerException, while preserving the existing parsing and trimming behavior for non-null values.api-sheriff/src/main/java/de/cuioss/sheriff/api/asset/UpstreamAssetSource.java (1)
141-176: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOptional: HEAD triggers a full upstream body download. The
UpstreamFetcheralways issuesGET, so aHEADrequest fetches (up tomaxBytesof) the upstream body only to discard it (serveBodyis false for HEAD). For large assets this is wasted bandwidth/latency. Additionally,fetched.body()clones the (potentially 10 MiB) array on both Line 168 and Line 174; binding it to a local once avoids the redundant copy.🤖 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/asset/UpstreamAssetSource.java` around lines 141 - 176, Update the upstream fetch flow in serve and the UpstreamFetcher API so HEAD requests use an upstream HEAD request and do not download a response body, while GET retains the existing body behavior. Bind fetched.body() to a local byte array once before the size check and response construction, reusing it to avoid repeated accessor calls and redundant copying.
🤖 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/test/java/de/cuioss/sheriff/api/asset/UpstreamAssetSourceTest.java`:
- Around line 236-248: Update the `@DisplayName` on shouldBuildResolvableUri to
describe successful URI construction and upstream fetching for a resolvable
remainder, matching the assertions and fetcher behavior in the test body.
---
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/asset/UpstreamAssetSource.java`:
- Around line 141-176: Update the upstream fetch flow in serve and the
UpstreamFetcher API so HEAD requests use an upstream HEAD request and do not
download a response body, while GET retains the existing body behavior. Bind
fetched.body() to a local byte array once before the size check and response
construction, reusing it to avoid repeated accessor calls and redundant copying.
In
`@integration-tests/src/test/java/de/cuioss/sheriff/api/integration/DirectoryAssetServingIT.java`:
- Around line 104-107: Update problemType to handle a null contentType before
calling indexOf; return a suitable fallback string for null so the assertEquals
caller reports the original test failure instead of throwing a
NullPointerException, while preserving the existing parsing and trimming
behavior for non-null values.
🪄 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: 1d1babec-27f2-4b4a-8213-6db3d66a11d9
⛔ Files ignored due to path filters (1)
integration-tests/src/main/docker/asset-origin/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (56)
api-sheriff/src/main/java/de/cuioss/sheriff/api/asset/AssetResponseEnvelope.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/asset/AssetSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/asset/DirectoryAssetSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/asset/PathConfinement.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/asset/UpstreamAssetSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/asset/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/AccessLevel.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/AnchorConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/AnchorType.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/AssetConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedAsset.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/DispatchStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssembler.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigModelReflection.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/RouteRuntime.javaapi-sheriff/src/main/resources/schema/endpoint.schema.jsonapi-sheriff/src/main/resources/schema/gateway.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/api/asset/AssetResponseEnvelopeTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/asset/DirectoryAssetSourceTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/asset/PathConfinementTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/asset/UpstreamAssetSourceTest.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/model/ConfigModelContractTest.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/GatewayEdgePipelineTest.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/quarkus/ConfigProducerTest.javaapi-sheriff/src/test/resources/config/anchor-squatter/gateway.yamlapi-sheriff/src/test/resources/config/anchored/gateway.yamldoc/README.adocdoc/adr/0013-anchor-type-access-axes.adocdoc/adr/0014-asset-serving-terminal-action.adocdoc/architecture.adocdoc/configuration.adocdoc/development/README.adocdoc/plan/10-anchor-types-assets.adocdoc/plan/README.adocdoc/security-threat-model.adocdoc/user/README.adocintegration-tests/docker-compose.ymlintegration-tests/src/main/docker/assets/app.cssintegration-tests/src/main/docker/sheriff-config/endpoints/assets-secure.yamlintegration-tests/src/main/docker/sheriff-config/endpoints/assets.yamlintegration-tests/src/main/docker/sheriff-config/endpoints/httpbin.yamlintegration-tests/src/main/docker/sheriff-config/gateway.yamlintegration-tests/src/main/docker/sheriff-config/topology.propertiesintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/DirectoryAssetServingIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/UpstreamAssetServingIT.java
The native binary refused to start (ApiSheriff-200 binding failure for EndpointConfig at /routes/0/asset) because the new AssetConfig record and its nested Source enum were absent from ConfigModelReflection's @RegisterForReflection targets. JVM-mode tests need no registration, which is why every local gate was green while the CI native IT failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
ConfigProducer passed only the tls.passthrough_sni values as additional topology aliases, so a source: upstream asset route's alias (e.g. ASSET_ORIGIN) never reached TopologyResolver and boot refused to start with ApiSheriff-200 "does not resolve in the topology" — the CI-native failure at PR #84. Collect every enabled endpoint's upstream asset-route alias into the additional-alias union. Add the missing boot-path coverage: a positive ConfigProducerTest fixture proving an upstream asset alias declared in topology.properties resolves through the startup event, and a negative fixture pinning the exact ApiSheriff-200 rejection when the alias is undeclared. Confirmed by a clean native rebuild + full Docker IT run (both asset ITs green). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
…ture The verify-invalid-config-fails.sh Case 2 fixture predates the mandatory anchor axes: its api/apiv1 anchors carried only path_prefix + auth, so the config now dies at the schema layer (missing type/access) before the ADR-0007 pairwise-disjoint rule runs, and the asserted marker never appears. Give the fixture anchors type: proxy + access: public (dropping the auth block the ADR-0013 matrix forbids on a public anchor) so the sole remaining violation is the pairwise-disjoint overlap the negative test exists to assert. Locally the script reuses an existing api-sheriff:distroless tag, which is why a stale image masked this. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
Address the accepted review findings from the unified triage: fast-return in the capped body subscriber's onNext once the cap has tripped (bounds over-cap buffering during cancel propagation), correct a copied @DisplayName on the resolvable-URI upstream test, and null-guard the problemType extraction in the directory-asset IT for cleaner negative-path diagnostics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
OliverWolffGIP
enabled auto-merge
July 21, 2026 05:12
Fix the three MAJOR reliability findings: content-aware equals/hashCode and a length-only toString for the byte[]-bearing AssetSource.Served and UpstreamFetcher.Fetched records (S6218, body bytes never rendered), and a provably-guarded Optional access in ConfigValidator's anchor consistency rule (S3655). Close the new-code coverage gap (77.2% -> ~85%): exercise the real SSRF-guarded transport fetcher and the streaming size-cap subscriber against an in-process MockWebServer (header mapping, mid-flight abort at maxBytes, read-timeout mapping), add record equals/hashCode/toString contract tests, and cover the transport-IOException-to-502 path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HkVXPmpTBDyHu3L7eHrBiL
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api-sheriff/src/test/java/de/cuioss/sheriff/api/asset/UpstreamAssetSourceTest.java (1)
267-267: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSwap arguments to properly invoke
Fetched.equals().In JUnit 5,
assertNotEquals(unexpected, actual)evaluatesunexpected.equals(actual). By passingBODY(abyte[]) as the first argument, the assertion executesbyte[].equals(first), which relies on array reference equality and trivially returns false. This means the unrelated-type check insideFetched.equals(Object)is never actually tested.To ensure
Fetched.equals()is invoked and the branch is covered, swap the arguments.♻️ Proposed fix
- () -> assertNotEquals(BODY, first, "a Fetched never equals an unrelated type")); + () -> assertNotEquals(first, BODY, "a Fetched never equals an unrelated type"));🤖 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/asset/UpstreamAssetSourceTest.java` at line 267, Swap the arguments in the assertNotEquals call within UpstreamAssetSourceTest so the Fetched instance first is passed as the unexpected argument and BODY is passed as actual, ensuring Fetched.equals(Object) handles the unrelated type.
🤖 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/asset/UpstreamAssetSourceTest.java`:
- Line 267: Swap the arguments in the assertNotEquals call within
UpstreamAssetSourceTest so the Fetched instance first is passed as the
unexpected argument and BODY is passed as actual, ensuring
Fetched.equals(Object) handles the unrelated type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e186244-0fad-4e06-8bca-638ef2b8d19a
📒 Files selected for processing (7)
api-sheriff/src/main/java/de/cuioss/sheriff/api/asset/AssetSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/asset/UpstreamAssetSource.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/asset/AssetSourceServedTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/asset/UpstreamAssetSourceHttpFetcherTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/asset/UpstreamAssetSourceTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/DirectoryAssetServingIT.java
🚧 Files skipped from review as they are similar to previous changes (4)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/asset/AssetSource.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.java
- integration-tests/src/test/java/de/cuioss/sheriff/api/integration/DirectoryAssetServingIT.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/asset/UpstreamAssetSource.java
5 tasks
OliverWolffGIP
added a commit
that referenced
this pull request
Jul 21, 2026
Auditability layer for the protocol-processors work, authored doc-first: - doc/adr/0015: WebSocket Origin fail-closed exact-match allowlist decision (GW-09/CSWSH rationale, no "any origin" default, why a new field over CORS, rejected alternatives). - doc/adr/0016: gRPC trailers-only rejection status-mapping decision (why a gRPC client cannot consume problem+json, canonical mapping, h2 failure -> UNAVAILABLE, rejected alternatives). - doc/plan/05-protocol-processors.adoc: orchestration notes (k6 supersedes the ghz/k6 split per Clarification 1, config model final via PR #84, parent-POM 1.5.4, two folded work items). - doc/variants/01-base-gateway.adoc: protocol scope complete (ADR-0002 promise closed with WS + gRPC). - doc/plan/README.adoc, doc/README.adoc: Plan 05 delivered; ADR-0015/0016 rows. - doc/LogMessages.adoc: doc-first WS LogRecords (INFO relay established / idle reclaim; WARN Origin rejection), %s-only templates in range. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Adds two explicit
type(proxy|bff|asset) andaccess(public|authenticated) axes to everygateway.yamlanchor — replacingpassthroughwithproxy+access: publicas a clean-slate breaking rename — and introduces asset serving as a new terminal action with two sources (directoryandupstream). Fail-closed boot validation covers the full access/auth matrix, wired through the startup-event path so validation cannot be bypassed by CDI-proxy laziness. Asset responses go through a gateway-owned envelope (fixed content-type map,nosniff, governed caching, strippedSet-Cookie, GET/HEAD only), with canonicalize-and-confine path safety applied to both sources before the source is touched. The path-traversal threat-model entry is reopened and closed with a generator-driven traversal/encoding regression corpus. The change also bootstraps thedoc/user/anddoc/development/documentation tree and records the design rationale in two new ADRs.Changes
AnchorType,AccessLevel,AnchorConfig,RouteConfig,RouteTableBuilder,ConfigValidator,ConfigModelReflection, and thegateway.schema.json/endpoint.schema.jsonschemas now require and enforcetype+accesson every anchor;passthroughis renamed toproxy+access: public.ConfigValidatorenforces the access→auth matrix (access: authenticatedrequires a fully-backed auth posture;access: public+ auth block fails boot;type: bffrequiresaccess: authenticated), asserted via the startup-event path inConfigValidatorTest/ConfigProducerTest(not a direct-call test), per the PR feat(benchmarks): replace wrk with k6, add APISIX comparative baseline #82 lazy-CDI-proxy lesson.de.cuioss.sheriff.api.assetpackage:AssetSource,DirectoryAssetSource,UpstreamAssetSource,PathConfinement,AssetResponseEnvelope, plusAssetConfig/ResolvedAssetconfig-model support andRouteRuntime/GatewayEdgeRoute/DispatchStage/RouteRuntimeAssemblerwiring.UpstreamAssetSourcereuses the existing upstream client, circuit breaker, and SSRF controls (scheme allowlist,EgressPolicy,followRedirects(NEVER), bounded size/timeout) rather than a parallel fetch stack.PathConfinementapplies canonicalize-and-confine to both sources before the source is touched, with auth-before-source-resolution ordering enforced in the edge pipeline.doc/security-threat-model.adocpath-traversal entry reopened and closed against the canonicalize-then-contain design and the new adversarial traversal corpus (DirectoryAssetSourceTest,UpstreamAssetSourceTest,PathConfinementTest).doc/adr/0013-anchor-type-access-axes.adoc,doc/adr/0014-asset-serving-terminal-action.adoc, plus updates todoc/configuration.adocanddoc/architecture.adoc.doc/user/README.adocanddoc/development/README.adoc, a fifth Conventions bullet indoc/plan/README.adocfor the three-layer documentation convention, and adoc/README.adocclassification fix forconfiguration.adoc's dual reference/design role.DirectoryAssetServingITandUpstreamAssetServingITagainst a real mounted directory and a real secondary origin server, with supportingintegration-testsdocker-compose / sheriff-config fixtures (assets.yaml,assets-secure.yaml, asset-origin content).Test Plan
python3 .plan/execute-script.py plan-marshall:build-maven:maven run --command-args "verify -Ppre-commit")python3 .plan/execute-script.py plan-marshall:build-maven:maven run --command-args "verify")Related Issues
None.
Generated by plan-finalize skill
Summary by CodeRabbit
New Features
assetterminal action for routes, supportingdirectoryandupstreamsources with confined paths, size limits, SSRF protections, and auth-before-resolution behavior.X-Content-Type-Options: nosniff,Set-Cookiestripping, andCache-Control: no-storefor authenticated access.GET/HEADare allowed; upstream redirects and upstream failures are handled safely.Documentation
type/accessfail-closed validation.Tests