fix(rest,runtime)!: a /data PATCH or DELETE refuses a version token that can never carry a version, on both doors

Claude-Session: https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU
Co-authored-by: Claude <noreply@anthropic.com>

Author: claude

packages/rest/src/rest-server.ts

@@ -1733,6 +1733,54 @@ function sendMetaItemAbsent(res: any): void {
     });
 }
 
+/**
+ * [#23492] What a `/data` write door does with the optimistic-concurrency
+ * token it read — the body's `expectedVersion` (`PATCH`) or `?expectedVersion`
+ * (`DELETE`), else `If-Match` — before the protocol is asked. One of three:
+ *
+ *  - ABSENT: the spelling consulted is `undefined` or `null`. The body key or
+ *    the query name falls through to `If-Match`, and no `If-Match` either is
+ *    the legal unguarded write.
+ *  - JUDGED: a string with something in it, or a finite number. The door
+ *    forwards its string, and the protocol compares it (`409
+ *    CONCURRENT_UPDATE` on a mismatch; the quoted-empty tag `""` is the
+ *    protocol's own `400`, `MalformedVersionTokenError`).
+ *  - REFUSED `400 VALIDATION_FAILED`: anything else the caller put in a token
+ *    position — an empty or blank string, a boolean, an array, an object. It
+ *    can never carry a version, so it is the protocol's ruled answer to `""`
+ *    (maintainer decision #20 ①: a token shape that can never carry a version
+ *    is refused, neither read as "no token" nor answered `409`), given here
+ *    because the protocol never sees these: it reads a blank string as no
+ *    token.
+ *
+ * What it replaces is a truthiness test. The door forwarded a token only when
+ * it was truthy, and `bodyVersion ?? If-Match` reads `If-Match` only when the
+ * body key is nullish, so `expectedVersion: ''` or `false` — not nullish, not
+ * truthy — hid a stale `If-Match`, and the write ran unguarded with a `200`.
+ * `?expectedVersion=` did the same to a `DELETE`, and an empty `If-Match`
+ * alone, the caller asking for a guard and sending nothing to judge, ran
+ * unguarded too.
+ *
+ * Returns the refusal's sentence, or `undefined` when the token is absent or
+ * judged. The runtime dispatcher's `/data` domain carries this function
+ * statement for statement (`versionTokenRefusal` in
+ * `packages/runtime/src/domains/data.ts`), so the two doors answer every token
+ * alike; `data-update-expected-version.test.ts` and
+ * `data-delete-expected-version.test.ts` hold them to it.
+ */
+function versionTokenRefusal(token: unknown, spelling: string): string | undefined {
+    if (token === undefined || token === null) return undefined;
+    if (typeof token === 'string' && token.trim() !== '') return undefined;
+    if (typeof token === 'number' && Number.isFinite(token)) return undefined;
+    const shape = typeof token === 'string' ? 'empty'
+        : Array.isArray(token) ? 'an array'
+        : typeof token === 'object' ? 'an object'
+        : `the ${typeof token} \`${String(token)}\``;
+    return `${spelling} is ${shape}, which can never carry a version, so it is refused rather than read as `
+        + 'no token. Send the version you read (the record\'s `updated_at`), or omit both expectedVersion and '
+        + 'If-Match to perform an unguarded write.';
+}
+
 /**
  * [#20338] May this caller read PENDING metadata — a `sys_metadata` row in
  * `state: 'draft'`, unpublished authoring work? THE question every door that
@@ -9067,6 +9115,23 @@ export class RestServer {
                             const { expectedVersion: _drop, ...rest } = data as any;
                             data = rest;
                         }
+                        // [#23492] A token that can never carry a version is
+                        // refused here, before anything is asked, and every
+                        // other present token is forwarded below: no token the
+                        // caller sent is read as "no token".
+                        const versionRefusal = versionTokenRefusal(
+                            expectedVersion,
+                            bodyVersion != null ? 'The body\'s `expectedVersion`' : 'The `If-Match` header',
+                        );
+                        if (versionRefusal) {
+                            res.status(400).json({
+                                error: versionRefusal,
+                                code: 'VALIDATION_FAILED',
+                                fields: [],
+                                object: req.params?.object,
+                            });
+                            return;
+                        }
                         if (await this.enforceApiAccess(req, res, p, environmentId, 'update')) return;
                         // [#3899] Same gate as create: the wire body is the bare
                         // field patch (`expectedVersion` already stripped above),
@@ -9078,7 +9143,7 @@ export class RestServer {
                             object: req.params.object,
                             id: req.params.id,
                             data: data ?? {},
-                            ...(expectedVersion ? { expectedVersion: String(expectedVersion) } : {}),
+                            ...(expectedVersion != null ? { expectedVersion: String(expectedVersion) } : {}),
                         };
                         const parsedUpdateOne = (UpdateDataRequestSchema as any).safeParse(updateInput);
                         if (!parsedUpdateOne.success) {
@@ -9094,7 +9159,7 @@ export class RestServer {
                             object: req.params.object,
                             id: req.params.id,
                             data: data ?? {},
-                            ...(expectedVersion ? { expectedVersion: String(expectedVersion) } : {}),
+                            ...(expectedVersion != null ? { expectedVersion: String(expectedVersion) } : {}),
                             ...(environmentId ? { environmentId } : {}),
                             ...(context ? { context } : {}),
                         };
@@ -9142,11 +9207,27 @@ export class RestServer {
                             ? (req.query as any).expectedVersion
                             : undefined;
                         const expectedVersion = queryVersion ?? ifMatchHeader;
+                        // [#23492] The `PATCH` above's rule: a token that can
+                        // never carry a version is refused, every other present
+                        // token is forwarded.
+                        const versionRefusal = versionTokenRefusal(
+                            expectedVersion,
+                            queryVersion != null ? 'The `expectedVersion` query parameter' : 'The `If-Match` header',
+                        );
+                        if (versionRefusal) {
+                            res.status(400).json({
+                                error: versionRefusal,
+                                code: 'VALIDATION_FAILED',
+                                fields: [],
+                                object: req.params?.object,
+                            });
+                            return;
+                        }
                         if (await this.enforceApiAccess(req, res, p, environmentId, 'delete')) return;
                         const deleteRequest: ServerScopedDataRequest<DeleteDataRequest> = {
                             object: req.params.object,
                             id: req.params.id,
-                            ...(expectedVersion ? { expectedVersion: String(expectedVersion) } : {}),
+                            ...(expectedVersion != null ? { expectedVersion: String(expectedVersion) } : {}),
                             ...(environmentId ? { environmentId } : {}),
                             ...(context ? { context } : {}),
                         };

packages/runtime/src/domains/data.ts

@@ -34,6 +34,9 @@ import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.
 // [#23265] …and the repo's one sentence for a repeated single-valued
 // parameter, which the delete's `?expectedVersion` answers.
 import { assertFilterParamSuppliedOnce, repeatedQueryParamMessage } from '@objectstack/rest';
+// [#23492] The dispatcher's one constructor for a `400 VALIDATION_FAILED`
+// answer, which a refused version token returns in this transport's envelope.
+import { validationFailure, VALIDATION_FAILED_STATUS } from '../validation-failure.js';
 
 /**
  * [#23265] The request's `If-Match`, from either header shape a host hands
@@ -50,6 +53,37 @@ function requestIfMatch(headers: unknown): unknown {
     return h['if-match'] ?? h['If-Match'];
 }
 
+/**
+ * [#23492] What this domain's `PATCH` and `DELETE` do with the
+ * optimistic-concurrency token they read, before `callData` is asked: absent
+ * (`undefined` or `null`, which falls through to `If-Match`), judged (a string
+ * with something in it, or a finite number, forwarded as its string), or
+ * refused `400 VALIDATION_FAILED` (an empty or blank string, a boolean, an
+ * array, an object: a token shape that can never carry a version, maintainer
+ * decision #20 ①). It replaces a truthiness test that let `expectedVersion: ''`
+ * or `false`, and `?expectedVersion=`, hide a stale `If-Match` and run the
+ * write unguarded.
+ *
+ * `RestServer`'s twin routes ask the same function, statement for statement
+ * (`versionTokenRefusal` in `packages/rest/src/rest-server.ts`, whose docblock
+ * carries the full argument), so the two doors answer every token alike;
+ * `data-update-expected-version.test.ts` and
+ * `data-delete-expected-version.test.ts` hold them to it. Returns the refusal's
+ * sentence, or `undefined` when the token is absent or judged.
+ */
+function versionTokenRefusal(token: unknown, spelling: string): string | undefined {
+    if (token === undefined || token === null) return undefined;
+    if (typeof token === 'string' && token.trim() !== '') return undefined;
+    if (typeof token === 'number' && Number.isFinite(token)) return undefined;
+    const shape = typeof token === 'string' ? 'empty'
+        : Array.isArray(token) ? 'an array'
+        : typeof token === 'object' ? 'an object'
+        : `the ${typeof token} \`${String(token)}\``;
+    return `${spelling} is ${shape}, which can never carry a version, so it is refused rather than read as `
+        + 'no token. Send the version you read (the record\'s `updated_at`), or omit both expectedVersion and '
+        + 'If-Match to perform an unguarded write.';
+}
+
 export function createDataDomain(deps: DomainHandlerDeps): DomainRoute {
     return {
         prefix: '/data',
@@ -170,22 +204,39 @@ export async function handleDataRequest(deps: DomainHandlerDeps, path: string, m
             // whole body over as the patch, so through the `@objectstack/hono`
             // catch-all a stale `If-Match` update was APPLIED, and a body
             // `expectedVersion` reached the engine as a field it refused as
-            // unknown. The key is stripped from the patch whatever its value,
-            // and a token is forwarded only when truthy: the twin's own
-            // statements, so both doors send `updateData` the same thing.
+            // unknown. The key is stripped from the patch whatever its value:
+            // the twin's own statements, so both doors send `updateData` the
+            // same thing.
+            //
+            // [#23492] …and every token the caller sent is judged or refused:
+            // one that can never carry a version is refused `400` before
+            // `callData` is asked (`versionTokenRefusal`), and every other
+            // present token is forwarded. A truthiness test used to decide
+            // the forward, so `expectedVersion: ''` or `false` hid a stale
+            // `If-Match` and the update ran unguarded, on both doors.
             const bodyVersion = (body && typeof body === 'object') ? body.expectedVersion : undefined;
             const expectedVersion = bodyVersion ?? requestIfMatch(_context.request?.headers);
             let data = body;
             if (body && typeof body === 'object' && 'expectedVersion' in body) {
                 const { expectedVersion: _token, ...patch } = body;
                 data = patch;
             }
+            const versionRefusal = versionTokenRefusal(
+                expectedVersion,
+                bodyVersion != null ? 'The body\'s `expectedVersion`' : 'The `If-Match` header',
+            );
+            if (versionRefusal) {
+                return {
+                    handled: true,
+                    response: deps.errorFromThrown(validationFailure(versionRefusal, []), VALIDATION_FAILED_STATUS),
+                };
+            }
             // Spec: returns UpdateDataResponse = { object, id, record }
             const result = await actionExec.callData(deps, _context, 'update', {
                 object: objectName,
                 id,
                 data,
-                ...(expectedVersion ? { expectedVersion: String(expectedVersion) } : {}),
+                ...(expectedVersion != null ? { expectedVersion: String(expectedVersion) } : {}),
             }, _context.dataDriver, _context.environmentId, _context.executionContext);
             return { handled: true, response: deps.success(result) };
         }
@@ -210,8 +261,13 @@ export async function handleDataRequest(deps: DomainHandlerDeps, path: string, m
             // request that sent no token. A repeated `?expectedVersion` is
             // refused first, as the twin's arity gate refuses it, before the
             // service is asked; a one-element array is one occurrence and an
-            // empty one is none. A token is forwarded only when non-empty, the
-            // twin's own test, so both doors send `deleteData` the same thing.
+            // empty one is none.
+            //
+            // [#23492] The `PATCH` above's rule, the twin's own: a token that
+            // can never carry a version is refused `400` before `callData` is
+            // asked, and every other present token is forwarded. A truthiness
+            // test used to decide the forward, so `?expectedVersion=` hid a
+            // stale `If-Match` and the delete ran unguarded, on both doors.
             const rawVersion = query?.expectedVersion;
             if (Array.isArray(rawVersion) && rawVersion.length > 1) {
                 return {
@@ -221,10 +277,20 @@ export async function handleDataRequest(deps: DomainHandlerDeps, path: string, m
             }
             const queryVersion = Array.isArray(rawVersion) ? rawVersion[0] : rawVersion;
             const expectedVersion = queryVersion ?? requestIfMatch(_context.request?.headers);
+            const versionRefusal = versionTokenRefusal(
+                expectedVersion,
+                queryVersion != null ? 'The `expectedVersion` query parameter' : 'The `If-Match` header',
+            );
+            if (versionRefusal) {
+                return {
+                    handled: true,
+                    response: deps.errorFromThrown(validationFailure(versionRefusal, []), VALIDATION_FAILED_STATUS),
+                };
+            }
             const result = await actionExec.callData(deps, _context, 'delete', {
                 object: objectName,
                 id,
-                ...(expectedVersion ? { expectedVersion: String(expectedVersion) } : {}),
+                ...(expectedVersion != null ? { expectedVersion: String(expectedVersion) } : {}),
             }, _context.dataDriver, _context.environmentId, _context.executionContext);
             return { handled: true, response: deps.success(result) };
         }