You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(wac): require Control on protected resource for POST-created .acl/.meta sidecars (JavaScriptSolidServer#580)
* fix(wac): require Control on protected resource for POST-created .acl/.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>
* test(wac): regression for POST-created .acl/.meta sidecar injection
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.
* fix(wac): use buildResourceUrl for sidecar Control check; clarify .meta 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.
* fix(wac): don't debit ledger in the secondary sidecar Control check
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.
* docs(wac): document full checkAccess() return shape
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.
---------
Co-authored-by: Melvin Carvalho <melvincarvalho@gmail.com>
0 commit comments