From 4a3bc8e86f69296ce18413e2225e6f35557a3b1c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 22:28:54 +0000 Subject: [PATCH 1/4] feat(core): wire parsers for the remaining spec'd endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every endpoint core specifies now has a ./wire parser, so a server no longer copies core's param and field names into its own lists, where they drift: parseAuthChallengeBody, parseAuthTokenBody, parseEntityPatchBody, parseTypeBody, parseMigrationBody, parseDeleteParams, parseDownloadParams and parseAssociationParams. Each refuses an unknown name or malformed boolean with 400 and a wrong-typed known body field with 422, following the split POST /records already makes. The body helpers are shared with wire-record.ts. The spec's § Unrecognized input now tables the parser for each endpoint, § Types names the keys POST /types accepts and ignores, and two error fixtures pin a non-boolean purge and an unknown /auth/token key. Unknown keys inside grant and association elements are left for a separate decision, as the issue notes. Refs #354 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014ah52eTkayJ1JxPuEmgVGe --- .../wire-parsers-remaining-endpoints.md | 6 + docs/spec/wire-format.md | 23 ++- .../adapter-api/tests/conformance.test.ts | 2 + packages/conformance-fixtures/src/index.ts | 34 ++++ packages/core/src/wire-body.ts | 180 ++++++++++++++++++ packages/core/src/wire-entry.ts | 31 ++- packages/core/src/wire-record.ts | 22 +-- packages/core/src/wire-request.ts | 65 +++++++ packages/core/tests/wire-body.test.ts | 168 ++++++++++++++++ packages/core/tests/wire-request.test.ts | 71 +++++++ 10 files changed, 580 insertions(+), 22 deletions(-) create mode 100644 .changeset/wire-parsers-remaining-endpoints.md create mode 100644 packages/core/src/wire-body.ts create mode 100644 packages/core/tests/wire-body.test.ts diff --git a/.changeset/wire-parsers-remaining-endpoints.md b/.changeset/wire-parsers-remaining-endpoints.md new file mode 100644 index 00000000..c5f0076f --- /dev/null +++ b/.changeset/wire-parsers-remaining-endpoints.md @@ -0,0 +1,6 @@ +--- +'@haverstack/core': minor +'@haverstack/conformance-fixtures': minor +--- + +`@haverstack/core/wire` adds a parser for every remaining endpoint core specifies, so a server no longer keeps its own list of their param and field names: `parseAuthChallengeBody()`, `parseAuthTokenBody()`, `parseEntityPatchBody()`, `parseTypeBody()`, `parseMigrationBody()`, `parseDeleteParams()`, `parseDownloadParams()` and `parseAssociationParams()`. Each refuses an unknown name or a malformed boolean with `StackBadRequestError`, and a known body field of the wrong type with `StackValidationError`. Two new error fixtures pin a non-boolean `purge` and an unknown key on `POST /auth/token`. diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index 96591c57..882ac8d4 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -193,7 +193,26 @@ Three things sit outside it: - **Headers.** A request carries headers the application never sees — proxies and browsers add them — so an unrecognized one is ignored, as is `If-Match` on the association endpoints (see [Records](#records)). - **Responses.** A client reading a server's response ignores what it doesn't recognize, the way it ignores an [unrecognized frame](./change-feed.md). Strictness is the server's side of the contract, where the input is the client's intent. -A server built on core reaches this through the wire parsers in `@haverstack/core/wire` — `parseQueryParams()`, `parseQueryBody()`, `parseChangeParams()`, `parseJournalParams()`, `createOptionsFromWireRecord()` and `changesFromWireBody()` — each of which refuses what its endpoint does not define. Endpoints a server parses itself — `/auth/token` among them — owe the same refusal. +A server built on core reaches this through the wire parsers in `@haverstack/core/wire`, one per endpoint that takes input, each of which refuses what its endpoint does not define: + +| Endpoint | Parser | +| ------------------------------- | ------------------------------- | +| `GET /records` | `parseQueryParams()` | +| `POST /records/query` | `parseQueryBody()` | +| `POST /records` | `createOptionsFromWireRecord()` | +| `PATCH /records/:id` | `changesFromWireBody()` | +| `DELETE /records/:id` | `parseDeleteParams()` | +| `POST /records/:id/migrate` | `parseMigrationBody()` | +| `GET /records/:id/journal` | `parseJournalParams()` | +| `GET /records/:id/associations` | `parseAssociationParams()` | +| `GET /changes` | `parseChangeParams()` | +| `GET /attachments/:fileId` | `parseDownloadParams()` | +| `POST /types` | `parseTypeBody()` | +| `PATCH /entity` | `parseEntityPatchBody()` | +| `POST /auth/challenge` | `parseAuthChallengeBody()` | +| `POST /auth/token` | `parseAuthTokenBody()` | + +The body parsers split errors the way [Records](#records) does for a create: a body that is not an object, carries a key its endpoint does not define, or lacks a field the endpoint requires is not that request at all, so it is **400**; a known field whose value is the wrong type is **422**, carrying the field's path. Endpoints a server defines beyond this spec owe the same refusal, parsed by the server itself. ### The taxonomy root @@ -577,6 +596,8 @@ GET /types/:id — get one type definition (id is URL-encoded) POST /types — register a type, or evolve an existing one in place ``` +**The body is a whole Type**, as `GET /types/:id` returns one. `id`, `name` and `schema` are read, as is `migratesFrom` when present; `baseId`, `version`, `schemaHash` and `createdAt` are accepted and ignored, since `defineType()` derives or stamps each of them. Any other key is refused (see [Unrecognized input](#unrecognized-input)). + `POST /types` on an `id` that already has a stored Type runs the same [schema drift check](./data-model.md#schema-drift-detection) as `Stack.defineType()` — the server-side storage layer never blindly overwrites a Type definition; legality is decided once, in the same invariant layer both the local and wire paths share. **A malformed schema answers 422** (code `validation`) rather than failing inside the server: a body here is parsed JSON, so a definition that is not an object, one naming no `kind` or an unrecognized one, a non-boolean `required`/`open`, and a container declaring neither its interior nor `open` — or both — are each reported against the field they sit on. See [Data model § Types](./data-model.md#types). diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index 3c26cc11..9d9ea515 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -751,6 +751,8 @@ const SERVER_ONLY_ERROR_FIXTURES = new Set([ 'error-bad-request-non-boolean-param', 'error-bad-request-unknown-query-body-key', 'error-bad-request-unknown-record-key', + 'error-bad-request-non-boolean-purge', + 'error-bad-request-unknown-auth-token-key', ]); describe('error response fixtures', () => { diff --git a/packages/conformance-fixtures/src/index.ts b/packages/conformance-fixtures/src/index.ts index f2fb39bd..c039cf5d 100644 --- a/packages/conformance-fixtures/src/index.ts +++ b/packages/conformance-fixtures/src/index.ts @@ -2196,6 +2196,40 @@ export const errorResponseFixtures: ConformanceFixture[] = [ responseStatus: 400, responseBody: { error: { code: 'bad_request', message: 'Unknown record key: title' } }, }, + { + name: 'error-bad-request-non-boolean-purge', + description: + 'DELETE /records/:id takes purge only as "true" or "false"; any other value returns 400 ' + + 'with code "bad_request" and deletes nothing. Read as false, a purge the caller meant ' + + 'becomes a soft delete that answers 200. See docs/spec/wire-format.md § Unrecognized input.', + method: 'DELETE', + path: '/records/1hk153x00001?purge=1', + responseStatus: 400, + responseBody: { + error: { code: 'bad_request', message: 'Invalid purge: expected true or false, got "1"' }, + }, + }, + { + name: 'error-bad-request-unknown-auth-token-key', + description: + 'POST /auth/token takes did, nonce and signature and nothing else; any other key — here ' + + 'a subjectId naming whom the token should act for — returns 400 with code "bad_request" ' + + 'and issues no token. Ignoring it would answer with a token the client did not ask for. ' + + 'See docs/spec/wire-format.md § Unrecognized input.', + method: 'POST', + path: '/auth/token', + requestBody: { + did: 'did:key:z6Mkfsz9oK6i2355mvEwtDYdAmqCN6kmQETThJtARfj9iGum', + nonce: 'k7Qm2ZxRt9vLbNc4Hy8Wf3', + signature: + 'CIvHvqS75hEpPDZi7hwLFOMM44-UCMuF5HzZ9_OIAMQvsGAYGsvXXpXQTP3KaPH2qKnQxl2j3xcB_v-axIx8Bg', + subjectId: 'did:key:z6Mktp5FtRqj2M7JxnPz9JWGMCUTE5o3XGt1br11TczKGp7B', + }, + responseStatus: 400, + responseBody: { + error: { code: 'bad_request', message: 'Unknown key in auth token body: subjectId' }, + }, + }, { name: 'error-validation-failed', description: diff --git a/packages/core/src/wire-body.ts b/packages/core/src/wire-body.ts new file mode 100644 index 00000000..a55c908b --- /dev/null +++ b/packages/core/src/wire-body.ts @@ -0,0 +1,180 @@ +/** + * Stack — Wire Request Bodies + * ------------------------------------------------------- + * The JSON bodies of the endpoints core specifies but runs no server for: + * the auth handshake, `PATCH /entity`, `POST /types` and + * `POST /records/:id/migrate`. Each parser names every key its endpoint + * defines, so a server refuses the rest without keeping its own copy of + * the list — a copy that drifts the first time core adds a field. + * + * The error split is the one `POST /records` makes: a body that is not an + * object, carries a key its endpoint does not define, or lacks a field the + * endpoint requires is not that request at all, so it is + * `StackBadRequestError` (400). A known field whose value is the wrong + * type is `StackValidationError` (422), which carries the field's path. + * Beyond type, values are `Stack`'s to judge — a schema's legality, a + * DID's form — so judging them here would let two answers drift. + * See docs/spec/wire-format.md § Unrecognized input. + */ + +import { StackBadRequestError, StackValidationError } from './errors.js'; +import type { DefineTypeOptions } from './stack.js'; +import type { StackType, TypeId, TypeSchema } from './types.js'; + +export function requireBody(body: unknown, label: string): Record { + if (typeof body !== 'object' || body === null || Array.isArray(body)) + throw new StackBadRequestError(`Invalid ${label}: expected an object`); + return body as Record; +} + +/** + * The refusal a present field earns when its value is the wrong shape. + * `StackValidationError` because the failure names a field, and only that + * class carries the path. + */ +export function fieldError(path: string, message: string): never { + throw new StackValidationError([{ path, message }]); +} + +/** A body carrying only `keys`; anything else is refused rather than ignored. */ +function requireKnownBody( + body: unknown, + keys: readonly string[], + label: string, +): Record { + const obj = requireBody(body, label); + const unknown = Object.keys(obj).filter((key) => !keys.includes(key)); + if (unknown.length > 0) + throw new StackBadRequestError( + `Unknown key${unknown.length > 1 ? 's' : ''} in ${label}: ${unknown.join(', ')}`, + ); + return obj; +} + +function requiredString(body: Record, key: string, label: string): string { + const value = body[key]; + if (value === undefined) throw new StackBadRequestError(`Invalid ${label}: ${key} is required`); + if (typeof value !== 'string') fieldError(key, `${key} must be a string`); + return value; +} + +function requiredObject( + body: Record, + key: string, + label: string, +): Record { + const value = body[key]; + if (value === undefined) throw new StackBadRequestError(`Invalid ${label}: ${key} is required`); + if (typeof value !== 'object' || value === null || Array.isArray(value)) + fieldError(key, `${key} must be an object`); + return value as Record; +} + +// ------------------------------------------------------- +// POST /auth/challenge, POST /auth/token +// ------------------------------------------------------- + +export type WireAuthChallengeRequest = { did: string }; +export type WireAuthTokenRequest = { did: string; nonce: string; signature: string }; + +/** + * Parse a `POST /auth/challenge` body. Whether `did` is a DID this server + * can verify is the handshake's `invalid_did`, not a parse failure. + * See docs/spec/wire-format.md § The handshake. + */ +export function parseAuthChallengeBody(body: unknown): WireAuthChallengeRequest { + const label = 'auth challenge body'; + const b = requireKnownBody(body, ['did'], label); + return { did: requiredString(b, 'did', label) }; +} + +/** + * Parse a `POST /auth/token` body. The three fields are the only client + * input the signing payload takes, so a fourth — a `subjectId`, say — is + * refused rather than read as a request the handshake cannot grant. + */ +export function parseAuthTokenBody(body: unknown): WireAuthTokenRequest { + const label = 'auth token body'; + const b = requireKnownBody(body, ['did', 'nonce', 'signature'], label); + return { + did: requiredString(b, 'did', label), + nonce: requiredString(b, 'nonce', label), + signature: requiredString(b, 'signature', label), + }; +} + +// ------------------------------------------------------- +// PATCH /entity +// ------------------------------------------------------- + +/** A content patch for the owner entity, in the shape `patchContent()` takes. */ +export type WireEntityPatch = { content: Record }; + +/** Parse a `PATCH /entity` body. See docs/spec/wire-format.md § Entity. */ +export function parseEntityPatchBody(body: unknown): WireEntityPatch { + const label = 'entity body'; + const b = requireKnownBody(body, ['content'], label); + return { content: requiredObject(b, 'content', label) }; +} + +// ------------------------------------------------------- +// POST /types +// ------------------------------------------------------- + +/** + * Every key a wire Type carries — a `StackType`'s, so a new field fails to + * compile here until it is listed. A client posts the whole Type, and the + * derived keys are accepted and ignored since `defineType()` computes them. + */ +const WIRE_TYPE_KEYS: readonly string[] = Object.keys({ + id: true, + baseId: true, + version: true, + name: true, + schema: true, + schemaHash: true, + migratesFrom: true, + createdAt: true, +} satisfies Record); + +/** + * Parse a `POST /types` body into the options `defineType()` takes. + * `schema` passes through unexamined: its legality is `defineType()`'s to + * report, field by field. See docs/spec/wire-format.md § Types. + */ +export function parseTypeBody(body: unknown): DefineTypeOptions { + const label = 'type body'; + const b = requireKnownBody(body, WIRE_TYPE_KEYS, label); + const id = requiredString(b, 'id', label); + const name = requiredString(b, 'name', label); + if (b.schema === undefined) + throw new StackBadRequestError(`Invalid ${label}: schema is required`); + const options: DefineTypeOptions = { id, name, schema: b.schema as TypeSchema }; + if (b.migratesFrom !== undefined) { + if (typeof b.migratesFrom !== 'string') + fieldError('migratesFrom', 'migratesFrom must be a string'); + options.migratesFrom = b.migratesFrom; + } + return options; +} + +// ------------------------------------------------------- +// POST /records/:id/migrate +// ------------------------------------------------------- + +/** The two arguments `commitMigration()` takes after the record id. */ +export type WireMigrationRequest = { toTypeId: TypeId; content: Record }; + +/** + * Parse a `POST /records/:id/migrate` body. `content` is the whole + * post-migration content, validated by `commitMigration()` against + * `toTypeId`'s schema. See docs/spec/wire-format.md § Migration commit. + */ +export function parseMigrationBody(body: unknown): WireMigrationRequest { + const label = 'migration body'; + const b = requireKnownBody(body, ['toTypeId', 'content'], label); + return { + toTypeId: requiredString(b, 'toTypeId', label), + content: requiredObject(b, 'content', label), + }; +} diff --git a/packages/core/src/wire-entry.ts b/packages/core/src/wire-entry.ts index 38b8f04c..19529848 100644 --- a/packages/core/src/wire-entry.ts +++ b/packages/core/src/wire-entry.ts @@ -52,8 +52,9 @@ export type { StackTokenStore, TokenInfo, TokenSession } from './types.js'; // No in-repo caller: the parse half of the request encoding adapter-api // builds, so a server decodes GET /records, POST /records/query, -// GET /changes and GET /records/:id/journal with these rather than -// transcribing the parameter table. +// GET /changes, GET /records/:id/journal, DELETE /records/:id, +// GET /attachments/:fileId and GET /records/:id/associations with these +// rather than transcribing the parameter table. // Contract, not internal — and the round trip against the builders is // pinned by a test, which is what having both halves here buys. export { @@ -61,6 +62,9 @@ export { parseQueryBody, parseChangeParams, parseJournalParams, + parseDeleteParams, + parseDownloadParams, + parseAssociationParams, parseIfMatch, parseUploadFilename, parsePositiveInt, @@ -70,7 +74,11 @@ export { // them before encoding, so which encoding a server's content reach selects // never decides whether a family query widens or is refused. export { assertQueryTravels } from './wire-request.js'; -export type { ParsedChangeParams } from './wire-request.js'; +export type { + ParsedChangeParams, + ParsedDownloadParams, + ParsedAssociationParams, +} from './wire-request.js'; // Re-exported by @haverstack/wire-types for the response side: one wire // date decoding, used by both halves. @@ -82,6 +90,23 @@ export { parseDate } from './wire-request.js'; export { createOptionsFromWireRecord, changesFromWireBody } from './wire-record.js'; export type { WireCreateRequest } from './wire-record.js'; +// No in-repo caller: the bodies of the remaining endpoints core specifies, +// so a server refuses a key they do not define without keeping its own +// list of the ones they do. See docs/spec/wire-format.md § Unrecognized input. +export { + parseAuthChallengeBody, + parseAuthTokenBody, + parseEntityPatchBody, + parseTypeBody, + parseMigrationBody, +} from './wire-body.js'; +export type { + WireAuthChallengeRequest, + WireAuthTokenRequest, + WireEntityPatch, + WireMigrationRequest, +} from './wire-body.js'; + // The tier gating purge, commitMigration() and includeUnlisted, which // a server must decide for those routes itself. Computing it as "is the // owner" is a privilege bug with no symptom. diff --git a/packages/core/src/wire-record.ts b/packages/core/src/wire-record.ts index c25ca2f2..1644e7d3 100644 --- a/packages/core/src/wire-record.ts +++ b/packages/core/src/wire-record.ts @@ -12,7 +12,8 @@ */ import { isOwnerActingAlone } from './access.js'; -import { StackBadRequestError, StackValidationError } from './errors.js'; +import { StackBadRequestError } from './errors.js'; +import { fieldError, requireBody } from './wire-body.js'; import type { BackdatableCreateRecordOptions } from './stack.js'; import { RECORD_CHANGE_SET_KEYS } from './types.js'; import type { @@ -59,21 +60,6 @@ const WIRE_RECORD_KEYS: readonly string[] = Object.keys({ associations: true, } satisfies Record); -function requireBody(body: unknown): Record { - if (typeof body !== 'object' || body === null || Array.isArray(body)) - throw new StackBadRequestError('Invalid record body: expected an object'); - return body as Record; -} - -/** - * The refusal a present field earns when its value is the wrong shape. - * `StackValidationError` because the failure names a field of the record - * being written, and only that class carries the path. - */ -function fieldError(path: string, message: string): never { - throw new StackValidationError([{ path, message }]); -} - function optionalString(body: Record, key: string): string | undefined { const value = body[key]; if (value === undefined) return undefined; @@ -117,7 +103,7 @@ export function createOptionsFromWireRecord( session: TokenSession, ownerEntityId: EntityId, ): WireCreateRequest { - const record = requireBody(body); + const record = requireBody(body, 'record body'); const unknown = Object.keys(record).filter((key) => !WIRE_RECORD_KEYS.includes(key)); if (unknown.length > 0) throw new StackBadRequestError( @@ -174,7 +160,7 @@ export function createOptionsFromWireRecord( * See docs/spec/wire-format.md § Records. */ export function changesFromWireBody(body: unknown): RecordChangeSet { - const envelope = requireBody(body); + const envelope = requireBody(body, 'record body'); const unknown = Object.keys(envelope).filter( (key) => !(RECORD_CHANGE_SET_KEYS as readonly string[]).includes(key), diff --git a/packages/core/src/wire-request.ts b/packages/core/src/wire-request.ts index 59bfb01c..0e8e5db7 100644 --- a/packages/core/src/wire-request.ts +++ b/packages/core/src/wire-request.ts @@ -28,6 +28,7 @@ import { StackBadRequestError } from './errors.js'; import { NATIVE_SORT_FIELDS } from './types.js'; import type { + DataAssociation, ChangeFilter, ChangeKind, JournalQuery, @@ -55,6 +56,7 @@ const POSITIVE_INTEGER = /^\d+$/; const SORT_FIELDS: ReadonlySet = new Set(NATIVE_SORT_FIELDS); const SORT_DIRECTIONS: ReadonlySet> = new Set(['asc', 'desc']); const CHANGE_KINDS: ReadonlySet = new Set(['created', 'changed', 'deleted', 'purged']); +const ASSOCIATION_KINDS: ReadonlySet = new Set(['tag', 'attachment', 'relationship']); const TARGET_KINDS: ReadonlySet = new Set(['record', 'entity', 'external']); const TARGET_KEYS: Record = { record: ['kind', 'recordId', 'stackUrl'], @@ -700,6 +702,69 @@ export function parseJournalParams(url: URL): JournalQuery { return query; } +// ------------------------------------------------------- +// DELETE /records/:id +// ------------------------------------------------------- + +/** + * Parse `DELETE /records/:id`'s query params into the `purge` flag. A + * misspelled or non-boolean `purge` is refused rather than read as a soft + * delete. See docs/spec/wire-format.md § Records. + */ +export function parseDeleteParams(url: URL): { purge: boolean } { + requireKnownParams(url, ['purge']); + return { purge: booleanParam(url, 'purge') }; +} + +// ------------------------------------------------------- +// GET /attachments/:fileId +// ------------------------------------------------------- + +/** + * The download params, in the names `resolveAttachmentDownloadContentType()` + * takes them by. An empty value stays empty rather than absent, so the + * resolution treats it exactly as it would an omitted one. + */ +export type ParsedDownloadParams = { contentTypeParam?: string; filenameParam?: string }; + +/** Parse `GET /attachments/:fileId`'s query params. See docs/spec/wire-format.md § Download. */ +export function parseDownloadParams(url: URL): ParsedDownloadParams { + requireKnownParams(url, ['contentType', 'filename']); + const contentType = url.searchParams.get('contentType'); + const filename = url.searchParams.get('filename'); + return { + ...(contentType !== null && { contentTypeParam: contentType }), + ...(filename !== null && { filenameParam: filename }), + }; +} + +// ------------------------------------------------------- +// GET /records/:id/associations +// ------------------------------------------------------- + +export type ParsedAssociationParams = { kind?: DataAssociation['kind']; label?: string }; + +/** + * Parse `GET /records/:id/associations`' query params. An unrecognized + * `kind` is refused, since reading it as absent would answer with every + * kind. `kind` repeats on `GET /changes` but names one value here, so a + * repeat is refused rather than read as its first. + * See docs/spec/wire-format.md § Associations. + */ +export function parseAssociationParams(url: URL): ParsedAssociationParams { + requireKnownParams(url, ['kind', 'label']); + if (url.searchParams.getAll('kind').length > 1) + throw new StackBadRequestError('Repeated query param: kind'); + const kind = url.searchParams.get('kind'); + if (kind !== null && !ASSOCIATION_KINDS.has(kind)) + throw new StackBadRequestError(`Invalid kind: "${kind}"`); + const label = url.searchParams.get('label'); + return { + ...(kind !== null && { kind: kind as DataAssociation['kind'] }), + ...(label !== null && { label }), + }; +} + // ------------------------------------------------------- // Headers // ------------------------------------------------------- diff --git a/packages/core/tests/wire-body.test.ts b/packages/core/tests/wire-body.test.ts new file mode 100644 index 00000000..8ca3106c --- /dev/null +++ b/packages/core/tests/wire-body.test.ts @@ -0,0 +1,168 @@ +import { describe, test, expect } from 'vitest'; +import { + parseAuthChallengeBody, + parseAuthTokenBody, + parseEntityPatchBody, + parseTypeBody, + parseMigrationBody, +} from '../src/wire-entry.js'; +import { Stack } from '../src/stack.js'; +import { StackBadRequestError, StackValidationError } from '../src/errors.js'; +import { MemoryAdapter } from '../src/testing.js'; +import type { TypeSchema } from '../src/types.js'; + +const DID = 'did:key:z6Mkfsz9oK6i2355mvEwtDYdAmqCN6kmQETThJtARfj9iGum'; + +/** The path a StackValidationError reports, for asserting the 422 names the field. */ +const pathOf = (fn: () => unknown): string | undefined => { + try { + fn(); + } catch (err) { + if (err instanceof StackValidationError) return err.errors[0]?.path; + throw err; + } + return undefined; +}; + +describe('body parsers — shared refusals', () => { + const parsers = { + parseAuthChallengeBody, + parseAuthTokenBody, + parseEntityPatchBody, + parseTypeBody, + parseMigrationBody, + }; + + for (const [name, parse] of Object.entries(parsers)) { + test(`${name} refuses a body that is not an object with 400`, () => { + for (const body of [undefined, null, [], 'x', 1]) + expect(() => parse(body)).toThrow(StackBadRequestError); + }); + } +}); + +describe('parseAuthChallengeBody', () => { + test('reads did', () => { + expect(parseAuthChallengeBody({ did: DID })).toEqual({ did: DID }); + }); + + test('an unknown key is 400', () => { + expect(() => parseAuthChallengeBody({ did: DID, origin: 'x' })).toThrow( + new StackBadRequestError('Unknown key in auth challenge body: origin'), + ); + }); + + test('an absent did is 400; a non-string one is 422 naming the field', () => { + expect(() => parseAuthChallengeBody({})).toThrow(StackBadRequestError); + expect(pathOf(() => parseAuthChallengeBody({ did: 1 }))).toBe('did'); + }); + + test('the form of the DID is not judged here', () => { + expect(parseAuthChallengeBody({ did: 'not-a-did' })).toEqual({ did: 'not-a-did' }); + }); +}); + +describe('parseAuthTokenBody', () => { + const body = { did: DID, nonce: 'k7Qm2ZxRt9vLbNc4Hy8Wf3', signature: 'CIvHvqS' }; + + test('reads did, nonce and signature', () => { + expect(parseAuthTokenBody(body)).toEqual(body); + }); + + test('a key naming a subject is refused rather than ignored', () => { + expect(() => parseAuthTokenBody({ ...body, subjectId: 'did:key:other' })).toThrow( + new StackBadRequestError('Unknown key in auth token body: subjectId'), + ); + }); + + test('each field is required, and must be a string', () => { + for (const key of ['did', 'nonce', 'signature'] as const) { + const { [key]: _, ...rest } = body; + expect(() => parseAuthTokenBody(rest)).toThrow(StackBadRequestError); + expect(pathOf(() => parseAuthTokenBody({ ...body, [key]: null }))).toBe(key); + } + }); +}); + +describe('parseEntityPatchBody', () => { + test('reads content, keeping null as a removal', () => { + expect(parseEntityPatchBody({ content: { name: 'Jane', handle: null } })).toEqual({ + content: { name: 'Jane', handle: null }, + }); + }); + + test('an unknown key is 400', () => { + expect(() => parseEntityPatchBody({ content: {}, did: DID })).toThrow(StackBadRequestError); + }); + + test('an absent content is 400; a non-object one is 422 naming the field', () => { + expect(() => parseEntityPatchBody({})).toThrow(StackBadRequestError); + expect(pathOf(() => parseEntityPatchBody({ content: [] }))).toBe('content'); + expect(pathOf(() => parseEntityPatchBody({ content: null }))).toBe('content'); + }); +}); + +describe('parseTypeBody', () => { + const schema: TypeSchema = { title: { kind: 'string', required: true } }; + + test('a whole Type as a client serializes one parses to defineType() options', async () => { + const stack = await Stack.open( + new MemoryAdapter({ ownerEntityId: 'did:key:owner', timezone: 'UTC' }), + ); + const type = await stack.defineType({ id: 'com.example/note@1', name: 'Note', schema }); + const wire = JSON.parse(JSON.stringify(type)) as unknown; + expect(parseTypeBody(wire)).toEqual({ id: 'com.example/note@1', name: 'Note', schema }); + }); + + test('migratesFrom is read when present', () => { + expect( + parseTypeBody({ id: 'com.example/note@2', name: 'Note', schema, migratesFrom: 'x@1' }), + ).toMatchObject({ migratesFrom: 'x@1' }); + expect(pathOf(() => parseTypeBody({ id: 'a@1', name: 'A', schema, migratesFrom: 1 }))).toBe( + 'migratesFrom', + ); + }); + + test('a key no Type carries is 400', () => { + expect(() => parseTypeBody({ id: 'a@1', name: 'A', schema, description: 'x' })).toThrow( + new StackBadRequestError('Unknown key in type body: description'), + ); + }); + + test('id, name and schema are required', () => { + expect(() => parseTypeBody({ name: 'A', schema })).toThrow(StackBadRequestError); + expect(() => parseTypeBody({ id: 'a@1', schema })).toThrow(StackBadRequestError); + expect(() => parseTypeBody({ id: 'a@1', name: 'A' })).toThrow(StackBadRequestError); + expect(pathOf(() => parseTypeBody({ id: 1, name: 'A', schema }))).toBe('id'); + }); + + test('a malformed schema passes through for defineType() to answer with 422', async () => { + const options = parseTypeBody({ id: 'a@1', name: 'A', schema: 'not a schema' }); + const stack = await Stack.open( + new MemoryAdapter({ ownerEntityId: 'did:key:owner', timezone: 'UTC' }), + ); + await expect(stack.defineType(options)).rejects.toThrow(StackValidationError); + }); +}); + +describe('parseMigrationBody', () => { + test('reads toTypeId and content', () => { + expect(parseMigrationBody({ toTypeId: 'a@2', content: { title: 'x' } })).toEqual({ + toTypeId: 'a@2', + content: { title: 'x' }, + }); + }); + + test('an unknown key is 400', () => { + expect(() => parseMigrationBody({ toTypeId: 'a@2', content: {}, fromTypeId: 'a@1' })).toThrow( + StackBadRequestError, + ); + }); + + test('both fields are required; a wrong-typed one is 422 naming the field', () => { + expect(() => parseMigrationBody({ content: {} })).toThrow(StackBadRequestError); + expect(() => parseMigrationBody({ toTypeId: 'a@2' })).toThrow(StackBadRequestError); + expect(pathOf(() => parseMigrationBody({ toTypeId: 2, content: {} }))).toBe('toTypeId'); + expect(pathOf(() => parseMigrationBody({ toTypeId: 'a@2', content: 'x' }))).toBe('content'); + }); +}); diff --git a/packages/core/tests/wire-request.test.ts b/packages/core/tests/wire-request.test.ts index 9c31b652..b8ca6622 100644 --- a/packages/core/tests/wire-request.test.ts +++ b/packages/core/tests/wire-request.test.ts @@ -4,6 +4,9 @@ import { parseQueryBody, parseChangeParams, parseJournalParams, + parseDeleteParams, + parseDownloadParams, + parseAssociationParams, parseIfMatch, parseUploadFilename, parsePositiveInt, @@ -498,3 +501,71 @@ describe('parseDate', () => { expect(parseDate(undefined)).toBeUndefined(); }); }); + +// ------------------------------------------------------- +// DELETE /records/:id, GET /attachments/:fileId, GET /records/:id/associations +// ------------------------------------------------------- + +describe('parseDeleteParams', () => { + const del = (qs: string): URL => new URL(`https://stack.example.com/records/rec-1${qs}`); + + test('purge is false unless it is "true"', () => { + expect(parseDeleteParams(del(''))).toEqual({ purge: false }); + expect(parseDeleteParams(del('?purge=false'))).toEqual({ purge: false }); + expect(parseDeleteParams(del('?purge=true'))).toEqual({ purge: true }); + }); + + test('a non-boolean, misspelled or repeated purge is refused rather than read as a soft delete', () => { + expect(() => parseDeleteParams(del('?purge=1'))).toThrow( + new StackBadRequestError('Invalid purge: expected true or false, got "1"'), + ); + expect(() => parseDeleteParams(del('?purg=true'))).toThrow(StackBadRequestError); + expect(() => parseDeleteParams(del('?purge=true&purge=false'))).toThrow(StackBadRequestError); + }); +}); + +describe('parseDownloadParams', () => { + const dl = (qs: string): URL => new URL(`https://stack.example.com/attachments/abc${qs}`); + + test('reads contentType and filename under the names the resolution takes', () => { + expect(parseDownloadParams(dl(''))).toEqual({}); + expect(parseDownloadParams(dl('?contentType=image%2Fpng&filename=photo.png'))).toEqual({ + contentTypeParam: 'image/png', + filenameParam: 'photo.png', + }); + }); + + test('an unknown or repeated param is refused', () => { + expect(() => parseDownloadParams(dl('?attachmentRecordId=1hk153x00001'))).toThrow( + StackBadRequestError, + ); + expect(() => parseDownloadParams(dl('?filename=a&filename=b'))).toThrow(StackBadRequestError); + }); +}); + +describe('parseAssociationParams', () => { + const assoc = (qs: string): URL => + new URL(`https://stack.example.com/records/rec-1/associations${qs}`); + + test('reads kind and label', () => { + expect(parseAssociationParams(assoc(''))).toEqual({}); + expect(parseAssociationParams(assoc('?kind=tag'))).toEqual({ kind: 'tag' }); + expect(parseAssociationParams(assoc('?label=avatar'))).toEqual({ label: 'avatar' }); + }); + + test('a kind that names no data association is refused rather than read as every kind', () => { + expect(() => parseAssociationParams(assoc('?kind=grant'))).toThrow( + new StackBadRequestError('Invalid kind: "grant"'), + ); + }); + + test('kind names one value here, so a repeat is refused', () => { + expect(() => parseAssociationParams(assoc('?kind=tag&kind=attachment'))).toThrow( + new StackBadRequestError('Repeated query param: kind'), + ); + }); + + test('an unknown param is refused', () => { + expect(() => parseAssociationParams(assoc('?type=tag'))).toThrow(StackBadRequestError); + }); +}); From 467cce3d2b133b4f1e5f9c0506b85d2a85dfaeac Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 23:23:44 +0000 Subject: [PATCH 2/4] feat(core): refuse unknown keys on association, permission and grant-target elements validateAssociation() and validateGrantTarget() checked that required fields were present but not that others were absent, so { kind: 'entity', entityId, scope } on a grantee stored and answered 200. The SQLite adapters then dropped the extra key because they have no column for it, while MemoryAdapter kept it, so the two disagreed about the same write. Every element is now held to its own kind's keys, at every depth, with 400. The key tables are typed against each union arm, so a new field fails to compile until it is listed. _grant content applies the same keys to its grantee as a 422. wire-request.ts reuses the shared target-key table. Refusing on write strands nothing: SQLite never stored such keys, and there is no install base. Refs #354 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014ah52eTkayJ1JxPuEmgVGe --- .../wire-parsers-remaining-endpoints.md | 4 +- docs/spec/data-model.md | 2 + docs/spec/wire-format.md | 2 +- .../adapter-api/tests/conformance.test.ts | 1 + packages/conformance-fixtures/src/index.ts | 19 +++ packages/core/src/grants.ts | 14 +- packages/core/src/query-validation.ts | 71 ++++++++++ packages/core/src/wire-request.ts | 6 +- packages/core/tests/element-keys.test.ts | 129 ++++++++++++++++++ 9 files changed, 240 insertions(+), 8 deletions(-) create mode 100644 packages/core/tests/element-keys.test.ts diff --git a/.changeset/wire-parsers-remaining-endpoints.md b/.changeset/wire-parsers-remaining-endpoints.md index c5f0076f..c8e54456 100644 --- a/.changeset/wire-parsers-remaining-endpoints.md +++ b/.changeset/wire-parsers-remaining-endpoints.md @@ -3,4 +3,6 @@ '@haverstack/conformance-fixtures': minor --- -`@haverstack/core/wire` adds a parser for every remaining endpoint core specifies, so a server no longer keeps its own list of their param and field names: `parseAuthChallengeBody()`, `parseAuthTokenBody()`, `parseEntityPatchBody()`, `parseTypeBody()`, `parseMigrationBody()`, `parseDeleteParams()`, `parseDownloadParams()` and `parseAssociationParams()`. Each refuses an unknown name or a malformed boolean with `StackBadRequestError`, and a known body field of the wrong type with `StackValidationError`. Two new error fixtures pin a non-boolean `purge` and an unknown key on `POST /auth/token`. +`@haverstack/core/wire` adds a parser for every remaining endpoint core specifies, so a server no longer keeps its own list of their param and field names: `parseAuthChallengeBody()`, `parseAuthTokenBody()`, `parseEntityPatchBody()`, `parseTypeBody()`, `parseMigrationBody()`, `parseDeleteParams()`, `parseDownloadParams()` and `parseAssociationParams()`. Each refuses an unknown name or a malformed boolean with `StackBadRequestError`, and a known body field of the wrong type with `StackValidationError`. Two error fixtures pin a non-boolean `purge` and an unknown key on `POST /auth/token`. + +`Stack` refuses an association, permission or grant-target element carrying a key its kind does not define, at every depth, with `StackBadRequestError`. This applies on every write that takes an element, on a `relatedTo` filter target, and on `grantType()`, `revokeType()` and `listTypeGrants()`. A `_grant` record's `grantee` is held to the same keys and refused with `StackValidationError`. A new error fixture pins an unknown key on a permission grantee. diff --git a/docs/spec/data-model.md b/docs/spec/data-model.md index 3937d373..971748a3 100644 --- a/docs/spec/data-model.md +++ b/docs/spec/data-model.md @@ -176,6 +176,8 @@ type RelationshipTarget = **Association identity is `(kind, label)` plus the payload that names a referent** — `fileId` for an attachment, the target for a relationship. An attachment's `attachmentRecordId` is outside it: the pointer [annotates a reference rather than naming one](./attachments.md#naming-the-upload-a-reference-came-from). Identity is what `dissociate()` matches on, what a second `associate()` of the same reference lands on, and the primary key every adapter stores an association under. +**An element carries only the keys its kind defines**, at every depth: the keys in the types above, a relationship target's keys for its own `kind`, and a permission grantee's for its own (see [Access control § Record-level permissions](./access-control.md#record-level-permissions)). Any other key is refused with `StackBadRequestError` (wire: **400**) on every write that takes an element — `create()`, `mutate()`, `associate()`, `dissociate()`, `grantAccess()`, `revokeAccess()` — and on a `relatedTo` filter target. A key from another arm counts as unknown: `role` on an entity grantee is refused, not ignored. An adapter stores only the keys it has a place for, so an extra key would otherwise answer 200 and then disappear. The same holds for a [type-level grant](./access-control.md#type-level-grants) target given to `grantType()`, `revokeType()` or `listTypeGrants()`, and for a `_grant` record's `grantee`, which as content is refused with `StackValidationError` (**422**). + **An association list holds distinct identities.** A list naming one identity twice — `create()`'s `associations`, or a change set's — is refused with `StackValidationError` (wire: **400**), not collapsed to the last entry. Two entries under one identity describe a state no store can hold, since a store keys them; accepting the list would leave which of the two the record ends up with to whichever adapter is underneath, and would leave [a journal entry](./journal.md#the-entry) reporting a prior state twice with no way to say which edit displaced it. Every way of producing such a list is a caller bug, which is why it is refused on the same terms as [an empty change set](#mutations). ### Reparenting diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index 882ac8d4..218b8418 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -185,7 +185,7 @@ The distinction between **400** and **422** matters for write endpoints (`POST / ### Unrecognized input -**A query param or JSON body key an endpoint does not define is refused with 400 (`bad_request`), never ignored.** Ignoring it answers a different request than the one sent, and the difference is always in the direction the caller didn't ask for: a misspelled filter widens a query, a misspelled `purge` soft-deletes, a misspelled field on a token request mints a token for someone else. A 400 tells the caller at once; a 200 for the wrong request never does. The rule holds at every depth of a body: a filter's `createdBy`, a sort, a relationship target. The same goes for values: a boolean param takes only `true` or `false`, and a param that names one value appears at most once, since reading a repeat as its first value ignores the rest. +**A query param or JSON body key an endpoint does not define is refused with 400 (`bad_request`), never ignored.** Ignoring it answers a different request than the one sent, and the difference is always in the direction the caller didn't ask for: a misspelled filter widens a query, a misspelled `purge` soft-deletes, a misspelled field on a token request mints a token for someone else. A 400 tells the caller at once; a 200 for the wrong request never does. The rule holds at every depth of a body: a filter's `createdBy`, a sort, a relationship target, an association or permission element (see [Data model § Associations](./data-model.md#associations)). The same goes for values: a boolean param takes only `true` or `false`, and a param that names one value appears at most once, since reading a repeat as its first value ignores the rest. Three things sit outside it: diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index 9d9ea515..dbda4fa7 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -751,6 +751,7 @@ const SERVER_ONLY_ERROR_FIXTURES = new Set([ 'error-bad-request-non-boolean-param', 'error-bad-request-unknown-query-body-key', 'error-bad-request-unknown-record-key', + 'error-bad-request-unknown-grantee-key', 'error-bad-request-non-boolean-purge', 'error-bad-request-unknown-auth-token-key', ]); diff --git a/packages/conformance-fixtures/src/index.ts b/packages/conformance-fixtures/src/index.ts index c039cf5d..d4dbb579 100644 --- a/packages/conformance-fixtures/src/index.ts +++ b/packages/conformance-fixtures/src/index.ts @@ -2196,6 +2196,25 @@ export const errorResponseFixtures: ConformanceFixture[] = [ responseStatus: 400, responseBody: { error: { code: 'bad_request', message: 'Unknown record key: title' } }, }, + { + name: 'error-bad-request-unknown-grantee-key', + description: + 'A permission element carries only the keys its grantee kind defines; any other — here a ' + + 'scope on an entity grantee — returns 400 with code "bad_request" and grants nothing. ' + + 'Ignored, it would store a narrower-looking grant than the one that applies. See ' + + 'docs/spec/data-model.md § Associations.', + method: 'POST', + path: '/records/1hk153x00001/permissions', + requestBody: { + kind: 'permission', + label: 'read', + grantee: { kind: 'entity', entityId: 'did:key:z6MkMember', scope: 'comments' }, + }, + responseStatus: 400, + responseBody: { + error: { code: 'bad_request', message: 'Unknown key in permission.grantee: scope' }, + }, + }, { name: 'error-bad-request-non-boolean-purge', description: diff --git a/packages/core/src/grants.ts b/packages/core/src/grants.ts index 4e31aeea..c9651ebf 100644 --- a/packages/core/src/grants.ts +++ b/packages/core/src/grants.ts @@ -11,6 +11,7 @@ import { baseIdOf } from './schema.js'; import { StackBadRequestError } from './errors.js'; +import { assertKnownKeys, GRANTEE_KEYS, unknownKeys } from './query-validation.js'; import { SYSTEM_TYPES, GRANT_ACTIONS } from './types.js'; import { carriesRoster, groupRoleFromAssociations } from './access.js'; import type { @@ -117,11 +118,14 @@ export function matchesGrantTarget(content: GrantContent, target: GrantQuery): b * empty groupId or entityId reaches no one, so storing it would leave a * grant that can only ever deny while looking like a share that worked. * Read as data, not as the type: a target reaching Stack from a request - * body or an import has whatever shape it arrived with. `allowAny` admits + * body or an import has whatever shape it arrived with, so a key its tier + * does not define is refused too. `allowAny` admits * the listing-only `role: 'any'`, which grantType() and revokeType() refuse. */ export function validateGrantTarget(target: GrantQuery, allowAny = false): void { const t = target as Partial> | null; + if (t?.kind === 'authenticated' || t?.kind === 'entity' || t?.kind === 'group') + assertKnownKeys(t, GRANTEE_KEYS[t.kind], 'grant target'); switch (t?.kind) { case 'authenticated': return; @@ -163,6 +167,14 @@ export function validateGrantee(typeId: TypeId, content: unknown): ValidationErr const g = c?.grantee as Partial> | null; // An absent or non-object grantee is the schema's to refuse, and it does. if (!g || typeof g !== 'object') return []; + if (g.kind === 'authenticated' || g.kind === 'entity' || g.kind === 'group') { + const unknown = unknownKeys(g, GRANTEE_KEYS[g.kind]); + if (unknown.length > 0) + return unknown.map((key) => ({ + path: `grantee.${key}`, + message: `A ${String(g.kind)} grantee does not carry ${key}`, + })); + } switch (g.kind) { case 'authenticated': return []; diff --git a/packages/core/src/query-validation.ts b/packages/core/src/query-validation.ts index 0afd0b75..5956f9bc 100644 --- a/packages/core/src/query-validation.ts +++ b/packages/core/src/query-validation.ts @@ -23,11 +23,20 @@ import { CONTENT_SEGMENT_METACHARACTERS, SEGMENT_METACHARACTER_RE } from './vali import type { ValidationError } from './validate.js'; import { NATIVE_SORT_FIELDS } from './types.js'; import type { + AnyoneAssociation, Association, + AttachmentAssociation, AuthorityAssociation, DataAssociation, + EntityTarget, + ExternalTarget, + GrantGrantee, JournalQuery, Grantee, + PermissionAssociation, + RecordTarget, + RelationshipAssociation, + TagAssociation, QuerySort, RecordFilter, RelationshipTarget, @@ -214,6 +223,60 @@ export function assertSortCapability( /** The identifier spaces a relationship target may name. */ const TARGET_KINDS = new Set(['record', 'entity', 'external']); +/** + * The keys of one arm of a union, listed as an object so the compiler + * holds the list to the type: a field added to the arm fails to compile + * here until it is listed. + */ +const keysOf = (keys: Record): readonly string[] => Object.keys(keys); + +/** Every key each relationship target arm defines. */ +export const TARGET_KEYS: Record = { + record: keysOf({ kind: true, recordId: true, stackUrl: true }), + entity: keysOf({ kind: true, entityId: true }), + external: keysOf({ kind: true, ns: true, id: true }), +}; + +/** Every key each association kind defines. */ +const ASSOCIATION_KEYS: Record = { + tag: keysOf({ kind: true, label: true }), + attachment: keysOf({ + kind: true, + label: true, + fileId: true, + attachmentRecordId: true, + }), + relationship: keysOf({ kind: true, label: true, target: true }), + permission: keysOf({ kind: true, label: true, grantee: true }), + anyone: keysOf({ kind: true, label: true }), +}; + +/** Every key each grantee arm defines, the default tier included. */ +export const GRANTEE_KEYS: Record = { + entity: keysOf>({ kind: true, entityId: true }), + group: keysOf>({ kind: true, groupId: true, role: true }), + authenticated: keysOf>({ kind: true }), +}; + +/** + * The keys of `value` its arm does not define. An element carrying one is + * refused rather than stored: an adapter keeps only the keys it has + * columns for, so the rest would answer 200 and then vanish. + * See docs/spec/data-model.md § Associations. + */ +export function unknownKeys(value: object, keys: readonly string[]): string[] { + return Object.keys(value).filter((key) => !keys.includes(key)); +} + +/** unknownKeys() as a refusal. 400, not 422: the key addresses nothing. */ +export function assertKnownKeys(value: object, keys: readonly string[], path: string): void { + const unknown = unknownKeys(value, keys); + if (unknown.length > 0) + throw new StackBadRequestError( + `Unknown key${unknown.length > 1 ? 's' : ''} in ${path}: ${unknown.join(', ')}`, + ); +} + /** * Collect what makes a relationship target malformed. Absence is * meaningful on `stackUrl` and an external `id` — this stack, and the @@ -234,6 +297,7 @@ function targetErrors( `Unknown relationship target kind "${target.kind}": expected "record", "entity" or "external".`, ); } + assertKnownKeys(target, TARGET_KEYS[target.kind], path); if (target.kind === 'record') { if (!target.recordId) return fail('A record target requires a non-empty recordId.'); if (target.stackUrl !== undefined && !target.stackUrl) { @@ -258,6 +322,10 @@ function targetErrors( * unrecognized kind would otherwise be stored under the one arm that * names a Record in this stack. See docs/spec/data-model.md * § Relationship targets. + * + * A key the element's kind does not define is thrown as a 400 rather than + * collected, at every depth: it addresses nothing, the way an unknown key + * on a request body does. See docs/spec/data-model.md § Associations. */ export function validateAssociation( association: Association, @@ -271,6 +339,7 @@ export function validateAssociation( }, ]; } + assertKnownKeys(association, ASSOCIATION_KEYS[association.kind], path); if (association.kind === 'permission') { return granteeErrors(association, `${path}.grantee`); } @@ -335,6 +404,8 @@ function granteeErrors( } const grantee = association.grantee; if (!grantee || typeof grantee !== 'object') return fail('A permission requires a grantee.'); + if (grantee.kind === 'entity' || grantee.kind === 'group') + assertKnownKeys(grantee, GRANTEE_KEYS[grantee.kind], path); if (grantee.kind === 'entity') { return grantee.entityId ? [] : fail('An entity grantee requires a non-empty entityId.'); } diff --git a/packages/core/src/wire-request.ts b/packages/core/src/wire-request.ts index 0e8e5db7..0fab966b 100644 --- a/packages/core/src/wire-request.ts +++ b/packages/core/src/wire-request.ts @@ -26,6 +26,7 @@ */ import { StackBadRequestError } from './errors.js'; +import { TARGET_KEYS } from './query-validation.js'; import { NATIVE_SORT_FIELDS } from './types.js'; import type { DataAssociation, @@ -58,11 +59,6 @@ const SORT_DIRECTIONS: ReadonlySet> = new Se const CHANGE_KINDS: ReadonlySet = new Set(['created', 'changed', 'deleted', 'purged']); const ASSOCIATION_KINDS: ReadonlySet = new Set(['tag', 'attachment', 'relationship']); const TARGET_KINDS: ReadonlySet = new Set(['record', 'entity', 'external']); -const TARGET_KEYS: Record = { - record: ['kind', 'recordId', 'stackUrl'], - entity: ['kind', 'entityId'], - external: ['kind', 'ns', 'id'], -}; /** Every param `GET /records` defines. See docs/spec/wire-format.md § Records. */ const RECORD_QUERY_PARAMS = [ diff --git a/packages/core/tests/element-keys.test.ts b/packages/core/tests/element-keys.test.ts new file mode 100644 index 00000000..7eb795d5 --- /dev/null +++ b/packages/core/tests/element-keys.test.ts @@ -0,0 +1,129 @@ +import { describe, test, expect, beforeEach } from 'vitest'; +import { Stack } from '../src/stack.js'; +import { StackBadRequestError, StackValidationError } from '../src/errors.js'; +import { MemoryAdapter } from '../src/testing.js'; +import type { Association, DataAssociation, AuthorityAssociation } from '../src/types.js'; + +const NOTE = 'com.example.test/note@1'; +const OWNER = 'did:key:owner'; + +let stack: Stack; +let recordId: string; + +beforeEach(async () => { + stack = await Stack.open(new MemoryAdapter({ ownerEntityId: OWNER, timezone: 'UTC' })); + await stack.defineType({ id: NOTE, name: 'Note', schema: { text: { kind: 'text' } } }); + recordId = (await stack.create(NOTE, { text: 'x' })).id; +}); + +/** An element carrying a key its kind does not define. */ +const extra = (a: T, key: string, value: unknown = 'x'): T => + ({ ...a, [key]: value }) as T; + +describe('an association element carries only the keys its kind defines', () => { + const data: DataAssociation[] = [ + { kind: 'tag', label: 'starred' }, + { kind: 'attachment', label: 'avatar', fileId: 'abc' }, + { kind: 'relationship', label: 'reply-to', target: { kind: 'record', recordId: 'r1' } }, + ]; + + for (const a of data) { + test(`a ${a.kind} with an unknown key is refused on create, mutate and associate`, async () => { + const bad = extra(a, 'scope'); + const message = 'Unknown key in associations[0]: scope'; + await expect(stack.create(NOTE, { text: 'y' }, { associations: [bad] })).rejects.toThrow( + new StackBadRequestError(message), + ); + await expect(stack.mutate(recordId, { associations: [bad] })).rejects.toThrow( + new StackBadRequestError(message), + ); + await expect(stack.associate(recordId, bad)).rejects.toThrow( + new StackBadRequestError('Unknown key in association: scope'), + ); + }); + } + + test('an attachment keeps attachmentRecordId, the one optional key it defines', async () => { + const upload = await stack.putAttachment(new Uint8Array([1]), { mimeType: 'text/plain' }); + const { fileId } = upload.content as { fileId: string }; + const a = { kind: 'attachment', label: 'avatar', fileId, attachmentRecordId: upload.id }; + await expect(stack.associate(recordId, a as DataAssociation)).resolves.toBeDefined(); + }); + + test('a key its target arm does not define is refused, even one another arm defines', async () => { + const a = { + kind: 'relationship', + label: 'author', + target: { kind: 'entity', entityId: 'did:key:a', ns: 'atproto' }, + } as DataAssociation; + await expect(stack.associate(recordId, a)).rejects.toThrow( + new StackBadRequestError('Unknown key in association.target: ns'), + ); + }); + + test('a relationship filter target is held to the same keys', async () => { + await expect( + stack.query({ + filter: { + relatedTo: { target: { kind: 'entity', entityId: 'did:key:a', recordId: 'r1' } as never }, + }, + }), + ).rejects.toThrow(StackBadRequestError); + }); +}); + +describe('a permission element carries only the keys its kind and grantee define', () => { + const read = (grantee: object): AuthorityAssociation => + ({ kind: 'permission', label: 'read', grantee }) as AuthorityAssociation; + + test('an unknown key on the grantee is refused', async () => { + const p = read({ kind: 'entity', entityId: 'did:key:a', scope: 'all' }); + await expect(stack.grantAccess(recordId, p)).rejects.toThrow( + new StackBadRequestError('Unknown key in permission.grantee: scope'), + ); + await expect(stack.mutate(recordId, { permissions: [p] })).rejects.toThrow( + new StackBadRequestError('Unknown key in permissions[0].grantee: scope'), + ); + }); + + test('a key from the other grantee arm is refused rather than ignored', async () => { + const p = read({ kind: 'entity', entityId: 'did:key:a', role: 'admin' }); + await expect(stack.grantAccess(recordId, p)).rejects.toThrow(StackBadRequestError); + }); + + test('an unknown key on the element itself is refused', async () => { + const anyone = extra({ kind: 'anyone', label: 'read' } as AuthorityAssociation, 'until'); + await expect(stack.grantAccess(recordId, anyone)).rejects.toThrow( + new StackBadRequestError('Unknown key in permission: until'), + ); + }); +}); + +describe('a grant target carries only the keys its tier defines', () => { + test('grantType() and revokeType() refuse an unknown key', async () => { + const grantee = { kind: 'entity', entityId: 'did:key:a', scope: 'all' } as never; + const message = 'Unknown key in grant target: scope'; + await expect(stack.grantType(NOTE, { actions: ['read-any'], grantee })).rejects.toThrow( + new StackBadRequestError(message), + ); + await expect(stack.revokeType(NOTE, { actions: ['read-any'], grantee })).rejects.toThrow( + new StackBadRequestError(message), + ); + }); + + test('listTypeGrants() refuses one too, so a misspelled query narrows nothing', async () => { + await expect( + stack.listTypeGrants({ kind: 'authenticated', entityId: 'did:key:a' } as never), + ).rejects.toThrow(StackBadRequestError); + }); + + test('a _grant written directly is held to the same keys, as content', async () => { + await expect( + stack.create('_grant@1', { + typeId: NOTE, + actions: ['read-any'], + grantee: { kind: 'authenticated', entityId: 'did:key:a' }, + }), + ).rejects.toThrow(StackValidationError); + }); +}); From f42fff1653809b545e5e5158116ce3d9b73ac2b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 02:16:01 +0000 Subject: [PATCH 3/4] docs(spec): define the PATCH /entity body shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseEntityPatchBody() reads { content }, but § Entity named no body, so a client copying PATCH /records/:id's contentPatch got a 400 the spec did not explain. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01STGzVnzxKKwGh3bs6RU5Fe --- docs/spec/wire-format.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index 218b8418..a358ccd1 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -695,6 +695,8 @@ PATCH /entity — update it A convenience alias for the owner entity rather than requiring clients to look it up by ID. +**The `PATCH` body is `{ "content": { … } }`** and nothing else: `content` is a content patch, merged at the top level exactly as a `PATCH /records/:id` `contentPatch` is — omitted keeps, `null` removes. It is not the records change set, so `contentPatch` and the envelope's native keys are refused with **400** like any other unrecognized key (see [Unrecognized input](#unrecognized-input)); a missing or non-object `content` is **400** and **422** respectively. + ## Change feed `GET /changes` streams record changes over SSE. Its frames, resumption rules and the obligations that fall on a server are [Change feed](./change-feed.md); the model it encodes is [Change events](./events.md). From 73e98ded742b834d304d40631fe1c13be333acc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 11:08:21 +0000 Subject: [PATCH 4/4] feat(core): name the PATCH /entity body key contentPatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit content on the wire means whole content (POST /records, migrate), so a merging key spelled content was the trap data-model.md § Mutations names. The parser is new in this PR, so its existing changeset covers it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01STGzVnzxKKwGh3bs6RU5Fe --- docs/spec/wire-format.md | 2 +- packages/core/src/wire-body.ts | 6 +++--- packages/core/tests/wire-body.test.ts | 19 +++++++++++-------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index a358ccd1..9218360a 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -695,7 +695,7 @@ PATCH /entity — update it A convenience alias for the owner entity rather than requiring clients to look it up by ID. -**The `PATCH` body is `{ "content": { … } }`** and nothing else: `content` is a content patch, merged at the top level exactly as a `PATCH /records/:id` `contentPatch` is — omitted keeps, `null` removes. It is not the records change set, so `contentPatch` and the envelope's native keys are refused with **400** like any other unrecognized key (see [Unrecognized input](#unrecognized-input)); a missing or non-object `content` is **400** and **422** respectively. +**The `PATCH` body is `{ "contentPatch": { … } }`** and nothing else, merged at the top level exactly as the same key on `PATCH /records/:id` is — omitted keeps, `null` removes. It carries that key's name because `content` on the wire means whole content, as on `POST /records` and `POST /records/:id/migrate`; see [Data model § Mutations](./data-model.md#mutations). The records envelope's other keys, and `content`, are refused with **400** like any other unrecognized key (see [Unrecognized input](#unrecognized-input)); a missing or non-object `contentPatch` is **400** and **422** respectively. ## Change feed diff --git a/packages/core/src/wire-body.ts b/packages/core/src/wire-body.ts index a55c908b..e38a775c 100644 --- a/packages/core/src/wire-body.ts +++ b/packages/core/src/wire-body.ts @@ -108,13 +108,13 @@ export function parseAuthTokenBody(body: unknown): WireAuthTokenRequest { // ------------------------------------------------------- /** A content patch for the owner entity, in the shape `patchContent()` takes. */ -export type WireEntityPatch = { content: Record }; +export type WireEntityPatch = { contentPatch: Record }; /** Parse a `PATCH /entity` body. See docs/spec/wire-format.md § Entity. */ export function parseEntityPatchBody(body: unknown): WireEntityPatch { const label = 'entity body'; - const b = requireKnownBody(body, ['content'], label); - return { content: requiredObject(b, 'content', label) }; + const b = requireKnownBody(body, ['contentPatch'], label); + return { contentPatch: requiredObject(b, 'contentPatch', label) }; } // ------------------------------------------------------- diff --git a/packages/core/tests/wire-body.test.ts b/packages/core/tests/wire-body.test.ts index 8ca3106c..c5a364c5 100644 --- a/packages/core/tests/wire-body.test.ts +++ b/packages/core/tests/wire-body.test.ts @@ -85,20 +85,23 @@ describe('parseAuthTokenBody', () => { }); describe('parseEntityPatchBody', () => { - test('reads content, keeping null as a removal', () => { - expect(parseEntityPatchBody({ content: { name: 'Jane', handle: null } })).toEqual({ - content: { name: 'Jane', handle: null }, + test('reads contentPatch, keeping null as a removal', () => { + expect(parseEntityPatchBody({ contentPatch: { name: 'Jane', handle: null } })).toEqual({ + contentPatch: { name: 'Jane', handle: null }, }); }); - test('an unknown key is 400', () => { - expect(() => parseEntityPatchBody({ content: {}, did: DID })).toThrow(StackBadRequestError); + test('an unknown key is 400, content included', () => { + expect(() => parseEntityPatchBody({ contentPatch: {}, did: DID })).toThrow( + StackBadRequestError, + ); + expect(() => parseEntityPatchBody({ content: {} })).toThrow(StackBadRequestError); }); - test('an absent content is 400; a non-object one is 422 naming the field', () => { + test('an absent contentPatch is 400; a non-object one is 422 naming the field', () => { expect(() => parseEntityPatchBody({})).toThrow(StackBadRequestError); - expect(pathOf(() => parseEntityPatchBody({ content: [] }))).toBe('content'); - expect(pathOf(() => parseEntityPatchBody({ content: null }))).toBe('content'); + expect(pathOf(() => parseEntityPatchBody({ contentPatch: [] }))).toBe('contentPatch'); + expect(pathOf(() => parseEntityPatchBody({ contentPatch: null }))).toBe('contentPatch'); }); });