diff --git a/.changeset/secret-mask-single-source.md b/.changeset/secret-mask-single-source.md new file mode 100644 index 0000000000..fdb8072bf0 --- /dev/null +++ b/.changeset/secret-mask-single-source.md @@ -0,0 +1,53 @@ +--- +"@objectstack/spec": minor +"@objectstack/objectql": patch +"@objectstack/service-settings": patch +--- + +fix(spec): declare the eight-bullet credential read mask once, in spec (#7572) + +The string a client sees in place of a credential it may not read back was +declared **twice**, byte-identical by convention only: + +- `SECRET_MASK` in `@objectstack/objectql` — the encrypted-**field** read mask on + the generic CRUD path (ADR-0100 §A/§B); +- `SETTINGS_SECRET_MASK` in `@objectstack/service-settings` — the settings REST + read boundary, added by #7522. + +Nothing bound them. An edit to either literal would desynchronise the two masked +reads a console sees, and the break would be invisible from both sides: each +package asserted against its own copy, so both suites stay green while the two +surfaces disagree. That matters more than a cosmetic mismatch — the console +renders "configured vs not configured" from this value and echoes it back +unchanged on save, and both write paths read that echo as "unchanged" +(ADR-0100 §B3). A drifted mask silently turns an unchanged form round-trip into a +real write of the mask's literal text over a live credential. + +**What changed.** The mask is declared once, in `@objectstack/spec` — the +contract face both sides already depend on — as `SECRET_MASK` in +`spec/src/data/secret-mask.ts`, alongside the rest of the ADR-0100 surface +(`data/field.zod.ts`, `data/object.zod.ts`). Both readers now import that one +declaration: + +- `@objectstack/objectql` **re-exports** it, so its public API is byte-for-byte + unchanged — `SECRET_MASK` is still exported from the package root and from + `core`, with the same name, value and literal type. No consumer changes. +- `@objectstack/service-settings` aliases it as `SETTINGS_SECRET_MASK`, keeping + the name that package publishes and every existing import of it working. + +**The framework-agnostic property of the settings service is intact.** #7522 +declined to import the constant because reaching it meant depending on +`@objectstack/objectql`, the whole data engine — that reasoning was right and +still holds; no objectql import was added. It never applied to `@objectstack/spec`, +which is already a dependency of the package and already in its runtime graph +(`manifest.ts` → `@objectstack/platform-objects/system` → `@objectstack/spec/data`). + +**New public API:** `SECRET_MASK` on `@objectstack/spec/data`. Additive — nothing +was removed or renamed on any package. + +The literal keeps its deliberate spelling (eight U+2022 BULLETs written out, not +an escape or a `.repeat(8)`), so a grep for the mask a client actually received +still lands on the declaration; a source-level pin holds that, next to a byte pin +on the value. The far-side literal pins in `plugin-audit` and `driver-memory` are +deliberately left restating the mask — a pin whose job is to catch the constant +changing must not import the constant. diff --git a/packages/objectql/src/secret-fields.test.ts b/packages/objectql/src/secret-fields.test.ts index aad8ec0f5e..6de46a044b 100644 --- a/packages/objectql/src/secret-fields.test.ts +++ b/packages/objectql/src/secret-fields.test.ts @@ -13,6 +13,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { ObjectQL } from './engine.js'; import { SECRET_MASK, isSecretRef } from './secret-fields.js'; +import { SECRET_MASK as SPEC_SECRET_MASK } from '@objectstack/spec/data'; import type { ICryptoProvider, CryptoHandle, CryptoContext } from '@objectstack/spec/contracts'; // ---- minimal stub driver (equality-only WHERE) ---------------------------- @@ -146,6 +147,27 @@ async function buildEngine(withCrypto: boolean) { return { engine, stores, crypto, driver }; } +/** + * [#7572] This package no longer DECLARES the read mask — it re-exports the one + * `@objectstack/spec` declares, so the encrypted-field path and the settings + * REST path cannot serve two different masks. + * + * The pin restates the literal on purpose. Every assertion below compares an + * engine read against the imported `SECRET_MASK`, which stays green whatever + * that constant says; only a restated copy can catch the mask's bytes changing + * under this package, and only the identity check can catch the re-export being + * quietly replaced by a fresh local literal — the exact shape #7572 removed. + */ +describe('objectql SECRET_MASK re-export (#7572)', () => { + it('is the spec declaration, not a copy', () => { + expect(SECRET_MASK).toBe(SPEC_SECRET_MASK); + }); + + it('is the eight-bullet mask ADR-0100 pins', () => { + expect(SECRET_MASK).toBe('••••••••'); + }); +}); + describe('objectql secret-field channel', () => { let ctx: Awaited>; beforeEach(async () => { ctx = await buildEngine(true); }); diff --git a/packages/objectql/src/secret-fields.ts b/packages/objectql/src/secret-fields.ts index 15f743e254..5cf965a3ab 100644 --- a/packages/objectql/src/secret-fields.ts +++ b/packages/objectql/src/secret-fields.ts @@ -38,8 +38,17 @@ export const SECRET_REF_PREFIX = 'secret:'; * Value returned in place of a secret field on a normal read. Indicates * "a secret is set" without leaking the handle id or the plaintext. A field * with no stored secret resolves to `null` instead. + * + * Declared in `@objectstack/spec` and re-exported here (#7572), because the + * same mask is the contract on a second surface this package cannot see: the + * settings REST read boundary in `@objectstack/service-settings`, which is + * deliberately framework-agnostic and does not depend on objectql. Two + * byte-identical literals bound by convention were what #7572 removed — the + * re-export keeps this package's public API unchanged while leaving exactly one + * definition. ⛔ Do not restate the literal here; edit it in + * `spec/src/data/secret-mask.ts`, where it is pinned. */ -export const SECRET_MASK = '••••••••'; +export { SECRET_MASK } from '@objectstack/spec/data'; /** Wrap a `sys_secret` handle id as the opaque ref persisted on the row. */ export function makeSecretRef(handleId: string): string { diff --git a/packages/services/service-settings/src/settings-secret-redaction.test.ts b/packages/services/service-settings/src/settings-secret-redaction.test.ts new file mode 100644 index 0000000000..6922f86f93 --- /dev/null +++ b/packages/services/service-settings/src/settings-secret-redaction.test.ts @@ -0,0 +1,57 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7572] `SETTINGS_SECRET_MASK` is the ADR-0100 credential read mask under this + * package's published name — the SAME declaration, not a copy of it. + * + * #7522 shipped it as a second byte-identical literal, bound to objectql's + * `SECRET_MASK` by convention alone. Nothing compared them, and nothing could + * have: each package asserted against its own constant, so an edit to either + * literal would desynchronise the mask a console sees on the settings REST read + * from the mask it sees on an encrypted FIELD read, with both suites still green. + * + * The hoist into `@objectstack/spec` removed the second declaration. This pin + * holds what the hoist alone cannot: that this export still resolves to it. A + * re-introduced local literal — the natural next edit for anyone who reads the + * framework-agnostic note in the module header and stops there — passes every + * behaviour test in `settings-routes.test.ts` (those compare a route response + * against this very constant, so they move with it) and fails HERE the moment + * its bytes differ. + * + * The literal is restated below for the same reason the spec-side pin restates + * it: a check that imported the constant to describe the constant would be true + * by construction. The bytes and the grep-findable spelling are pinned at the + * declaration itself, in `spec/src/data/secret-mask.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; + +import { SECRET_MASK } from '@objectstack/spec/data'; + +import { SETTINGS_SECRET_MASK, dropEchoedSecretMasks, redactSecretValues } from './settings-secret-redaction.js'; + +describe('SETTINGS_SECRET_MASK is the shared ADR-0100 mask (#7572)', () => { + it('is the spec declaration, not a copy', () => { + expect(SETTINGS_SECRET_MASK).toBe(SECRET_MASK); + }); + + it('is the eight-bullet mask', () => { + expect(SETTINGS_SECRET_MASK).toBe('••••••••'); + }); + + it('serves the shared mask on read — a client comparing against the FIELD mask recognises it', () => { + const out = redactSecretValues( + { smtp_password: { value: 'hunter2', source: 'global', locked: false } }, + new Set(['smtp_password']), + ); + expect(out.smtp_password.value).toBe(SECRET_MASK); + }); + + it('reads an echo of the shared mask as "unchanged" — the write half of the same contract', () => { + // What a console echoes back is whatever it was SERVED, which is the shared + // constant; the drop has to key off that same string or the echo becomes a + // real write of eight bullets over a live secret. + const patch = dropEchoedSecretMasks({ smtp_password: SECRET_MASK }, new Set(['smtp_password'])); + expect(patch).toEqual({}); + }); +}); diff --git a/packages/services/service-settings/src/settings-secret-redaction.ts b/packages/services/service-settings/src/settings-secret-redaction.ts index 262ba08bf7..115ea2cdfe 100644 --- a/packages/services/service-settings/src/settings-secret-redaction.ts +++ b/packages/services/service-settings/src/settings-secret-redaction.ts @@ -20,9 +20,10 @@ * it is still an exposure, and the REST response is the one surface that should * never carry the cleartext. * - * The mask shape is NOT invented here — it mirrors the encrypted-**field** - * convention ADR-0100 pins for `secret` / `password` columns on the generic CRUD - * path (`SECRET_MASK` in `@objectstack/objectql`, exercised by the + * The mask shape is NOT invented here — it IS the encrypted-**field** convention + * ADR-0100 pins for `secret` / `password` columns on the generic CRUD path + * (`SECRET_MASK`, declared in `@objectstack/spec` and re-exported by + * `@objectstack/objectql`, exercised by the * `records-forms.encrypted-field-behavior` checklist item): * * - **read**: a set value becomes the mask; an unset one stays `null`, so the @@ -33,17 +34,21 @@ * key is DROPPED from the patch, so a form round-trip that echoes the mask * does not overwrite the stored secret with the mask's literal text. * - * The constant is redeclared rather than imported because this service is - * deliberately framework-agnostic (see the `settings-service.ts` header): it - * defines its own minimal `SettingsEngine` instead of importing `IDataEngine`, - * and does not depend on `@objectstack/objectql` at all. Taking a runtime - * dependency on the whole data engine to reach one string would undo that. The - * long-term fix is to hoist the mask into a package both sides already depend on - * (`@objectstack/spec`) and have objectql re-export it — recorded on #7522 as - * follow-up rather than done here, since it is a cross-package move on a - * security card. + * The mask itself is now IMPORTED, not redeclared (#7572). #7522 shipped a + * second byte-identical literal here and said why: this service is deliberately + * framework-agnostic (see the `settings-service.ts` header) — it defines its own + * minimal `SettingsEngine` instead of importing `IDataEngine`, and does not + * depend on `@objectstack/objectql` at all, so taking a runtime dependency on + * the whole data engine to reach one string was not worth it. That reasoning + * held against depending on **objectql**, and it still does. It does not apply + * to `@objectstack/spec`, which is where the mask now lives: spec is already a + * dependency of this package, and already in its runtime graph (`manifest.ts` + * → `@objectstack/platform-objects/system` → `@objectstack/spec/data`). The + * framework-agnostic property is untouched — no objectql import was added here + * or anywhere in this package. */ +import { SECRET_MASK } from '@objectstack/spec/data'; import type { ResolvedSettingValue } from '@objectstack/spec/system'; /** @@ -51,12 +56,14 @@ import type { ResolvedSettingValue } from '@objectstack/spec/system'; * is set" without leaking its cleartext; an unset secret resolves to `null` * instead, so set-vs-unset stays observable. * - * Byte-identical to `SECRET_MASK` in `@objectstack/objectql` (ADR-0100) — eight - * U+2022 BULLET characters — so one client-side comparison recognises a masked - * read from either surface. Spelled as the literal, not an escape, so a grep for - * the mask finds both declarations. + * The ADR-0100 credential read mask under the name this package publishes — the + * SAME declaration objectql re-exports as `SECRET_MASK`, not a copy of it + * (#7572), so one client-side comparison recognises a masked read from either + * surface and no edit can leave the two disagreeing. The literal, its eight + * U+2022 BULLET characters and its grep-findable spelling are pinned at the + * declaration in `@objectstack/spec` (`data/secret-mask.ts`). */ -export const SETTINGS_SECRET_MASK = '••••••••'; +export const SETTINGS_SECRET_MASK = SECRET_MASK; /** Mask one resolved value: the effective value AND every cascade entry. */ function maskResolved(resolved: ResolvedSettingValue): ResolvedSettingValue { diff --git a/packages/spec/api-surface/data.json b/packages/spec/api-surface/data.json index 7c34172f05..5db2c195e1 100644 --- a/packages/spec/api-surface/data.json +++ b/packages/spec/api-surface/data.json @@ -484,6 +484,7 @@ "SEARCH_AUTO_EXCLUDED_FIELDS (const)", "SEARCH_AUTO_EXCLUDED_TYPES (const)", "SEARCH_VIRTUAL_TYPES (const)", + "SECRET_MASK (const)", "SINGLE_OPTION_TYPES (const)", "SQLDialect (type)", "SQLDialectSchema (const)", diff --git a/packages/spec/export-origins/data.json b/packages/spec/export-origins/data.json index 601ca826fa..96501936ad 100644 --- a/packages/spec/export-origins/data.json +++ b/packages/spec/export-origins/data.json @@ -484,6 +484,7 @@ "SEARCH_AUTO_EXCLUDED_FIELDS": "src/data/search-fields.ts#SEARCH_AUTO_EXCLUDED_FIELDS (const)", "SEARCH_AUTO_EXCLUDED_TYPES": "src/data/search-fields.ts#SEARCH_AUTO_EXCLUDED_TYPES (const)", "SEARCH_VIRTUAL_TYPES": "src/data/search-fields.ts#SEARCH_VIRTUAL_TYPES (const)", + "SECRET_MASK": "src/data/secret-mask.ts#SECRET_MASK (const)", "SINGLE_OPTION_TYPES": "src/data/field-value.zod.ts#SINGLE_OPTION_TYPES (const)", "SQLDialect": "src/data/driver-sql.zod.ts#SQLDialect (type)", "SQLDialectSchema": "src/data/driver-sql.zod.ts#SQLDialectSchema (const)", diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index d3ad441c31..c9f6ff3975 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -54,6 +54,12 @@ export * from './object.zod'; // `enable.apiMethods` whitelist into its effective operation set (#3391). export * from './api-derivation'; export * from './field.zod'; +// The credential read mask (ADR-0100) — the ONE string a masked read serves, in +// place of the two byte-identical literals objectql and service-settings each +// declared until #7572. Both now import this one, so the mask a console sees on +// the encrypted-FIELD path cannot desynchronise from the mask it sees on the +// settings REST path. +export * from './secret-mask'; // The unknown-authoring-key lint's CORE — comparator, finding shape, curated // guidance tables (#3786). Kept frontend-safe: the stack WALKER that imports // every schema lives in kernel/metadata-authoring-lint.ts, so this subpath's diff --git a/packages/spec/src/data/secret-mask.test.ts b/packages/spec/src/data/secret-mask.test.ts new file mode 100644 index 0000000000..d755c87252 --- /dev/null +++ b/packages/spec/src/data/secret-mask.test.ts @@ -0,0 +1,55 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7572] The credential read mask's two load-bearing properties, pinned where + * the mask is now declared: its BYTES and its SPELLING. + * + * Hoisting the constant into `spec` removed the drift *between* the two former + * copies — objectql's `SECRET_MASK` and service-settings' `SETTINGS_SECRET_MASK` + * are the same declaration now, so they cannot desynchronise. What a single + * declaration does NOT remove is an edit to that declaration, and this mask has + * a wider blast radius than its one line suggests: a console renders + * "configured vs not configured" from it, echoes it back unchanged on save, and + * both write paths read that echo as "unchanged" (ADR-0100 §B3). Change the + * bytes and every already-rendered form's echo stops being recognised — it + * becomes a genuine write of eight bullets over a live credential. + * + * So this file keeps its OWN copy of the literal and compares. That restatement + * is the point: a pin that imported the constant to check the constant would be + * green by construction. The far-side pins in plugin-audit and driver-memory + * restate it for the same reason and are deliberately left alone (#7572). + * + * The spelling half is not decoration either. #7572 records it as a property to + * preserve: the mask is written as the literal — not `'•'.repeat(8)`, not + * an escape — so that someone who greps for the eight bullets a client actually + * received lands on the declaration. Nothing but a source read can hold that, + * since every spelling produces the identical value. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import url from 'node:url'; + +import { describe, it, expect } from 'vitest'; + +import { SECRET_MASK } from './secret-mask'; + +const HERE = path.dirname(url.fileURLToPath(import.meta.url)); +const SOURCE = path.resolve(HERE, 'secret-mask.ts'); + +describe('SECRET_MASK — the credential read mask (ADR-0100 / #7572)', () => { + it('is exactly eight U+2022 BULLET characters', () => { + // Restated on purpose — see the module header. + expect(SECRET_MASK).toBe('••••••••'); + expect([...SECRET_MASK]).toHaveLength(8); + expect(new Set([...SECRET_MASK].map((c) => c.codePointAt(0)))).toEqual(new Set([0x2022])); + // No surrogate pairs, so the UTF-16 length matches the code-point count — + // the property every `value.length === 8` reader downstream relies on. + expect(SECRET_MASK).toHaveLength(8); + }); + + it('is spelled as the literal in source, so a grep for the mask finds it', () => { + const source = fs.readFileSync(SOURCE, 'utf8'); + expect(source).toContain("export const SECRET_MASK = '••••••••';"); + }); +}); diff --git a/packages/spec/src/data/secret-mask.ts b/packages/spec/src/data/secret-mask.ts new file mode 100644 index 0000000000..af0c58bf1e --- /dev/null +++ b/packages/spec/src/data/secret-mask.ts @@ -0,0 +1,61 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The credential read mask (ADR-0100) — the one string a client sees in place + * of a value it is not allowed to read back. + * + * # Why this lives in `spec` and not next to either reader + * + * "A masked read looks like THIS" is a **client-facing contract**, and it is + * consumed by two independent surfaces that never call each other: + * + * - the encrypted-**field** read path in `@objectstack/objectql` — `secret` + * and generic `password` columns masked on `find`/`findOne`/`$expand` + * (ADR-0100 §A/§B), re-exported from that package as `SECRET_MASK` for + * hosts and privileged consumers; + * - the settings **REST boundary** in `@objectstack/service-settings`, whose + * `SETTINGS_SECRET_MASK` is this same constant under the name that package + * publishes (#7522). + * + * Until #7572 each of those declared its own byte-identical literal, bound by + * nothing but convention. That duplication is invisible while it holds and + * silent when it breaks: an edit to either literal desynchronises the two + * masked-read faces a console sees, and *both* packages' suites stay green, + * because each asserts against its own copy. The console cannot be shown two + * different masks — it renders "configured vs not configured" from this value + * and echoes it back unchanged on save (the echoed-mask write guard, ADR-0100 + * §B3), so a drifted mask silently turns "unchanged" into "overwrite". + * + * `spec` is the contract face both sides already depend on, so the mask is + * declared here **once** and imported. Neither reader takes a new dependency to + * reach it: objectql is built on spec, and service-settings — which is + * deliberately framework-agnostic and does NOT depend on objectql — already + * loads `@objectstack/spec/data` at runtime through + * `@objectstack/platform-objects`' object declarations. + * + * # The literal is spelled out on purpose + * + * Eight U+2022 BULLET characters, written as the literal rather than as a + * `•` escape or a `.repeat(8)`, so that a plain grep for the mask a client + * actually received finds this declaration. Pinned by `secret-mask.test.ts` in + * both directions — the bytes AND the spelling — because "spelled so it can be + * found" is a property no type can carry. + */ + +/** + * Value served in place of a credential on a masked read: `secret` and generic + * `password` fields on the engine's generic read path (ADR-0100), and secret- + * backed settings on the settings REST read path (#7522). + * + * Says "a value is set" without leaking the plaintext or the storage handle. An + * UNSET credential reads back `null` instead, never this mask — the masking is + * presence-preserving on both surfaces, which is what lets a console render + * "configured" vs "not configured" at all. + * + * Writing this exact string back is treated as "unchanged" and dropped by both + * write paths, so a form round-trip that echoes what it read cannot overwrite + * the stored credential with the mask's literal text. Accepted cost, recorded in + * ADR-0100 §B3: eight bullets cannot itself be stored as a credential value + * through an echoing client. + */ +export const SECRET_MASK = '••••••••';