diff --git a/.changeset/21955-spec-redaction-driver-identity.md b/.changeset/21955-spec-redaction-driver-identity.md new file mode 100644 index 00000000000..759ef51e8b8 --- /dev/null +++ b/.changeset/21955-spec-redaction-driver-identity.md @@ -0,0 +1,12 @@ +--- +'@objectstack/spec': patch +--- + +The datasource read redaction resolves a driver's identity the way its sibling helper does. The per-driver half of `redactableConfigKeys` now looks a driver up through `resolveDriverId`, the resolver `passthroughSecretPaths` and the write door's contract lookup already use. So every spelling the write door accepts as a builtin driver is redacted as that driver. + +Clause-②: no + +- The still-writable credential key is withheld on the datasource admin read (`GET /api/v1/datasources/:name`) and on the metadata read under every accepted spelling of its driver, and `redactedConfigKeys` names it. +- A crafted driver id that made the read throw now answers as a driver the platform ships no contract for: the canonical credential spellings and the former aliases are withheld, and the read succeeds. +- `restoreRedactedConfig` and the credential migration read the same list, so an untouched Save still restores the stored value under every accepted spelling, and the migration names the still-writable key as residue there too. +- Unchanged: what the write door accepts, every export and its type, and the answer for a canonical spelling. 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 index 635dada2872..bcdf1226c84 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-config-redaction.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-config-redaction.test.ts @@ -39,7 +39,7 @@ */ import { describe, it, expect } from 'vitest'; -import { validateDriverConfig, getDriverConfigSchema, BUILTIN_DRIVER_IDS } from '@objectstack/spec/data'; +import { validateDriverConfig, getDriverConfigSchema, BUILTIN_DRIVER_IDS, DRIVER_ID_ALIASES } from '@objectstack/spec/data'; import { redactDatasourceConfig, restoreRedactedConfig, @@ -603,3 +603,77 @@ describe('nested credential positions OFF the passthrough table — the class co expect((records[0].config!.replication as any).url).toBe('postgresql://svc@other-replica/db'); }); }); + +// Lockstep with the spec fix that makes the per-driver half of +// `redactableConfigKeys` resolve a driver's identity through `resolveDriverId` +// (#21955). This door reaches it with no source change: `getDatasource()` and +// `restoreRedactedConfig` both go through `redactDatasourceConfig`. Spellings +// are DERIVED from the alias table and the resolver's own folding, never +// listed by hand. +describe('every accepted spelling of a builtin driver, at the service door', () => { + const variantsOf = (spelling: string): string[] => [ + spelling, + spelling.toUpperCase(), + `${spelling.charAt(0).toUpperCase()}${spelling.slice(1)}`, + ` ${spelling} `, + ]; + /** The driver-specific keys beyond the contract derivation and the contract-less fallback. */ + const stillWritableOf = (id: string): string[] => { + const fallback = new Set(redactableConfigKeys('a-driver-with-no-contract')); + const refused = new Set(refusedCredentialKeys(id)); + return redactableConfigKeys(id).filter((key) => !fallback.has(key) && !refused.has(key)); + }; + const CASES = Object.entries(DRIVER_ID_ALIASES).flatMap(([spelling, id]) => + variantsOf(spelling).flatMap((variant) => stillWritableOf(id).map((key) => ({ variant, key }))), + ); + const spelledRow = (variant: string, key: string): StoredDatasource => ({ + name: 'spelled_ds', + driver: variant, + origin: 'runtime', + config: { url: 'file:./fixture.db', [key]: 'fixture-secret' }, + }); + + it('getDatasource() withholds the still-writable credential key under each accepted spelling, and names it', async () => { + // Population floor: the class is not empty today, so a derivation that + // silently found nothing fails here instead of passing over zero rows. + expect(CASES.length).toBeGreaterThan(0); + for (const { variant, key } of CASES) { + const { service } = makeService([spelledRow(variant, key)]); + const ds = await service.getDatasource('spelled_ds'); + expect(ds!.config, JSON.stringify(variant)).toEqual({ url: 'file:./fixture.db' }); + expect(ds!.redactedConfigKeys, JSON.stringify(variant)).toEqual([key]); + } + }); + + it('an untouched Save restores the stored value under each accepted spelling, so redaction never turns a save into deletion', async () => { + for (const { variant, key } of CASES) { + const { service, records } = makeService([spelledRow(variant, key)]); + // Exactly what the edit form does: GET, then PATCH the config it was given. + const read = await service.getDatasource('spelled_ds'); + expect(read!.config, JSON.stringify(variant)).not.toHaveProperty(key); + await service.updateDatasource('spelled_ds', { config: read!.config, label: 'Renamed' }); + expect(records[0].label).toBe('Renamed'); + expect(records[0].config, JSON.stringify(variant)).toEqual({ url: 'file:./fixture.db', [key]: 'fixture-secret' }); + } + }); + + it('a crafted driver id is read with its credentials withheld, and an untouched Save keeps them', async () => { + // Every name an object literal inherits: none is a builtin spelling, so + // each answers as a driver with no shipped contract, never with a throw. + const crafted = Object.getOwnPropertyNames(Object.prototype); + expect(crafted.length).toBeGreaterThan(0); + for (const name of crafted) { + const { service, records } = makeService([{ + name: 'crafted_ds', + driver: name, + origin: 'runtime', + config: { host: 'fixture-host', password: 'fixture-secret' }, + }]); + const ds = await service.getDatasource('crafted_ds'); + expect(ds!.config, name).toEqual({ host: 'fixture-host' }); + expect(ds!.redactedConfigKeys, name).toEqual(['password']); + await service.updateDatasource('crafted_ds', { config: ds!.config }); + expect(records[0].config, name).toEqual({ host: 'fixture-host', password: 'fixture-secret' }); + } + }); +}); diff --git a/packages/services/service-datasource/src/__tests__/datasource-credential-migration.test.ts b/packages/services/service-datasource/src/__tests__/datasource-credential-migration.test.ts index be0e01524de..e673d5f2e94 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-credential-migration.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-credential-migration.test.ts @@ -16,7 +16,7 @@ */ import { describe, it, expect } from 'vitest'; -import { BUILTIN_DRIVER_IDS, refusedCredentialKeys } from '@objectstack/spec/data'; +import { BUILTIN_DRIVER_IDS, DRIVER_ID_ALIASES, redactableConfigKeys, refusedCredentialKeys } from '@objectstack/spec/data'; import { planCredentialMigration, urlCredentialKeys } from '../datasource-credential-migration.js'; import type { StoredDatasource } from '../datasource-admin-service.js'; @@ -289,3 +289,53 @@ describe('#9040 — the passthrough spelling at the planner door', () => { expect(plan).toEqual({ action: 'none', status: 'nothing-to-migrate', remaining: [] }); }); }); + +// Lockstep with the spec fix that makes the per-driver half of +// `redactableConfigKeys` resolve a driver's identity through `resolveDriverId` +// (#21955): the planner's unbindable residue is derived from that same list, +// so a row whose driver is written in any accepted spelling must be planned +// exactly as the canonical spelling's row is. Spellings are DERIVED from the +// alias table and the resolver's own folding, never listed by hand. +describe('planCredentialMigration reads the same list under every accepted spelling of a builtin driver', () => { + const variantsOf = (spelling: string): string[] => [ + spelling, + spelling.toUpperCase(), + `${spelling.charAt(0).toUpperCase()}${spelling.slice(1)}`, + ` ${spelling} `, + ]; + /** The driver-specific keys beyond the contract derivation and the contract-less fallback. */ + const stillWritableOf = (id: string): string[] => { + const fallback = new Set(redactableConfigKeys('a-driver-with-no-contract')); + const refused = new Set(refusedCredentialKeys(id)); + return redactableConfigKeys(id).filter((key) => !fallback.has(key) && !refused.has(key)); + }; + const CASES = Object.entries(DRIVER_ID_ALIASES).flatMap(([spelling, id]) => + variantsOf(spelling).flatMap((variant) => stillWritableOf(id).map((key) => ({ variant, id, key }))), + ); + + it('a bindable row names the still-writable credential key as its residue, byte-equal to the canonical spelling', () => { + // Population floor: the class is not empty today. + expect(CASES.length).toBeGreaterThan(0); + for (const { variant, id, key } of CASES) { + const [slot] = refusedCredentialKeys(id); + expect(slot, id).toBeDefined(); + const config = { url: 'file:./fixture.db', [slot as string]: 'fixture-token', [key]: 'fixture-secret' }; + const canonical = planCredentialMigration(row({ driver: id, config })); + expect(canonical).toEqual({ action: 'bind', key: slot, value: 'fixture-token', remaining: [key] }); + expect(planCredentialMigration(row({ driver: variant, config })), JSON.stringify(variant)).toEqual(canonical); + } + }); + + it('a row holding only the still-writable credential key is refused with that key named, as under the canonical spelling', () => { + for (const { variant, id, key } of CASES) { + const config = { url: 'file:./fixture.db', [key]: 'fixture-secret' }; + const canonical = planCredentialMigration(row({ driver: id, config })); + const spelled = planCredentialMigration(row({ driver: variant, config })); + expect(canonical.action).toBe('refuse'); + expect(spelled.action, JSON.stringify(variant)).toBe('refuse'); + if (spelled.action !== 'refuse') throw new Error('unreachable'); + expect(spelled.reason, JSON.stringify(variant)).toContain(`config.${key}`); + expect(spelled.remedy.length).toBeGreaterThan(0); + } + }); +}); diff --git a/packages/spec/src/data/datasource-credential-redaction.test.ts b/packages/spec/src/data/datasource-credential-redaction.test.ts index 8ee4e2a1371..0bebbc1d24c 100644 --- a/packages/spec/src/data/datasource-credential-redaction.test.ts +++ b/packages/spec/src/data/datasource-credential-redaction.test.ts @@ -32,10 +32,13 @@ import { BUILTIN_DRIVER_IDS, CREDENTIAL_KEY_SPELLINGS, CREDENTIAL_URL_QUERY_PARAM_NAMES, + DRIVER_ID_ALIASES, getDriverConfigSchema, + resolveDriverId, urlCredentialQueryParams, urlUserinfoPassword, urlUserinfoUsername, + validateDriverConfig, } from './driver/index'; import { passthroughSecretPaths, @@ -617,3 +620,91 @@ describe('refusedCredentialPaths — the schema derivation, walked at depth', () } }); }); + +// The per-driver half of `redactableConfigKeys` resolves a driver's identity +// through `resolveDriverId`, the resolver its sibling helper +// `passthroughSecretPaths` and the write door's contract lookup already use +// (#21955). Every spelling below is DERIVED from the alias table and the +// resolver's own folding (trim + lower-case), never listed by hand, so a +// spelling added to the vocabulary is pinned the day it lands. +describe('driver identity: every accepted spelling of a builtin driver is redacted as that driver', () => { + /** The spellings the resolver folds onto one alias: as-is, upper, capitalised, padded. */ + const variantsOf = (spelling: string): string[] => [ + spelling, + spelling.toUpperCase(), + `${spelling.charAt(0).toUpperCase()}${spelling.slice(1)}`, + ` ${spelling} `, + ]; + const ACCEPTED = Object.entries(DRIVER_ID_ALIASES).flatMap(([spelling, id]) => + variantsOf(spelling).map((variant) => [variant, id] as const), + ); + /** The driver-specific keys beyond the contract derivation and the contract-less fallback. */ + const stillWritableOf = (id: string): string[] => { + const fallback = new Set(redactableConfigKeys('a-driver-with-no-contract')); + const refused = new Set(refusedCredentialKeys(id)); + return redactableConfigKeys(id).filter((key) => !fallback.has(key) && !refused.has(key)); + }; + + it('each accepted spelling is judged by the write door against its builtin driver contract', () => { + // The premise of the pins below: the write door does not treat these as + // unknown plugin drivers, so the read door must not either. + expect(ACCEPTED.length).toBe(Object.keys(DRIVER_ID_ALIASES).length * 4); + for (const [variant, id] of ACCEPTED) { + expect(resolveDriverId(variant), JSON.stringify(variant)).toBe(id); + expect(validateDriverConfig(variant, {}).known, JSON.stringify(variant)).toBe(true); + } + }); + + it('each accepted spelling answers its canonical driver\'s redactable set, byte-equal', () => { + for (const [variant, id] of ACCEPTED) { + expect(redactableConfigKeys(variant), JSON.stringify(variant)).toEqual(redactableConfigKeys(id)); + } + }); + + it('the still-writable credential key is withheld under each accepted spelling and named as withheld', () => { + let judged = 0; + for (const [variant, id] of ACCEPTED) { + for (const key of stillWritableOf(id)) { + const stored = { url: 'file:./fixture.db', [key]: 'fixture-secret' }; + const { config, redactedKeys } = redactDatasourceConfig(variant, stored); + expect(config, JSON.stringify(variant)).toEqual({ url: 'file:./fixture.db' }); + expect(redactedKeys, JSON.stringify(variant)).toEqual([key]); + judged += 1; + } + } + // Population floor: the still-writable class is not empty today, so a + // derivation that silently found nothing would fail here instead of + // passing over zero spellings. + expect(judged).toBeGreaterThan(0); + }); + + it('control: a canonical spelling withholds the still-writable credential key exactly as before', () => { + let judged = 0; + for (const id of BUILTIN_DRIVER_IDS as readonly string[]) { + for (const key of stillWritableOf(id)) { + const { config, redactedKeys } = redactDatasourceConfig(id, { url: 'file:./fixture.db', [key]: 'fixture-secret' }); + expect(config, id).toEqual({ url: 'file:./fixture.db' }); + expect(redactedKeys, id).toEqual([key]); + judged += 1; + } + } + expect(judged).toBeGreaterThan(0); + }); + + it('a crafted driver id answers as a driver with no shipped contract: credentials withheld, never a throw', () => { + // Every name an object literal inherits. None is a builtin spelling, so + // each must resolve to no driver and take the contract-less fallback, not + // index an inherited member of the still-writable table. + const crafted = Object.getOwnPropertyNames(Object.prototype); + expect(crafted.length).toBeGreaterThan(0); + const fallback = redactableConfigKeys('a-driver-with-no-contract'); + for (const name of crafted) { + expect(resolveDriverId(name), name).toBeUndefined(); + expect(() => redactableConfigKeys(name), name).not.toThrow(); + expect(redactableConfigKeys(name), name).toEqual(fallback); + const { config, redactedKeys } = redactDatasourceConfig(name, { host: 'fixture-host', password: 'fixture-secret' }); + expect(config, name).toEqual({ host: 'fixture-host' }); + expect(redactedKeys, name).toEqual(['password']); + } + }); +}); diff --git a/packages/spec/src/data/datasource-credential-redaction.ts b/packages/spec/src/data/datasource-credential-redaction.ts index 43066bf2622..b543590328e 100644 --- a/packages/spec/src/data/datasource-credential-redaction.ts +++ b/packages/spec/src/data/datasource-credential-redaction.ts @@ -127,6 +127,11 @@ import { getDriverConfigSchema, resolveDriverId } from './driver/config-registry * 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. + * + * Keyed by CANONICAL driver id and looked up through {@link resolveDriverId}, + * exactly like {@link PASSTHROUGH_SECRET_PATHS} below: every spelling the + * write door judges against a driver's contract is redacted as that driver, + * and an id that resolves to no builtin indexes nothing (#21955). */ const STILL_WRITABLE_CREDENTIAL_KEYS: Record = { turso: ['encryptionKey'], @@ -370,7 +375,8 @@ export function refusedCredentialKeys(driver: unknown): string[] { 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] ?? []) : []; + const id = resolveDriverId(driver); + const stillWritable = id ? (STILL_WRITABLE_CREDENTIAL_KEYS[id] ?? []) : []; return [...new Set([...canonical, ...FORMER_CREDENTIAL_ALIASES, ...stillWritable])]; }