diff --git a/.changeset/27582-secret-field-own-ref-only.md b/.changeset/27582-secret-field-own-ref-only.md new file mode 100644 index 00000000000..d1d76f4304f --- /dev/null +++ b/.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 + + + +**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. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 8e75e9ed503..1afe01b4b54 100644 --- a/content/docs/permissions/system-context.mdx +++ b/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 | diff --git a/packages/objectql/src/engine-secret-internal-fields.ts b/packages/objectql/src/engine-secret-internal-fields.ts index 621c5ec3c95..8796f407712 100644 --- a/packages/objectql/src/engine-secret-internal-fields.ts +++ b/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,6 +112,53 @@ 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 | 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, @@ -113,10 +166,12 @@ export async function encryptSecretFields( row: Record, context: ExecutionContext | undefined, driverOptions: unknown, + storedRows: StoredRowsReader, ): Promise { const pending: PendingSecretRow[] = []; if (!row || typeof row !== 'object') return pending; const schema = this._registry.getObject(object); + let ownRows: readonly (Record | 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( diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 68aa844759f..0af248405c8 100644 --- a/packages/objectql/src/engine.ts +++ b/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, context: ExecutionContext | undefined, driverOptions: unknown, + storedRows: StoredRowsReader, ): Promise { // [#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); - sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record, 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, 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); - sealedSecrets = await this.encryptSecretFields(object, hookContext.input.data as Record, 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, 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 diff --git a/packages/objectql/src/error-http-status-spelling.test.ts b/packages/objectql/src/error-http-status-spelling.test.ts index ae9f61140b2..65acd022bdb 100644 --- a/packages/objectql/src/error-http-status-spelling.test.ts +++ b/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], diff --git a/packages/objectql/src/secret-field-own-ref-write.test.ts b/packages/objectql/src/secret-field-own-ref-write.test.ts new file mode 100644 index 00000000000..6b825ebe586 --- /dev/null +++ b/packages/objectql/src/secret-field-own-ref-write.test.ts @@ -0,0 +1,321 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27582] A non-system write may only seal a new value or keep its own + * record's ref. + * + * A `secret` field's stored value is an opaque `secret:` ref. On a write that + * is not the system principal's, a value already spelled as a ref is accepted + * only when it is the ref that same record's same column already holds, and + * then it changes nothing (like the echoed read mask). Any other ref is + * refused with the ADR-0112 `VALIDATION_ERROR` / 400 pair, before any + * credential row or record is written, and with one answer whatever the ref + * names. A system write keeps the pass-through. + * + * Driven through a real {@link ObjectQL} over an in-memory stub driver, on + * every engine write door the credential pass sits on: insert (single, batch + * and the per-row `insertMany`), the by-id update and the predicate update. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import type { ICryptoProvider, CryptoHandle } from '@objectstack/spec/contracts'; +import { ObjectQL } from './engine.js'; +import { SECRET_MASK, makeSecretRef, parseSecretRef } from './secret-fields.js'; + +type Row = Record; + +/** An in-memory stub driver whose stores can be read back directly. */ +function makeDriver() { + const stores = new Map>(); + const storeFor = (o: string) => stores.get(o) ?? (stores.set(o, new Map()), stores.get(o)!); + const matches = (row: Row, where: any) => + !where || Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) return true; + if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(row[k]); + return (row[k] ?? null) === (v ?? null); + }); + const copy = (r: Row | undefined) => (r == null ? r : { ...r }); + /** The payload of every record statement, in order: which columns a write actually rewrote. */ + const statements: Array<{ object: string; data: Row }> = []; + let n = 0; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async syncSchema() {}, + async find(o: string, ast: any) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + const page = typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + return page.map(copy); + }, + async findOne(o: string, ast: any) { return (await this.find(o, ast))[0] ?? null; }, + async create(o: string, data: Row) { + const row = { ...data, id: (data.id as string) ?? `${o}_${++n}` }; + storeFor(o).set(String(row.id), row); + return copy(row); + }, + async update(o: string, id: string, data: Row) { + statements.push({ object: o, data: { ...data } }); + const row = { ...storeFor(o).get(String(id)), ...data, id }; + storeFor(o).set(String(id), row); + return copy(row); + }, + async updateMany(o: string, ast: any, data: Row) { + statements.push({ object: o, data: { ...data } }); + let k = 0; + for (const [id, row] of storeFor(o)) { + if (!matches(row, ast?.where)) continue; + storeFor(o).set(id, { ...row, ...data, id }); + k += 1; + } + return k; + }, + async delete(o: string, id: string) { return storeFor(o).delete(String(id)); }, + async count(o: string, ast: any) { return (await this.find(o, ast)).length; }, + async bulkCreate(o: string, rows: Row[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return {}; }, async commit() {}, async rollback() {}, + }; + return { + driver, + rowsOf: (o: string) => Array.from(storeFor(o).values()), + lastStatement: (o: string) => [...statements].reverse().find((s) => s.object === o)?.data, + }; +} + +function makeCrypto(): ICryptoProvider & { encrypts: () => number } { + let n = 0; + return { + encrypts: () => n, + async encrypt(plain: string): Promise { + n += 1; + return { id: `sec_${n}`, kmsKeyId: 'local', alg: 'test-b64', version: 1, ciphertext: Buffer.from(plain).toString('base64') }; + }, + async decrypt(handle: CryptoHandle) { return Buffer.from(handle.ciphertext, 'base64').toString('utf8'); }, + async rotateKey(handle: CryptoHandle) { return handle; }, + digest(plain: string) { return `d:${plain.length}`; }, + async keyedDigest(plain: string) { return `k:${plain.length}`; }, + }; +} + +const sysSecret = { + name: 'sys_secret', label: 'Secret', systemFields: { tenant: false }, + fields: { + id: { name: 'id', type: 'text' }, namespace: { name: 'namespace', type: 'text' }, key: { name: 'key', type: 'text' }, + kms_key_id: { name: 'kms_key_id', type: 'text' }, alg: { name: 'alg', type: 'text' }, + version: { name: 'version', type: 'number' }, ciphertext: { name: 'ciphertext', type: 'text' }, + created_at: { name: 'created_at', type: 'datetime' }, + }, +}; +const VAULT = 'qa_ref_vault'; +const vault = { + name: VAULT, label: 'Vault', + fields: { + id: { name: 'id', type: 'text' }, + name: { name: 'name', type: 'text' }, + tier: { name: 'tier', type: 'text' }, + token: { name: 'token', type: 'secret' }, + }, +}; + +const SYSTEM = { context: { isSystem: true } } as any; +const USER = { context: { isSystem: false, userId: 'u1' } } as any; +/** A ref spelled like a stored one, naming no `sys_secret` row at all. */ +const UNKNOWN_REF = makeSecretRef('sec_none'); + +describe('[#27582] a non-system write may only seal a new value or keep its own record\'s ref', () => { + let engine: ObjectQL; + let d: ReturnType; + let crypto: ReturnType; + + const secrets = () => d.rowsOf('sys_secret'); + const stored = (id: string) => d.rowsOf(VAULT).find((r) => r.id === id) as Row | undefined; + const refOf = (id: string) => stored(id)!.token as string; + const seed = async (id: string, token: string, tier = 'gold') => { + await engine.insert(VAULT, { id, name: id, tier, token }, SYSTEM); + return refOf(id); + }; + /** The refusal envelope: the ADR-0112 pair and the located column. */ + const refusal = (field = 'token') => ({ code: 'VALIDATION_ERROR', status: 400, httpStatus: 400, object: VAULT, field }); + const caught = (p: Promise) => p.then(() => null, (e: unknown) => e as Error & Row); + + beforeEach(async () => { + engine = new ObjectQL(); + d = makeDriver(); + engine.registerDriver(d.driver, true); + await engine.init(); + engine.registry.registerObject(sysSecret as any); + engine.registry.registerObject(vault as any); + crypto = makeCrypto(); + engine.setCryptoProvider(crypto); + }); + + describe('create: a new record has no ref of its own', () => { + it('a create carrying another record\'s ref is refused before anything is written', async () => { + const theirs = await seed('r1', 'theirs'); + const before = { secrets: secrets().length, encrypts: crypto.encrypts() }; + const err = await caught(engine.insert(VAULT, { name: 'mine', token: theirs }, USER)); + expect(err).toMatchObject(refusal()); + expect(d.rowsOf(VAULT).map((r) => r.id)).toEqual(['r1']); + expect(secrets().length).toBe(before.secrets); + expect(crypto.encrypts()).toBe(before.encrypts); + }); + + it('a ref that names an existing row and one that names nothing get the same answer', async () => { + const theirs = await seed('r1', 'theirs'); + const named = await caught(engine.insert(VAULT, { name: 'a', token: theirs }, USER)); + const unknown = await caught(engine.insert(VAULT, { name: 'b', token: UNKNOWN_REF }, USER)); + expect(named).toMatchObject(refusal()); + expect(unknown).toMatchObject(refusal()); + expect(unknown!.message).toBe(named!.message); + expect(named!.message).not.toContain(parseSecretRef(theirs)!); + expect(d.rowsOf(VAULT)).toHaveLength(1); + }); + + it('a create with no context is not the system principal', async () => { + const theirs = await seed('r1', 'theirs'); + expect(await caught(engine.insert(VAULT, { name: 'a', token: theirs }))).toMatchObject(refusal()); + }); + + it('a batch create carrying a ref is refused whole', async () => { + const theirs = await seed('r1', 'theirs'); + const err = await caught(engine.insert(VAULT, [{ name: 'a', token: 'fresh' }, { name: 'b', token: theirs }], USER)); + expect(err).toMatchObject(refusal()); + expect(d.rowsOf(VAULT).map((r) => r.id)).toEqual(['r1']); + expect(secrets()).toHaveLength(1); + }); + + it('insertMany refuses that row alone and lands the others', async () => { + const theirs = await seed('r1', 'theirs'); + const outcomes = await engine.insertMany(VAULT, [{ name: 'a', token: 'fresh' }, { name: 'b', token: theirs }], USER); + expect(outcomes.map((o) => o.ok)).toEqual([true, false]); + expect((outcomes[1] as any).error).toMatchObject(refusal()); + const landed = d.rowsOf(VAULT).find((r) => r.name === 'a')!; + expect(await engine.resolveSecretField(VAULT, String(landed.id), 'token')).toBe('fresh'); + expect(d.rowsOf(VAULT).some((r) => r.name === 'b')).toBe(false); + }); + + it('a beforeInsert hook that writes a ref onto the row is judged the same way', async () => { + const theirs = await seed('r1', 'theirs'); + engine.registerHook('beforeInsert', (ctx: any) => { ctx.input.data.token = theirs; }, { object: VAULT }); + expect(await caught(engine.insert(VAULT, { name: 'a' }, USER))).toMatchObject(refusal()); + expect(d.rowsOf(VAULT)).toHaveLength(1); + }); + + it('a new value is still sealed, and the system principal keeps the pass-through', async () => { + const created = await engine.insert(VAULT, { name: 'a', token: 'fresh' }, USER); + expect(await engine.resolveSecretField(VAULT, String(created.id), 'token')).toBe('fresh'); + const theirs = await seed('r1', 'theirs'); + await engine.insert(VAULT, { id: 'copy', name: 'copy', token: theirs }, SYSTEM); + expect(refOf('copy')).toBe(theirs); + }); + }); + + describe('by-id update: only the ref this record already holds', () => { + it('keeping its own ref is accepted and changes nothing', async () => { + const own = await seed('r1', 'mine'); + const before = secrets().map((s) => s.id); + await engine.update(VAULT, { id: 'r1', name: 'renamed', token: own }, USER); + expect(stored('r1')).toMatchObject({ name: 'renamed', token: own }); + // Kept, not rewritten: the statement leaves the column alone, as for the echoed mask. + expect(d.lastStatement(VAULT)).toMatchObject({ name: 'renamed' }); + expect(d.lastStatement(VAULT)).not.toHaveProperty('token'); + expect(secrets().map((s) => s.id)).toEqual(before); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); + }); + + it('keeping its own ref as the only key is accepted too', async () => { + const own = await seed('r1', 'mine'); + await engine.update(VAULT, { id: 'r1', token: own }, USER); + expect(refOf('r1')).toBe(own); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); + }); + + it('the echoed read mask is still a no-op', async () => { + const own = await seed('r1', 'mine'); + await engine.update(VAULT, { id: 'r1', name: 'renamed', token: SECRET_MASK }, USER); + expect(refOf('r1')).toBe(own); + }); + + it('a beforeUpdate hook that writes the stored row back keeps its own ref', async () => { + const own = await seed('r1', 'mine'); + engine.registerHook('beforeUpdate', (ctx: any) => { + ctx.input.data = { ...ctx.previous, ...ctx.input.data }; + }, { object: VAULT }); + await engine.update(VAULT, { id: 'r1', name: 'renamed' }, USER); + expect(stored('r1')).toMatchObject({ name: 'renamed', token: own }); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); + }); + + it('another record\'s ref is refused and the stored value is untouched', async () => { + const own = await seed('r1', 'mine'); + const theirs = await seed('r2', 'theirs'); + const before = secrets().map((s) => s.id); + const err = await caught(engine.update(VAULT, { id: 'r1', name: 'renamed', token: theirs }, USER)); + expect(err).toMatchObject(refusal()); + expect(stored('r1')).toMatchObject({ name: 'r1', token: own }); + expect(secrets().map((s) => s.id)).toEqual(before); + expect(await engine.resolveSecretField(VAULT, 'r1', 'token')).toBe('mine'); + }); + + it('another record\'s ref and a ref naming nothing get the same answer', async () => { + await seed('r1', 'mine'); + const theirs = await seed('r2', 'theirs'); + const named = await caught(engine.update(VAULT, { id: 'r1', token: theirs }, USER)); + const unknown = await caught(engine.update(VAULT, { id: 'r1', token: UNKNOWN_REF }, USER)); + expect(named).toMatchObject(refusal()); + expect(unknown).toMatchObject(refusal()); + expect(unknown!.message).toBe(named!.message); + }); + + it('a record whose column is empty has no ref to keep', async () => { + await engine.insert(VAULT, { id: 'r1', name: 'r1' }, SYSTEM); + const theirs = await seed('r2', 'theirs'); + expect(await caught(engine.update(VAULT, { id: 'r1', token: theirs }, USER))).toMatchObject(refusal()); + expect(stored('r1')!.token ?? null).toBeNull(); + }); + + it('the system principal keeps the pass-through', async () => { + await seed('r1', 'mine'); + const theirs = await seed('r2', 'theirs'); + await engine.update(VAULT, { id: 'r1', token: theirs }, SYSTEM); + expect(refOf('r1')).toBe(theirs); + }); + }); + + describe('predicate update: only a ref every matched record already holds', () => { + const multi = (where: Row) => ({ ...USER, where, multi: true }); + + it('a ref every matched row already holds is accepted and changes nothing', async () => { + await seed('r1', 'one'); + await seed('r2', 'two'); + await engine.update(VAULT, { token: 'shared' }, { ...SYSTEM, where: { tier: 'gold' }, multi: true }); + const shared = refOf('r1'); + expect(refOf('r2')).toBe(shared); + await engine.update(VAULT, { name: 'renamed', token: shared }, multi({ tier: 'gold' })); + expect(d.rowsOf(VAULT).map((r) => [r.name, r.token])).toEqual([['renamed', shared], ['renamed', shared]]); + expect(d.lastStatement(VAULT)).not.toHaveProperty('token'); + expect(await engine.resolveSecretField(VAULT, 'r2', 'token')).toBe('shared'); + }); + + it('a ref one matched row does not hold is refused, and nothing is written', async () => { + const one = await seed('r1', 'one'); + const two = await seed('r2', 'two'); + const err = await caught(engine.update(VAULT, { name: 'renamed', token: one }, multi({ tier: 'gold' }))); + expect(err).toMatchObject(refusal()); + expect(d.rowsOf(VAULT).map((r) => [r.name, r.token])).toEqual([['r1', one], ['r2', two]]); + }); + + it('a ref on a predicate that matches no row is refused', async () => { + const one = await seed('r1', 'one'); + const err = await caught(engine.update(VAULT, { token: one }, multi({ tier: 'silver' }))); + expect(err).toMatchObject(refusal()); + }); + + it('the system principal keeps the pass-through', async () => { + const one = await seed('r1', 'one'); + await seed('r2', 'two', 'silver'); + await engine.update(VAULT, { token: one }, { ...SYSTEM, where: { tier: 'silver' }, multi: true }); + expect(refOf('r2')).toBe(one); + }); + }); +}); diff --git a/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts b/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts new file mode 100644 index 00000000000..8371108c030 --- /dev/null +++ b/packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#27582] A non-system write may only seal a new value or keep its own +// record's ref — pinned at the REST data door of a booted stack with a crypto +// provider wired (`bootStack` wires one LocalCryptoProvider, as +// `objectstack dev` / `serve` do). Every HTTP request is assembled as a +// non-system caller. `packages/objectql/src/secret-field-own-ref-write.test.ts` +// drives each engine write door; this file holds the same rule through the +// doors a client writes through: +// +// - a create whose `secret` field holds a ref is refused (`VALIDATION_ERROR` / +// 400), on the single-record door, the bulk door and the import door; +// - an update that keeps the record's own value is accepted and changes +// nothing — the read mask the form echoes, and the record's own ref; +// - an update naming any other ref is refused with one answer, whatever that +// ref names, and the stored value stays; +// - the system principal keeps the pass-through. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { SECRET_MASK } from '@objectstack/objectql'; + +const VAULT = 'sor_vault'; + +const sorStack = defineStack({ + manifest: { + id: 'com.dogfood.secret-field-own-ref-write', namespace: 'sor', version: '0.0.0', type: 'app', + name: 'Secret field own-ref fixture', description: 'A secret field beside a name.', + }, + objects: [ + ObjectSchema.create({ + name: VAULT, label: 'Vault', pluralLabel: 'Vaults', sharingModel: 'public_read_write', + fields: { + name: Field.text({ label: 'Name', required: true }), + token: Field.secret({ label: 'Token' }), + }, + }), + ], +} as never); + +type Answer = { status: number; json: any; text: string }; + +describe('[#27582] secret field: a non-system write may only seal a new value or keep its own record\'s ref — over REST', () => { + let stack: VerifyStack; + let ql: any; + let token = ''; + + const call = async (method: string, path: string, body?: unknown): Promise => { + const res = await stack.apiAs(token, method, path, body); + const text = await res.text(); + let json: any = {}; + try { json = JSON.parse(text); } catch { /* not JSON */ } + return { status: res.status, json, text }; + }; + /** The value the business row holds, as the driver stores it. */ + const storedRef = async (id: string) => { + const [row] = await ql.getDriver(VAULT).find(VAULT, { where: { id } }); + return row?.token as string | null | undefined; + }; + const secretRowCount = async () => ((await ql.getDriver('sys_secret').find('sys_secret', {})) ?? []).length; + const create = async (name: string, value: string) => { + const res = await call('POST', `/data/${VAULT}`, { name, token: value }); + expect(res.status, res.text).toBe(201); + const [row] = await stack.rows(VAULT, { name }); + return String(row.id); + }; + /** The parts of a refusal a client can branch on or read. */ + const answerOf = (a: Answer) => ({ status: a.status, code: a.json.code, error: a.json.error, message: a.json.message }); + + let otherId = ''; + let otherRef = ''; + const unknownRef = 'secret:sor_handle_names_no_row'; + + beforeAll(async () => { + stack = await bootStack(sorStack); + token = await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + otherId = await create('sor-other', 'sor-other-value'); + otherRef = String(await storedRef(otherId)); + expect(otherRef).toMatch(/^secret:/); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('a create whose secret field holds a ref is refused before anything is written', async () => { + const before = await secretRowCount(); + const refused = await call('POST', `/data/${VAULT}`, { name: 'sor-create', token: otherRef }); + expect(refused.status, refused.text).toBe(400); + expect(refused.json.code, refused.text).toBe('VALIDATION_ERROR'); + expect(refused.text).not.toContain(otherRef); + expect(await stack.rows(VAULT, { name: 'sor-create' })).toHaveLength(0); + expect(await secretRowCount()).toBe(before); + }); + + it('the bulk create door and the import door refuse that row too', async () => { + const bulk = await call('POST', `/data/${VAULT}/createMany`, [{ name: 'sor-bulk', token: otherRef }]); + expect(bulk.status, bulk.text).toBe(400); + expect(bulk.json.code, bulk.text).toBe('VALIDATION_ERROR'); + expect(await stack.rows(VAULT, { name: 'sor-bulk' })).toHaveLength(0); + + const imported = await call('POST', `/data/${VAULT}/import`, { format: 'json', rows: [{ name: 'sor-import', token: otherRef }] }); + expect(imported.status, imported.text).toBe(200); + expect(imported.json.created, imported.text).toBe(0); + expect(imported.json.errors, imported.text).toBe(1); + expect(await stack.rows(VAULT, { name: 'sor-import' })).toHaveLength(0); + }); + + it('an update that echoes the read mask, or the record\'s own ref, is accepted and changes nothing', async () => { + const id = await create('sor-own', 'sor-own-value'); + const own = String(await storedRef(id)); + const before = await secretRowCount(); + + const read = await call('GET', `/data/${VAULT}/${id}`); + const echoed = (read.json.record ?? read.json).token; + expect(echoed).toBe(SECRET_MASK); + const masked = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-own-renamed', token: echoed }); + expect(masked.status, masked.text).toBe(200); + + const kept = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-own-again', token: own }); + expect(kept.status, kept.text).toBe(200); + + expect(await storedRef(id)).toBe(own); + expect(await secretRowCount()).toBe(before); + expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sor-own-value'); + expect((await stack.rows(VAULT, { id }))[0].name).toBe('sor-own-again'); + }); + + it('an update naming any other ref is refused with one answer, and the stored value stays', async () => { + const id = await create('sor-keep', 'sor-keep-value'); + const own = String(await storedRef(id)); + const before = await secretRowCount(); + + const named = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-keep-renamed', token: otherRef }); + expect(named.status, named.text).toBe(400); + expect(named.json.code, named.text).toBe('VALIDATION_ERROR'); + expect(named.text).not.toContain(otherRef); + + const unknown = await call('PATCH', `/data/${VAULT}/${id}`, { name: 'sor-keep-renamed', token: unknownRef }); + expect(answerOf(unknown)).toEqual(answerOf(named)); + + expect(await storedRef(id)).toBe(own); + expect(await secretRowCount()).toBe(before); + expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sor-keep-value'); + expect((await stack.rows(VAULT, { id }))[0].name).toBe('sor-keep'); + }); + + it('a new value still seals, and the system principal keeps the pass-through', async () => { + const id = await create('sor-new', 'sor-first'); + const replaced = await call('PATCH', `/data/${VAULT}/${id}`, { token: 'sor-second' }); + expect(replaced.status, replaced.text).toBe(200); + expect(await ql.resolveSecretField(VAULT, id, 'token')).toBe('sor-second'); + + const copy = await ql.insert(VAULT, { name: 'sor-system-copy', token: otherRef }, { context: { isSystem: true } }); + expect(await storedRef(String(copy.id))).toBe(otherRef); + await ql.update(VAULT, { id: String(copy.id), token: null }, { context: { isSystem: true } }); + expect(await ql.resolveSecretField(VAULT, otherId, 'token')).toBe('sor-other-value'); + }); +});