Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/21787-write-response-credential-mask.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@objectstack/core': patch
'@objectstack/runtime': patch
---

Credential-class field values are now masked on every write response, as on reads.

Clause-②: no

- A `secret` field, and a `password` field on an object that is not `managedBy: 'better-auth'`, already read back as `SECRET_MASK` (`null` when unset) on the generic read path (ADR-0100). Every write response that returns a record (REST, batch and MCP) now answers the same way.
- The shared write-response helper every write door already calls (`omitInternalFieldsFromWriteResponse`, `@objectstack/core`) now applies the credential mask before it omits `internal: true` fields. New exports beside it: `maskCredentialFieldsInWriteResponse` and `collectCredentialWriteResponseFields`, which read the same `isMaskedOnReadFieldType` declaration as the engine's read mask.
- `callData`'s fallback create and update arms (`@objectstack/runtime`, used when no protocol service is registered) now pass their response record through the same helper.
- Unchanged: the engine's own write results still return the stored row whole to privileged server-side callers, and the echoed-mask write guard still treats a `SECRET_MASK` value as "leave unchanged", so a client that saves back a write response does not overwrite the stored credential.
76 changes: 76 additions & 0 deletions packages/core/src/utils/internal-write-response.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { SECRET_MASK } from '@objectstack/spec/data';
import {
collectCredentialWriteResponseFields,
maskCredentialFieldsInWriteResponse,
omitInternalFieldsFromWriteResponse,
} from './internal-write-response.js';

const STORED_PASSWORD = 'stored-password-value-never-returned';
const STORED_REF = 'secret:handle-never-returned';

const SCHEMA = {
name: 'cred_holder',
fields: {
id: { type: 'text' },
name: { type: 'text' },
login_password: { type: 'password' },
api_token: { type: 'secret' },
hidden_hash: { type: 'text', internal: true },
both: { type: 'secret', internal: true },
},
};

const row = () => ({
id: 'r1',
name: 'visible',
login_password: STORED_PASSWORD,
api_token: STORED_REF,
hidden_hash: 'h',
both: 'secret:x',
});

describe('write-response credential mask', () => {
it('collects the read-mask set: secret always, password outside the exempt bucket', () => {
expect(collectCredentialWriteResponseFields(SCHEMA).sort()).toEqual(['api_token', 'both', 'login_password']);
expect(collectCredentialWriteResponseFields({ ...SCHEMA, managedBy: 'better-auth' }).sort())
.toEqual(['api_token', 'both']);
expect(collectCredentialWriteResponseFields(undefined)).toEqual([]);
expect(collectCredentialWriteResponseFields({ name: 'x' })).toEqual([]);
});

it('masks a set credential, keeps an unset one null, never adds a key', () => {
const r: Record<string, unknown> = { id: 'r1', login_password: STORED_PASSWORD, api_token: null };
maskCredentialFieldsInWriteResponse(SCHEMA, r);
expect(r).toEqual({ id: 'r1', login_password: SECRET_MASK, api_token: null });
// Idempotent.
maskCredentialFieldsInWriteResponse(SCHEMA, r);
expect(r).toEqual({ id: 'r1', login_password: SECRET_MASK, api_token: null });
});

it('the shared write-response helper masks credentials and omits internal fields, on every row', () => {
const rows = [row(), null, 7, row()];
omitInternalFieldsFromWriteResponse(SCHEMA, rows);
for (const r of [rows[0], rows[3]] as Array<Record<string, unknown>>) {
expect(r).toEqual({ id: 'r1', name: 'visible', login_password: SECRET_MASK, api_token: SECRET_MASK });
}
const wire = JSON.stringify(rows);
expect(wire.includes(STORED_PASSWORD)).toBe(false);
expect(wire.includes('secret:')).toBe(false);
});

it('a field removed upstream (field-level security) stays absent', () => {
const r: Record<string, unknown> = { id: 'r1', name: 'visible' };
omitInternalFieldsFromWriteResponse(SCHEMA, r);
expect(r).toEqual({ id: 'r1', name: 'visible' });
});

it('a better-auth object keeps its password column, still masks its secret column', () => {
const r = row();
omitInternalFieldsFromWriteResponse({ ...SCHEMA, managedBy: 'better-auth' }, r);
expect(r.login_password).toBe(STORED_PASSWORD);
expect(r.api_token).toBe(SECRET_MASK);
});
});
79 changes: 75 additions & 4 deletions packages/core/src/utils/internal-write-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,29 @@
* columns are `required`, so a mask carries zero bits while still shipping a
* value under a field whose description promises none.
*
* ## The credential-class mask rides the same helper
*
* The engine masks credential-class fields (`secret`, and `password` outside
* the exempt `managedBy` buckets — ADR-0100) on its READ path only, and keeps
* its write results whole for the same reason it keeps `internal` columns:
* a privileged server-side writer may read the stored value back off the
* result. Every external write response therefore owes the credential mask
* too, at the same mouths. It is applied by the same helper, ahead of the
* omission, so no mouth can hold one guarantee and miss the other, and the
* three tripwires named above hold both by one enumeration.
*
* Deletion is IN PLACE and idempotent: records that already lack the field
* (a re-stripped read result, a fake engine that never returned it) pass
* through unchanged, and non-record values (`null`, an affected-row count, a
* driver's boolean delete verdict) are skipped rather than judged.
*/

import { SECRET_MASK, isMaskedOnReadFieldType } from '@objectstack/spec/data';

/** Minimal view of an object schema this module reads — the field map only. */
interface SchemaWithFields {
fields?: Record<string, { internal?: unknown } | undefined> | undefined;
fields?: Record<string, { internal?: unknown; type?: unknown } | undefined> | undefined;
managedBy?: unknown;
}

/**
Expand All @@ -118,9 +132,65 @@ export function collectInternalWriteResponseFields(schema: unknown): string[] {
}

/**
* Drop every `internal: true` field from a write response's record(s), in
* place. THE single helper every external write mouth goes through — see the
* module header; the three tripwires enforce the "every".
* Collect the names of the credential-class fields the engine masks on read
* (ADR-0100: every `secret` field, every `password` field outside the exempt
* `managedBy` buckets) — the write-response half of that mask reads the SAME
* set, so a write answers what a read of the same row would.
*
* Not restated: the type set and its per-type `managedBy` exemptions are
* declared once in `@objectstack/spec/data` and asked through
* `isMaskedOnReadFieldType`, exactly as objectql's `collectMaskedReadFields`
* asks it. ⛔ Do not add a `def.type === …` arm here; change the declaration.
*/
export function collectCredentialWriteResponseFields(schema: unknown): string[] {
const s = schema as SchemaWithFields | null | undefined;
const fields = s?.fields;
if (!fields || typeof fields !== 'object') return [];
const managedBy: unknown = s?.managedBy;
const out: string[] = [];
for (const [name, def] of Object.entries(fields)) {
if (def && isMaskedOnReadFieldType(def.type, managedBy)) out.push(name);
}
return out;
}

/**
* Replace every credential-class field in a write response's record(s) with
* `SECRET_MASK`, in place — the write-response mirror of the engine's read
* mask (`maskSecretFields`). A set value becomes the mask; an unset one
* (`null` / `undefined`) becomes `null`, so "a credential is set" is the only
* bit a response carries. A field the record does not carry (never written, or
* already removed by field-level security upstream) stays absent: this mask
* never ADDS a key, so it composes with the security plugin's field masking
* instead of re-exposing what that removed.
*
* Idempotent; non-objects are skipped, arrays are walked.
*/
export function maskCredentialFieldsInWriteResponse(schema: unknown, records: unknown): void {
if (!records) return;
const credentialFields = collectCredentialWriteResponseFields(schema);
if (credentialFields.length === 0) return;
const list = Array.isArray(records) ? records : [records];
for (const row of list) {
if (!row || typeof row !== 'object') continue;
const r = row as Record<string, unknown>;
for (const field of credentialFields) {
if (!(field in r)) continue;
r[field] = r[field] == null ? null : SECRET_MASK;
}
}
}

/**
* Apply the generic-data-path non-exposure rules to a write response's
* record(s), in place: credential-class fields are MASKED
* ({@link maskCredentialFieldsInWriteResponse}), then `internal: true` fields
* are OMITTED. THE single helper every external write mouth goes through —
* see the module header; the three tripwires enforce the "every".
*
* Mask first, omit second — the engine read path's order, so a field that is
* both credential-typed and `internal` ends up omitted (the stricter
* disposition wins).
*
* @param schema The registered object schema (`engine.registry.getObject(...)`
* / the protocol's own registry view / `metadataService
Expand All @@ -133,6 +203,7 @@ export function collectInternalWriteResponseFields(schema: unknown): string[] {
*/
export function omitInternalFieldsFromWriteResponse(schema: unknown, records: unknown): void {
if (!records) return;
maskCredentialFieldsInWriteResponse(schema, records);
const internalFields = collectInternalWriteResponseFields(schema);
if (internalFields.length === 0) return;
const list = Array.isArray(records) ? records : [records];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFin
const SENTINEL = 'INTERNAL-SENTINEL-8497-NEVER-SERIALIZED';
/** The value that MUST appear wherever a record was promised (falsifiability). */
const CONTROL = 'CONTROL-VALUE-8497-RECORD-FLOWED';
/** Credential-class stored values: a `password` plaintext and a `secret` handle ref. */
const CREDENTIAL_SENTINELS = ['PASSWORD-SENTINEL-NEVER-SERIALIZED', 'secret:HANDLE-SENTINEL-NEVER-SERIALIZED'];
const NEVER_SERIALIZED = [SENTINEL, ...CREDENTIAL_SENTINELS];
const leaks = (wire: string) => NEVER_SERIALIZED.filter((v) => wire.includes(v));

const VAULT = {
name: 'vault',
Expand All @@ -60,6 +64,8 @@ const VAULT = {
id: { name: 'id', type: 'text' },
name: { name: 'name', type: 'text' },
vault_secret: { name: 'vault_secret', type: 'text', internal: true },
vault_password: { name: 'vault_password', type: 'password' },
vault_token: { name: 'vault_token', type: 'secret' },
},
// No `apiEnabled`/`apiMethods` narrowing: the ADR-0049 exposure gate must let
// every verb through, or a recipe would be measuring a 404 instead of a body.
Expand All @@ -78,6 +84,8 @@ function makeSentinelEngine(): IDataEngine {
id,
name: (data?.name as string) ?? CONTROL,
vault_secret: SENTINEL,
vault_password: CREDENTIAL_SENTINELS[0],
vault_token: CREDENTIAL_SENTINELS[1],
});
return {
find: vi.fn(async () => [storedRow()]),
Expand Down Expand Up @@ -157,7 +165,7 @@ const RECIPES: Record<string, Recipe> = {
remove: { invoke: (b) => b.remove('vault', 'row-1'), writesRecords: false },
};

describe('#8497 tripwire: no MCP write response carries an `internal: true` value', () => {
describe('#8497 tripwire: no MCP write response carries an `internal: true` value or a credential-class stored value', () => {
it('the enumeration is real: it sees the bridge write verbs', () => {
const faces = enumerateBridgeFaces(makeBridge());
expect(faces).toEqual(expect.arrayContaining(['create', 'update', 'remove']));
Expand All @@ -181,10 +189,10 @@ describe('#8497 tripwire: no MCP write response carries an `internal: true` valu
});

for (const [name, recipe] of Object.entries(RECIPES)) {
it(`${name}: response never carries the internal sentinel${recipe.writesRecords ? ', and really returned a record' : ''}`, async () => {
it(`${name}: response never carries the internal or credential sentinels${recipe.writesRecords ? ', and really returned a record' : ''}`, async () => {
const bridge = makeBridge();
const wire = JSON.stringify((await recipe.invoke(bridge)) ?? null);
expect(wire.includes(SENTINEL), `${name} leaked an internal field: ${wire}`).toBe(false);
expect(leaks(wire), `${name} leaked an internal or credential-class field: ${wire}`).toEqual([]);
if (recipe.writesRecords) {
expect(
wire.includes(CONTROL),
Expand All @@ -207,6 +215,16 @@ describe('#8497 tripwire: no MCP write response carries an `internal: true` valu
expect(wire.includes(CONTROL)).toBe(true); // still a real record echo
});

it('the caller cannot read their own credential write back in clear from the update echo', async () => {
const bridge = makeBridge();
const wire = JSON.stringify(await bridge.update('vault', 'row-1', {
name: CONTROL,
vault_password: 'caller-sent-password',
}));
expect(wire.includes('caller-sent-password')).toBe(false);
expect(wire.includes(CONTROL)).toBe(true);
});

it('NEGATIVE CONTROL: the machinery goes red on a write mouth that skips the helper', async () => {
// Exactly the defect this file was written after: an engine-only mouth that
// echoes `engine.insert`'s (whole) result. Reintroduce it locally and prove
Expand All @@ -221,10 +239,10 @@ describe('#8497 tripwire: no MCP write response carries an `internal: true` valu
};

const leaked = await leaky.create('vault', { name: CONTROL });
expect(JSON.stringify(leaked).includes(SENTINEL)).toBe(true); // the scan bites
expect(leaks(JSON.stringify(leaked))).toEqual(NEVER_SERIALIZED); // the scan bites

omitInternalFieldsFromWriteResponse(VAULT, (leaked as any).record);
expect(JSON.stringify(leaked).includes(SENTINEL)).toBe(false); // the helper closes it
expect(leaks(JSON.stringify(leaked))).toEqual([]); // the helper closes it
expect(JSON.stringify(leaked).includes(CONTROL)).toBe(true); // …without eating the record
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,24 @@ import {
const SENTINEL = 'INTERNAL-SENTINEL-7823-NEVER-SERIALIZED';
/** The value that MUST appear wherever a record was promised (falsifiability). */
const CONTROL = 'CONTROL-VALUE-7823-RECORD-FLOWED';
/**
* Credential-class stored values (a `password` field's plaintext and a
* `secret` field's handle ref) — masked on read by the engine, and owed the
* same mask on every write response by the shared helper.
*/
const CREDENTIAL_SENTINELS = ['PASSWORD-SENTINEL-NEVER-SERIALIZED', 'secret:HANDLE-SENTINEL-NEVER-SERIALIZED'];
/** Every stored value no write response may carry. */
const NEVER_SERIALIZED = [SENTINEL, ...CREDENTIAL_SENTINELS];
const leaks = (wire: string) => NEVER_SERIALIZED.filter((v) => wire.includes(v));

const VAULT_SCHEMA = {
name: 'vault',
fields: {
id: { name: 'id', type: 'text' },
name: { name: 'name', type: 'text' },
vault_secret: { name: 'vault_secret', type: 'text', internal: true },
vault_password: { name: 'vault_password', type: 'password' },
vault_token: { name: 'vault_token', type: 'secret' },
},
enable: { clone: true },
};
Expand All @@ -82,6 +93,8 @@ function makeSentinelEngine() {
id,
name: (data as any)?.name ?? CONTROL,
vault_secret: SENTINEL,
vault_password: CREDENTIAL_SENTINELS[0],
vault_token: CREDENTIAL_SENTINELS[1],
});
let nextId = 1;
const handle = { id: 'trx-1' };
Expand Down Expand Up @@ -241,7 +254,7 @@ const RECIPES: Record<string, Recipe> = {
runAtomicBatchData: { coveredVia: 'batchData' },
};

describe('#7823 tripwire: every generic data ingress strips `internal: true` from its write response', () => {
describe('#7823 tripwire: every generic data ingress strips `internal: true` and masks credential-class fields in its write response', () => {
const enumerated = enumerateDataMethods(ObjectStackProtocolImplementation.prototype);

it('the enumeration is real: it sees the three ruling-named ingresses', () => {
Expand Down Expand Up @@ -276,12 +289,12 @@ describe('#7823 tripwire: every generic data ingress strips `internal: true` fro
for (const name of enumerated) {
const recipe = RECIPES[name];
if (!recipe || 'coveredVia' in recipe) continue;
it(`${name}: response never carries the internal sentinel${recipe.expectRecord ? ', and really returned a record' : ''}`, async () => {
it(`${name}: response never carries the internal or credential sentinels${recipe.expectRecord ? ', and really returned a record' : ''}`, async () => {
for (const invoke of recipe.invocations) {
const p = new ObjectStackProtocolImplementation(makeSentinelEngine());
const response = await invoke(p);
const wire = JSON.stringify(response ?? null);
expect(wire.includes(SENTINEL), `${name} leaked an internal field: ${wire}`).toBe(false);
expect(leaks(wire), `${name} leaked an internal or credential-class field: ${wire}`).toEqual([]);
if (recipe.expectRecord) {
expect(wire.includes(CONTROL), `${name} returned no record at all — the probe is blind: ${wire}`).toBe(true);
}
Expand All @@ -305,12 +318,12 @@ describe('#7823 tripwire: every generic data ingress strips `internal: true` fro

const p = new LeakyProtocol(makeSentinelEngine());
const wire = JSON.stringify(await p.leakyData({ object: 'vault', id: 'row-1', data: { name: 'x' } }));
expect(wire.includes(SENTINEL)).toBe(true); // half 2: the scan detects the leak
expect(leaks(wire)).toEqual(NEVER_SERIALIZED); // half 2: the scan detects every leak

// And the helper is exactly what closes it — same response, one call.
const fixed = await p.leakyData({ object: 'vault', id: 'row-1', data: { name: 'x' } });
omitInternalFieldsFromWriteResponse(VAULT_SCHEMA, (fixed as any).record);
expect(JSON.stringify(fixed).includes(SENTINEL)).toBe(false);
expect(leaks(JSON.stringify(fixed))).toEqual([]);
});

it('the collector agrees with the engine rule: strict `internal === true` only', () => {
Expand Down
Loading
Loading