fix(mcp): redact credentials split across analyze lines (WALM-687)

[ducnmm]

Ticket

WALM-687 — https://linear.app/mysten-labs/issue/WALM-687/bug-memwal-analyze-stores-a-credential-split-across-lines-gh-994
GH #994 — #994

What changed?

• sanitizePassage screens segment texts with sanitizeFactBatch (blank lines as empty entries) instead of sanitizeFact on each line alone.
• A dropped segment, including a bare key refused as credential-only, still contributes its kinds and count. dropped stays line and reason.

Why is this needed?

memwal_analyze stored a credential verbatim when the label and the value were on different lines. The same split was already redacted on memwal_remember_bulk.

Scope

Passage redaction catches a credential split across lines, the same way bulk entries already do.

Out of scope

None

How was this tested?

• Unit tests
• Integration tests
• End-to-end tests
• Manual testing
• Not applicable

Commands: from services/server/scripts, node --test --import tsx './mcp/__tests__/secret-redaction.test.ts' './mcp/__tests__/write-path-redaction.test.ts' (53 pass). npm test 398 pass; 1 fail in integration.test.ts (stdio bridge cannot resolve @noble/ed25519 because packages/mcp is not installed in this worktree). That test and package are untouched.

How can the reviewer verify it?

1. From services/server/scripts, run node --test --import tsx './mcp/__tests__/secret-redaction.test.ts'.
2. The new tests cover a delegate key split across two lines, a non-hex opaque token, a delegatePrivateKey JSON value on its own line, and a passage of digests with no credential label (unchanged).

Risks and dependencies

None

Author checklist

• This pull request maps to one ticket and one logical outcome.
• I reviewed the complete diff myself.
• I removed unrelated, debug, and temporary changes.
• I ran the relevant tests.
• CI is green.
• The branch is up to date with its target branch.
• I added or updated tests where appropriate.
• I documented any important risk, dependency, rollout, or follow-up.
• I provided clear verification steps.
• The pull request is ready for review and is no longer a Draft.

[ducnmm]

@nikola0x0 gentle ping — still waiting on review of b5c250f.

[nikola0x0]

Issue counts by severity
bugs: 1
suggestions: 1
nits: 0

The fix closes the split-label case from GH #994, and all 53 tests pass locally. The one bug is a new false positive that the cross-entry pass brings with it once it runs on a whole passage instead of a 20-entry batch. It's a regression against dev. Details and a tested diff are inline on redaction.ts:826.

[nikola0x0]

[bug] sanitizeFactBatch screens a bare token (fewer than 3 words around it) against the whole batch (const context = bare ? wholeBatch : neighbourhood). That's reasonable for memwal_remember_bulk's 20 entries. A passage, though, is up to 200k characters, so one label anywhere in it (delegate key, secret, seed, api key, all common in MemWal transcripts) makes every lone identifier line in the passage count as labelled key material.

Reproduced, with the label on line 1 and the identifiers 30+ lines later:

user: I rotated my delegate key yesterday, all good now.
assistant: step 0 done            (x30)
user: the blob it wrote is
Xj9vKq2mP7nR4tW8yB1cE5gH0dF3sA6uZ2xN8qL4kM7
assistant: and the release commit
4f3c2b1a9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b
user: package 0xe80f2feec1c139616a86c9f71210152e2a7ca552b20841f2e192f99f75864437

• dev: dropped: [], and all three identifiers are kept.
• This PR: lines 33, 35 and 36 are dropped as credential-only, with kinds: ['labelled-key-material']. The blob id, the SHA and the package id are all gone, and the agent is told they were credentials.

The new test on line 577 doesn't catch this because its passage has no label at all, so the whole-batch gate returns early.

Suggestion: in passage mode, screen bare tokens against their neighbours only. Also leave blank lines out of the batch, so a label and a value with a blank line between them still count as neighbours:

-export function sanitizeFactBatch(inputs: string[]): SanitizedText[] {
+export function sanitizeFactBatch(
+    inputs: string[],
+    { bareScope = "batch" }: { bareScope?: "batch" | "neighbours" } = {},
+): SanitizedText[] {
@@
-                const bare = wordsAround(own, match) < 3;
+                const bare = bareScope === "batch" && wordsAround(own, match) < 3;
@@ export function sanitizePassage
-    // Blank lines are empty entries, so the batch screen sees the same breaks
-    // the passage does. They are not facts and never carry a refusal.
-    const results = sanitizeFactBatch(segments.map((s) => (s.blank ? "" : s.text)));
+    // Blank lines are left out, so a label and a value with a blank line
+    // between them are still neighbours. A bare token is screened against its
+    // neighbours only: one label in a 200k-character passage must not take
+    // every lone blob id or SHA with it.
+    const screened = sanitizeFactBatch(
+        segments.filter((s) => !s.blank).map((s) => s.text),
+        { bareScope: "neighbours" },
+    );
+    let next = 0;
+    const results = segments.map((s) =>
+        s.blank ? { text: "", changed: false, kinds: [], count: 0 } : screened[next++],
+    );

Checked locally with that applied: all 53 tests in secret-redaction + write-path-redaction still pass, the passage above comes back with dropped: [], and my delegate private key for mainnet:\n\n<seed>\n\nthanks still drops the seed line (the blank-line case). The bulk path is unchanged, since bareScope defaults to "batch".

[nikola0x0]

[suggestion] This test can't catch the case above, since there is no label anywhere, so sanitizeFactBatch returns at the whole-batch gate. Worth adding the one that would have:

test("a lone identifier far from a credential label survives a passage", () => {
    const lines = [
        "user: I rotated my delegate key yesterday, all good now.",
        ...Array.from({ length: 10 }, (_, i) => `assistant: step ${i} done`),
        "Xj9vKq2mP7nR4tW8yB1cE5gH0dF3sA6uZ2xN8qL4kM7",
        "4f3c2b1a9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b",
    ];
    const out = sanitizePassage(lines.join("\n"));
    assert.deepEqual(out.dropped, []);
    assert.ok(out.text.includes("Xj9vKq2mP7nR4tW8yB1cE5gH0dF3sA6uZ2xN8qL4kM7"));
    assert.ok(out.text.includes("4f3c2b1a9e8d7c6b5a4f3e2d1c0b9a8f7e6d5c4b"));
});

Plus the blank-line case, label\n\n<seed>, as a positive test.

[ducnmm]

@nikola0x0 confirmed. A label anywhere in a passage makes every bare token in that passage look labelled, which the 20-entry batch does not do. Holding the code change so the batch path stays strict.