From a43116869768b7f6e7f7a7bb9712d6821d831498 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 19:25:55 +0000 Subject: [PATCH 1/2] feat: refuse unrecognized wire input instead of ignoring it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ./wire request parsers now refuse a query param or body key their endpoint does not define, at any depth, and take only true/false for boolean params. An ignored name answers a different request than the one sent: a misspelled filter widens a query, a stale field mints the wrong token. Specified in wire-format.md § Unrecognized input, with four new error fixtures. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018rAh3A4jeexocb6UcZRAwa --- .changeset/refuse-unrecognized-input.md | 6 + docs/spec/wire-format.md | 12 ++ .../adapter-api/tests/conformance.test.ts | 13 +- packages/conformance-fixtures/src/index.ts | 60 +++++++ packages/core/src/wire-record.ts | 27 +++ packages/core/src/wire-request.ts | 162 +++++++++++++++--- packages/core/tests/wire-record.test.ts | 7 + packages/core/tests/wire-request.test.ts | 55 +++++- 8 files changed, 309 insertions(+), 33 deletions(-) create mode 100644 .changeset/refuse-unrecognized-input.md diff --git a/.changeset/refuse-unrecognized-input.md b/.changeset/refuse-unrecognized-input.md new file mode 100644 index 00000000..a01e5769 --- /dev/null +++ b/.changeset/refuse-unrecognized-input.md @@ -0,0 +1,6 @@ +--- +'@haverstack/core': minor +'@haverstack/conformance-fixtures': minor +--- + +The `./wire` request parsers refuse input their endpoint does not define instead of ignoring it. `parseQueryParams()`, `parseChangeParams()` and `parseJournalParams()` throw `StackBadRequestError` for an unknown query param or a boolean param other than `true`/`false`. `parseQueryBody()` does the same for an unknown key at any depth, a non-object body, a non-boolean `includeDeleted`/`includeUnlisted` or a non-string `cursor`. `createOptionsFromWireRecord()` refuses a key no wire record carries. The rule is specified in docs/spec/wire-format.md § Unrecognized input, and four new error fixtures cover it. diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index d3b40c94..8e005a85 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -183,6 +183,18 @@ Empty is not an encoding of anything — nor is a literal `null`. Absence is `40 The distinction between **400** and **422** matters for write endpoints (`POST /records`, `PATCH /records/:id`, `POST /records/:id/migrate`, `POST /types`): a 400 covers a request that couldn't be parsed at all, or that is well-formed but addresses something that doesn't exist (an undefined `typeId`, a malformed TypeId, search text the engine cannot parse); a 422 means the server understood the request and what it addressed, but the content didn't satisfy the type schema. The distinguishing question is whether there was a schema to fail against: `StackValidationError.details` names the fields that broke, and an unknown type has no such detail to report — `Stack.create()` resolves the type and throws before the validation block runs for exactly this reason. +### 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 — and a boolean param takes only `true` or `false`, since any other value is a mistake of the same kind. + +Three things sit outside it: + +- **Keys an endpoint defines as ignored are not unrecognized.** A create body is a whole record, so it may carry every key a wire record has, and the server-assigned ones are dropped as [Records](#records) requires rather than refused. +- **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 owe the same refusal. + ### The taxonomy root Every class in the table above extends the abstract `StackError`, so `err instanceof StackError` answers the one question a server's error middleware asks first: is this a Stack-domain failure with a wire representation, or an ordinary bug that should surface as a bare 500? Membership is exactly that guarantee — a `StackError` always has a `code`, and every code has a status. Errors with no wire mapping (`IdGenerationError`, `InvalidDidError`, `UseAfterCloseError`, `InvalidAdapterError`, `RelayScopeError`) stay outside the hierarchy for that reason. diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index 85020918..3c26cc11 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -741,10 +741,17 @@ const ERROR_CLASS_FOR_CODE = { /** * Fixtures no conformant client can originate: `QuerySort` is a union, so * a request naming both sort parameters is unrepresentable before it ever - * reaches the wire. The rule is a server's to enforce, and the fixture - * exists for server implementations to test against. + * reaches the wire, and APIAdapter never sends a name the wire does not + * define. The rules are a server's to enforce, and the fixtures exist for + * server implementations to test against. */ -const SERVER_ONLY_ERROR_FIXTURES = new Set(['error-bad-request-both-sort-parameters']); +const SERVER_ONLY_ERROR_FIXTURES = new Set([ + 'error-bad-request-both-sort-parameters', + 'error-bad-request-unknown-query-param', + 'error-bad-request-non-boolean-param', + 'error-bad-request-unknown-query-body-key', + 'error-bad-request-unknown-record-key', +]); describe('error response fixtures', () => { for (const fixture of errorResponseFixtures.filter( diff --git a/packages/conformance-fixtures/src/index.ts b/packages/conformance-fixtures/src/index.ts index a469c4e9..f2fb39bd 100644 --- a/packages/conformance-fixtures/src/index.ts +++ b/packages/conformance-fixtures/src/index.ts @@ -2136,6 +2136,66 @@ export const errorResponseFixtures: ConformanceFixture[] = [ }, }, }, + { + name: 'error-bad-request-unknown-query-param', + description: + 'A query param the endpoint does not define returns 400 with code "bad_request" rather ' + + 'than being ignored: an ignored filter param answers a wider query than the one sent, ' + + 'with a 200 that never says so. The same holds on every endpoint. See ' + + 'docs/spec/wire-format.md § Unrecognized input.', + method: 'GET', + path: '/records?authorId=did%3Akey%3Az6MkMember', + responseStatus: 400, + responseBody: { + error: { code: 'bad_request', message: 'Unknown query param: authorId' }, + }, + }, + { + name: 'error-bad-request-non-boolean-param', + description: + 'A boolean query param takes only "true" or "false"; any other value returns 400 with ' + + 'code "bad_request" rather than reading as false. See docs/spec/wire-format.md ' + + '§ Unrecognized input.', + method: 'GET', + path: '/records?includeDeleted=1', + responseStatus: 400, + responseBody: { + error: { + code: 'bad_request', + message: 'Invalid includeDeleted: expected true or false, got "1"', + }, + }, + }, + { + name: 'error-bad-request-unknown-query-body-key', + description: + 'POST /records/query refuses a key it does not define at any depth of the body — here ' + + 'inside filter.createdBy — with 400 / code "bad_request". See docs/spec/wire-format.md ' + + '§ Unrecognized input.', + method: 'POST', + path: '/records/query', + requestBody: { filter: { createdBy: { authorId: 'did:key:z6MkMember' } } }, + responseStatus: 400, + responseBody: { + error: { code: 'bad_request', message: 'Unknown key in filter.createdBy: authorId' }, + }, + }, + { + name: 'error-bad-request-unknown-record-key', + description: + 'POST /records accepts every key a wire record carries — dropping the server-assigned ' + + 'ones — and refuses any other with 400 / code "bad_request". See ' + + 'docs/spec/wire-format.md § Unrecognized input.', + method: 'POST', + path: '/records', + requestBody: { + typeId: 'com.example/note@1', + content: { title: 'Hello' }, + title: 'Hello', + }, + responseStatus: 400, + responseBody: { error: { code: 'bad_request', message: 'Unknown record key: title' } }, + }, { name: 'error-validation-failed', description: diff --git a/packages/core/src/wire-record.ts b/packages/core/src/wire-record.ts index 8145fce4..bab08d12 100644 --- a/packages/core/src/wire-record.ts +++ b/packages/core/src/wire-record.ts @@ -35,6 +35,28 @@ export type WireCreateRequest = { options: Omit; }; +/** + * Every top-level key a wire record carries. A create body is a whole + * record, so each of these is accepted — stamped ones are then dropped — + * and anything else is refused. See docs/spec/wire-format.md § Unrecognized input. + */ +const WIRE_RECORD_KEYS: readonly string[] = [ + 'id', + 'typeId', + 'createdAt', + 'updatedAt', + 'content', + 'version', + 'parentId', + 'appId', + 'createdBy', + 'updatedBy', + 'deletedAt', + 'unlistedAt', + 'permissions', + 'associations', +]; + function requireBody(body: unknown): Record { if (typeof body !== 'object' || body === null || Array.isArray(body)) throw new StackBadRequestError('Invalid record body: expected an object'); @@ -94,6 +116,11 @@ export function createOptionsFromWireRecord( ownerEntityId: EntityId, ): WireCreateRequest { const record = requireBody(body); + const unknown = Object.keys(record).filter((key) => !WIRE_RECORD_KEYS.includes(key)); + if (unknown.length > 0) + throw new StackBadRequestError( + `Unknown record key${unknown.length > 1 ? 's' : ''}: ${unknown.join(', ')}`, + ); const typeId = record.typeId; if (typeof typeId !== 'string' || typeId === '') diff --git a/packages/core/src/wire-request.ts b/packages/core/src/wire-request.ts index bee7a1c6..d4c95636 100644 --- a/packages/core/src/wire-request.ts +++ b/packages/core/src/wire-request.ts @@ -56,6 +56,76 @@ 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 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 = [ + 'typeId', + 'parentId', + 'appId', + 'createdBySubject', + 'createdByPrincipal', + 'tag', + 'attachmentLabel', + 'attachmentFileId', + 'referencesFileId', + 'relatedTo', + 'relatedToStack', + 'relatedToEntity', + 'relatedToNs', + 'relatedToId', + 'relatedToLabel', + 'search', + 'createdBefore', + 'createdAfter', + 'updatedBefore', + 'updatedAfter', + 'includeDeleted', + 'includeUnlisted', + 'sort', + 'sortContent', + 'direction', + 'limit', + 'cursor', +] as const; + +const QUERY_BODY_FILTER_KEYS = [ + 'typeId', + 'parentId', + 'appId', + 'createdBy', + 'tags', + 'attachment', + 'referencesFileId', + 'relatedTo', + 'content', + 'contentPresent', + 'search', + 'includeDeleted', + 'includeUnlisted', + 'createdAt', + 'updatedAt', +] as const; + +/** + * Every param `GET /changes` defines. `since` is the resume cursor, which + * the server reads itself — see parseChangeParams(). + */ +const CHANGE_PARAMS = [ + 'typeId', + 'baseId', + 'parentId', + 'createdBySubject', + 'createdByPrincipal', + 'kind', + 'include', + 'includeUnlisted', + 'since', +] as const; /** * Strict positive-integer parse for a URL param — rejects "1abc", "2.7", @@ -138,6 +208,49 @@ function requirePlainObject(raw: unknown, label: string): Record; } +/** + * A plain object carrying only `keys`. An unrecognized key is refused + * rather than ignored, since ignoring it answers a different request than + * the one sent. See docs/spec/wire-format.md § Unrecognized input. + */ +function requireKnownKeys( + raw: unknown, + keys: readonly string[], + label: string, +): Record { + const obj = requirePlainObject(raw, 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; +} + +/** The URL-param form of requireKnownKeys(). */ +function requireKnownParams(url: URL, names: readonly string[]): void { + const unknown = [...new Set(url.searchParams.keys())].filter((name) => !names.includes(name)); + if (unknown.length > 0) + throw new StackBadRequestError( + `Unknown query param${unknown.length > 1 ? 's' : ''}: ${unknown.join(', ')}`, + ); +} + +/** A boolean URL param: absent is false, and only `true`/`false` are values. */ +function booleanParam(url: URL, name: string): boolean { + const value = url.searchParams.get(name); + if (value === null || value === 'false') return false; + if (value === 'true') return true; + throw new StackBadRequestError(`Invalid ${name}: expected true or false, got "${value}"`); +} + +function optionalBoolean(raw: unknown, label: string): boolean | undefined { + if (raw === undefined) return undefined; + if (typeof raw !== 'boolean') + throw new StackBadRequestError(`Invalid ${label}: expected a boolean`); + return raw; +} + // ------------------------------------------------------- // Filter fields that never travel // ------------------------------------------------------- @@ -190,11 +303,11 @@ export function assertQueryTravels(query: StackQuery): void { * a kind it does not recognize. */ function parseRelatedToTarget(raw: unknown): RelationshipTargetPattern { - const t = requirePlainObject(raw, 'filter.relatedTo.target'); - if (typeof t.kind !== 'string' || !TARGET_KINDS.has(t.kind)) - throw new StackBadRequestError( - `Invalid filter.relatedTo.target.kind: ${JSON.stringify(t.kind)}`, - ); + const label = 'filter.relatedTo.target'; + const kind = requirePlainObject(raw, label).kind; + if (typeof kind !== 'string' || !TARGET_KINDS.has(kind)) + throw new StackBadRequestError(`Invalid filter.relatedTo.target.kind: ${JSON.stringify(kind)}`); + const t = requireKnownKeys(raw, TARGET_KEYS[kind as RelationshipTargetPattern['kind']], label); if (t.kind === 'record') { return { kind: 'record', @@ -219,7 +332,7 @@ function parseRelatedToTarget(raw: unknown): RelationshipTargetPattern { /** A label, a target, or both — the body form of the same filter. */ function parseRelatedToBody(raw: unknown): RelatedToFilter { - const r = requirePlainObject(raw, 'filter.relatedTo'); + const r = requireKnownKeys(raw, ['label', 'target'], 'filter.relatedTo'); const label = r.label !== undefined ? requireString(r.label, 'filter.relatedTo.label') : undefined; const target = r.target !== undefined ? parseRelatedToTarget(r.target) : undefined; @@ -295,7 +408,7 @@ function parseAttachmentParams(url: URL): AttachmentFilter | undefined { } function parseAttachmentBody(raw: unknown): AttachmentFilter { - const a = requirePlainObject(raw, 'filter.attachment'); + const a = requireKnownKeys(raw, ['label', 'fileId'], 'filter.attachment'); return { ...(a.label !== undefined && { label: requireString(a.label, 'filter.attachment.label') }), ...(a.fileId !== undefined && { fileId: requireString(a.fileId, 'filter.attachment.fileId') }), @@ -328,6 +441,7 @@ function parseCreatedByParams(url: URL): RecordFilter['createdBy'] { */ export function parseQueryParams(url: URL): StackQuery { assertNoUntravelableFields(url.searchParams.has('baseId'), url.searchParams.has('presentAt')); + requireKnownParams(url, RECORD_QUERY_PARAMS); const filter: RecordFilter = {}; @@ -376,8 +490,8 @@ export function parseQueryParams(url: URL): StackQuery { }; } - if (url.searchParams.get('includeDeleted') === 'true') filter.includeDeleted = true; - if (url.searchParams.get('includeUnlisted') === 'true') filter.includeUnlisted = true; + if (booleanParam(url, 'includeDeleted')) filter.includeDeleted = true; + if (booleanParam(url, 'includeUnlisted')) filter.includeUnlisted = true; const query: StackQuery = {}; if (Object.keys(filter).length) query.filter = filter; @@ -416,10 +530,12 @@ function parseLimitValue(raw: unknown): number { * `Date`. See docs/spec/wire-format.md § Records. */ export function parseQueryBody(raw: unknown): StackQuery { - if (!raw || typeof raw !== 'object') return {}; - const body = raw as Record; + if (raw === undefined) return {}; + const body = requirePlainObject(raw, 'query body'); const f = body.filter !== undefined ? requirePlainObject(body.filter, 'filter') : undefined; assertNoUntravelableFields(f !== undefined && 'baseId' in f, 'presentAt' in body); + requireKnownKeys(body, ['filter', 'sort', 'limit', 'cursor'], 'query body'); + if (f) requireKnownKeys(f, QUERY_BODY_FILTER_KEYS, 'filter'); const query: StackQuery = {}; @@ -431,7 +547,7 @@ export function parseQueryBody(raw: unknown): StackQuery { filter.parentId = f.parentId === null ? null : requireString(f.parentId, 'filter.parentId'); if (f.appId !== undefined) filter.appId = requireStringOrArray(f.appId, 'filter.appId'); if (f.createdBy !== undefined) { - const by = requirePlainObject(f.createdBy, 'filter.createdBy'); + const by = requireKnownKeys(f.createdBy, ['subjectId', 'principalId'], 'filter.createdBy'); filter.createdBy = { ...(by.subjectId !== undefined && { subjectId: requireStringOrArray(by.subjectId, 'filter.createdBy.subjectId'), @@ -450,18 +566,18 @@ export function parseQueryBody(raw: unknown): StackQuery { if (f.contentPresent !== undefined) filter.contentPresent = requireStringArray(f.contentPresent, 'filter.contentPresent'); if (f.search !== undefined) filter.search = requireString(f.search, 'filter.search'); - if (f.includeDeleted) filter.includeDeleted = true; - if (f.includeUnlisted) filter.includeUnlisted = true; + if (optionalBoolean(f.includeDeleted, 'filter.includeDeleted')) filter.includeDeleted = true; + if (optionalBoolean(f.includeUnlisted, 'filter.includeUnlisted')) filter.includeUnlisted = true; - if (f.createdAt) { - const r = requirePlainObject(f.createdAt, 'filter.createdAt'); + if (f.createdAt !== undefined) { + const r = requireKnownKeys(f.createdAt, ['before', 'after'], 'filter.createdAt'); filter.createdAt = { ...(r.before !== undefined && { before: requireDate(r.before, 'filter.createdAt.before') }), ...(r.after !== undefined && { after: requireDate(r.after, 'filter.createdAt.after') }), }; } - if (f.updatedAt) { - const r = requirePlainObject(f.updatedAt, 'filter.updatedAt'); + if (f.updatedAt !== undefined) { + const r = requireKnownKeys(f.updatedAt, ['before', 'after'], 'filter.updatedAt'); filter.updatedAt = { ...(r.before !== undefined && { before: requireDate(r.before, 'filter.updatedAt.before') }), ...(r.after !== undefined && { after: requireDate(r.after, 'filter.updatedAt.after') }), @@ -471,15 +587,15 @@ export function parseQueryBody(raw: unknown): StackQuery { query.filter = filter; } - if (body.sort) { - const s = requirePlainObject(body.sort, 'sort'); + if (body.sort !== undefined) { + const s = requireKnownKeys(body.sort, ['field', 'contentField', 'direction'], 'sort'); const sort = buildSort(s.field, s.contentField, s.direction); if (!sort) throw new StackBadRequestError('Invalid sort: expected a field or a contentField.'); query.sort = sort; } if (body.limit !== undefined) query.limit = parseLimitValue(body.limit); - if (typeof body.cursor === 'string') query.cursor = body.cursor; + if (body.cursor !== undefined) query.cursor = requireString(body.cursor, 'cursor'); return query; } @@ -506,6 +622,7 @@ export type ParsedChangeParams = { * the server's own resumption machinery rather than request encoding. */ export function parseChangeParams(url: URL): ParsedChangeParams { + requireKnownParams(url, CHANGE_PARAMS); const filter: ChangeFilter = {}; const typeIds = url.searchParams.getAll('typeId'); @@ -536,7 +653,7 @@ export function parseChangeParams(url: URL): ParsedChangeParams { return { filter, includeRecords: include === 'record', - includeUnlisted: url.searchParams.get('includeUnlisted') === 'true', + includeUnlisted: booleanParam(url, 'includeUnlisted'), }; } @@ -550,6 +667,7 @@ export function parseChangeParams(url: URL): ParsedChangeParams { * change feed's opaque cursor. See docs/spec/wire-format.md § Journal. */ export function parseJournalParams(url: URL): JournalQuery { + requireKnownParams(url, ['afterSeq', 'limit']); const query: JournalQuery = {}; const afterSeq = url.searchParams.get('afterSeq'); if (afterSeq !== null) query.afterSeq = parsePositiveInt(afterSeq, 'afterSeq'); diff --git a/packages/core/tests/wire-record.test.ts b/packages/core/tests/wire-record.test.ts index 941ee8c5..4b348e23 100644 --- a/packages/core/tests/wire-record.test.ts +++ b/packages/core/tests/wire-record.test.ts @@ -65,6 +65,13 @@ describe('createOptionsFromWireRecord — stamped fields', () => { }); }); +describe('createOptionsFromWireRecord — unrecognized keys', () => { + test('a key no wire record carries is refused rather than ignored', () => { + expect(() => parse(wireBody({ entityId: OWNER }))).toThrow(StackBadRequestError); + expect(() => parse(wireBody({ entityId: OWNER }))).toThrow(/entityId/); + }); +}); + // ------------------------------------------------------- // The clock fields // ------------------------------------------------------- diff --git a/packages/core/tests/wire-request.test.ts b/packages/core/tests/wire-request.test.ts index c4dcb61c..02c4a989 100644 --- a/packages/core/tests/wire-request.test.ts +++ b/packages/core/tests/wire-request.test.ts @@ -35,11 +35,16 @@ describe('parseQueryParams', () => { expect(parseQueryParams(url('')).filter).toBeUndefined(); }); - test('includeDeleted and includeUnlisted are set only by the literal "true"', () => { + test('includeDeleted and includeUnlisted take only "true" or "false"', () => { expect(parseQueryParams(url('?includeDeleted=true')).filter?.includeDeleted).toBe(true); - expect(parseQueryParams(url('?includeDeleted=1')).filter?.includeDeleted).toBeUndefined(); + expect(parseQueryParams(url('?includeDeleted=false')).filter?.includeDeleted).toBeUndefined(); expect(parseQueryParams(url('?includeUnlisted=true')).filter?.includeUnlisted).toBe(true); - expect(parseQueryParams(url('?includeUnlisted=yes')).filter?.includeUnlisted).toBeUndefined(); + expect(() => parseQueryParams(url('?includeDeleted=1'))).toThrow(StackBadRequestError); + expect(() => parseQueryParams(url('?includeUnlisted=yes'))).toThrow(StackBadRequestError); + }); + + test('an unrecognized param is refused rather than ignored', () => { + expect(() => parseQueryParams(url('?entityId=did:key:x'))).toThrow(/Unknown query param/); }); test('a malformed date bound is refused rather than dropped', () => { @@ -145,10 +150,32 @@ describe('parseQueryParams', () => { // ------------------------------------------------------- describe('parseQueryBody', () => { - test('a non-object body parses to an empty query', () => { + test('an absent body is an empty query, and a non-object body is refused', () => { expect(parseQueryBody(undefined)).toEqual({}); - expect(parseQueryBody(null)).toEqual({}); - expect(parseQueryBody('nonsense')).toEqual({}); + expect(() => parseQueryBody(null)).toThrow(StackBadRequestError); + expect(() => parseQueryBody('nonsense')).toThrow(StackBadRequestError); + }); + + test('an unrecognized key is refused at every level', () => { + for (const body of [ + { filters: {} }, + { filter: { entityId: 'x' } }, + { filter: { createdBy: { entityId: 'x' } } }, + { filter: { attachment: { label: 'a', mime: 'x' } } }, + { filter: { createdAt: { before: '2024-01-01', on: '2024-01-01' } } }, + { filter: { relatedTo: { label: 'x', scope: 'entity' } } }, + { filter: { relatedTo: { target: { kind: 'entity', entityId: 'e', ns: 'x' } } } }, + { sort: { field: 'createdAt', order: 'asc' } }, + ]) { + expect(() => parseQueryBody(body), JSON.stringify(body)).toThrow(/Unknown key/); + } + }); + + test('a non-boolean includeDeleted or a non-string cursor is refused', () => { + expect(() => parseQueryBody({ filter: { includeDeleted: 'true' } })).toThrow( + StackBadRequestError, + ); + expect(() => parseQueryBody({ cursor: 5 })).toThrow(StackBadRequestError); }); test('ISO date strings decode back to Date objects', () => { @@ -319,9 +346,15 @@ describe('parseChangeParams', () => { expect(() => parseChangeParams(changes('?include=everything'))).toThrow(StackBadRequestError); }); - test('includeUnlisted is set only by the literal "true"', () => { + test('includeUnlisted takes only "true" or "false"', () => { expect(parseChangeParams(changes('?includeUnlisted=true')).includeUnlisted).toBe(true); - expect(parseChangeParams(changes('?includeUnlisted=1')).includeUnlisted).toBe(false); + expect(parseChangeParams(changes('?includeUnlisted=false')).includeUnlisted).toBe(false); + expect(() => parseChangeParams(changes('?includeUnlisted=1'))).toThrow(StackBadRequestError); + }); + + test('an unrecognized param is refused, and the resume cursor is not one', () => { + expect(() => parseChangeParams(changes('?token=abc'))).toThrow(/Unknown query param/); + expect(() => parseChangeParams(changes('?since=abc'))).not.toThrow(); }); }); @@ -404,6 +437,12 @@ describe('parseJournalParams', () => { expect(parseJournalParams(journal('/records/r1/journal'))).toEqual({}); }); + test('an unrecognized param is refused rather than ignored', () => { + expect(() => parseJournalParams(journal('/records/r1/journal?sinceSeq=1'))).toThrow( + /Unknown query param/, + ); + }); + test('round-trips a window', () => { expect(parseJournalParams(journal('/records/r1/journal?afterSeq=4&limit=10'))).toEqual({ afterSeq: 4, From 1a27505038f6d0d88057252f4b1343471ab45bca Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 22:16:01 +0000 Subject: [PATCH 2/2] fix(core): refuse repeated single-value params and pin wire record keys A single-value query param repeated (?includeDeleted=true&includeDeleted=junk) was read as its first value with the rest silently dropped, which the Unrecognized input rule forbids. Only the filter lists (typeId, baseId, appId, createdBySubject, createdByPrincipal, tag, kind) may repeat now. WIRE_RECORD_KEYS is checked against keyof StackRecord, so a new record field fails to compile until the create body accepts it. Also document that parseQueryBody() takes an absent body as undefined, and name /auth/token among the endpoints a server parses itself. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0193gggj973cZHEHdWJHXGTQ --- .changeset/refuse-unrecognized-input.md | 2 +- docs/spec/wire-format.md | 4 +-- packages/core/src/wire-record.ts | 36 +++++++++++++----------- packages/core/src/wire-request.ts | 30 ++++++++++++++++++-- packages/core/tests/wire-request.test.ts | 24 ++++++++++++++++ 5 files changed, 73 insertions(+), 23 deletions(-) diff --git a/.changeset/refuse-unrecognized-input.md b/.changeset/refuse-unrecognized-input.md index a01e5769..c5ecffba 100644 --- a/.changeset/refuse-unrecognized-input.md +++ b/.changeset/refuse-unrecognized-input.md @@ -3,4 +3,4 @@ '@haverstack/conformance-fixtures': minor --- -The `./wire` request parsers refuse input their endpoint does not define instead of ignoring it. `parseQueryParams()`, `parseChangeParams()` and `parseJournalParams()` throw `StackBadRequestError` for an unknown query param or a boolean param other than `true`/`false`. `parseQueryBody()` does the same for an unknown key at any depth, a non-object body, a non-boolean `includeDeleted`/`includeUnlisted` or a non-string `cursor`. `createOptionsFromWireRecord()` refuses a key no wire record carries. The rule is specified in docs/spec/wire-format.md § Unrecognized input, and four new error fixtures cover it. +The `./wire` request parsers refuse input their endpoint does not define instead of ignoring it. `parseQueryParams()`, `parseChangeParams()` and `parseJournalParams()` throw `StackBadRequestError` for an unknown query param, a repeat of a param that names one value, or a boolean param other than `true`/`false`. `parseQueryBody()` does the same for an unknown key at any depth, a non-object body, a non-boolean `includeDeleted`/`includeUnlisted` or a non-string `cursor`. `createOptionsFromWireRecord()` refuses a key no wire record carries. The rule is specified in docs/spec/wire-format.md § Unrecognized input, and four new error fixtures cover it. diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index 8e005a85..96591c57 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 — and a boolean param takes only `true` or `false`, since any other value is a mistake of the same kind. +**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. Three things sit outside it: @@ -193,7 +193,7 @@ 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 owe the same refusal. +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. ### The taxonomy root diff --git a/packages/core/src/wire-record.ts b/packages/core/src/wire-record.ts index bab08d12..c25ca2f2 100644 --- a/packages/core/src/wire-record.ts +++ b/packages/core/src/wire-record.ts @@ -20,6 +20,7 @@ import type { AuthorityAssociation, DataAssociation, RecordChangeSet, + StackRecord, TokenSession, TypeId, } from './types.js'; @@ -36,26 +37,27 @@ export type WireCreateRequest = { }; /** - * Every top-level key a wire record carries. A create body is a whole + * Every top-level key a wire record carries — a `StackRecord`'s, so a new + * field fails to compile here until it is listed. A create body is a whole * record, so each of these is accepted — stamped ones are then dropped — * and anything else is refused. See docs/spec/wire-format.md § Unrecognized input. */ -const WIRE_RECORD_KEYS: readonly string[] = [ - 'id', - 'typeId', - 'createdAt', - 'updatedAt', - 'content', - 'version', - 'parentId', - 'appId', - 'createdBy', - 'updatedBy', - 'deletedAt', - 'unlistedAt', - 'permissions', - 'associations', -]; +const WIRE_RECORD_KEYS: readonly string[] = Object.keys({ + id: true, + typeId: true, + createdAt: true, + updatedAt: true, + content: true, + version: true, + parentId: true, + appId: true, + createdBy: true, + updatedBy: true, + deletedAt: true, + unlistedAt: true, + permissions: true, + associations: true, +} satisfies Record); function requireBody(body: unknown): Record { if (typeof body !== 'object' || body === null || Array.isArray(body)) diff --git a/packages/core/src/wire-request.ts b/packages/core/src/wire-request.ts index d4c95636..59bfb01c 100644 --- a/packages/core/src/wire-request.ts +++ b/packages/core/src/wire-request.ts @@ -227,13 +227,35 @@ function requireKnownKeys( return obj; } -/** The URL-param form of requireKnownKeys(). */ +/** + * Params a filter repeats to name several values. Any other param names + * one value, so a repeat of it is refused rather than read as its first. + */ +const REPEATABLE_PARAMS: ReadonlySet = new Set([ + 'typeId', + 'baseId', + 'appId', + 'createdBySubject', + 'createdByPrincipal', + 'tag', + 'kind', +]); + +/** The URL-param form of requireKnownKeys(), which also refuses a repeat. */ function requireKnownParams(url: URL, names: readonly string[]): void { - const unknown = [...new Set(url.searchParams.keys())].filter((name) => !names.includes(name)); + const present = [...new Set(url.searchParams.keys())]; + const unknown = present.filter((name) => !names.includes(name)); if (unknown.length > 0) throw new StackBadRequestError( `Unknown query param${unknown.length > 1 ? 's' : ''}: ${unknown.join(', ')}`, ); + const repeated = present.filter( + (name) => !REPEATABLE_PARAMS.has(name) && url.searchParams.getAll(name).length > 1, + ); + if (repeated.length > 0) + throw new StackBadRequestError( + `Repeated query param${repeated.length > 1 ? 's' : ''}: ${repeated.join(', ')}`, + ); } /** A boolean URL param: absent is false, and only `true`/`false` are values. */ @@ -527,7 +549,9 @@ function parseLimitValue(raw: unknown): number { * Build a `StackQuery` from a `POST /records/query` JSON body — the * superset form, which additionally carries `filter.content`. Dates arrive * as the ISO strings `JSON.stringify` made of them and are decoded back to - * `Date`. See docs/spec/wire-format.md § Records. + * `Date`. `undefined` is the absent body; `null` or any other non-object + * is refused, so a server hands over no body as `undefined`. + * See docs/spec/wire-format.md § Records. */ export function parseQueryBody(raw: unknown): StackQuery { if (raw === undefined) return {}; diff --git a/packages/core/tests/wire-request.test.ts b/packages/core/tests/wire-request.test.ts index 02c4a989..9c31b652 100644 --- a/packages/core/tests/wire-request.test.ts +++ b/packages/core/tests/wire-request.test.ts @@ -47,6 +47,17 @@ describe('parseQueryParams', () => { expect(() => parseQueryParams(url('?entityId=did:key:x'))).toThrow(/Unknown query param/); }); + test('a single-value param appears at most once; a filter list may repeat', () => { + expect(() => parseQueryParams(url('?includeDeleted=true&includeDeleted=junk'))).toThrow( + /Repeated query param: includeDeleted/, + ); + expect(() => parseQueryParams(url('?limit=5&limit=500'))).toThrow(/Repeated query param/); + expect(parseQueryParams(url('?typeId=a/b@1&typeId=a/c@1&tag=x&tag=y')).filter).toMatchObject({ + typeId: ['a/b@1', 'a/c@1'], + tags: ['x', 'y'], + }); + }); + test('a malformed date bound is refused rather than dropped', () => { expect(() => parseQueryParams(url('?createdBefore=not-a-date'))).toThrow(StackBadRequestError); }); @@ -356,6 +367,13 @@ describe('parseChangeParams', () => { expect(() => parseChangeParams(changes('?token=abc'))).toThrow(/Unknown query param/); expect(() => parseChangeParams(changes('?since=abc'))).not.toThrow(); }); + + test('a single-value param appears at most once; a filter list may repeat', () => { + expect(() => parseChangeParams(changes('?include=record&include=record'))).toThrow( + /Repeated query param/, + ); + expect(() => parseChangeParams(changes('?kind=created&kind=deleted'))).not.toThrow(); + }); }); // ------------------------------------------------------- @@ -443,6 +461,12 @@ describe('parseJournalParams', () => { ); }); + test('a repeated param is refused', () => { + expect(() => parseJournalParams(journal('/records/r1/journal?afterSeq=1&afterSeq=9'))).toThrow( + /Repeated query param/, + ); + }); + test('round-trips a window', () => { expect(parseJournalParams(journal('/records/r1/journal?afterSeq=4&limit=10'))).toEqual({ afterSeq: 4,