Skip to content

Commit 4420662

Browse files
committed
fix(service-datasource): carry a withheld value forward only where the read path still withholds it
restoreRedactedConfig re-redacts the merged config and keeps only the grafts whose landing path is still withheld, repeating until nothing more drops (the /meta carry-forward's loop). An untouched Save skips the second walk: the merged config is the stored one. Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2320178 commit 4420662

2 files changed

Lines changed: 99 additions & 6 deletions

File tree

‎packages/services/service-datasource/src/__tests__/datasource-contractless-credentials.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,60 @@ describe('a withheld array value is carried only onto the element it came from',
261261
});
262262
});
263263

264+
describe('a value is carried forward only where the read path would still withhold it', () => {
265+
// A pair's `value` is credential material only while a label names a
266+
// credential, so an edit to the LABEL changes whether the value beside it is
267+
// withheld. The pair sits under a plain (non-credential) record key, so the
268+
// array identity rule does not apply and only the re-judgment can decide.
269+
const STORED = {
270+
host: 'wh.internal',
271+
proxyHeader: { name: 'Authorization', value: 'Bearer px-1' },
272+
probeHeader: { key: 'X-Api-Key', value: 'pk-2' },
273+
};
274+
const SERVED = { host: 'wh.internal', proxyHeader: { name: 'Authorization' }, probeHeader: { key: 'X-Api-Key' } };
275+
const restore = (patch: Record<string, unknown>) => restoreRedactedConfig(DRIVER, patch, STORED) as Record<string, unknown>;
276+
277+
it('control: an untouched Save carries both values forward', () => {
278+
expect(restore(structuredClone(SERVED))).toEqual(STORED);
279+
});
280+
281+
it('a label renamed to a non-credential name: the value is dropped, not served', () => {
282+
const out = restore({ ...structuredClone(SERVED), proxyHeader: { name: 'X-Trace' } });
283+
expect(out.proxyHeader).toEqual({ name: 'X-Trace' });
284+
expect(out.probeHeader).toEqual(STORED.probeHeader);
285+
expect(JSON.stringify(out)).not.toContain('px-1');
286+
});
287+
288+
it('a label deleted: the value is dropped, not served', () => {
289+
const out = restore({ ...structuredClone(SERVED), proxyHeader: {} });
290+
expect(out.proxyHeader).toEqual({});
291+
expect(JSON.stringify(out)).not.toContain('px-1');
292+
});
293+
294+
it('a `key:` label renamed: the value is dropped, not served', () => {
295+
const out = restore({ ...structuredClone(SERVED), probeHeader: { key: 'Accept' } });
296+
expect(out.probeHeader).toEqual({ key: 'Accept' });
297+
expect(out.proxyHeader).toEqual(STORED.proxyHeader);
298+
expect(JSON.stringify(out)).not.toContain('pk-2');
299+
});
300+
301+
it('a label renamed to ANOTHER credential name still carries the value (it stays withheld)', () => {
302+
const out = restore({ ...structuredClone(SERVED), proxyHeader: { name: 'Proxy-Authorization' } });
303+
expect(out.proxyHeader).toEqual({ name: 'Proxy-Authorization', value: 'Bearer px-1' });
304+
});
305+
306+
it('whatever survives the restore is withheld again on the next read', async () => {
307+
const { service } = makeService([{ name: 'w', driver: DRIVER, origin: 'runtime', config: structuredClone(STORED) }]);
308+
const read = await service.getDatasource('w');
309+
const config = structuredClone(read!.config) as Record<string, unknown>;
310+
config.proxyHeader = { name: 'X-Trace' };
311+
config.probeHeader = {};
312+
await service.updateDatasource('w', { config });
313+
const again = await service.getDatasource('w');
314+
expect(JSON.stringify(again)).not.toMatch(/px-1|pk-2/);
315+
});
316+
});
317+
264318
describe('planCredentialMigration names a contractless row\'s credentials as residue', () => {
265319
it('refuses with a remedy instead of reporting nothing-to-migrate', () => {
266320
const plan = planCredentialMigration({

‎packages/services/service-datasource/src/datasource-config-redaction.ts‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,17 @@ export {
9797
* raw-headers list's value) is carried only when the patch's array equals the
9898
* served array.
9999
*
100+
* ## Only where the read path would still withhold it
101+
*
102+
* Whether a position is withheld can depend on its siblings: the `value` of a
103+
* `{ name, value }` pair is credential material only while a label names a
104+
* credential. A value carried forward beside an EDITED sibling (the label
105+
* renamed, deleted, or moved to another label key) could therefore land where
106+
* the read path would serve it. So the grafted config is redacted again, and a
107+
* graft survives only when the redaction still withholds its landing path —
108+
* repeated until nothing more drops. A dropped graft leaves the patch as the
109+
* author sent it there.
110+
*
100111
* What this does NOT do is let a patch set a refused key: `assertValidConfig`
101112
* still runs on the merged record, so a caller that types `password` into the
102113
* config gets #8078's refusal exactly as it would without this function.
@@ -110,20 +121,48 @@ export function restoreRedactedConfig(
110121
if (!stored || typeof stored !== 'object') return patch;
111122

112123
const served = redactDatasourceConfig(driver, stored);
113-
let out: Record<string, unknown> = patch;
114-
124+
const grafts: Array<{ landing: string[]; value: unknown }> = [];
115125
for (const path of served.redactedPaths) {
116126
const storedLeaf = valueAt(stored, path);
117127
if (storedLeaf === undefined) continue;
118128
const landing = landingPath(served.config, patch, path);
119-
if (!landing) continue;
120-
if (out === patch) out = { ...patch };
121-
graftAt(out, landing, storedLeaf);
129+
if (landing) grafts.push({ landing, value: storedLeaf });
122130
}
131+
if (grafts.length === 0) return patch;
132+
133+
const graftAll = (kept: readonly { landing: string[]; value: unknown }[]): Record<string, unknown> => {
134+
const out: Record<string, unknown> = { ...patch };
135+
for (const graft of kept) graftAt(out, graft.landing, graft.value);
136+
return out;
137+
};
123138

124-
return out;
139+
// An untouched Save — the patch IS the served projection — grafts every
140+
// withheld value back onto exactly what it was withheld from, so the merged
141+
// config is the stored one and the read path withholds the same positions:
142+
// no second walk is owed.
143+
if (grafts.length === served.redactedPaths.length && sameValue(patch, served.config)) return graftAll(grafts);
144+
145+
// Otherwise keep only what the read path would STILL withhold where it
146+
// lands. The judgment of a position can depend on its siblings — a pair's
147+
// `value` is a credential only while a label names one — so a value carried
148+
// under an edited sibling may land where the read path would serve it.
149+
// Each pass re-grafts the survivors onto the untouched patch and drops every
150+
// graft the merged config's redaction no longer withholds AT its landing
151+
// path; the set only shrinks, so this settles within one pass per graft.
152+
// Same loop as the `/meta` carry-forward's (#20590).
153+
let kept = grafts;
154+
for (;;) {
155+
const out = graftAll(kept);
156+
const withheld = new Set(redactDatasourceConfig(driver, out).redactedPaths.map(pathKey));
157+
const next = kept.filter((graft) => withheld.has(pathKey(graft.landing)));
158+
if (next.length === kept.length) return next.length === 0 ? patch : out;
159+
kept = next;
160+
}
125161
}
126162

163+
/** A path as one comparable string (segments may hold any character, so JSON, not a join). */
164+
const pathKey = (path: readonly string[]): string => JSON.stringify(path);
165+
127166
/** An array position, as the redactor spells it in a path (its decimal index). */
128167
const isIndex = (segment: string): boolean => /^(0|[1-9][0-9]*)$/.test(segment);
129168

0 commit comments

Comments
 (0)