From e0e106eb5b126b48f65a33027c7f3f404b4a0440 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E6=80=A1?= Date: Wed, 12 Aug 2026 17:02:14 +0000 Subject: [PATCH] fix(service-datasource): stop serving stored credentials on the datasource read path, and fix the false "credential-stripped" claim (#8081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getDatasource()` returned the driver `config` verbatim while its own doc comment promised "with the credential stripped" and described `config` as "non-sensitive — credentials live in `sys_secret`, never in config". Nothing stripped anything. The comment was load-bearing: it is why the gap survived a 26-surface credential survey. #8078 closed the WRITE door (`config.password` / `config.authToken` are `z.never()` on every driver that has them) and deliberately left stored rows alone. Those rows still hold cleartext, and `GET /api/v1/datasources/:name` handed it to every caller. The refused-key set is DERIVED from each driver's contract — #8078 spells a refused inline credential as `z.never()`, so the schema is the list — plus the pre-#8078 alias spellings (a stored row never met the parse that renamed them) and turso's still-writable `encryptionKey`. A driver with no shipped contract keeps the canonical spellings hidden by name. A credential embedded in `config.url` is redacted too, at the password component of the userinfo only. Refusing such a URL at the write door stays UNRULED (#7990) and untouched: redacting on the way out is not refusing on the way in, and a scrub that dropped `config.password` while serving the same secret one key over would be a scrub in name only. `updateDatasource` carries the hidden material forward across a same-driver round-trip, after the validation gate rather than before it — the gate judges what the author wrote, and this is material the author never saw. Without it the scrub would have turned every untouched "Save" into silent credential deletion. It also repairs a regression #8078 introduced and this card measured: the form was served `config.password` verbatim, posted it back, and the gate refused it — so editing any legacy datasource answered 400 for a value the server itself had just supplied. The stored record is never mutated; the connect path is unaffected. Reverse verification: 9 of 21 pins fail on `origin/main` (each naming the defect it carries), 21/21 pass here. The two guard pins — #8078's refusal with its guidance intact, and URL credentials still ACCEPTED at the write door — pass on both sides by design. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WCoM9vVXw6yAVgEc4oio4m --- .changeset/datasource-credential-read-path.md | 67 ++++ .../datasource-config-redaction.test.ts | 320 ++++++++++++++++++ .../service-datasource/src/admin-routes.ts | 11 +- .../src/datasource-admin-service.ts | 74 +++- .../src/datasource-config-redaction.ts | 275 +++++++++++++++ 5 files changed, 737 insertions(+), 10 deletions(-) create mode 100644 .changeset/datasource-credential-read-path.md create mode 100644 packages/services/service-datasource/src/__tests__/datasource-config-redaction.test.ts create mode 100644 packages/services/service-datasource/src/datasource-config-redaction.ts diff --git a/.changeset/datasource-credential-read-path.md b/.changeset/datasource-credential-read-path.md new file mode 100644 index 0000000000..785cd88e39 --- /dev/null +++ b/.changeset/datasource-credential-read-path.md @@ -0,0 +1,67 @@ +--- +"@objectstack/service-datasource": patch +--- + +fix(service-datasource): the datasource read path stops serving stored credentials in cleartext, and the "credential-stripped" comment stops lying (#8081) + +`GET /api/v1/datasources/:name` returned the driver `config` **verbatim**, while +the method producing it carried a doc comment promising the opposite — +"with the credential stripped", `config` described as "non-sensitive — +credentials live in `sys_secret`, never in config". Nothing stripped anything. + +The comment was not merely stale, it was **load-bearing**: it is the reason the +gap survived a 26-surface credential survey. A safety claim that no code performs +is worse than no claim, because it stops the next reader from looking. + +#8078 closed the WRITE door — `config.password` / `config.authToken` are +declared-unwritable on every driver that has them, so no new row can carry an +inline credential. It deliberately did not touch rows already stored. Those rows +still hold cleartext, and until now the admin read path handed it to every caller +of that route. + +**What is redacted.** The refused-key set is DERIVED from each driver's own +contract rather than retyped here: #8078 spells a refused inline credential as +`z.never()`, so the schema *is* the list, and a credential key refused tomorrow +is covered the day it lands. Three sources feed the scrub — the derived keys, the +pre-#8078 alias spellings (`passwd`/`pwd`/`token`/`jwt`/`auth_token`/`authtoken`, +which a stored row can still hold verbatim because the wizard persists through +`metadata.register` and never met the parse that would have renamed them), and +turso's `encryptionKey`, an AES-256 key that remains writable because the secret +binder has no slot for it. A driver the platform ships no contract for still has +the canonical spellings hidden by name: declining to *refuse* an unrecognised key +is a boundary choice about authoring, while serving a key literally named +`password` back in cleartext is a leak under any boundary. + +**URL-embedded credentials.** A `postgresql://user:pass@host/db` in `config.url` +carries the same secret as `config.password`, and a scrub that dropped one while +serving the other one key over would be a scrub in name only. The read path now +redacts the **password component of a URL's userinfo**, preserving the scheme, +the username and everything from the host onward. Refusing such a URL at the +write door remains deliberately **unruled** (#7990) and is untouched: redacting a +value on the way out is not the same act as refusing it on the way in. + +**The response says what it withheld.** `getDatasource()` gains +`redactedConfigKeys`, so a caller knows a credential is being held back rather +than inferring it from an absence — the same courtesy the existing `hasSecret` +flag pays for the bound `sys_secret` handle. + +**A round-trip no longer destroys the credential — and no longer 400s.** The +edit form reads this config and patches it straight back, so a scrub without an +inverse would have turned every untouched "Save" into silent credential deletion. +`updateDatasource` therefore carries the hidden material forward when a patch is +round-tripping the same driver's config, after the validation gate rather than +before it: the gate judges what the *author* wrote, and this material is +something the author never saw and is not asking to change. Restoring it is the +same rule the `credentialsRef` beside it has always followed. + +This also repairs a regression that arrived with #8078 and is measured here for +the first time: on `main` the form was served `config.password` verbatim, posted +it back unchanged, and the write gate refused it — so **editing any legacy +datasource through the wizard answered 400 for a value the server itself had +just supplied**, including the `active: false` that takes a misconfigured +datasource out of service. + +**Not changed.** The stored record is never mutated: redaction is a read-path act +only, the connect path reads the raw record, and a legacy datasource keeps +authenticating exactly as before. Getting cleartext *out* of the store is a +migration with its own decision to make and is not attempted here. diff --git a/packages/services/service-datasource/src/__tests__/datasource-config-redaction.test.ts b/packages/services/service-datasource/src/__tests__/datasource-config-redaction.test.ts new file mode 100644 index 0000000000..8ca1e7da6d --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/datasource-config-redaction.test.ts @@ -0,0 +1,320 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #8081 — the services half of #7990: the datasource credential READ path. + * + * ## What each pin reads on `origin/main` (reverse verification) + * + * RED on main (carries the defect this card fixes): + * - "serves a stored row's cleartext password" — `getDatasource()` returned + * `config` verbatim, so `config.password: 'hunter2'` came back on + * `GET /api/v1/datasources/:name`. Measured on `origin/main` before the fix. + * - "serves the credential embedded in a connection URL" — the same read + * returned `postgresql://admin:hunter2@db.internal:5432/app`. This is the + * half a `config.password`-only scrub would have left open. + * - "an untouched Save does not destroy the stored credential" — red on main + * for a REASON WORTH RECORDING, and not the one it was written for. On main + * the edit form is served `config.password` verbatim, posts it straight + * back, and #8078's write gate refuses it — so editing any legacy row + * through the wizard answers 400 for a value the server itself supplied. + * Redacting the read is what makes that round-trip legal again; the restore + * is what stops it deleting the credential instead. Measured, not inferred. + * - "the SERVED config is redacted while the STORED record keeps its + * credential" — pins both halves at once, because the store-side half alone + * is green on main for the worst reason (nothing redacts there). + * + * GREEN on main (guards behaviour that must NOT change): + * - "#8078's parse refusal still fires, with its guidance intact" — guards the + * spec half. The refusal message names the secret binder and + * `external.credentialsRef`; this card must not weaken it while adding a + * read-path scrub next to it. Passes on main and must keep passing. + * - "a URL-embedded credential is still ACCEPTED at the write door" — guards + * the deliberately UNRULED boundary (#7990): #8078 pinned URL credentials as + * a fact rather than refusing them. Redacting on read must not drift into + * refusing on write. Passes on main and must keep passing. + * - "the connect path still sees the stored credential" — guards the fact that + * redaction is a read-path act only, so a legacy datasource keeps working. + * Passes on main trivially and would go red if the scrub ever mutated the + * stored record. + */ + +import { describe, it, expect } from 'vitest'; +import { validateDriverConfig, getDriverConfigSchema, BUILTIN_DRIVER_IDS } from '@objectstack/spec/data'; +import { + redactDatasourceConfig, + restoreRedactedConfig, + redactUrlPassword, + refusedCredentialKeys, + redactableConfigKeys, +} from '../datasource-config-redaction.js'; +import { + DatasourceAdminService, + type DatasourceAdminServiceConfig, + type StoredDatasource, +} from '../datasource-admin-service.js'; + +/** A legacy row: written before #8078, so it carries inline cleartext. */ +const LEGACY_PG: StoredDatasource = { + name: 'legacy_pg', + driver: 'postgres', + origin: 'runtime', + config: { + host: 'db.internal', + database: 'app', + username: 'admin', + password: 'hunter2', + url: 'postgresql://admin:hunter2@db.internal:5432/app', + }, + external: { credentialsRef: 'sys_secret:abc' }, +}; + +function makeService(seed: StoredDatasource[]) { + const records: StoredDatasource[] = seed.map((r) => ({ ...r, config: { ...r.config } })); + const cfg: DatasourceAdminServiceConfig = { + probe: async () => ({ ok: true }), + listDatasourceRecords: async () => records, + getDatasourceRecord: async (n) => records.find((r) => r.name === n), + putDatasourceRecord: async (rec) => { + const i = records.findIndex((r) => r.name === rec.name); + if (i >= 0) records[i] = rec; + else records.push(rec); + }, + deleteDatasourceRecord: async () => {}, + writeSecret: async () => 'sys_secret:new', + countBoundObjects: async () => 0, + }; + return { records, service: new DatasourceAdminService(cfg) }; +} + +describe('the refusal set is DERIVED from the driver contracts, not retyped', () => { + it('every `z.never()` config key is found by the derivation', () => { + // The property that makes this module self-extending: #8078 spelled a + // refused inline credential as `z.never()`, so the schema IS the list. + expect(refusedCredentialKeys('postgres')).toEqual(['password']); + expect(refusedCredentialKeys('mysql')).toEqual(['password']); + expect(refusedCredentialKeys('mongodb')).toEqual(['password']); + expect(refusedCredentialKeys('turso')).toEqual(['authToken']); + // Credential-less drivers declare none, and must not acquire one by accident. + expect(refusedCredentialKeys('sqlite')).toEqual([]); + expect(refusedCredentialKeys('memory')).toEqual([]); + }); + + it('the unknown-driver fallback covers every spelling the contracts refuse', () => { + // Guards the one hand-written list in the module: if a driver refuses a NEW + // credential key, the fallback used for contract-less drivers must learn it + // too, or an unknown driver's config would serve that spelling in cleartext. + const declared = new Set(); + for (const id of BUILTIN_DRIVER_IDS as readonly string[]) { + for (const key of refusedCredentialKeys(id)) declared.add(key); + } + const fallback = new Set(redactableConfigKeys('a-driver-with-no-contract')); + for (const key of declared) expect(fallback.has(key)).toBe(true); + }); + + it('a driver with no shipped contract still has its canonical credentials hidden', () => { + expect(getDriverConfigSchema('not-a-real-driver' as never)).toBeUndefined(); + const { config, redactedKeys } = redactDatasourceConfig('not-a-real-driver', { + host: 'h', + password: 'hunter2', + authToken: 'jwt', + }); + expect(config).toEqual({ host: 'h' }); + expect(redactedKeys).toEqual(['authToken', 'password']); + }); +}); + +describe('read path: getDatasource() no longer serves a stored credential', () => { + it('RED ON MAIN — a stored row\'s cleartext password does not reach the caller', async () => { + const { service } = makeService([LEGACY_PG]); + const ds = await service.getDatasource('legacy_pg'); + expect(ds).toBeDefined(); + expect(ds!.config).not.toHaveProperty('password'); + expect(JSON.stringify(ds!.config)).not.toContain('hunter2'); + // The non-credential half is untouched — this is a redaction, not a purge. + expect(ds!.config).toMatchObject({ host: 'db.internal', database: 'app', username: 'admin' }); + }); + + it('RED ON MAIN — the credential embedded in `config.url` does not reach the caller either', async () => { + const { service } = makeService([LEGACY_PG]); + const ds = await service.getDatasource('legacy_pg'); + // The scrub that stops at `config.password` is the one that only looks + // finished: on main this same read served the password twice over. + expect(ds!.config!.url).toBe('postgresql://admin@db.internal:5432/app'); + expect(ds!.redactedConfigKeys).toEqual(['password', 'url']); + }); + + it('names what it withheld, so a caller is not left inferring it from an absence', async () => { + const { service } = makeService([ + { name: 'clean', driver: 'postgres', origin: 'runtime', config: { host: 'h', database: 'd' } }, + ]); + const ds = await service.getDatasource('clean'); + expect(ds!.redactedConfigKeys).toEqual([]); + expect(ds!.config).toEqual({ host: 'h', database: 'd' }); + }); + + it('turso: the still-writable `encryptionKey` is redacted on read as well', async () => { + const { service } = makeService([{ + name: 'turso_ds', driver: 'turso', origin: 'runtime', + config: { url: 'libsql://db.turso.io', authToken: 'jwt-token', encryptionKey: 'aes-256-key' }, + }]); + const ds = await service.getDatasource('turso_ds'); + expect(ds!.config).toEqual({ url: 'libsql://db.turso.io' }); + expect(ds!.redactedConfigKeys).toEqual(['authToken', 'encryptionKey']); + }); + + it('pre-#8078 ALIAS spellings are redacted — a stored row never met the parse that renamed them', async () => { + const { service } = makeService([{ + name: 'ancient', driver: 'postgres', origin: 'runtime', + config: { host: 'h', database: 'd', passwd: 'hunter2', pwd: 'hunter2' }, + }]); + const ds = await service.getDatasource('ancient'); + expect(ds!.config).toEqual({ host: 'h', database: 'd' }); + expect(ds!.redactedConfigKeys).toEqual(['passwd', 'pwd']); + }); + + it('RED ON MAIN — the SERVED config is redacted while the STORED record keeps its credential', async () => { + const { service, records } = makeService([LEGACY_PG]); + const ds = await service.getDatasource('legacy_pg'); + + // Both halves in one pin, deliberately. Asserting only that the store is + // untouched passes on `origin/main` for the worst possible reason — nothing + // redacts there, so served and stored are the same object's contents and + // the pin is green while the defect is live. Pairing it with the served + // side makes the pin fail on main and pass here, which is the only version + // of it that pins anything (#7801). + expect(ds!.config).not.toHaveProperty('password'); + // The connect path reads the raw record (`getDatasourceRecord`), not this + // projection. If redaction ever mutated in place, a legacy datasource would + // stop authenticating the moment someone opened its edit form. + expect(records[0].config).toMatchObject({ + password: 'hunter2', + url: 'postgresql://admin:hunter2@db.internal:5432/app', + }); + }); +}); + +describe('URL redaction is surgical', () => { + it('removes only the password component of userinfo', () => { + expect(redactUrlPassword('postgresql://admin:hunter2@db:5432/app')) + .toBe('postgresql://admin@db:5432/app'); + expect(redactUrlPassword('mongodb://u:p@a.example.com:27017/db?replicaSet=rs0')) + .toBe('mongodb://u@a.example.com:27017/db?replicaSet=rs0'); + }); + + it('leaves a URL with no embedded password exactly as it was', () => { + for (const url of [ + 'postgresql://admin@db:5432/app', + 'postgresql://db:5432/app', + 'libsql://my-db.turso.io', + 'file:./data/objectstack.db', + ':memory:', + ]) { + expect(redactUrlPassword(url)).toBe(url); + } + }); + + it('a malformed password containing `@` is redacted WHOLE, not split at the first one', () => { + // The userinfo boundary is the LAST `@` before the path. Matching the first + // would leave `ss@host` behind — a redaction that publishes part of the + // password while looking like it worked. + expect(redactUrlPassword('postgres://u:p@ss@host/db')).toBe('postgres://u@host/db'); + expect(redactUrlPassword('postgres://u:p@ss@host/db')).not.toContain('ss'); + }); + + it('does not mistake a colon in a path or query for userinfo', () => { + // `@` and `:` after the first `/` are not userinfo, and a value that is not + // a URL at all must survive byte-for-byte. + expect(redactUrlPassword('https://host/a:b@c')).toBe('https://host/a:b@c'); + expect(redactUrlPassword('https://host/p?to=a:b@c')).toBe('https://host/p?to=a:b@c'); + expect(redactUrlPassword('public')).toBe('public'); + expect(redactUrlPassword('a:b@c')).toBe('a:b@c'); + }); +}); + +describe('write path: the scrub must not turn "Save" into credential deletion', () => { + it('RED WITHOUT THE RESTORE — an untouched round-trip keeps the stored credential', async () => { + const { service, records } = makeService([LEGACY_PG]); + // Exactly what the edit form does: GET, then PATCH the config it was given. + const read = await service.getDatasource('legacy_pg'); + await service.updateDatasource('legacy_pg', { config: read!.config, label: 'Renamed' }); + + expect(records[0].label).toBe('Renamed'); + expect(records[0].config).toMatchObject({ + password: 'hunter2', + url: 'postgresql://admin:hunter2@db.internal:5432/app', + }); + }); + + it('an author who edits a NON-credential field still gets that edit', async () => { + const { service, records } = makeService([LEGACY_PG]); + const read = await service.getDatasource('legacy_pg'); + await service.updateDatasource('legacy_pg', { + config: { ...read!.config, database: 'app_v2' }, + }); + expect(records[0].config).toMatchObject({ database: 'app_v2', password: 'hunter2' }); + }); + + it('an author who rewrites the URL by hand WINS — the restore never overrides an edit', async () => { + const { service, records } = makeService([LEGACY_PG]); + await service.updateDatasource('legacy_pg', { + config: { host: 'db.internal', database: 'app', username: 'admin', url: 'postgresql://admin@elsewhere:5432/app' }, + }); + // Not the stored URL: the patch differs from the redaction of it, so it is + // an edit, not a round-trip. + expect(records[0].config!.url).toBe('postgresql://admin@elsewhere:5432/app'); + }); + + it('changing the DRIVER does not carry the old driver\'s credential across', async () => { + const { service, records } = makeService([LEGACY_PG]); + const read = await service.getDatasource('legacy_pg'); + await service.updateDatasource('legacy_pg', { + driver: 'mysql', + config: { host: 'db.internal', database: 'app', username: 'admin' }, + }); + expect(read!.config).not.toHaveProperty('password'); + expect(records[0].driver).toBe('mysql'); + expect(records[0].config).not.toHaveProperty('password'); + }); + + it('restoreRedactedConfig is a no-op when there is nothing stored to restore', () => { + expect(restoreRedactedConfig('postgres', { host: 'h' }, undefined)).toEqual({ host: 'h' }); + expect(restoreRedactedConfig('postgres', { host: 'h' }, { host: 'h' })).toEqual({ host: 'h' }); + }); +}); + +describe('GREEN ON MAIN — #8078 is not weakened by anything above', () => { + it('the parse refusal still fires, and its guidance still reaches the caller', async () => { + const { service } = makeService([]); + await expect( + service.createDatasource({ + name: 'nope', driver: 'postgres', + config: { host: 'h', database: 'd', password: 'hunter2' }, + } as never), + ).rejects.toThrow(/is a credential and is not accepted inline/); + + // The guidance names BOTH mechanisms the refusal diverts to. A refusal that + // said only "not allowed" would leave the author with no next move. + const issues = validateDriverConfig('postgres', { host: 'h', password: 'x' }); + expect(issues).toMatchObject({ known: true }); + const message = (issues as { issues: Array<{ message: string }> }).issues[0].message; + expect(message).toContain('external.credentialsRef'); + expect(message).toContain('secret binder'); + }); + + it('an author who types a refused key into a PATCH is still refused', async () => { + const { service } = makeService([LEGACY_PG]); + await expect( + service.updateDatasource('legacy_pg', { + config: { host: 'h', database: 'd', password: 'newpassword' }, + } as never), + ).rejects.toThrow(/is a credential and is not accepted inline/); + }); + + it('GREEN ON MAIN — a URL-embedded credential is still ACCEPTED at the write door', () => { + // #7990 left refusing these UNRULED and #8078 pinned the acceptance as a + // FACT. Redacting on the way out must not become refusing on the way in; + // this pin fails the moment that boundary moves without a ruling. + expect(validateDriverConfig('postgres', { url: 'postgresql://u:pass@h:5432/d' })) + .toEqual({ known: true, issues: [] }); + }); +}); diff --git a/packages/services/service-datasource/src/admin-routes.ts b/packages/services/service-datasource/src/admin-routes.ts index 55a028f80a..02503eb5cb 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -49,7 +49,7 @@ const SERVICE_ERROR_CODE: Record = { * Served by `datasource-admin`: * * GET /datasources → listDatasources (provenance + health) - * GET /datasources/:name → getDatasource (credential-stripped) + * GET /datasources/:name → getDatasource (config credential-redacted) * POST /datasources/test → testConnection (no persistence) * POST /datasources → createDatasource (origin: 'runtime') * PATCH /datasources/:name → updateDatasource (runtime only) @@ -229,9 +229,12 @@ export function registerDatasourceAdminRoutes( // `datasource` `test_connection` action). Distinct from `POST /datasources/test` // which probes an unsaved draft carried inline. Registered before the generic // `:name` mutation routes. - // Read one datasource's full detail for the edit form (credential stripped; - // `config` is non-sensitive, plus a `hasSecret` flag). Registered after the - // static `/drivers` route so that literal segment is never captured as a name. + // Read one datasource's full detail for the edit form. `config` is redacted + // of every stored credential — including one embedded in a connection URL — + // and the response names what was withheld in `redactedConfigKeys`, alongside + // the `hasSecret` flag for the bound `sys_secret` handle (#8081). Registered + // after the static `/drivers` route so that literal segment is never captured + // as a name. server.get(`${root}/:name`, async (req: any, res: any) => { const svc = resolve(res, 'datasource-admin', 'getDatasource'); if (!svc) return; diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index 29345036a4..1c23e56042 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -18,11 +18,18 @@ * - Credentials never persist in cleartext: the cleartext {@link SecretInput} * transits create/update/test only; create/update write it to the secret * store and persist only the returned `credentialsRef`. + * - The read path never SERVES a credential, whatever a stored row holds + * (#8081): `getDatasource` redacts driver `config` through + * `datasource-config-redaction.ts`. That invariant is about what leaves this + * service, and is separate from the one above — rows written before #8078 + * can and do hold inline cleartext, which is why it is stated on its own + * rather than treated as a consequence. * - Removal is refused while objects are still bound to the datasource. */ import { validateDriverConfig } from '@objectstack/spec/data'; import { assertDatasourcePoolSupported } from './datasource-pool-support.js'; +import { redactDatasourceConfig, restoreRedactedConfig } from './datasource-config-redaction.js'; import type { IDatasourceAdminService, DatasourceDraft, @@ -176,31 +183,59 @@ export class DatasourceAdminService implements IDatasourceAdminService { } /** - * Read one datasource's full detail for editing, with the credential stripped. - * Returns `config` (non-sensitive — credentials live in `sys_secret`, never in - * config), `origin`, and a `hasSecret` flag so the UI can show "leave blank to - * keep" without ever receiving the `credentialsRef` or any cleartext. Returns - * `undefined` when the name is unknown. + * Read one datasource's full detail for editing, with every stored credential + * redacted out of `config` (#8081). + * + * Returns `config`, `origin`, a `hasSecret` flag so the UI can show "leave + * blank to keep" without ever receiving the `credentialsRef`, and + * `redactedConfigKeys` naming what was withheld. Returns `undefined` when the + * name is unknown. + * + * ## The claim this comment used to make + * + * It said "with the credential stripped", and described `config` as + * "non-sensitive — credentials live in `sys_secret`, never in config". Both + * halves were false, and load-bearing: nothing here stripped anything, and + * `config` was returned verbatim. A datasource row written before #8078 + * carries `config.password` / `config.authToken` in cleartext (that is the + * whole reason #8078 exists), and this method served it — to every caller of + * `GET /api/v1/datasources/:name` — under a comment asserting it could not. + * A safety claim that no code performs is worse than no claim: it is what + * stops the next reader from looking. + * + * What makes the claim true now is {@link redactDatasourceConfig}, which also + * covers the credential the old sentence could not have described — the one + * embedded in a `postgresql://user:pass@host` URL, which lives in `config` + * and is not `config.password`. Refusing such a URL at the WRITE door remains + * unruled (#7990) and is deliberately untouched here; hiding it on the way + * out is a separate act and is what this method owes its callers. + * + * The stored record is not modified. Cleartext already at rest stays at rest + * until the migration this card proposes runs — closing the read path is what + * stops it being SERVED, not what removes it. */ async getDatasource(name: string): Promise< | (Pick & { origin: 'code' | 'runtime'; hasSecret: boolean; + redactedConfigKeys: string[]; }) | undefined > { const rec = await this.config.getDatasourceRecord(name); if (!rec) return undefined; const hasSecret = Boolean(rec.external?.credentialsRef); + const { config, redactedKeys } = redactDatasourceConfig(rec.driver, rec.config); return { name: rec.name, label: rec.label, driver: rec.driver, schemaMode: rec.schemaMode ?? 'managed', - config: rec.config ?? {}, + config, active: rec.active ?? true, origin: rec.origin === 'runtime' ? 'runtime' : 'code', hasSecret, + redactedConfigKeys: redactedKeys, ...(rec.definedIn ? { definedIn: rec.definedIn } : {}), }; } @@ -321,6 +356,33 @@ export class DatasourceAdminService implements IDatasourceAdminService { if (prevRef && prevRef !== credentialsRef) await this.tryRemoveSecret(prevRef); } + // Carry forward the credential material `getDatasource()` redacts (#8081) + // — AFTER the gate above, and only when this patch is round-tripping the + // same driver's config back. + // + // After, because the two judgements have different subjects. The gate + // judges what the AUTHOR wrote, and #8078's refusal of an inline credential + // is aimed at exactly that. This restore replaces material the author never + // saw, was never offered the chance to write, and is not asking to change; + // running the gate over it would refuse a legacy row for the contents of + // its own stored config and make `active: false` — the way a misconfigured + // datasource is taken out of service — unreachable on the rows most likely + // to need it. Same shape as the `credentialsRef` preserved a few lines up, + // which is likewise carried across a patch without being re-judged. + // + // Only when the driver is unchanged: a patch that re-points a datasource at + // a different driver is rewiring the connection, and one driver's stored + // credential is not evidence about another's. + // + // This preserves cleartext already at rest; it does not create any. Getting + // that cleartext OUT of the store is the migration this card proposes + // (scope item 3) and is deliberately not attempted here — a write path that + // quietly dropped a credential the operator still depends on would be the + // destructive sweep that decision is reserved for. + if (patch.config !== undefined && merged.driver === existing.driver) { + merged.config = restoreRedactedConfig(existing.driver, merged.config, existing.config); + } + await this.config.putDatasourceRecord(merged); await this.tryRegisterPool(merged); return this.toSummary(merged); diff --git a/packages/services/service-datasource/src/datasource-config-redaction.ts b/packages/services/service-datasource/src/datasource-config-redaction.ts new file mode 100644 index 0000000000..b4417f2585 --- /dev/null +++ b/packages/services/service-datasource/src/datasource-config-redaction.ts @@ -0,0 +1,275 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Read-path credential redaction for a datasource's driver `config` (#8081, + * the services half of #7990). + * + * #8078 closed the WRITE door: `config.password` / `config.authToken` are + * declared-unwritable (`z.never()`) on every driver that has them, so no new + * row can carry an inline credential. It could not close the READ door, and it + * did not try: rows written before it still carry cleartext, and + * `DatasourceAdminService.getDatasource()` handed `config` back verbatim while + * its own doc comment claimed the credential had been stripped. This module is + * the strip that comment described. + * + * ## What counts as a credential here + * + * Three sources, in descending order of authority: + * + * 1. **Derived from the driver's own contract** — a config key whose schema is + * `z.never()` IS the shape #8078 gave a refused inline credential + * (`refusedInlineCredentialKey`), so reading the schema is reading the + * refusal list rather than re-typing it. A driver that refuses a new + * credential key tomorrow is covered here the day it lands, which a + * hand-maintained list in this package would not be. + * 2. **Former alias spellings** ({@link FORMER_CREDENTIAL_ALIASES}) — `passwd` + * / `pwd` / `token` / `jwt` / `auth_token` / `authtoken` used to be + * `aliases` that the parse RENAMED onto the canonical key; #8078 moved them + * to `guidance`, which refuses them. Neither spelling appears in the schema + * shape, and a stored row never went through the parse that would have + * renamed it — the wizard persists through `metadata.register`, whose + * validation is a structural name/label check. So the only place these can + * still be found is exactly the place this module reads: a stored row. + * 3. **Credential-shaped keys that are still WRITABLE** ({@link + * STILL_WRITABLE_CREDENTIAL_KEYS}) — today just turso's `encryptionKey`, an + * AES-256 key the binder has no slot for (#8081 scope item 4, which owns + * the decision about giving it one). Redacting it on READ neither grants + * nor removes that slot: the key stays writable, stays stored, and stays + * injected at connect. It simply stops being served back in cleartext, + * which is the one question this module answers. + * + * For a driver the platform ships no contract for, source 1 is empty — the + * registry is saying "nothing to check against", not "nothing to protect". The + * canonical spellings are therefore redacted by NAME for unknown drivers too. + * That asymmetry with the write gate (which deliberately lets an unknown + * driver's config through untouched) is intentional: declining to REFUSE an + * unrecognised key is a boundary choice about authoring, while serving a key + * literally named `password` back in cleartext is a leak under any boundary. + * + * ## URL-embedded credentials + * + * A `postgresql://user:pass@host/db` in `config.url` carries the same secret as + * `config.password`, and #7990/#8078 left refusing it explicitly UNRULED — the + * spec half pinned the behaviour as a fact rather than rejecting it. This + * module does not disturb that: nothing here refuses a URL, at any door. But a + * scrub that dropped `config.password` and then served the identical credential + * one key over would be a scrub in name only — the same "claims a protection it + * does not perform" shape #8081 exists to end. So the read path redacts the + * PASSWORD COMPONENT of a URL's userinfo and leaves everything else, including + * the username, byte-for-byte. Redacting a value on the way out is not the same + * act as refusing it on the way in, and only the second one is unruled. + * + * ## Why redaction must be reversible + * + * `getDatasource()` feeds the Studio edit form, and `updateDatasource()` takes + * that form's `config` back as a whole-object patch. A scrub with no inverse + * would therefore turn every "Save" on an unmodified form into silent credential + * DELETION — trading a disclosure bug for a data-loss bug. {@link + * restoreRedactedConfig} is that inverse, and it is the same rule the secret + * path next to it has always used ("preserve the existing `credentialsRef` + * unless a new secret rewraps it"), applied to the material this module hides. + */ + +import { getDriverConfigSchema } from '@objectstack/spec/data'; + +/** + * Canonical inline-credential spellings, used for a driver whose contract this + * platform does not ship. Kept in sync with the schemas by + * `datasource-config-redaction.test.ts`, which asserts every `z.never()` key + * across every builtin driver appears here — so a new refused key cannot land + * without this fallback learning it. + */ +const CANONICAL_CREDENTIAL_KEYS = ['password', 'authToken'] as const; + +/** + * Pre-#8078 alias spellings of the keys above. A row written through the wizard + * (which does not parse) can hold these verbatim; a row written through an + * authoring door had them renamed onto the canonical key before storage. + */ +const FORMER_CREDENTIAL_ALIASES = [ + 'passwd', + 'pwd', + 'token', + 'jwt', + 'auth_token', + 'authtoken', +] as const; + +/** + * Credential-shaped config keys that remain WRITABLE by deliberate spec choice, + * and so are never found by the `z.never()` derivation. + * + * `encryptionKey` (turso) is an AES-256 key for the local database file. #8078 + * left it writable because the datasource secret binder injects exactly one + * secret slot and `external.credentialsRef` resolution cannot target a second + * one; giving it a slot is #8081 scope item 4 and is NOT decided here. + */ +const STILL_WRITABLE_CREDENTIAL_KEYS: Record = { + turso: ['encryptionKey'], +}; + +/** Unwrap `.optional()` / `.default()` / `.nullable()` down to the base type. */ +function baseTypeOf(schema: unknown): string | undefined { + let node: any = schema; + for (let depth = 0; node && depth < 10; depth += 1) { + const def = node.def ?? node._def; + const type: string | undefined = def?.type; + if (!type) return undefined; + if (type === 'optional' || type === 'default' || type === 'nullable' || type === 'readonly') { + node = def.innerType; + continue; + } + return type; + } + return undefined; +} + +/** + * The inline-credential keys a driver's own contract declares unwritable. + * + * Empty for a driver with no shipped contract — see the module note on why the + * canonical spellings are still redacted in that case. + */ +export function refusedCredentialKeys(driver: unknown): string[] { + let shape: Record | undefined; + try { + const schema: any = getDriverConfigSchema(driver as never); + const raw = schema?.shape; + shape = typeof raw === 'function' ? raw() : raw; + } catch { + return []; + } + if (!shape) return []; + return Object.entries(shape) + .filter(([, member]) => baseTypeOf(member) === 'never') + .map(([key]) => key); +} + +/** Every config key this module hides for `driver`, canonical + alias + writable-but-secret. */ +export function redactableConfigKeys(driver: unknown): string[] { + const derived = refusedCredentialKeys(driver); + const canonical = derived.length > 0 ? derived : [...CANONICAL_CREDENTIAL_KEYS]; + const stillWritable = typeof driver === 'string' ? (STILL_WRITABLE_CREDENTIAL_KEYS[driver] ?? []) : []; + return [...new Set([...canonical, ...FORMER_CREDENTIAL_ALIASES, ...stillWritable])]; +} + +/** + * `scheme://[user[:password]@]rest`. Anchored, and every class excludes `/?#` + * so a password-looking substring in a path or query cannot be mistaken for one + * — `https://host/a:b@c` has no userinfo and must come back untouched. + * + * The password group deliberately ALLOWS `@` and is greedy, which (with + * backtracking) makes the match end at the LAST `@` before the path — the + * userinfo boundary RFC 3986 actually defines. A lazier class stopping at the + * first `@` would split `postgres://u:p@ss@host/db` after `p`, leave `ss@host` + * in place, and publish a fragment of the password while looking redacted. + * Such a URL is malformed (a literal `@` in userinfo must be `%40`), which is + * precisely why it must not be the case that decides how much leaks. + */ +const URL_USERINFO_RE = /^([a-z][a-z0-9+.\-]*:\/\/)([^/?#@:]*)(:[^/?#]*)@/i; + +/** + * Strip the password component from a URL's userinfo, preserving the scheme, + * the username, and everything from the host onward. + * + * Returns the input unchanged when there is nothing to strip, which is what + * makes "did this value change?" a usable test for whether a credential was + * present. + */ +export function redactUrlPassword(value: string): string { + return value.replace(URL_USERINFO_RE, (_m, scheme: string, user: string) => `${scheme}${user}@`); +} + +/** A driver `config` with its credential material removed, and what was removed. */ +export interface RedactedDatasourceConfig { + config: Record; + /** + * Config keys whose value was removed or rewritten, sorted. Serving this + * alongside the redacted config is the difference between a caller that knows + * a credential is being withheld and one that infers it from an absence. + */ + redactedKeys: string[]; +} + +/** + * Remove every stored credential from a driver `config` for serving on a read + * path. + * + * Pure: the input object is never mutated, so a caller holding the stored + * record (the connect path does) is unaffected. + */ +export function redactDatasourceConfig( + driver: unknown, + config: Record | undefined, +): RedactedDatasourceConfig { + if (!config || typeof config !== 'object') return { config: {}, redactedKeys: [] }; + + const hidden = new Set(redactableConfigKeys(driver)); + const out: Record = {}; + const redactedKeys: string[] = []; + + for (const [key, value] of Object.entries(config)) { + if (hidden.has(key)) { + // Dropped, not masked. A mask would round-trip back through the wizard as + // a literal new password, and post-#8078 the canonical spellings would + // then be REFUSED at the write door — turning an untouched "Save" into an + // error the author cannot act on. An absent key is the shape the form + // already understands from `hasSecret`. + if (value !== undefined) redactedKeys.push(key); + continue; + } + if (typeof value === 'string') { + const redacted = redactUrlPassword(value); + if (redacted !== value) { + out[key] = redacted; + redactedKeys.push(key); + continue; + } + } + out[key] = value; + } + + return { config: out, redactedKeys: redactedKeys.sort() }; +} + +/** + * Re-apply the credential material {@link redactDatasourceConfig} hid, for a + * patch that is round-tripping a previously-read config back to the store. + * + * The rule is deliberately narrow: stored material is carried forward ONLY + * where the patch is indistinguishable from what the read path served — an + * absent key, or a URL that matches the stored URL once redacted. Anything the + * author actually changed wins, including clearing a URL's password by hand. + * + * What this does NOT do is let a patch set a refused key: `assertValidConfig` + * still runs on the merged record, so a caller that types `password` into the + * config gets #8078's refusal exactly as it would without this function. + */ +export function restoreRedactedConfig( + driver: unknown, + patch: Record | undefined, + stored: Record | undefined, +): Record | undefined { + if (!patch || typeof patch !== 'object') return patch; + if (!stored || typeof stored !== 'object') return patch; + + const hidden = new Set(redactableConfigKeys(driver)); + const out: Record = { ...patch }; + + for (const key of hidden) { + // Only when the patch does not speak to the key at all. A patch that DOES + // carry it is the author's word, and (for a refused spelling) is about to + // be refused on its own merits rather than quietly overwritten here. + if (!(key in out) && stored[key] !== undefined) out[key] = stored[key]; + } + + for (const [key, storedValue] of Object.entries(stored)) { + if (hidden.has(key) || typeof storedValue !== 'string') continue; + const redactedStored = redactUrlPassword(storedValue); + // Unchanged by redaction ⇒ it carried no credential ⇒ nothing to restore. + if (redactedStored === storedValue) continue; + if (out[key] === redactedStored) out[key] = storedValue; + } + + return out; +}