fix(objectql)!: a non-system write to a secret field may only seal a new value or keep its own record's ref

.changeset/27582-secret-field-own-ref-only.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql)!: a non-system write to a `secret` field may only seal a new value or keep its own record's reference (#27582)
+
+Clause-②: no (narrowing) — a non-system write to a `secret` field refuses a value already spelled `secret:` unless it is the reference that record's column already holds (so a create always refuses one), where it stored such a value as given before. Nothing is widened.
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: a write value the engine used to store as given is refused unless it is the record's own reference. The fix for a caller is to send the secret's value, write null, or omit the field, which the refusal says; there is nothing for a migration to rewrite and no tombstone. -->
+
+**BREAKING** accept-set narrowing in the credential write path (`@objectstack/objectql`), served by every engine write door and every REST data door that writes through one.
+
+- **Before.** A `secret` field value that starts with `secret:` is read as a reference to a credential already stored, not as a credential. Such a value was stored as given, from any caller, on a create or an update.
+- **Now.** From any caller but the system principal, such a value is accepted only when it is the reference that record's column already holds. On an update, every record the write lands on must already hold exactly that value, and the column is then left as it is, as an echoed read mask is. A create has no reference of its own, so it never takes one. Anything else is refused with `400 VALIDATION_ERROR`, naming the object and the field and never the value, before any credential row or record is written. The answer is the same whatever the reference names. This holds on the single and batch create, `insertMany` (the refused row is reported on its own and the others land), the by-id update and the predicate update, so on every REST door that writes through them: create, bulk, import and update.
+- **Unchanged.** The system principal (`isSystem: true`: seed replay, migration) still stores such a value as given. A new value is sealed as before, `null` clears, an echoed read mask changes nothing, and reads still mask. The Console's password field sends back the mask it read, so the form round trip is unaffected.
+- **Who sees it.** A caller without the system context (an API client, an import, a hook or script) that sends a `secret:` value to a `secret` field other than the reference that record already holds.
+
+**The remedy.** To set the secret, send its value; to clear it, write `null`; to keep it, omit the field or echo the read mask.

content/docs/permissions/system-context.mdx

@@ -136,6 +136,7 @@ bugs — it is the protection or the side effect that silently does not happen.
 | 23b | **Position-catalog check skipped** — a `sys_user_position` write whose `position` names no position in the security catalog is not refused | plugin-security | Get: a system writer can store an assignment naming no position — the seed loader writes `stack.data` from `AppPlugin.start()` under a system context, as invitation acceptance and the platform's own bootstraps do. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no position of that name: the environment registry (ADR-0131 D3), one per deployment, which takes no organization, so a name only a `sys_position` row carries — any organization's — is refused like one nobody carries. It is the same stand-down as the engine's referential-integrity check for a lookup column | `assertPositionNamesCatalogRow` |
 | 23c | **Grant-name catalog check skipped** — a `sys_user_permission_set` write whose `permission_set` names no permission set in the security catalog is not refused | plugin-security | Get: a system writer can store a grant naming a set not yet registered — seed replay and boot provisioning may write the grant before the definition it names is registered. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `permission_set`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no set by that name; a grant that names no set at all is refused `required` for every caller. The grant names its set by name alone, and nothing derives the name from `permission_set_id` (ADR-0131 D4, D10) | `packages/plugins/plugin-security/src/grant-permission-set-name.ts#onInsert`, `packages/plugins/plugin-security/src/grant-permission-set-name.ts#onUpdate` |
 | 23d | **Grant-holder membership check skipped** — a `sys_user_position` or `sys_user_permission_set` write naming a user with no `sys_member` row in the row's organization is not refused | plugin-security | Get: a system writer can store such a grant — invitation acceptance writes the membership and its placement together, and seed replay, the organization-admin reconcile and the platform bootstraps write grants as the system. Lose: such a row takes effect once its user is admitted to the organization. A non-system insert, or a non-system update that moves `user_id` or `organization_id`, is refused `400 VALIDATION_FAILED` / `reference_not_found` at `user_id` instead, for every caller, a platform administrator included; the membership is read as `{ isSystem: true, tenantId }` with `tenantId` the row's own organization, never another one. A row stored with no organization is not judged. Under `single`, 18.0 stamps a new grant with the Default Organization, so such a row is one an earlier release wrote | `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onInsert`, `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onUpdate` |
+| 23e | **A `secret` field takes a stored reference as given** — a value already spelled `secret:` is stored unchanged | objectql | Get: a system writer may copy a `secret:` reference onto a record as it stands (seed replay, a migration). Every other caller may only seal a new value or keep its own record's ref: on a create any such value is refused, and on an update it is accepted only when every record the write lands on already holds exactly that reference in that column, which is then left as it is, as an echoed read mask is. Any other reference is refused `400 VALIDATION_ERROR` before anything is written, with one answer whatever it names. Lose: the check that the reference is the record's own — a system writer that copies one is trusted to mean it. Inbound HTTP never carries the flag | `encryptSecretFields` |
 | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `buildDriverOptions` |
 | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `packages/plugins/plugin-security/src/system-write-guard.ts#assertEngineOwnedWriteAllowed` |
 | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` |
@@ -532,6 +533,7 @@ and CI refuses one that differs from what the tree produces.
 | `packages/objectql/src/engine-master-detail-related.ts#resolveOneHopRelated` | 1 | 29b |
 | `packages/objectql/src/engine-referential-integrity.ts#assertReferencesResolve` | 1 | 23 |
 | `packages/objectql/src/engine-referential-integrity.ts#masterDetailHeadersCallerReads` | 1 | 29c |
+| `packages/objectql/src/engine-secret-internal-fields.ts#encryptSecretFields` | 1 | 23e |
 | `packages/objectql/src/engine-secret-internal-fields.ts#stripSearchCompanionFromRead` | 1 | 27 |
 | `packages/objectql/src/engine-validate.ts#validate` | 1 | 21b |
 | `packages/objectql/src/engine.ts#dependentCountIsDisclosable` | 1 | 28 |

packages/objectql/src/engine-secret-internal-fields.ts

@@ -23,6 +23,12 @@
  * column off it, has left it unreferenced. [#27581] A committed delete hands
  * the same release every handle the deleted rows held (`deletedSecretSlots`).
  *
+ * [#27582] And one on the value already spelled as a ref: a non-system write
+ * may only seal a new value or keep its own record's ref. `encryptSecretFields`
+ * takes the write's own stored rows ({@link StoredRowsReader}) and refuses any
+ * other ref with {@link SecretRefWriteError}; the system principal keeps the
+ * pass-through.
+ *
  * ## How the moved bodies reach the class
  *
  * The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts`
@@ -106,17 +112,66 @@ export interface SecretHandleSlot {
   readonly handleId: string;
 }
 
+/**
+ * [#27582] The rows a write's own record(s) hold in storage now, read for one
+ * question: is a ref on a non-system payload the value that record's column
+ * already holds? Each call site says what its own rows are — none for an
+ * insert (a new record holds no ref of its own), the pre-image for a by-id
+ * update, the matched rows for a predicate update — and the reader is called
+ * only when a non-system payload carries a ref, so every other write reads
+ * nothing more than it did.
+ */
+export type StoredRowsReader = () => Promise<readonly (Record<string, unknown> | null | undefined)[] | null>;
+
+/**
+ * [#27582] The ADR-0112 pair carried by the refusal of a ref that is not the
+ * record's own. `VALIDATION_ERROR` / 400 is the standard-catalog member the
+ * sibling credential refusals use (`EMPTY_CREDENTIAL_REFUSAL_CODE`): the
+ * payload is wrong and the caller can fix it, so no ledger registration is
+ * needed.
+ */
+export const SECRET_REF_WRITE_REFUSAL_CODE = 'VALIDATION_ERROR';
+export const SECRET_REF_WRITE_REFUSAL_STATUS = 400;
+
+/**
+ * [#27582] Refusal of a `secret:` ref on a non-system write that is not the
+ * value the record's own column already holds. Fail closed, with one answer:
+ * the message names the column and never the ref, and nothing about the handle
+ * the ref names is looked up, so the answer is the same whatever it names.
+ */
+export class SecretRefWriteError extends Error {
+  readonly code = SECRET_REF_WRITE_REFUSAL_CODE;
+  readonly status = SECRET_REF_WRITE_REFUSAL_STATUS;
+  /** The same number under ADR-0112 D5's spelling, as `EmptyCredentialWriteError` carries it. */
+  readonly httpStatus = SECRET_REF_WRITE_REFUSAL_STATUS;
+  readonly object: string;
+  readonly field: string;
+  constructor(object: string, field: string) {
+    super(
+      `Stored-reference value refused for secret field "${object}.${field}": a value that starts with "secret:" `
+        + 'is read as a reference to a credential already stored, not as a credential, and a write may only keep '
+        + 'the reference this record already holds. To set the secret, send its value; to clear it, write null; '
+        + 'to leave it unchanged, omit the field. Nothing was written.',
+    );
+    this.name = 'SecretRefWriteError';
+    this.object = object;
+    this.field = field;
+  }
+}
+
 /** The body of `ObjectQL#encryptSecretFields`; the method's docblock there is its contract. */
 export async function encryptSecretFields(
   this: Pick<SecretInternalFieldsHost, '_registry' | 'cryptoProvider' | 'getDriver'>,
   object: string,
   row: Record<string, unknown>,
   context: ExecutionContext | undefined,
   driverOptions: unknown,
+  storedRows: StoredRowsReader,
 ): Promise<PendingSecretRow[]> {
   const pending: PendingSecretRow[] = [];
   if (!row || typeof row !== 'object') return pending;
   const schema = this._registry.getObject(object);
+  let ownRows: readonly (Record<string, unknown> | null | undefined)[] | undefined;
 
   // Echoed-mask drop for every field the read path masks (secret + generic
   // password). The read path returns SECRET_MASK; a client that PATCHes it
@@ -141,7 +196,22 @@ export async function encryptSecretFields(
     // on the payload, so it must not depend on whether a CryptoProvider or
     // sys_secret store happens to be wired.
     if (value === '') throw new EmptyCredentialWriteError(object, field, 'secret');
-    if (isSecretRef(value)) continue; // already encrypted ref
+    if (isSecretRef(value)) {
+      // The system principal's ref passes through as it always has: a system
+      // writer may copy a ref on purpose (seed replay, migration).
+      if (context?.isSystem === true) continue;
+      // [#27582] A non-system write may only seal a new value or keep its own
+      // record's ref: every row this write lands on must already hold exactly
+      // this value in this column. A write with no own row (an insert, a
+      // predicate that matches nothing) has nothing to keep. Kept, the key is
+      // dropped, as the echoed mask is: the column is not rewritten at all, so
+      // the verdict cannot go stale before the statement runs.
+      ownRows ??= (await storedRows()) ?? [];
+      const keepsOwn = ownRows.length > 0 && ownRows.every((own) => own?.[field] === value);
+      if (!keepsOwn) throw new SecretRefWriteError(object, field);
+      delete row[field];
+      continue;
+    }
 
     if (!this.cryptoProvider) {
       throw new Error(

packages/objectql/src/engine.ts

@@ -125,6 +125,7 @@ import {
   stripSearchCompanionFromRead,
   type PendingSecretRow,
   type SecretHandleSlot,
+  type StoredRowsReader,
 } from './engine-secret-internal-fields.js';
 // [#25522 cut 6] The predicate-write hook dispatch bodies; the methods of the
 // same names below hand each body the members it reads (`HookDispatchHost`).
@@ -10575,7 +10576,16 @@ export class ObjectQL implements IObjectQLEngine {
    *    (maintainer ruling 2026-08-13 on #8559). The `password` half of the same
    *    ruling lands at its own seam — {@link refuseEmptyPasswordFields} — since
    *    the two types share the read mask but not this encryption path.
-   *  - Secret value already a ref (re-save of an unchanged ref) ⇒ left as-is.
+   *  - Secret value already a ref ⇒ [#27582] a non-system write may only seal
+   *    a new value or keep its own record's ref. From the system principal
+   *    (`context.isSystem === true`) it is left as-is, as it always was. From
+   *    any other caller it is kept only when every row the write lands on
+   *    already holds exactly that ref in that column — read from `storedRows`,
+   *    which each call site supplies (none for an insert, the pre-image for a
+   *    by-id update, the matched rows for a predicate update) — and then the
+   *    key is dropped, as the echoed mask is. Any other ref is REFUSED with
+   *    {@link SecretRefWriteError} (ADR-0112 `VALIDATION_ERROR`/400), one
+   *    answer whatever the ref names.
    *  - **Fail-closed:** any other secret value with no CryptoProvider registered,
    *    or no reachable `sys_secret` store, THROWS — never persists cleartext.
    *    (A `password` field needs no CryptoProvider — it is stored as-is.)
@@ -10585,6 +10595,7 @@ export class ObjectQL implements IObjectQLEngine {
     row: Record<string, unknown>,
     context: ExecutionContext | undefined,
     driverOptions: unknown,
+    storedRows: StoredRowsReader,
   ): Promise<PendingSecretRow[]> {
     // [#25522 cut 5] The body moved to `engine-secret-internal-fields.ts` (see `SecretInternalFieldsHost`).
     const readRegistry = () => this._registry;
@@ -10594,7 +10605,7 @@ export class ObjectQL implements IObjectQLEngine {
       get cryptoProvider() { return readCryptoProvider(); },
       getDriver: (...a) => this.getDriver(...a),
     };
-    return encryptSecretFields.call(host, object, row, context, driverOptions);
+    return encryptSecretFields.call(host, object, row, context, driverOptions, storedRows);
   }
 
   /**
@@ -15374,7 +15385,8 @@ export class ObjectQL implements IObjectQLEngine {
             // carries the secret-arm refusal). Same try, so partial mode
             // reports either refusal per-row instead of killing the batch.
             this.refuseEmptyPasswordFields(object, rows[i]);
-            sealedSecrets[i] = await this.encryptSecretFields(object, rows[i], opCtx.context, driverOptions);
+            // [#27582] A new record holds no ref of its own: no stored rows.
+            sealedSecrets[i] = await this.encryptSecretFields(object, rows[i], opCtx.context, driverOptions, async () => []);
           } catch (e) {
             if (!partialMode) throw e;
             rowErrors[i] = e;
@@ -17029,7 +17041,8 @@ export class ObjectQL implements IObjectQLEngine {
                // [#8559] Password half of the credential write door, then the
                // secret channel (which carries the secret-arm refusal).
                this.refuseEmptyPasswordFields(object, hookContext.input.data as Record<string, unknown>);
-               sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record<string, unknown>, opCtx.context, hookContext.input.options);
+               // [#27582] This record's own stored row is the pre-image read above.
+               sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record<string, unknown>, opCtx.context, hookContext.input.options, async () => [priorRecord]);
                // [#21663] Scope `'skip'` — the public `validateRecord` IS that
                // scope: the readonly strip has NOT run yet, so a readonly value
                // here may be a caller's the strip is about to drop — judged, a
@@ -17426,7 +17439,9 @@ export class ObjectQL implements IObjectQLEngine {
                // [#8559] Password half of the credential write door, then the
                // secret channel (which carries the secret-arm refusal).
                this.refuseEmptyPasswordFields(object, hookContext.input.data as Record<string, unknown>);
-               sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record<string, unknown>, opCtx.context, hookContext.input.options);
+               // [#27582] The rows this write lands on are the matched rows: the
+               // D7 memo, read here only when a non-system payload carries a ref.
+               sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record<string, unknown>, opCtx.context, hookContext.input.options, readPriorRows);
                // [#21663] Scope `'skip'` — the public `validateRecord` IS that
                // scope: the readonly strip has NOT run yet, so a readonly value
                // here may be a caller's the strip is about to drop — judged, a

packages/objectql/src/error-http-status-spelling.test.ts

@@ -37,6 +37,7 @@ import { DuplicateRecordError } from './duplicate-record-error.js';
 import { HookUnscopedDataAccessError } from './hook-run-as.js';
 import { MultiUpdateHookKeyDivergenceError } from './multi-update-hook-key-divergence.js';
 import { EmptyCredentialWriteError } from './secret-fields.js';
+import { SecretRefWriteError } from './engine-secret-internal-fields.js';
 import { SystemWriteOrganizationRequiredError } from './tenancy/system-write-organization.js';
 import { NamespaceConflictError, ArtifactObjectNameConflictError, ObjectOwnershipConflictError } from './registry.js';
 import { invalidFilterError } from './filter-comparand-shape.js';
@@ -52,6 +53,7 @@ const CONSTRUCTED: Array<[string, () => unknown, number]> = [
   ['HookUnscopedDataAccessError', () => new HookUnscopedDataAccessError({ hook: 'beforeFind', object: 'customer', event: 'beforeFind' }), 403],
   ['MultiUpdateHookKeyDivergenceError', () => new MultiUpdateHookKeyDivergenceError('customer', ['a', 'b'], 2), 400],
   ['EmptyCredentialWriteError', () => new EmptyCredentialWriteError('datasource', 'secret_key', 'secret'), 400],
+  ['SecretRefWriteError', () => new SecretRefWriteError('datasource', 'secret_key'), 400],
   ['SystemWriteOrganizationRequiredError', () => new SystemWriteOrganizationRequiredError('sys_job', 'group', 'walled-posture'), 500],
   ['NamespaceConflictError', () => new NamespaceConflictError('crm', 'pkg_a', 'pkg_b'), 422],
   ['ArtifactObjectNameConflictError', () => new ArtifactObjectNameConflictError('customer', 'pkg_a', 'pkg_b'), 422],