secret-guard: count a secret once when bash returns the same output in content and structuredContent
Measured in a full-extension session: one `echo "$SECRET"` was reported as "redacted ... x2". pi's bash tool returns its output both as content text and as structuredContent.output, and both are (correctly) redacted. The count now takes, per name, the highest count from any one field instead of the sum. A test pins it: it failed before the change and passes after. Re-run in the full-extension TUI: the warning now reads "redacted SG_TEST_TOKEN from bash output"; 0 raw copies sent, 0 in the session file. Suite: 46/46.
This commit is contained in:
@@ -304,6 +304,9 @@ Read the module header for the incident and for what is not covered.
|
|||||||
identity for untouched values so this is cheap to detect. A handler that
|
identity for untouched values so this is cheap to detect. A handler that
|
||||||
always returned a copy would rewrite every tool result and every request
|
always returned a copy would rewrite every tool result and every request
|
||||||
context, for nothing.
|
context, for nothing.
|
||||||
|
- **Hits are counted per field, then the highest count per name is reported.**
|
||||||
|
pi's bash returns its output twice (content text and `structuredContent.output`),
|
||||||
|
so a sum would report one `echo` as "×2".
|
||||||
- **The `context` pass walks the whole conversation on every request.** Measured:
|
- **The `context` pass walks the whole conversation on every request.** Measured:
|
||||||
36 ms on a 53 MB session, under 1 ms on a typical one.
|
36 ms on a 53 MB session, under 1 ms on a typical one.
|
||||||
- **End-to-end check** (2026-10-12, real pi on erdc/deepseek-v4.1-flash; the secret
|
- **End-to-end check** (2026-10-12, real pi on erdc/deepseek-v4.1-flash; the secret
|
||||||
|
|||||||
@@ -126,6 +126,13 @@ function addHits(into: Hits, from: Hits) {
|
|||||||
for (const [k, n] of from) into.set(k, (into.get(k) ?? 0) + n);
|
for (const [k, n] of from) into.set(k, (into.get(k) ?? 0) + n);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// One tool result can carry the same output in several fields: pi's bash returns
|
||||||
|
// it as content text AND structuredContent.output. Count a name by the field that
|
||||||
|
// holds it most often, not by the sum, so a single echo is reported once.
|
||||||
|
function maxHits(into: Hits, from: Hits) {
|
||||||
|
for (const [k, n] of from) into.set(k, Math.max(into.get(k) ?? 0, n));
|
||||||
|
}
|
||||||
|
|
||||||
/** Replace every known value. Text with no secret comes back as the same string. */
|
/** Replace every known value. Text with no secret comes back as the same string. */
|
||||||
export function redactText(text: string, known: Known): { text: string; hits: Hits } {
|
export function redactText(text: string, known: Known): { text: string; hits: Hits } {
|
||||||
const hits: Hits = new Map();
|
const hits: Hits = new Map();
|
||||||
@@ -185,6 +192,7 @@ interface ToolResultLike {
|
|||||||
export function toolResultChange(event: ToolResultLike, known: Known) {
|
export function toolResultChange(event: ToolResultLike, known: Known) {
|
||||||
if (known.length === 0) return undefined;
|
if (known.length === 0) return undefined;
|
||||||
const hits: Hits = new Map();
|
const hits: Hits = new Map();
|
||||||
|
const contentHits: Hits = new Map();
|
||||||
const result: Record<string, unknown> = {};
|
const result: Record<string, unknown> = {};
|
||||||
|
|
||||||
let contentChanged = false;
|
let contentChanged = false;
|
||||||
@@ -193,14 +201,15 @@ export function toolResultChange(event: ToolResultLike, known: Known) {
|
|||||||
const r = redactText(part.text, known);
|
const r = redactText(part.text, known);
|
||||||
if (r.hits.size === 0) return part;
|
if (r.hits.size === 0) return part;
|
||||||
contentChanged = true;
|
contentChanged = true;
|
||||||
addHits(hits, r.hits);
|
addHits(contentHits, r.hits);
|
||||||
return { ...part, text: r.text };
|
return { ...part, text: r.text };
|
||||||
});
|
});
|
||||||
if (contentChanged) result.content = content;
|
if (contentChanged) result.content = content;
|
||||||
|
maxHits(hits, contentHits);
|
||||||
|
|
||||||
if (event.structuredContent !== undefined) {
|
if (event.structuredContent !== undefined) {
|
||||||
const r = redactDeep(event.structuredContent, known);
|
const r = redactDeep(event.structuredContent, known);
|
||||||
addHits(hits, r.hits);
|
maxHits(hits, r.hits);
|
||||||
// Replacing content without returning structuredContent DROPS it (types.d.ts),
|
// Replacing content without returning structuredContent DROPS it (types.d.ts),
|
||||||
// so return it whenever content changed, redacted or not.
|
// so return it whenever content changed, redacted or not.
|
||||||
if (r.value !== event.structuredContent || contentChanged) result.structuredContent = r.value;
|
if (r.value !== event.structuredContent || contentChanged) result.structuredContent = r.value;
|
||||||
@@ -208,7 +217,7 @@ export function toolResultChange(event: ToolResultLike, known: Known) {
|
|||||||
if (event.details !== undefined && event.details !== null) {
|
if (event.details !== undefined && event.details !== null) {
|
||||||
const r = redactDeep(event.details, known);
|
const r = redactDeep(event.details, known);
|
||||||
if (r.value !== event.details) {
|
if (r.value !== event.details) {
|
||||||
addHits(hits, r.hits);
|
maxHits(hits, r.hits);
|
||||||
result.details = r.value;
|
result.details = r.value;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -191,6 +191,15 @@ test("toolResultChange: secrets in details or structuredContent alone are redact
|
|||||||
assert.ok(!("content" in ch.result), "unchanged content is not returned");
|
assert.ok(!("content" in ch.result), "unchanged content is not returned");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("toolResultChange: the same output in content AND structuredContent (bash) counts once", () => {
|
||||||
|
// pi's bash returns its output twice: content text and structuredContent.output (measured 2026-10-12).
|
||||||
|
const known = envSecrets(ENV);
|
||||||
|
const out = `val=${V.a}`;
|
||||||
|
const ch = toolResultChange(bashEvent(out, { structuredContent: { output: out, exit_code: 0 } }), known);
|
||||||
|
assert.equal(ch.result.structuredContent.output, "val=<redacted:MEMPALACE_REMOTE_TOKEN>");
|
||||||
|
assert.equal(ch.hits.get("MEMPALACE_REMOTE_TOKEN"), 1);
|
||||||
|
});
|
||||||
|
|
||||||
test("toolResultChange: no secret anywhere -> undefined (handler returns nothing)", () => {
|
test("toolResultChange: no secret anywhere -> undefined (handler returns nothing)", () => {
|
||||||
assert.equal(toolResultChange(bashEvent("total 12\ndrwx------ 2 developer"), envSecrets(ENV)), undefined);
|
assert.equal(toolResultChange(bashEvent("total 12\ndrwx------ 2 developer"), envSecrets(ENV)), undefined);
|
||||||
assert.equal(toolResultChange(bashEvent(`x ${V.a}`), []), undefined, "no known secrets -> nothing to do");
|
assert.equal(toolResultChange(bashEvent(`x ${V.a}`), []), undefined, "no known secrets -> nothing to do");
|
||||||
|
|||||||
Reference in New Issue
Block a user