From 77747706cabb48e7f6087f6e6b8206422f0f2c6b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 18:35:08 +0000 Subject: [PATCH] feat(core): protect _config from ownership hijack, deletion, and query leakage (#67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing guarded the stack's identity record: update('_config', { entityId }) silently re-anchored ownership out from under every running permission check, delete('_config') (soft or hard) left the stack unreadable or unopenable, and query exclusion was a hardcoded WHERE clause one adapter happened to remember. Per the issue's layering principle, these are integrity guards that belong in Stack (inherited by every adapter and by ScopedStack, which delegates) rather than adapter-specific conventions — query exclusion is the one exception, since it has to stay a WHERE predicate to avoid breaking pagination, and already lives in sqlite-shared for both real SQL adapters. - Stack.update('_config', ...) rejects a changed entityId (StackConflictError); other fields (timezone) update normally. - Stack.delete('_config') is rejected unconditionally, soft or hard. - Stack.restoreVersion('_config', ...) rejects restoring a snapshot whose entityId disagrees with the live record's. - MemoryAdapter now excludes _config from generic queries too, matching the real adapters and making the exclusion rule testable/enforceable at the core level. docs/spec.md: _config protections stated under Stack initialization, the query-exclusion rule made normative, and the 409 error-table row extended. Fixes #67 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KDmUW3oNxPSCLf2ftqaF8q --- docs/spec.md | 10 +++- packages/core/src/stack.ts | 45 ++++++++++++++ packages/core/src/testing.ts | 4 ++ packages/core/tests/stack.test.ts | 98 +++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 1 deletion(-) diff --git a/docs/spec.md b/docs/spec.md index c09539b..0446f9b 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -39,6 +39,12 @@ Plugin and extension code that doesn't need to know the underlying backend shoul **For the API adapter**, identity values are sourced from the discovery endpoint (`GET /.well-known/stack`) when the adapter is opened and cached for the session as adapter properties. +**`_config` is protected** (#67), by `Stack` itself rather than any one adapter — the same layering as schema validation, so every adapter and `ScopedStack` (which delegates to `Stack`) inherit it automatically: + +- **Addressable only by ID.** `get('_config')` works; a generic `query()` never returns it, regardless of filter — matching every other reserved-ID system record, but load-bearing here since `_config` is read at open and consulted by every permission check. +- **`entityId` is immutable.** `update('_config', { entityId: '...' })` throws `StackConflictError` — changing it would silently re-anchor stack ownership out from under a running system, and the stakes only rise once identity becomes a DID. Other fields (`timezone` today) update normally. `restoreVersion('_config', ...)` inherits the same rule: a snapshot whose `entityId` disagrees with the live record's cannot be restored. Ownership transfer, if it's ever added, is a deliberate future API with key-custody semantics — not a field write. +- **Never deletable**, soft or hard. `delete('_config')` always throws `StackConflictError`: a soft-deleted config is unreadable through normal paths, and a hard-deleted one leaves nothing to reopen the stack against. + --- ### Entity @@ -715,6 +721,8 @@ type DateRange = { By default, `query()` (like `get()`) returns Records exactly as stored — see [presentAt: 'latest'](#type-migrations) to migrate results in memory instead. +**`query()` never returns the `_config` record**, regardless of filter (#67) — it's addressable only by ID, via `get('_config')` or the adapter's own typed `ownerEntityId`/`timezone` properties. This is the one exception to "adapters are storage engines, `Stack` is the invariant layer": exclusion must live in the adapter's own query predicate (a `WHERE` clause, or the equivalent for an in-memory adapter) rather than be post-filtered by `Stack`, since post-filtering after the adapter applies `limit` would silently under-fill a page. Every adapter — including test doubles — implements this exclusion directly; it is not optional convention. + ### Sorting and pagination ```ts @@ -840,7 +848,7 @@ Standard HTTP status codes are used throughout: | **401** | Unauthorized | Missing or invalid bearer token | | **403** | Forbidden | `StackPermissionError` — record exists but the requester lacks access | | **404** | Not found | `StackNotFoundError` — record or version does not exist | -| **409** | Conflict | `StackConflictError` — operation blocked by a constraint violation (e.g. deleting an attachment still referenced by a record, a client-supplied `id` that already exists) | +| **409** | Conflict | `StackConflictError` — operation blocked by a constraint violation (e.g. deleting an attachment still referenced by a record, a client-supplied `id` that already exists, deleting `_config` or changing its `entityId` — see [Stack initialization](#stack-initialization)) | | **412** | Precondition failed | `StackVersionConflictError` (code `version_conflict`) — an `If-Match` precondition doesn't match the record's current version (see [Versions](#versions)). A distinct error type and status from `StackConflictError`/409, not a subtype of it — the two have different recovery stories | | **413** | Request entity too large | Attachment upload exceeds the server's size limit | | **422** | Unprocessable entity | `StackValidationError` — request is syntactically valid but content fails schema validation (e.g. a required field has the wrong type) | diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 061433e..79f58bb 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -39,6 +39,7 @@ import type { GrantAction, GrantContent, AttachmentContent, + ConfigContent, } from './types.js'; // ------------------------------------------------------- @@ -754,6 +755,13 @@ export class Stack implements StackClient { ); } + if (id === SYSTEM_TYPES.CONFIG) { + this.checkConfigEntityIdUnchanged( + (existing.content as ConfigContent).entityId, + (merged as ConfigContent).entityId, + ); + } + // Snapshot the raw stored state before overwriting await this.saveVersion(existing); @@ -832,8 +840,18 @@ export class Stack implements StackClient { * as update(); it's a no-op if the record is already deleted. Hard delete * destroys the record (and its version history) outright — there's * nothing to snapshot. + * + * `_config` can never be deleted, soft or hard (#67): it's the stack's + * identity record, read at open and consulted by every permission check. + * A soft-deleted `_config` is unreadable through normal paths; a + * hard-deleted one bricks the stack outright (nothing to reopen against). */ async delete(id: string, opts: DeleteRecordOptions = {}): Promise { + if (id === SYSTEM_TYPES.CONFIG) { + throw new StackConflictError( + "Cannot delete the _config record: it holds the stack's identity and is required for every permission check.", + ); + } if (opts.hard) { return this.adapter.deleteRecord(id, { hard: true, expectedVersion: opts.ifVersion }); } @@ -982,6 +1000,13 @@ export class Stack implements StackClient { throw new StackValidationError(errors); } + if (id === SYSTEM_TYPES.CONFIG) { + this.checkConfigEntityIdUnchanged( + (existing.content as ConfigContent).entityId, + (target.content as ConfigContent).entityId, + ); + } + // Snapshot current state before restoring await this.saveVersion(existing); @@ -1071,6 +1096,26 @@ export class Stack implements StackClient { } } + /** + * `_config.entityId` defines stack ownership — read once at open and + * consulted by every permission check thereafter (#67). Neither update() + * nor restoreVersion() may change it: a write that silently re-anchors + * ownership would desync every already-running owner check from the next + * reopen onward. This is a conflict with stack integrity, not a schema + * violation (the new value is a perfectly valid string) — hence + * StackConflictError, matching delete()'s guard on the same record. + * Ownership transfer, if it ever exists, is a deliberate future API with + * key-custody semantics (#49), not a field write. + */ + private checkConfigEntityIdUnchanged(existingEntityId: string, newEntityId: string): void { + if (newEntityId !== existingEntityId) { + throw new StackConflictError( + 'Cannot change _config.entityId: it defines stack ownership. ' + + 'Ownership transfer is not a supported operation.', + ); + } + } + /** * Store raw bytes and return the content-addressed file ID. * Does not create an _attachment@1 record — use putAttachment() or diff --git a/packages/core/src/testing.ts b/packages/core/src/testing.ts index c493c55..beb96b3 100644 --- a/packages/core/src/testing.ts +++ b/packages/core/src/testing.ts @@ -11,6 +11,7 @@ import type { AdapterCapabilities, BlobFileInfo, } from './types.js'; +import { SYSTEM_TYPES } from './types.js'; import { applyMergePatch } from './merge.js'; import { StackVersionConflictError, StackConflictError } from './stack.js'; @@ -127,6 +128,9 @@ export class MemoryAdapter implements StackAdapter { async queryRecords(query: StackQuery): Promise { const f = query.filter ?? {}; let results = this.order.map((id) => this.records.get(id)!); + // _config is addressable only by ID (getRecord), never returned by a + // generic query — mirroring the real SQL adapters' WHERE exclusion (#67). + results = results.filter((r) => r.id !== SYSTEM_TYPES.CONFIG); if (!f.includeDeleted) results = results.filter((r) => !r.deletedAt); if (f.typeId) { const ids = Array.isArray(f.typeId) ? f.typeId : [f.typeId]; diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 1549c43..b9a21c3 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -917,6 +917,104 @@ describe('delete', () => { }); }); +// ------------------------------------------------------- +// _config protections (#67) +// ------------------------------------------------------- + +describe('_config protections (#67)', () => { + const CONFIG_ID = '_config'; + const CONFIG_TYPE = '_config@1'; + + // MemoryAdapter never materializes a _config record on its own (ownerEntityId + // is a plain constructor field) — the guards under test operate on whatever + // record exists at id "_config", so tests seed one directly. + async function seedConfig(entityId = 'owner-123', timezone = 'UTC') { + return adapter.createRecord({ + id: CONFIG_ID, + typeId: CONFIG_TYPE, + createdAt: new Date(), + updatedAt: new Date(), + content: { entityId, timezone }, + version: 1, + }); + } + + test('update() rejects a change to entityId', async () => { + await seedConfig(); + await expect(stack.update(CONFIG_ID, { entityId: 'someone-else' })).rejects.toThrow( + StackConflictError, + ); + expect((await adapter.getRecord(CONFIG_ID))?.content.entityId).toBe('owner-123'); + }); + + test('update() allows changing timezone', async () => { + await seedConfig(); + const updated = await stack.update(CONFIG_ID, { timezone: 'America/New_York' }); + expect((updated.content as Record).timezone).toBe('America/New_York'); + }); + + test('setting entityId to its current value is a no-op, not an error', async () => { + await seedConfig('owner-123'); + await expect(stack.update(CONFIG_ID, { entityId: 'owner-123' })).resolves.toBeDefined(); + }); + + test('soft delete is rejected', async () => { + await seedConfig(); + await expect(stack.delete(CONFIG_ID)).rejects.toThrow(StackConflictError); + expect(await adapter.getRecord(CONFIG_ID)).not.toBeNull(); + }); + + test('hard delete is rejected', async () => { + await seedConfig(); + await expect(stack.delete(CONFIG_ID, { hard: true })).rejects.toThrow(StackConflictError); + expect(await adapter.getRecord(CONFIG_ID)).not.toBeNull(); + }); + + test('restoreVersion() rejects a snapshot with a different entityId', async () => { + await seedConfig('owner-123'); + // Simulates a snapshot that predates this guard, or a bypassed + // direct-adapter write — either way, a stored version whose entityId + // disagrees with the live record's must not be restorable. + await adapter.saveVersion(CONFIG_ID, { + version: 1, + typeId: CONFIG_TYPE, + content: { entityId: 'someone-else', timezone: 'UTC' }, + updatedAt: new Date(), + }); + await expect(stack.restoreVersion(CONFIG_ID, 1)).rejects.toThrow(StackConflictError); + }); + + test('restoreVersion() allows a snapshot with the same entityId', async () => { + await seedConfig('owner-123', 'UTC'); + await stack.update(CONFIG_ID, { timezone: 'America/New_York' }); + const restored = await stack.restoreVersion(CONFIG_ID, 1); + expect((restored.content as Record).timezone).toBe('UTC'); + }); + + test('generic query excludes _config', async () => { + await seedConfig(); + const result = await stack.query({ filter: { typeId: CONFIG_TYPE } }); + expect(result.records).toHaveLength(0); + }); + + test('_config is still addressable directly by ID', async () => { + await seedConfig(); + expect(await stack.get(CONFIG_ID)).not.toBeNull(); + }); + + test('ScopedStack delegation: the owner cannot change entityId via scoped update either', async () => { + await seedConfig('owner-123'); + await expect( + stack.asEntity('owner-123').update(CONFIG_ID, { entityId: 'someone-else' }), + ).rejects.toThrow(StackConflictError); + }); + + test('ScopedStack delegation: the owner cannot delete _config via scoped delete either', async () => { + await seedConfig('owner-123'); + await expect(stack.asEntity('owner-123').delete(CONFIG_ID)).rejects.toThrow(StackConflictError); + }); +}); + // ------------------------------------------------------- // undelete // -------------------------------------------------------