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; +}