Skip to content

Commit 7deed84

Browse files
melvincarvalhomelvincarvalho
committed
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.
1 parent 5eeed9e commit 7deed84

2 files changed

Lines changed: 19 additions & 10 deletions

File tree

‎src/handlers/container.js‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { isContainer, getEffectiveUrlPath, getPodName } from '../utils/url.js';
55
import { generateProfile, generatePreferences, generateTypeIndex, serialize } from '../webid/profile.js';
66
import { generateOwnerAcl, generatePrivateAcl, generateInboxAcl, generatePublicFolderAcl, serializeAcl, relativizeOwnerWebId, AccessMode } from '../wac/parser.js';
77
import { checkAccess } from '../wac/checker.js';
8+
import { buildResourceUrl } from '../auth/middleware.js';
89
import { provisionOwnerKey, assertProvisionKeysCompatible } from '../keys/provision.js';
910
import { createToken } from '../auth/token.js';
1011
import { canAcceptInput, toJsonLd, RDF_TYPES } from '../rdf/conneg.js';
@@ -89,20 +90,26 @@ export async function handlePost(request, reply) {
8990
const newStoragePath = storagePath + filename + (isCreatingContainer ? '/' : '');
9091
const resourceUrl = `${request.protocol}://${request.hostname}${newUrlPath}`;
9192

92-
// Security: a Slug that resolves to an `.acl`/`.meta` sidecar governs
93-
// ANOTHER resource's permissions. The authorize() preHandler only checked
94-
// Append/Write on the *container* (the request path), and its dedicated
95-
// `.acl` Control guard (authorizeAclAccess) never fires here because the
96-
// request path is the container, not the resolved sidecar. Without this an
97-
// agent with mere Append rights on a container could POST `Slug: victim.acl`
98-
// and self-grant Control on a sibling resource — privilege escalation.
93+
// Security: a Slug that resolves to an `.acl` sidecar governs ANOTHER
94+
// resource's permissions — the WAC checker searches for `*.acl`, so an
95+
// `.acl` written here becomes the authorization policy for its sibling.
96+
// The authorize() preHandler only checked Append/Write on the *container*
97+
// (the request path), and its dedicated `.acl` Control guard
98+
// (authorizeAclAccess) never fires here because the request path is the
99+
// container, not the resolved sidecar. Without this an agent with mere
100+
// Append rights on a container could POST `Slug: victim.acl` and self-grant
101+
// Control on a sibling resource — privilege escalation. `.meta` is not
102+
// consulted for WAC, but it is a protected Solid sidecar dotfile, so we gate
103+
// it the same way (defense in depth) rather than let it be minted by Append.
99104
// Mirror authorizeAclAccess: require acl:Control on the protected resource
100-
// before minting a sidecar via POST.
105+
// before minting a sidecar via POST. Build the resource URL with the same
106+
// buildResourceUrl() the auth middleware uses so this Control decision is
107+
// evaluated against the identical origin (host+port, subdomain-normalized).
101108
if (!isCreatingContainer && /\.(acl|meta)$/.test(filename)) {
102109
const protectedUrlPath = newUrlPath.replace(/\.(acl|meta)$/, '');
103110
const protectedStoragePath = newStoragePath.replace(/\.(acl|meta)$/, '');
104111
const { allowed } = await checkAccess({
105-
resourceUrl: `${request.protocol}://${request.hostname}${protectedUrlPath}`,
112+
resourceUrl: buildResourceUrl(request, protectedUrlPath),
106113
resourcePath: protectedStoragePath,
107114
isContainer: protectedUrlPath.endsWith('/'),
108115
agentWebId: request.webId,

‎test/auth.test.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,9 @@ describe('Authentication', () => {
216216
it('should deny POST-created .acl/.meta sidecars without Control on the protected resource', async () => {
217217
// Regression for the POST .acl sidecar injection: an agent holding only
218218
// acl:Append on a container (here, the public-append inbox) must not be
219-
// able to plant a sidecar that governs a sibling resource's permissions.
219+
// able to plant an .acl sidecar, which the WAC checker would then treat
220+
// as the authorization policy for the sibling resource. .meta is not a
221+
// WAC input, but is gated the same way as a protected Solid sidecar.
220222
await createTestPod('sidecarvictim');
221223

222224
const aclBody = JSON.stringify({

0 commit comments

Comments
 (0)