fix(service-settings): redact encrypted setting values at the REST read boundary (#7522) (#7554)

GET /api/settings/:namespace returned the plaintext of every encrypted setting
in the namespace — in values.<key>.value and repeated once more inside each
cascadeChain entry — for both specifier flavours (type: 'password' and an
explicit encrypted: true). Storage was always correct: sys_setting.value is
null, value_enc holds a sec_ handle, and sys_secret holds aes-256-gcm
ciphertext. The leak was entirely on the way out, where there was no redaction
step at all.

The endpoint requires setup.access, so this is defense-in-depth rather than
privilege escalation — but every operator, integration, proxy, browser cache
and HAR capture on that response path received the cleartext of every secret in
the namespace, defeating the point of the value_enc + sys_secret split.

The fix lives at the REST boundary and nowhere else, reusing the mask
convention ADR-0100 pins for encrypted FIELDS on the generic CRUD path rather
than inventing a sentinel:

  - read  — a set secret is served as SETTINGS_SECRET_MASK (the same eight
            bullets as objectql's SECRET_MASK); an unset one stays null, so the
            response is presence-preserving. cascadeChain is masked entry by
            entry. source, locked, lockedReason and the 409 SETTINGS_LOCKED
            env-lock behaviour are untouched.
  - write — a submitted value equal to the mask means "unchanged" and the key is
            dropped from the patch, so a form echoing what it read cannot
            overwrite the stored secret with the mask's literal text. Scoped to
            secret keys, so a plain setting whose value genuinely is eight
            bullets still writes verbatim. PUT's own response is redacted the
            same way — it carries resolved values too, including cascade entries
            the caller never submitted.

SettingsService still decrypts. materialiseRow(), get(), getNamespace(),
snapshotOf() and createClient() keep handing real plaintext to in-process
consumers, because the mail/sms/storage/auth plugins read their credentials
through exactly that path; a test pins that round-trip so this cannot be
"fixed" one layer down.

New public API: SETTINGS_SECRET_MASK, redactSecretValues, dropEchoedSecretMasks
and SettingsService.secretKeysOf(namespace) — the last one so the boundary reads
the SAME encrypted-key set setMany consults, which is what stops the two sides
drifting into "encrypted on write, cleartext on read".


Claude-Session: https://claude.ai/code/session_01CMcwKmYDRFjtfT8jCRRXwB

Co-authored-by: Claude <noreply@anthropic.com>

Author: huangyiirene

.changeset/settings-redact-encrypted-rest-read.md

@@ -0,0 +1,48 @@
+---
+"@objectstack/service-settings": patch
+---
+
+fix(service-settings): redact encrypted setting values at the REST read boundary (#7522)
+
+`GET /api/settings/:namespace` returned the **plaintext** of every encrypted
+setting in the namespace — in `values.<key>.value` and repeated once more in each
+`cascadeChain` entry. Both specifier flavours were affected: `type: 'password'`
+and an explicit `encrypted: true`. Storage was never the problem
+(`sys_setting.value` is null, `value_enc` holds a `sec_` handle, and `sys_secret`
+holds aes-256-gcm ciphertext); the leak was entirely on the way out.
+
+The endpoint requires `setup.access`, so this is defense-in-depth rather than
+privilege escalation — but every operator, integration, proxy, browser cache and
+HAR capture on that response path received the cleartext of every secret in the
+namespace, which is precisely what the `value_enc` + `sys_secret` split exists to
+prevent.
+
+**What changed.** The REST handlers now redact before the payload leaves the
+process, reusing the mask convention ADR-0100 already pins for encrypted
+*fields* on the generic CRUD path rather than inventing a sentinel:
+
+- **Read** — a set secret is served as `SETTINGS_SECRET_MASK` (`••••••••`, the
+  same eight bullets as objectql's `SECRET_MASK`); an unset one stays `null`. The
+  redaction is presence-preserving, so "configured vs not configured" is still
+  readable, and it covers `cascadeChain` entry by entry as well as the effective
+  value. `source`, `locked`, `lockedReason` and the `409 SETTINGS_LOCKED`
+  env-lock behaviour are untouched.
+- **Write** — a submitted value equal to the mask means "unchanged" and the key
+  is dropped from the patch, so a form round-trip that echoes what it read does
+  not overwrite the stored secret with the mask's literal text. The drop is
+  scoped to secret keys: a plain setting whose value genuinely is eight bullets
+  still writes verbatim. `PUT`'s own response is redacted the same way — it
+  carries resolved values too, including cascade entries the caller never
+  submitted.
+
+**What deliberately did not change.** `SettingsService` still decrypts.
+`materialiseRow()`, `get()`, `getNamespace()`, `snapshotOf()` and `createClient()`
+keep returning real plaintext, because the mail / sms / storage / auth plugins
+read their credentials through exactly that path. Redaction belongs to the REST
+boundary and nowhere else; a test pins the in-process round-trip so this cannot
+be "fixed" one layer down.
+
+New public API on `@objectstack/service-settings`: `SETTINGS_SECRET_MASK`,
+`redactSecretValues`, `dropEchoedSecretMasks`, and
+`SettingsService.secretKeysOf(namespace)` — published so a client can recognise a
+masked read instead of comparing against a hard-coded string.

packages/services/service-settings/src/index.ts

@@ -55,6 +55,17 @@ export {
   registerSettingsRoutes,
   type SettingsRoutesOptions,
 } from './settings-routes.js';
+// #7522 — the REST read mask for encrypted settings, plus the two halves of the
+// boundary it defines. Published because a client has to be able to RECOGNISE a
+// masked read: the console renders "configured" from it and echoes it back
+// unchanged on save, and comparing against a string hard-coded in the console is
+// exactly the drift this export prevents. The SERVICE layer is unaffected — it
+// still hands real plaintext to in-process consumers; see the module header.
+export {
+  SETTINGS_SECRET_MASK,
+  redactSecretValues,
+  dropEchoedSecretMasks,
+} from './settings-secret-redaction.js';
 export {
   settingsObjects,
   settingsPluginManifestHeader,

packages/services/service-settings/src/settings-routes.test.ts

@@ -6,6 +6,11 @@ import { SettingsService } from './settings-service.js';
 import { registerSettingsRoutes } from './settings-routes.js';
 import { brandingSettingsManifest } from './manifests/branding.manifest.js';
 import { localizationSettingsManifest } from './manifests/localization.manifest.js';
+import { SETTINGS_SECRET_MASK } from './settings-secret-redaction.js';
+// `InMemoryCryptoProvider` is a value-only alias (`export const … = LocalCryptoProvider`),
+// so the class itself is the one that can also be spelled as a type.
+import { LocalCryptoProvider } from './local-crypto-provider.js';
+import type { SettingsManifest } from '@objectstack/spec/system';
 
 class MockHttp implements IHttpServer {
   routes = new Map<string, RouteHandler>();
@@ -304,3 +309,310 @@ describe('settings-routes', () => {
     ]);
   });
 });
+
+// ─────────────────────────────────────────────────────────────────────────────
+// #7522 — encrypted settings are REDACTED at the REST read boundary.
+//
+// The service decrypts on purpose and keeps doing so; these cases pin the
+// boundary in both directions: nothing ciphertext-backed leaves over HTTP, and
+// an in-process consumer still receives the real plaintext. Both specifier
+// flavours are covered — `type: 'password'` (implicitly encrypted) and an
+// explicit `encrypted: true` on a non-password type — because `registerManifest`
+// folds them into one set and a fix that only saw one of them would still leak.
+// ─────────────────────────────────────────────────────────────────────────────
+
+const SMTP_PLAINTEXT = 'smtp-pa55word-plaintext';
+const TOKEN_PLAINTEXT = 'webhook-token-plaintext';
+const GLOBAL_PLAINTEXT = 'global-scope-plaintext';
+
+/** Two secret flavours + one plain control key, resolved down the full cascade. */
+const secretsManifest: SettingsManifest = {
+  namespace: 'secretsns',
+  version: 1,
+  label: 'Secrets',
+  // `user` so the cascade walks global → tenant → user and `cascadeChain`
+  // carries more than one entry to leak through.
+  scope: 'user',
+  readPermission: 'setup.access',
+  writePermission: 'setup.write',
+  specifiers: [
+    // Flavour A — implicitly encrypted because the TYPE is `password`.
+    { type: 'password', key: 'smtp_password', label: 'SMTP password', required: false },
+    // Flavour B — an ordinary type carrying an explicit `encrypted: true`.
+    { type: 'text', key: 'webhook_token', label: 'Webhook token', required: false, encrypted: true },
+    // Control — never encrypted, must survive the redaction untouched.
+    { type: 'text', key: 'smtp_host', label: 'Host', required: false, default: 'smtp.example.com' },
+  ],
+};
+
+const secretAdmin = () => ({
+  enforced: true,
+  permissions: ['setup.access', 'setup.write'],
+  userId: 'usr_1',
+});
+
+function makeSecretStack() {
+  const secretRows = new Map<string, any>();
+  const cryptoProvider = new LocalCryptoProvider();
+  const svc = new SettingsService({
+    env: {},
+    cryptoProvider,
+    secretStore: {
+      async insert(row) { secretRows.set(row.id, row); return { id: row.id }; },
+      async get(id) { return secretRows.get(id) ?? null; },
+      async update(id, patch) { secretRows.set(id, { ...secretRows.get(id), ...patch }); },
+    },
+  });
+  svc.registerManifest(secretsManifest);
+  const http = new MockHttp();
+  registerSettingsRoutes(http, svc, { contextFromRequest: secretAdmin });
+  return { svc, http, secretRows, cryptoProvider };
+}
+
+/**
+ * Seed an upper-scope (`global`) encrypted row directly, exactly as `setMany`
+ * would write it — ciphertext in the secret store, only the `sec_` handle on the
+ * setting row. The public write path resolves one scope per key, so this is the
+ * only way to give `cascadeChain` a second ciphertext-backed entry.
+ */
+async function seedGlobalSecret(
+  svc: SettingsService,
+  secretRows: Map<string, any>,
+  cryptoProvider: LocalCryptoProvider,
+  key: string,
+  plaintext: string,
+) {
+  const handle = await cryptoProvider.encrypt(plaintext, { namespace: 'secretsns', key });
+  secretRows.set(handle.id, {
+    id: handle.id,
+    namespace: 'secretsns',
+    key,
+    kms_key_id: handle.kmsKeyId,
+    alg: handle.alg,
+    version: handle.version,
+    ciphertext: handle.ciphertext,
+  });
+  await (svc as any).upsertRow({
+    namespace: 'secretsns',
+    key,
+    scope: 'global',
+    user_id: null,
+    value: null,
+    value_enc: handle.id,
+    encrypted: true,
+  });
+  return handle.id;
+}
+
+describe('settings-routes — #7522 encrypted values are redacted at the REST boundary', () => {
+  it('GET /:ns leaks no ciphertext-backed cleartext — both flavours, value AND cascadeChain', async () => {
+    const { svc, http, secretRows, cryptoProvider } = makeSecretStack();
+
+    // Written through the trusted in-process path, the way a seed/bootstrap or
+    // a plugin would.
+    await svc.set('secretsns', 'smtp_password', SMTP_PLAINTEXT, { userId: 'usr_1' });
+    await svc.set('secretsns', 'webhook_token', TOKEN_PLAINTEXT, { userId: 'usr_1' });
+    await seedGlobalSecret(svc, secretRows, cryptoProvider, 'smtp_password', GLOBAL_PLAINTEXT);
+
+    const h = http.routes.get('GET /api/settings/:namespace')!;
+    const { req, res, state } = makeReqRes({ params: { namespace: 'secretsns' } });
+    await h(req, res);
+
+    expect(state.status).toBe(200);
+
+    // The whole-body assertion is the one that cannot be satisfied by masking
+    // only the places we happened to think of.
+    const wire = JSON.stringify(state.body);
+    expect(wire).not.toContain(SMTP_PLAINTEXT);
+    expect(wire).not.toContain(TOKEN_PLAINTEXT);
+    expect(wire).not.toContain(GLOBAL_PLAINTEXT);
+
+    // …and the specific places the issue names, so a future regression says
+    // WHICH surface broke rather than just "a string appeared".
+    const values = state.body.data.values;
+    expect(values.smtp_password.value).toBe(SETTINGS_SECRET_MASK);
+    expect(values.webhook_token.value).toBe(SETTINGS_SECRET_MASK);
+    for (const key of ['smtp_password', 'webhook_token']) {
+      const chain = values[key].cascadeChain as Array<{ scope: string; value: unknown }>;
+      expect(chain.length).toBeGreaterThan(0);
+      for (const entry of chain) {
+        expect([SETTINGS_SECRET_MASK, null]).toContain(entry.value);
+      }
+    }
+    // The global entry is present and masked — a second ciphertext-backed
+    // scope, not an artefact of the user row being the only one.
+    expect(values.smtp_password.cascadeChain).toEqual(
+      expect.arrayContaining([expect.objectContaining({ scope: 'global', value: SETTINGS_SECRET_MASK })]),
+    );
+
+    // The non-encrypted control key is untouched.
+    expect(values.smtp_host.value).toBe('smtp.example.com');
+  });
+
+  it('redaction is presence-preserving: an UNSET secret stays null, not a mask', async () => {
+    const { svc, http } = makeSecretStack();
+    await svc.set('secretsns', 'smtp_password', SMTP_PLAINTEXT, { userId: 'usr_1' });
+
+    const h = http.routes.get('GET /api/settings/:namespace')!;
+    const { req, res, state } = makeReqRes({ params: { namespace: 'secretsns' } });
+    await h(req, res);
+
+    // Set vs unset stays observable — the console renders "configured" from it.
+    expect(state.body.data.values.smtp_password.value).toBe(SETTINGS_SECRET_MASK);
+    expect(state.body.data.values.webhook_token.value).toBeNull();
+  });
+
+  it('`source` and `locked` survive the redaction unchanged', async () => {
+    const { svc, http } = makeSecretStack();
+    await svc.set('secretsns', 'webhook_token', TOKEN_PLAINTEXT, { userId: 'usr_1' });
+
+    const h = http.routes.get('GET /api/settings/:namespace')!;
+    const { req, res, state } = makeReqRes({ params: { namespace: 'secretsns' } });
+    await h(req, res);
+
+    expect(state.body.data.values.webhook_token.source).toBe('user');
+    expect(state.body.data.values.webhook_token.locked).toBe(false);
+    expect(state.body.data.values.smtp_host.source).toBe('default');
+  });
+
+  it('an env-locked secret is masked while `source: env` / `locked` / 409 SETTINGS_LOCKED are unchanged', async () => {
+    // The env override is itself a secret value; it must not ride out on the
+    // read path either, and the lock affordances must keep working.
+    const svc = new SettingsService({ env: { OS_SECRETSNS_SMTP_PASSWORD: 'env-supplied-secret' } });
+    svc.registerManifest(secretsManifest);
+    const http = new MockHttp();
+    registerSettingsRoutes(http, svc, { contextFromRequest: secretAdmin });
+
+    const read = http.routes.get('GET /api/settings/:namespace')!;
+    const r1 = makeReqRes({ params: { namespace: 'secretsns' } });
+    await read(r1.req, r1.res);
+    expect(JSON.stringify(r1.state.body)).not.toContain('env-supplied-secret');
+    expect(r1.state.body.data.values.smtp_password.value).toBe(SETTINGS_SECRET_MASK);
+    expect(r1.state.body.data.values.smtp_password.source).toBe('env');
+    expect(r1.state.body.data.values.smtp_password.locked).toBe(true);
+    expect(r1.state.body.data.values.smtp_password.lockedReason).toContain('OS_SECRETSNS_SMTP_PASSWORD');
+    expect(r1.state.body.data.values.smtp_password.cascadeChain).toEqual([
+      expect.objectContaining({ scope: 'env', value: SETTINGS_SECRET_MASK, locked: true }),
+    ]);
+
+    const write = http.routes.get('PUT /api/settings/:namespace')!;
+    const r2 = makeReqRes({ params: { namespace: 'secretsns' }, body: { smtp_password: 'new' } });
+    await write(r2.req, r2.res);
+    expect(r2.state.status).toBe(409);
+    expect(r2.state.body.error.code).toBe('SETTINGS_LOCKED');
+  });
+
+  // ── the echoed-mask write, i.e. the second bug a redaction fix introduces ──
+
+  it('PUTting the echoed mask back is a NO-OP — the stored secret is not overwritten', async () => {
+    const { svc, http, secretRows } = makeSecretStack();
+    await svc.set('secretsns', 'smtp_password', SMTP_PLAINTEXT, { userId: 'usr_1' });
+    const handlesBefore = [...secretRows.keys()];
+
+    const h = http.routes.get('PUT /api/settings/:namespace')!;
+    const { req, res, state } = makeReqRes({
+      params: { namespace: 'secretsns' },
+      body: { smtp_password: SETTINGS_SECRET_MASK },
+    });
+    await h(req, res);
+
+    expect(state.status).toBe(200);
+    expect(state.body.error).toBeUndefined();
+    // No new ciphertext row: the mask was never encrypted and stored.
+    expect([...secretRows.keys()]).toEqual(handlesBefore);
+    // And the in-process read still yields the ORIGINAL plaintext — not the
+    // mask's literal text, which is what an unguarded write would have stored
+    // (and which would decrypt back to itself, so nothing would look wrong
+    // until the SMTP login failed).
+    const resolved = await svc.get<string>('secretsns', 'smtp_password', { userId: 'usr_1' });
+    expect(resolved.value).toBe(SMTP_PLAINTEXT);
+    expect(resolved.value).not.toBe(SETTINGS_SECRET_MASK);
+  });
+
+  it('the echoed mask inside the read-shape {values:{k:{value}}} envelope is a no-op too', async () => {
+    const { svc, http } = makeSecretStack();
+    await svc.set('secretsns', 'webhook_token', TOKEN_PLAINTEXT, { userId: 'usr_1' });
+
+    const h = http.routes.get('PUT /api/settings/:namespace')!;
+    // Exactly what GET now returns, echoed back wholesale by a form save.
+    const { req, res, state } = makeReqRes({
+      params: { namespace: 'secretsns' },
+      body: {
+        values: {
+          webhook_token: { value: SETTINGS_SECRET_MASK, source: 'user', locked: false },
+        },
+      },
+    });
+    await h(req, res);
+
+    expect(state.status).toBe(200);
+    expect((await svc.get<string>('secretsns', 'webhook_token', { userId: 'usr_1' })).value)
+      .toBe(TOKEN_PLAINTEXT);
+  });
+
+  it('a REAL new secret still writes, and the write RESPONSE is redacted too', async () => {
+    const { svc, http } = makeSecretStack();
+    await svc.set('secretsns', 'smtp_password', SMTP_PLAINTEXT, { userId: 'usr_1' });
+
+    const h = http.routes.get('PUT /api/settings/:namespace')!;
+    const { req, res, state } = makeReqRes({
+      params: { namespace: 'secretsns' },
+      body: { smtp_password: 'a-genuinely-new-secret' },
+    });
+    await h(req, res);
+
+    expect(state.status).toBe(200);
+    // The write took effect in the store…
+    expect((await svc.get<string>('secretsns', 'smtp_password', { userId: 'usr_1' })).value)
+      .toBe('a-genuinely-new-secret');
+    // …but the response body does not echo it back over the wire.
+    expect(JSON.stringify(state.body)).not.toContain('a-genuinely-new-secret');
+    expect(state.body.data.values.smtp_password.value).toBe(SETTINGS_SECRET_MASK);
+  });
+
+  it('a non-encrypted key whose value genuinely IS the mask is written verbatim', async () => {
+    // The drop is scoped to secret keys — it must not swallow a legal write.
+    const { svc, http } = makeSecretStack();
+
+    const h = http.routes.get('PUT /api/settings/:namespace')!;
+    const { req, res, state } = makeReqRes({
+      params: { namespace: 'secretsns' },
+      body: { smtp_host: SETTINGS_SECRET_MASK },
+    });
+    await h(req, res);
+
+    expect(state.status).toBe(200);
+    expect((await svc.get<string>('secretsns', 'smtp_host', { userId: 'usr_1' })).value)
+      .toBe(SETTINGS_SECRET_MASK);
+  });
+
+  // ── the other half of the boundary: the service layer is NOT redacted ──────
+
+  it('in-process consumers still receive REAL plaintext (createClient / snapshotOf)', async () => {
+    // This is the guard against someone later "fixing" #7522 in the service
+    // layer: the mail/sms/storage/auth plugins read their credentials through
+    // exactly this path, and a mask here would break every one of them.
+    const { svc } = makeSecretStack();
+    await svc.set('secretsns', 'smtp_password', SMTP_PLAINTEXT, { userId: 'usr_1' });
+    await svc.set('secretsns', 'webhook_token', TOKEN_PLAINTEXT, { userId: 'usr_1' });
+
+    const client = await svc.createClient('secretsns', { ctx: { userId: 'usr_1' } });
+    expect(client.current.smtp_password).toBe(SMTP_PLAINTEXT);
+    expect(client.get('webhook_token')).toBe(TOKEN_PLAINTEXT);
+
+    // …and so does the raw service read the routes wrap.
+    const payload = await svc.getNamespace('secretsns', { userId: 'usr_1' });
+    expect(payload.values.smtp_password.value).toBe(SMTP_PLAINTEXT);
+    expect(payload.values.webhook_token.value).toBe(TOKEN_PLAINTEXT);
+    client.dispose();
+  });
+
+  it('secretKeysOf reports both flavours and refuses an unknown namespace', async () => {
+    const { svc } = makeSecretStack();
+    expect([...svc.secretKeysOf('secretsns')].sort()).toEqual(['smtp_password', 'webhook_token']);
+    // Fail-closed: "unknown namespace" must never answer "nothing is secret".
+    expect(() => svc.secretKeysOf('nope')).toThrow(
+      expect.objectContaining({ code: 'SETTINGS_UNKNOWN_NAMESPACE' }),
+    );
+  });
+});