fix(wac): require Control on protected resource for POST-created .acl/.meta sidecars by jjohare · Pull Request #580 · JavaScriptSolidServer/JavaScriptSolidServer

jjohare · 2026-07-03T22:05:02Z

Summary

A POST whose Slug resolves to an .acl/.meta sidecar is currently authorized only against the container (the request path), not against the sidecar's protected resource. The dedicated ACL Control guard in auth/middleware.js (authorizeAclAccess) keys on urlPath.endsWith('.acl'), which never matches a container POST — the sidecar filename is only produced inside handlePost via generateUniqueFilename, after authorization has already run.

Impact — privilege escalation

An agent holding only acl:Append on a container (e.g. a public-append inbox/upload directory produced by generateInboxAcl) can:

POST to the container with header Slug: victim.acl
Body = an ACL granting the attacker acl:Control + acl:Read

The slug validator permits . (/^[a-zA-Z0-9._-]+$/), so victim.acl passes. The resulting /container/victim.acl then governs /container/victim, giving the attacker full control of a sibling resource it had no prior access to. The same trick works for .meta.

Fix

In handlePost, when the resolved child filename ends in .acl/.meta, require acl:Control on the protected resource (the sidecar path minus the suffix) via the existing checkAccess, before writing. This mirrors authorizeAclAccess. Owners (who hold Control) are unaffected; an Append-only agent now gets 403.

if (!isCreatingContainer && /\.(acl|meta)$/.test(filename)) {
  const protectedUrlPath = newUrlPath.replace(/\.(acl|meta)$/, '');
  const { allowed } = await checkAccess({
    resourceUrl: `${request.protocol}://${request.hostname}${protectedUrlPath}`,
    resourcePath: newStoragePath.replace(/\.(acl|meta)$/, ''),
    isContainer: protectedUrlPath.endsWith('/'),
    agentWebId: request.webId,
    requiredMode: AccessMode.CONTROL,
  });
  if (!allowed) return reply.code(403).send({ error: 'Forbidden', message: '...' });
}

Testing note (please validate in CI)

I could not run the full integration harness in my environment (the server bootstrap eagerly imports the passkey path, and @simplewebauthn/server is not installed there). The change is syntax- and import-verified and mirrors the existing authorizeAclAccess logic. A proposed regression test (integration style, using startTestServer + createTestPod): create a container whose ACL grants a second agent only acl:Append; as that agent POST Slug: x.acl; assert 403; assert as the owner (Control) the same POST succeeds.

Context

Found during a cross-implementation audit against the Rust port solid-pod-rs, which shared the same latent gap and is fixed in lockstep. Happy to adjust the approach (e.g. reject sidecar-via-POST outright vs. the Control-gate here) to whatever the maintainers prefer.

🤖 Generated by Claude Code

…/.meta sidecars A POST whose Slug resolves to an `.acl`/`.meta` sidecar is currently authorized only against the *container* (the request path), because the dedicated ACL Control guard in `auth/middleware.js` (`authorizeAclAccess`) keys on `urlPath.endsWith('.acl')` — which never matches a container POST. The sidecar filename is only produced *inside* `handlePost` via `generateUniqueFilename`, after authorization has run. Impact: an agent holding only `acl:Append` on a container (e.g. a public-append inbox/upload directory created by `generateInboxAcl`) can `POST` with `Slug: victim.acl` and write a resource ACL that grants itself `acl:Control`/`acl:Read` on a sibling — privilege escalation to full control of a resource it had no access to. The slug validator permits `.`, so `victim.acl` passes. Fix: in `handlePost`, when the resolved child filename ends in `.acl`/`.meta`, require `acl:Control` on the protected resource (the sidecar path minus the suffix) before writing, mirroring `authorizeAclAccess`. Owners (who hold Control) are unaffected; Append-only agents get 403. Found during a cross-implementation audit against the solid-pod-rs Rust port, which shared the same gap and is fixed in lockstep. Reproduction and a proposed regression test are in the PR description; the full integration harness could not be exercised in the contributor's environment (missing optional `@simplewebauthn/server` dep used by the passkey path at server bootstrap), so CI validation is requested. Co-Authored-By: jjohare <github@thedreamlab.uk>

Copilot

Pull request overview

This PR closes a WAC authorization gap where POST to a container could mint an .acl/.meta sidecar (via Slug) without requiring acl:Control on the protected resource, enabling privilege escalation. It adds a targeted authorization check in handlePost once the resolved child filename is known.

Changes:

Add a post-slug-resolution guard in handlePost to require acl:Control on the protected resource when creating *.acl/*.meta via POST.
Wire in WAC primitives (checkAccess, AccessMode) needed to perform that additional authorization check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

melvincarvalho · 2026-07-03T22:42:34Z

+    const protectedUrlPath = newUrlPath.replace(/\.(acl|meta)$/, '');
+    const protectedStoragePath = newStoragePath.replace(/\.(acl|meta)$/, '');
+    const { allowed } = await checkAccess({
+      resourceUrl: `${request.protocol}://${request.hostname}${protectedUrlPath}`,
+      resourcePath: protectedStoragePath,
+      isContainer: protectedUrlPath.endsWith('/'),
+      agentWebId: request.webId,
+      requiredMode: AccessMode.CONTROL
+    });


Fixed in 7deed84: the Control check now builds the protected-resource URL with buildResourceUrl(request, protectedUrlPath) — the same helper authorize()/authorizeAclAccess() use — so it evaluates against the identical origin (host+port, subdomain-normalized) as the rest of WAC.

melvincarvalho · 2026-07-03T22:42:39Z

+  if (!isCreatingContainer && /\.(acl|meta)$/.test(filename)) {
+    const protectedUrlPath = newUrlPath.replace(/\.(acl|meta)$/, '');
+    const protectedStoragePath = newStoragePath.replace(/\.(acl|meta)$/, '');
+    const { allowed } = await checkAccess({
+      resourceUrl: `${request.protocol}://${request.hostname}${protectedUrlPath}`,


A regression test was added in the same commit under review (5eeed9e), in test/auth.test.js: 'should deny POST-created .acl/.meta sidecars without Control on the protected resource' (asserts 403 for Slug victim.acl/victim.meta via the public-append inbox, 201 for a normal POST) plus 'should allow the owner (Control) to POST an .acl sidecar'. Verified it fails against the unpatched handler and passes with the guard.

Cover the privilege-escalation path fixed in this PR: an append-only agent (public inbox) POSTing Slug: victim.acl / victim.meta must get 403, a normal non-sidecar POST still gets 201, and the owner (Control) can still POST an .acl sidecar. The deny test fails against the unpatched handler and passes with the Control guard.

Copilot

Pull request overview

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

melvincarvalho · 2026-07-03T22:42:36Z

+    const protectedUrlPath = newUrlPath.replace(/\.(acl|meta)$/, '');
+    const protectedStoragePath = newStoragePath.replace(/\.(acl|meta)$/, '');
+    const { allowed } = await checkAccess({
+      resourceUrl: `${request.protocol}://${request.hostname}${protectedUrlPath}`,
+      resourcePath: protectedStoragePath,
+      isContainer: protectedUrlPath.endsWith('/'),


Fixed in 7deed84 by switching to buildResourceUrl(), which prefers the Host header (preserving the port) and applies subdomain normalization, matching the auth middleware.

melvincarvalho · 2026-07-03T22:42:37Z

+  // Security: a Slug that resolves to an `.acl`/`.meta` sidecar governs
+  // ANOTHER resource's permissions. The authorize() preHandler only checked
+  // Append/Write on the *container* (the request path), and its dedicated


Fixed in 7deed84: reworded to make clear only .acl is consulted by the WAC checker; .meta is gated as defense-in-depth (protected Solid sidecar dotfile), not because it governs permissions.

melvincarvalho · 2026-07-03T22:42:38Z

+      // Regression for the POST .acl sidecar injection: an agent holding only
+      // acl:Append on a container (here, the public-append inbox) must not be
+      // able to plant a sidecar that governs a sibling resource's permissions.


Fixed in 7deed84 — the test comment no longer implies .meta affects WAC; it notes .meta is not a WAC input and is gated as a protected sidecar.

…ta rationale Address review feedback on the POST .acl/.meta sidecar guard: - Build the protected-resource URL with buildResourceUrl() (the same helper authorize()/authorizeAclAccess() use) instead of a hand-rolled request.hostname string, so the Control decision is evaluated against the identical origin (host+port, subdomain-normalized) as the rest of WAC. - Reword the code and test comments: only .acl is consulted for WAC; .meta is gated as defense-in-depth (protected Solid sidecar), not because it governs permissions.

Copilot

Pull request overview

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

melvincarvalho · 2026-07-03T23:04:11Z

+  if (!isCreatingContainer && /\.(acl|meta)$/.test(filename)) {
+    const protectedUrlPath = newUrlPath.replace(/\.(acl|meta)$/, '');
+    const protectedStoragePath = newStoragePath.replace(/\.(acl|meta)$/, '');
+    const { allowed } = await checkAccess({
+      resourceUrl: buildResourceUrl(request, protectedUrlPath),
+      resourcePath: protectedStoragePath,
+      isContainer: protectedUrlPath.endsWith('/'),
+      agentWebId: request.webId,
+      requiredMode: AccessMode.CONTROL
+    });


Fixed in cfdf385. Added a noDebit option to checkAccess() (threaded into checkAuthorizations()): when set, a matching positive-cost PaymentCondition is treated as not-satisfied (returns paymentRequired) instead of debiting the ledger. The sidecar Control guard now passes noDebit: true, so this secondary check can't charge — no double debit and no silent charge; the authoritative debit stays in the primary authorize() hook. Owners hold unconditioned Control and are unaffected. Added test/wac.test.js coverage asserting the primary check debits while the noDebit check leaves the balance unchanged.

The POST .acl/.meta guard in handlePost runs a second checkAccess() on a request the global authorize() hook already evaluated (and possibly billed). Since checkAccess() debits the web ledger for a matching positive-cost PaymentCondition, a payment-gated Control grant could be charged inside the guard — a double debit, and a silent one (the guard ignores paid/ paymentRequired, so no X-Cost/X-Balance headers and 403 instead of 402). Add a noDebit option to checkAccess()/checkAuthorizations(): when set, a positive-cost paid grant is treated as not-satisfied (returns paymentRequired) rather than debited. Owners hold unconditioned Control and are unaffected; the authoritative debit stays in the primary authorize() path. The sidecar guard passes noDebit: true. Adds test/wac.test.js coverage asserting the primary check debits while the noDebit check leaves the balance unchanged.

Copilot

Pull request overview

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

melvincarvalho · 2026-07-03T23:17:24Z

+ *   Used by secondary/guard checks (e.g. the POST sidecar Control gate in
+ *   handlePost) so a single request cannot debit twice or charge silently;
+ *   the authoritative debit stays in the primary authorize() hook.
 * @returns {Promise<{allowed: boolean, wacAllow: string}>}


Fixed in aab92cd — the @returns for checkAccess() now documents the full shape (paymentRequired/paid/balance/currency alongside allowed/wacAllow), and notes which fields are present in the deny/debit/noDebit paths.

@returns

The @returns for checkAccess() listed only {allowed, wacAllow}, but the function also returns paymentRequired/paid/balance/currency, which callers (and the noDebit test) rely on. Document the full shape.

Copilot

Pull request overview

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

Security: require acl:Control on the protected resource for POST-created .acl/.meta sidecars (#580). Closes a WAC privilege-escalation gap where an agent with only acl:Append on a container (e.g. a public inbox) could POST Slug: victim.acl and self-grant Control over a sibling resource. The guard mirrors authorizeAclAccess, uses buildResourceUrl for origin-consistent WAC evaluation, and runs with noDebit so the secondary check never charges the web ledger.

jjohare requested a review from melvincarvalho as a code owner July 3, 2026 22:05

melvincarvalho requested a review from Copilot July 3, 2026 22:07

Copilot started reviewing on behalf of melvincarvalho July 3, 2026 22:07 View session