diff --git a/docs/spec.md b/docs/spec.md index 6b6ed9e..11cc87f 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -365,6 +365,8 @@ type Association = **Note:** `parentId` is a separate native field (not an Association) because hierarchical containment is fundamental enough to warrant indexing at the library level. Associations are for metadata and cross-references. +**Reference creation is gated on `ScopedStack`:** an `attachment` association or file-ref content field requires file access, and a `relationship` association or `parentId` requires read access to the target — see Reference-creation gating under [Permissions](#permissions) for the full rule and its `_group`-roster carve-out. Plain `Stack` is unscoped and does not apply this. + --- ## Permissions @@ -430,6 +432,17 @@ The core library ships a permission-enforcing wrapper so server implementations **`ScopedStack.create()`** additionally checks `_grant` records for a `'create'` action on the target type before allowing the Record to be written. Anonymous requesters are always denied. The owner always passes. The created Record's `entityId` is always set to the requester, so `-own` grants apply to it immediately. +**Reference-creation gating.** A `create` grant on a type authorizes writing Records of that type — it does not, by itself, authorize referencing arbitrary other Records or files through that Record. `ScopedStack.create()` and `ScopedStack.associate()` both additionally check that the requester may create the specific reference being written, since a reference elsewhere confers access (an `attachment` association or file-ref content field makes the referenced file downloadable via `getAttachment`; see [Attachment](#attachment)): + +- **`attachment` associations and file-ref content fields** require file access: the requester is the owner, uploaded the file themselves (holds an `_attachment@1` Record for it), or can already read some Record referencing it. This is exactly `getAttachment()`'s own access rule — reference creation requires what reference possession would grant. +- **`relationship` associations and `parentId`** require read access to the target Record. +- **`tag` associations** carry no reference and are never gated. +- **`_group` roster associations are exempt** from the `relationship` check above — a roster association's `recordId` names an Entity, not a readable Record (see [Group](#group)), and roster mutation is already gated by the stricter admin-or-owner rule there. + +A missing target and an existing-but-inaccessible one **always produce the same `StackPermissionError`**, with no distinguishing detail — otherwise the check itself becomes a confirmation oracle (e.g. for a guessed file hash: content-addressed `fileId`s mean a successful attach-then-read round-trip would otherwise confirm the stack holds those exact bytes). On `update()`, only file-ref fields actually present in the patch are checked — untouched fields carry no new reference. + +`appId` and `permissions` are deliberately **not** gated by this: `appId` is self-reported, untrusted metadata everywhere (no verification mechanism exists yet — a foundation for future enforcement, per [App](#app)), never a permission input. `permissions` at create time is consistent with `setPermissions()`'s existing owner-or-creator policy — a contributor authoring a Record in your Stack can already widen its access up to and including `public`; create-time is not a new capability, just the same one exercised earlier. + Reading or writing a Record that exists but isn't accessible throws `StackPermissionError`; a missing Record throws `StackNotFoundError`, so callers can distinguish "not found" from "forbidden" (typically 404 vs 403 at the HTTP layer). Plain `Stack` methods remain unscoped and perform no permission checks — correct for single-entity embedded use, where there's no requester distinct from the app itself. Use `asEntity()` when one `Stack` instance serves requests from multiple, possibly untrusted, entities, e.g. a server adapter. diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 0437b8f..9809044 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -1583,6 +1583,99 @@ export class ScopedStack implements StackClient { return record; } + /** + * Whether the requester may create a reference to `recordId` (as a + * `parentId` or a `relationship` association target). Missing and + * unreadable both return false — indistinguishable, so this can't be used + * to probe for a record's existence (#51). + */ + private async canReadReferent(recordId: string): Promise { + const record = await this.stack.get(recordId); + if (!record) return false; + return this.canRead(record); + } + + /** + * Whether the requester may create a reference (`attachment` association + * or file-ref content field) to `fileId` — the dual of getAttachment()'s + * access rule: reference creation requires exactly what reference + * possession would grant. A nonexistent fileId and an existing-but- + * inaccessible one are indistinguishable here (both false), so this can't + * become a confirmation oracle for guessed content hashes (#51). + */ + private async canAccessFile(fileId: string): Promise { + if (this.requesterEntityId === this.stack.ownerEntityId) return true; + + const refResult = await this.query({ filter: { attachmentFileId: fileId }, limit: 1 }); + if (refResult.records.length > 0) return true; + + if (!this.requesterEntityId) return false; + + return this.stack.features.contentFieldQuery + ? ( + await this.stack.query({ + filter: { + typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, + entityId: this.requesterEntityId, + content: { fileId }, + }, + limit: 1, + }) + ).records.length > 0 + : ( + await queryAllPages((q) => this.stack.query(q), { + filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, entityId: this.requesterEntityId }, + }) + ).some((r) => (r.content as AttachmentContent).fileId === fileId); + } + + /** Names of the type's top-level file-ref fields — the content-reference half of attachmentFileId matching (#63). */ + private async fileRefFieldNames(typeId: TypeId): Promise { + const type = await this.stack.getType(typeId); + if (!type) return []; + return Object.entries(type.schema) + .filter(([, def]) => def.kind === 'file-ref') + .map(([field]) => field); + } + + /** + * Gates file-ref content fields on file access, mirroring the attachment- + * association gate below — #63 made a file-ref field convey attachment + * access exactly like an `attachment` association does, so it needs the + * same reference-creation check (#51). Only fields actually present in + * `content` are checked: on update() that's a merge patch, so untouched + * fields carry no new reference. + */ + private async requireFileRefAccess( + typeId: TypeId, + content: Record, + ): Promise { + for (const field of await this.fileRefFieldNames(typeId)) { + const value = content[field]; + if (typeof value !== 'string') continue; + if (!(await this.canAccessFile(value))) throw new StackPermissionError(); + } + } + + /** + * Gates a single association's reference-creation check per #51: an + * `attachment` association requires file access, a `relationship` + * association requires read access to its target, a `tag` carries no + * reference and is unchecked. + * + * `_group` roster associations are exempt from the relationship check — + * their `recordId` is an entity ID, not a readable record, and roster + * mutation is already gated by isGroupManager() (#58), which is strictly + * tighter than "can read the target". + */ + private async requireAssociationAccess(typeId: TypeId, association: Association): Promise { + if (association.kind === 'attachment') { + if (!(await this.canAccessFile(association.fileId))) throw new StackPermissionError(); + } else if (association.kind === 'relationship' && baseIdOf(typeId) !== SYSTEM_TYPES.GROUP) { + if (!(await this.canReadReferent(association.recordId))) throw new StackPermissionError(); + } + } + /** * Create a new record on behalf of the authenticated requester. * Requires either an entity-specific _grant or a default _grant for @@ -1597,6 +1690,16 @@ export class ScopedStack implements StackClient { * `_group` records additionally get the creator stamped as their first * `admin` roster association, so a group is never management-orphaned — * without this, nobody could ever pass isGroupManager() to add themselves. + * + * `parentId`, `associations`, and file-ref content fields are all + * reference-creating options a caller could otherwise use to piggyback on + * a bare `create` grant: a `parentId` or `relationship` association + * requires read access to the target, and an `attachment` association or + * file-ref field requires file access — the same checks associate() + * applies post-create (#51). `permissions` and `appId` are deliberately + * left unchecked here: `permissions` is create-time-consistent with + * setPermissions() (owner/creator territory already), and `appId` is + * self-reported, untrusted metadata everywhere, not a permission input. */ async create = Record>( typeId: TypeId, @@ -1612,6 +1715,13 @@ export class ScopedStack implements StackClient { validateRecordId(opts.id); validateIdTimestampSkew(opts.id, this.idTimestampSkewMs); } + if (opts.parentId !== undefined && !(await this.canReadReferent(opts.parentId))) { + throw new StackPermissionError(); + } + for (const assoc of opts.associations ?? []) { + await this.requireAssociationAccess(typeId, assoc); + } + await this.requireFileRefAccess(typeId, content); const createOpts = baseIdOf(typeId) === SYSTEM_TYPES.GROUP ? { ...opts, associations: stampGroupAdmin(opts.associations, requester) } @@ -1672,7 +1782,8 @@ export class ScopedStack implements StackClient { content: Record, opts: IfVersionOptions = {}, ): Promise { - await this.requireUpdatable(id); + const record = await this.requireUpdatable(id); + await this.requireFileRefAccess(record.typeId, content); return this.stack.update(id, content, opts); } @@ -1681,7 +1792,8 @@ export class ScopedStack implements StackClient { association: Association, opts: IfVersionOptions = {}, ): Promise { - await this.requireUpdatable(id); + const record = await this.requireUpdatable(id); + await this.requireAssociationAccess(record.typeId, association); return this.stack.associate(id, association, opts); } @@ -1805,49 +1917,13 @@ export class ScopedStack implements StackClient { /** * Download attachment bytes. Accessible if the requester is the owner, * can read any record referencing the file, or uploaded the file themselves - * and it hasn't been associated with a record yet. + * and it hasn't been associated with a record yet. Shares its predicate + * with the reference-creation gate (canAccessFile, #51) — reference + * creation requires exactly what reference possession grants. */ async getAttachment(fileId: string): Promise { - if (this.requesterEntityId === this.stack.ownerEntityId) { - return this.stack.getAttachment(fileId); - } - - // Accessible if the requester can read any record that references this file - const refResult = await this.query({ filter: { attachmentFileId: fileId }, limit: 1 }); - if (refResult.records.length > 0) { - return this.stack.getAttachment(fileId); - } - - // Accessible if the requester uploaded it and it hasn't been associated yet. - // With contentFieldQuery, the filter narrows to this fileId server-side, so - // limit: 1 is a correct existence check. Without it, matching happens in - // memory below — limit: 1 there would only ever see the requester's single - // most recent upload, false-denying every earlier one, so that path - // cursor-walks all of the requester's uploads instead. - if (this.requesterEntityId) { - const hasUpload = this.stack.features.contentFieldQuery - ? ( - await this.stack.query({ - filter: { - typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, - entityId: this.requesterEntityId, - content: { fileId }, - }, - limit: 1, - }) - ).records.length > 0 - : ( - await queryAllPages((q) => this.stack.query(q), { - filter: { - typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, - entityId: this.requesterEntityId, - }, - }) - ).some((r) => (r.content as AttachmentContent).fileId === fileId); - if (hasUpload) return this.stack.getAttachment(fileId); - } - - throw new StackPermissionError(); + if (!(await this.canAccessFile(fileId))) throw new StackPermissionError(); + return this.stack.getAttachment(fileId); } /** diff --git a/packages/core/tests/scoped-stack.test.ts b/packages/core/tests/scoped-stack.test.ts index 2d3e7de..0728118 100644 --- a/packages/core/tests/scoped-stack.test.ts +++ b/packages/core/tests/scoped-stack.test.ts @@ -1111,3 +1111,359 @@ describe('Permission — group role restriction (#58)', () => { expect((await stack.asEntity(admin).get(record.id))?.id).toBe(record.id); }); }); + +// ------------------------------------------------------- +// ScopedStack.create()/associate() — reference-creation gating (#51) +// ------------------------------------------------------- +// +// ScopedStack.create() forwards parentId/associations from an untrusted +// caller. Creating a reference should require exactly what possessing that +// reference would grant: an attachment association or file-ref field +// requires file access, a relationship association or parentId requires +// read access to the target. Both gates must produce the same error for a +// nonexistent target as for an existing-but-forbidden one, so the check +// itself can't be used to probe for a record or file's existence. + +describe('ScopedStack.create — attachment association gating (#51)', () => { + beforeEach(async () => { + await stack.defineType(COMMENT, 'Comment', { text: { kind: 'text', required: true } }); + await stack.grant(MEMBER, [{ actions: ['create'], typeId: COMMENT }]); + }); + + test('attachment association referencing an inaccessible file is rejected', async () => { + await expect( + stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [ + { kind: 'attachment', label: 'x', fileId: 'unknown-file', mimeType: 'image/png' }, + ], + }, + ), + ).rejects.toThrow(StackPermissionError); + }); + + test('attachment association referencing a file the requester uploaded is allowed', async () => { + await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); + const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const record = await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [{ kind: 'attachment', label: 'x', fileId, mimeType: 'image/png' }], + }, + ); + expect(record.associations).toContainEqual({ + kind: 'attachment', + label: 'x', + fileId, + mimeType: 'image/png', + }); + }); + + test('attachment association referencing a file readable via another record is allowed', async () => { + const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const owned = await stack.create(NOTE, { text: 'owner note' }); + await stack.associate(owned.id, { + kind: 'attachment', + label: 'cover', + fileId, + mimeType: 'image/png', + }); + await stack.grant(MEMBER, [{ actions: ['read-any'], typeId: NOTE }]); + + const record = await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [{ kind: 'attachment', label: 'x', fileId, mimeType: 'image/png' }], + }, + ); + expect(record.associations).toContainEqual({ + kind: 'attachment', + label: 'x', + fileId, + mimeType: 'image/png', + }); + }); + + test('nonexistent and existing-but-forbidden fileIds produce indistinguishable errors', async () => { + const forbiddenFileId = await stack.putAttachment(new Uint8Array([9]), 'image/png'); + let nonexistentError: Error | undefined; + let forbiddenError: Error | undefined; + try { + await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [ + { kind: 'attachment', label: 'x', fileId: 'truly-nonexistent', mimeType: 'image/png' }, + ], + }, + ); + } catch (e) { + nonexistentError = e as Error; + } + try { + await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [ + { kind: 'attachment', label: 'x', fileId: forbiddenFileId, mimeType: 'image/png' }, + ], + }, + ); + } catch (e) { + forbiddenError = e as Error; + } + expect(nonexistentError).toBeInstanceOf(StackPermissionError); + expect(forbiddenError).toBeInstanceOf(StackPermissionError); + expect(nonexistentError?.message).toBe(forbiddenError?.message); + }); + + test('tag associations are never gated', async () => { + const record = await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [{ kind: 'tag', label: 'starred' }], + }, + ); + expect(record.associations).toContainEqual({ kind: 'tag', label: 'starred' }); + }); + + test('the owner is exempt from the attachment-association gate', async () => { + const record = await stack.create( + COMMENT, + { text: 'hi' }, + { + associations: [ + { kind: 'attachment', label: 'x', fileId: 'anything', mimeType: 'image/png' }, + ], + }, + ); + expect(record.associations).toHaveLength(1); + }); +}); + +describe('ScopedStack.create — relationship association and parentId gating (#51)', () => { + let readableNote: StackRecord; + let unreadableNote: StackRecord; + + beforeEach(async () => { + await stack.defineType(COMMENT, 'Comment', { text: { kind: 'text', required: true } }); + await stack.grant(MEMBER, [{ actions: ['create'], typeId: COMMENT }]); + readableNote = await adapter.createRecord( + makeRecord({ + permissions: [{ access: 'entity', entityId: MEMBER, read: true, write: false }], + }), + ); + unreadableNote = await adapter.createRecord(makeRecord()); + }); + + test('relationship association targeting an unreadable record is rejected', async () => { + await expect( + stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [{ kind: 'relationship', label: 'related', recordId: unreadableNote.id }], + }, + ), + ).rejects.toThrow(StackPermissionError); + }); + + test('relationship association targeting a readable record is allowed', async () => { + const record = await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [{ kind: 'relationship', label: 'related', recordId: readableNote.id }], + }, + ); + expect(record.associations).toContainEqual({ + kind: 'relationship', + label: 'related', + recordId: readableNote.id, + }); + }); + + test('missing and unreadable relationship targets produce indistinguishable errors', async () => { + let missingError: Error | undefined; + let unreadableError: Error | undefined; + try { + await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [ + { kind: 'relationship', label: 'related', recordId: 'nonexistent-record' }, + ], + }, + ); + } catch (e) { + missingError = e as Error; + } + try { + await stack.asEntity(MEMBER).create( + COMMENT, + { text: 'hi' }, + { + associations: [{ kind: 'relationship', label: 'related', recordId: unreadableNote.id }], + }, + ); + } catch (e) { + unreadableError = e as Error; + } + expect(missingError).toBeInstanceOf(StackPermissionError); + expect(unreadableError).toBeInstanceOf(StackPermissionError); + expect(missingError?.message).toBe(unreadableError?.message); + }); + + test('_group roster relationship associations are exempt from the target-read check', async () => { + await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_group@1' }]); + // MEMBER's own entityId has no corresponding readable record in this stack. + const group = await stack.asEntity(MEMBER).create('_group@1', { name: 'New Group' }); + expect(group.associations).toContainEqual({ + kind: 'relationship', + label: 'admin', + recordId: MEMBER, + }); + }); + + test('parentId requires read access to the parent', async () => { + await expect( + stack.asEntity(MEMBER).create(COMMENT, { text: 'hi' }, { parentId: unreadableNote.id }), + ).rejects.toThrow(StackPermissionError); + + const record = await stack + .asEntity(MEMBER) + .create(COMMENT, { text: 'hi' }, { parentId: readableNote.id }); + expect(record.parentId).toBe(readableNote.id); + }); + + test('the owner is exempt from parentId and relationship gates', async () => { + const record = await stack.create( + COMMENT, + { text: 'hi' }, + { + parentId: unreadableNote.id, + associations: [{ kind: 'relationship', label: 'related', recordId: unreadableNote.id }], + }, + ); + expect(record.parentId).toBe(unreadableNote.id); + }); +}); + +describe('ScopedStack.associate — reference-creation gating (#51)', () => { + let ownedRecord: StackRecord; + + beforeEach(async () => { + await stack.grant(MEMBER, [{ actions: ['create', 'update-own'], typeId: NOTE }]); + ownedRecord = await stack.asEntity(MEMBER).create(NOTE, { text: 'mine' }); + }); + + test('associate() rejects an attachment association to a file the requester cannot access', async () => { + await expect( + stack.asEntity(MEMBER).associate(ownedRecord.id, { + kind: 'attachment', + label: 'x', + fileId: 'unknown', + mimeType: 'image/png', + }), + ).rejects.toThrow(StackPermissionError); + }); + + test('associate() allows an attachment association to a file the requester uploaded', async () => { + await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); + const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + await stack + .asEntity(MEMBER) + .associate(ownedRecord.id, { kind: 'attachment', label: 'x', fileId, mimeType: 'image/png' }); + expect((await adapter.getRecord(ownedRecord.id))?.associations).toContainEqual({ + kind: 'attachment', + label: 'x', + fileId, + mimeType: 'image/png', + }); + }); + + test('associate() rejects a relationship association to an unreadable record', async () => { + const unreadableNote = await adapter.createRecord(makeRecord()); + await expect( + stack.asEntity(MEMBER).associate(ownedRecord.id, { + kind: 'relationship', + label: 'related', + recordId: unreadableNote.id, + }), + ).rejects.toThrow(StackPermissionError); + }); + + test('associate() never gates tag associations', async () => { + await stack.asEntity(MEMBER).associate(ownedRecord.id, { kind: 'tag', label: 'starred' }); + expect((await adapter.getRecord(ownedRecord.id))?.associations).toContainEqual({ + kind: 'tag', + label: 'starred', + }); + }); +}); + +describe('ScopedStack — file-ref content field gating (#51, extends #63)', () => { + const PHOTO_NOTE = 'com.example.test/photo-note-gating@1'; + const FILE_ID = 'c'.repeat(64); + + beforeEach(async () => { + await stack.defineType(PHOTO_NOTE, 'Photo note', { + coverFileId: { kind: 'file-ref' }, + title: { kind: 'string' }, + }); + await stack.grant(MEMBER, [{ actions: ['create', 'update-own'], typeId: PHOTO_NOTE }]); + }); + + test('create() rejects a file-ref field pointing at an inaccessible file', async () => { + await expect( + stack.asEntity(MEMBER).create(PHOTO_NOTE, { coverFileId: FILE_ID }), + ).rejects.toThrow(StackPermissionError); + }); + + test('create() allows a file-ref field pointing at a file the requester uploaded', async () => { + await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); + const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const record = await stack.asEntity(MEMBER).create(PHOTO_NOTE, { coverFileId: fileId }); + expect(record.content.coverFileId).toBe(fileId); + }); + + test('update() rejects changing a file-ref field to an inaccessible file', async () => { + await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); + const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const record = await stack.asEntity(MEMBER).create(PHOTO_NOTE, { coverFileId: fileId }); + + await expect( + stack.asEntity(MEMBER).update(record.id, { coverFileId: FILE_ID }), + ).rejects.toThrow(StackPermissionError); + }); + + test('update() leaving the file-ref field untouched is unaffected by its accessibility', async () => { + // Owner-created record with a file-ref the MEMBER updater can't independently access; + // a patch that never mentions coverFileId carries no new reference and isn't gated. + const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const record = await stack.create( + PHOTO_NOTE, + { coverFileId: fileId }, + { + permissions: [{ access: 'entity', entityId: MEMBER, read: true, write: true }], + }, + ); + + const updated = await stack.asEntity(MEMBER).update(record.id, { title: 'renamed' }); + expect(updated.content.title).toBe('renamed'); + expect(updated.content.coverFileId).toBe(fileId); + }); + + test('the owner is exempt from the file-ref gate', async () => { + const record = await stack.create(PHOTO_NOTE, { coverFileId: FILE_ID }); + expect(record.content.coverFileId).toBe(FILE_ID); + }); +});