fix(wac): advertise the ACL resource — Link rel="acl" on every resource response

[josepharuja]

The header WAC's "ACL Resource Discovery" section makes a MUST, found missing the honest way: the first harness run in the project's history to clear REGISTER CLIENTS stopped at PREPARE SERVER because it could not learn the test container's ACL URL. It fell through a ticket seam — T5.1's note deferred the emission to T5.3, and T5.3's DoD listed only WAC-Allow.

The fix

• AclLink (new, package-private): the one rendering of the link — the target's own ACL by the AclResource convention, whether or not it has a representation yet; an ACL resource advertises itself (editing it is governed by Control on what it protects; a .acl.acl names nothing in the model).
• AuthorizationFilter writes it up front — that value is what refusals (401/403) and error-mapped responses carry.
• The four handlers that emit their own Link values (Read, Options, Patch, Create) restate it, because a router response's headers replace that field wholesale on write — found empirically: the filter-only version passed on 401s and vanished on 200s. For POST the advertised ACL is the container posted to (the request target), not the created resource.
• The storage description stays exempt, deliberately: a virtual resource has no ACL to name.
• LinkRelation.ACL — the enum row the class javadoc already anticipated.

Tests

• Four new cases in AuthorizationHttpTest: document, container, ACL-self, and refusal (discovery does not require access).
• Twelve exact-list Link assertions across the existing HTTP tests gain the new trailing value.
• Full mvn verify green.

Evidence from the harness (provisional, NOT baseline)

With this fix and a locally-patched harness client (RFC 9449 ath added to its DPoP proofs — upstream bug, solid-contrib/conformance-test-harness#767/#786), the CTH executed its 41 test cases against Cistern for the first time:

MustFeatures passed: 24, failed: 11 · MustScenarios passed: 613, failed: 28

These numbers are evidence for this PR, not a recordable baseline — they were produced by a patched harness and the ruling on how to treat that is the architect's (ground rule 1). cth/BASELINE.md is deliberately untouched here. Failure clusters, for the record: CORS ×5 features (Vary: Origin, ACAO echo), WAC write-access ×3, method-not-allowed, content-type-reject, containment, named-graph negotiation.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Implements WAC “ACL Resource Discovery” by ensuring every resource-targeting HTTP response advertises the target’s ACL via Link: <…>; rel="acl" (including refusals), unblocking CTH discovery of container ACL URLs.

Changes:

• Introduces a single renderer for the ACL discovery link (AclLink) and adds it early in AuthorizationFilter so it’s present on 401/403 and error-mapped responses.
• Restates the ACL Link in handlers that emit their own Link values (Read, Options, Patch, Create) to avoid losing the filter-emitted header.
• Updates and extends HTTP tests to assert the new rel="acl" link alongside existing Link values.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/AclLink.java	Centralizes rendering of the WAC Link rel="acl" value (including ACL-self behavior).
cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/AuthorizationFilter.java	Adds the ACL discovery Link early so refusals/error-mapped responses carry it.
cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/ResourceReadHandler.java	Restates the ACL Link when building ServerResponse headers for reads.
cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/ResourcePatchHandler.java	Restates the ACL Link on PATCH responses (including create-via-patch).
cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/ResourceOptionsHandler.java	Passes request target through and restates the ACL Link for OPTIONS responses.
cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/ResourceCreateHandler.java	Advertises the POSTed-to container’s ACL in create responses (per PR description).
cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/LinkRelation.java	Adds LinkRelation.ACL enum entry with WAC discovery documentation.
cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/AuthorizationHttpTest.java	Adds focused ACL discovery tests (document, container, ACL-self, refusal).
cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/ResourceReadHttpTest.java	Updates exact Link header assertions to include the trailing ACL discovery link.
cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/ResourcePatchHttpTest.java	Updates create-via-patch Link expectations to include rel="acl".
cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/ResourceOptionsHttpTest.java	Updates OPTIONS Link list assertions to include the ACL discovery link.
cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/ResourceCreateHttpTest.java	Updates create tests to include the new rel="acl" link in Link assertions.

Suppressed comments (1)

cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/AuthorizationHttpTest.java:230

• This test seeds /.acl in @BeforeEach, so the GET should succeed. Adding an explicit expectStatus().isOk() ensures the test fails if the ACL resource becomes unreadable (or starts erroring) while still emitting a Link rel="acl" header.

    @DisplayName("an ACL resource advertises itself, not a .acl.acl outside the model")
    void aclResourceAdvertisesItself() {
        assertAclLink(asOwner(client.get().uri("/.acl")).exchange(), "/.acl");
    }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[josepharuja]

Review disposition (advisor review at level high — seven finder angles, all findings verified against the diff — plus Copilot):

Refuted the original design with its own diff. Five handlers emit their own Link values; the first commit patched four and missed PUT — the exact request a permissions editor makes — plus the storage description, and a slice test pinned the omission. Commit e49abc3 replaces the restatement contract with a single writer: a beforeCommit hook in AuthorizationFilter, the OriginVaryFilter shape (copy-then-put — add() mutates a list that a functional ServerResponse makes immutable), covering every handler current and future, refusals, and error-mapped responses uniformly.

Behavioural decisions the review forced, now explicit and tested: the storage description IS WAC-governed (its effective-ACL walk is real), so its 200 now advertises the ACL rather than only its 401; a 304 stays free of discovery links per the conditional-request contract; CORS preflights and unparseable-target 400s are documented exemptions. AclResource.aclFor() (idempotent — an ACL is its own ACL) moves the mapping into cistern-wac; webflux only renders.

Copilot's comment (missing status assertions on two tests): fixed — every discovery test now asserts status, and the helper asserts exactly one acl link via LinkHeader's own parser rather than containment, so duplicates and wrong targets fail too.

Flagged, pre-existing, not fixed here: asterisk-form OPTIONS * 400s in this filter before ResourceOptionsHandler's asterisk branch can answer — needs its own ticket.

Post-redesign verification: full mvn verify green; live server rebuilt; patched-harness re-run in flight to confirm the provisional 24/11 holds.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

cistern-webflux/src/main/java/com/enrichmeai/cistern/webflux/AuthorizationFilter.java:152

• This comment says the 400 for a malformed target is produced "just below", but paths.identifierFor(...) is invoked before this block and can throw. Rewording will avoid confusing the reader about why those 400s have no rel="acl" link.

        // wrote. Covers allowed, refused, and error-mapped responses alike. Two responses
        // deliberately go without: a CORS preflight (returned above — it answers "may this
        // origin ask?", not a request targeting the resource), and a 400 for a target
        // identifierFor refuses just below, which has no well-formed ACL to name.

cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/AuthorizationHttpTest.java:9

• Unused static import assertTrue causes compilation failure (javac rejects unused imports). Remove it or use the static import consistently.

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

cistern-webflux/src/test/java/com/enrichmeai/cistern/webflux/AuthorizationHttpTest.java:278

• PR description says "The storage description stays exempt" from Link rel="acl", but this new test (and AuthorizationFilter's current behavior) asserts the opposite. Please reconcile the intended behavior: either exempt /.well-known/solid in the filter and update this test, or update the PR description to match the new behavior.

    @Test
    @DisplayName("the storage description is WAC-governed and advertises its ACL like anything else")
    void storageDescriptionAdvertisesAcl() {
        assertAclLink(asOwner(client.get().uri("/.well-known/solid")).exchange()
                .expectStatus().isOk(), "/.well-known/solid" + AclResource.SUFFIX);
    }