Skip to content

Commit bcfa671

Browse files
silasjmatsonclaude
andcommitted
fix(reactotron-mcp): redact multiSet / multiMerge AsyncStorage payloads
The reactotron-react-native asyncStorage plugin emits multiSet and multiMerge as `{ pairs: [[key, value], ...] }` — an object with a `pairs` field holding 2-tuple arrays. The redactor's existing branches matched `{ key, value }` (setItem) and `{ "0": k, "1": v }` (a synthetic shape no plugin actually produces), so neither fired here: - The top-level object had neither `key` nor `"0"`, so the function returned the wrapper unchanged with no recursion into `pairs`. - Even if recursion had happened, the generic Array branch would have mapped over each tuple's two strings and returned them as-is. Result: bare key/value pairs leaked verbatim. The fourth pair in the example test happened to redact only because its value was a JSON-shaped string that fell into redactStringValue. Add explicit handling for both shapes (`{ pairs: [...] }` wrapper and `[key, value]` 2-tuple) and drop the dead `{0,1}` branch — JSON.stringify of `[k, v]` produces `["k","v"]`, never `{"0":"k","1":"v"}`, and no client code path produces it. Tests pivoted to use the actual wire format. Verified the new tests fail against the unfixed code (6 failures, all in the multiSet block) before applying the fix. Reported by joshuayoes in PR review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1385a0e commit bcfa671

2 files changed

Lines changed: 96 additions & 86 deletions

File tree

lib/reactotron-mcp/src/redaction.ts

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,10 @@ function storageKeyIsSensitive(storageKey: string, sensitiveKeysLower: Set<strin
432432
}
433433

434434
/**
435-
* Redact AsyncStorage mutation data. Storage payloads use positional keys
436-
* ("0" = storage key, "1" = value) that the generic redactor can't match.
437-
* This function splits storage keys on common separators and redacts the value
438-
* when any segment matches sensitiveKeys.
435+
* Redact AsyncStorage mutation payloads. The storage key carries the
436+
* sensitive name (not any payload field), so this branches by wire shape
437+
* (`{ key, value }`, `{ pairs: [[k, v], ...] }`) and tests the key against
438+
* sensitiveKeys before redacting the corresponding value.
439439
*/
440440
export function redactAsyncStorageData(data: unknown, rules: McpRedactionRules): unknown {
441441
if (data === null || data === undefined) return data
@@ -448,29 +448,25 @@ function redactAsyncStorageWithParsed(data: unknown, parsed: ParsedRules): unkno
448448
}
449449

450450
function redactAsyncStorageItem(data: unknown, ctx: RedactionContext): unknown {
451-
// multiSet / multiGet / multiRemove: array of pairs [{0: key, 1: value}, ...]
452-
if (Array.isArray(data)) {
453-
return data.map((item) => redactAsyncStorageItem(item, ctx))
451+
const sensitiveKeysLower = ctx.parsed.sensitiveKeysLower
452+
453+
// [key, value] tuple from a multiSet/multiMerge `pairs` entry.
454+
if (Array.isArray(data) && data.length === 2 && typeof data[0] === "string") {
455+
const [key, value] = data as [string, unknown]
456+
if (storageKeyIsSensitive(key, sensitiveKeysLower)) return [key, REDACTED]
457+
if (typeof value === "string") return [key, redactStringValue(value, ctx)]
458+
return data
454459
}
455460

456461
if (typeof data === "object" && data !== null) {
457462
const obj = data as Record<string, unknown>
458-
const sensitiveKeysLower = ctx.parsed.sensitiveKeysLower
459463

460-
// Pair shape: { 0: "auth:password", 1: "hunter2" }
461-
if ("0" in obj && typeof obj["0"] === "string") {
462-
const storageKey = obj["0"]
463-
if (storageKeyIsSensitive(storageKey, sensitiveKeysLower) && "1" in obj) {
464-
return { ...obj, "1": REDACTED }
465-
}
466-
// Even if the key isn't sensitive, the value might contain JSON with sensitive keys
467-
if ("1" in obj && typeof obj["1"] === "string") {
468-
return { ...obj, "1": redactStringValue(obj["1"], ctx) }
469-
}
470-
return obj
464+
// multiSet / multiMerge: { pairs: [[key, value], ...] }
465+
if (Array.isArray(obj.pairs)) {
466+
return { ...obj, pairs: obj.pairs.map((p) => redactAsyncStorageItem(p, ctx)) }
471467
}
472468

473-
// setItem / mergeItem: { key: "auth:password", value: "hunter2" }
469+
// setItem / mergeItem: { key, value }
474470
if ("key" in obj && typeof obj.key === "string") {
475471
const result = { ...obj }
476472
if (storageKeyIsSensitive(obj.key, sensitiveKeysLower) && "value" in obj) {

lib/reactotron-mcp/test/redaction.test.ts

Lines changed: 80 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -667,66 +667,80 @@ describe("redactAsyncStorageData()", () => {
667667
valuePatterns: ["Bearer\\s+[A-Za-z0-9\\-._~+/]+=*"],
668668
}
669669

670-
test("redacts values when storage key contains a sensitive segment (colon separator)", () => {
671-
const data = [
672-
{ "0": "auth:password", "1": "hunter2" },
673-
{ "0": "auth:api_key", "1": "leaked-api-key" },
674-
{ "0": "settings:theme", "1": "dark" },
675-
]
676-
const result = redactAsyncStorageData(data, rules) as any[]
677-
expect(result[0]["1"]).toBe(REDACTED)
678-
expect(result[1]["1"]).toBe(REDACTED)
679-
expect(result[2]["1"]).toBe("dark")
680-
})
681-
682-
test("redacts values when storage key uses dot separator", () => {
683-
const data = [{ "0": "user.accessToken", "1": "secret-token" }]
684-
const result = redactAsyncStorageData(data, rules) as any[]
685-
expect(result[0]["1"]).toBe(REDACTED)
686-
})
687-
688-
test("redacts values when storage key contains sensitive key as substring", () => {
689-
const data = [
690-
{ "0": "auth_password", "1": "hunter2" },
691-
{ "0": "persist:api_key", "1": "leaked" },
692-
{ "0": "myAccessToken", "1": "secret" },
693-
]
694-
const result = redactAsyncStorageData(data, rules) as any[]
695-
expect(result[0]["1"]).toBe(REDACTED)
696-
expect(result[1]["1"]).toBe(REDACTED)
697-
expect(result[2]["1"]).toBe(REDACTED)
698-
})
699-
700-
test("redacts values when storage key uses slash separator", () => {
701-
const data = [{ "0": "auth/password", "1": "hunter2" }]
702-
const result = redactAsyncStorageData(data, rules) as any[]
703-
expect(result[0]["1"]).toBe(REDACTED)
704-
})
705-
706-
test("applies JSON string redaction to values when storage key is not sensitive", () => {
707-
const jsonValue = JSON.stringify({ password: "hunter2", api_key: "leaked", note: "Bearer abcdef1234567890ABCDEFGH" })
708-
const data = [{ "0": "auth:session_data", "1": jsonValue }]
709-
const result = redactAsyncStorageData(data, rules) as any[]
710-
// Storage key "auth:session_data" doesn't match sensitiveKeys, but JSON contents do
711-
const parsed = JSON.parse(result[0]["1"])
712-
expect(parsed.password).toBe(REDACTED)
713-
expect(parsed.api_key).toBe(REDACTED)
714-
expect(parsed.note).toBe(REDACTED) // Bearer pattern match
715-
})
716-
717-
test("redacts JSON string values when storage key is not sensitive", () => {
718-
const jsonValue = JSON.stringify({ password: "hunter2", name: "alice" })
719-
const data = [{ "0": "cached:data", "1": jsonValue }]
720-
const result = redactAsyncStorageData(data, rules) as any[]
721-
const parsed = JSON.parse(result[0]["1"])
722-
expect(parsed.password).toBe(REDACTED)
723-
expect(parsed.name).toBe("alice")
724-
})
725-
726-
test("handles key-value shape with 'key' and 'value' fields", () => {
727-
const data = { key: "auth:password", value: "hunter2" }
728-
const result = redactAsyncStorageData(data, rules) as any
729-
expect(result.value).toBe(REDACTED)
670+
describe("setItem / mergeItem", () => {
671+
test("redacts value when storage key carries a sensitive segment", () => {
672+
const data = { key: "auth:password", value: "hunter2" }
673+
const result = redactAsyncStorageData(data, rules) as any
674+
expect(result.value).toBe(REDACTED)
675+
})
676+
677+
test("redacts JSON string values when storage key is not sensitive", () => {
678+
const jsonValue = JSON.stringify({ password: "hunter2", name: "alice" })
679+
const data = { key: "cached:data", value: jsonValue }
680+
const result = redactAsyncStorageData(data, rules) as any
681+
const parsed = JSON.parse(result.value)
682+
expect(parsed.password).toBe(REDACTED)
683+
expect(parsed.name).toBe("alice")
684+
})
685+
})
686+
687+
describe("multiSet / multiMerge", () => {
688+
test("redacts values when storage key carries a sensitive segment (colon)", () => {
689+
const data = {
690+
pairs: [
691+
["auth:password", "hunter2"],
692+
["auth:api_key", "leaked-api-key"],
693+
["settings:theme", "dark"],
694+
],
695+
}
696+
const result = redactAsyncStorageData(data, rules) as any
697+
expect(result.pairs[0]).toEqual(["auth:password", REDACTED])
698+
expect(result.pairs[1]).toEqual(["auth:api_key", REDACTED])
699+
expect(result.pairs[2]).toEqual(["settings:theme", "dark"])
700+
})
701+
702+
test("redacts when storage key uses dot separator", () => {
703+
const data = { pairs: [["user.accessToken", "secret-token"]] }
704+
const result = redactAsyncStorageData(data, rules) as any
705+
expect(result.pairs[0][1]).toBe(REDACTED)
706+
})
707+
708+
test("redacts when storage key uses slash separator", () => {
709+
const data = { pairs: [["auth/password", "hunter2"]] }
710+
const result = redactAsyncStorageData(data, rules) as any
711+
expect(result.pairs[0][1]).toBe(REDACTED)
712+
})
713+
714+
test("redacts when storage key contains sensitive key as substring", () => {
715+
const data = {
716+
pairs: [
717+
["auth_password", "hunter2"],
718+
["persist:api_key", "leaked"],
719+
["myAccessToken", "secret"],
720+
],
721+
}
722+
const result = redactAsyncStorageData(data, rules) as any
723+
expect(result.pairs[0][1]).toBe(REDACTED)
724+
expect(result.pairs[1][1]).toBe(REDACTED)
725+
expect(result.pairs[2][1]).toBe(REDACTED)
726+
})
727+
728+
test("applies JSON-string redaction to non-sensitive pair values", () => {
729+
const jsonValue = JSON.stringify({ password: "hunter2", api_key: "leaked", note: "Bearer abcdef1234567890ABCDEFGH" })
730+
const data = { pairs: [["auth:session_data", jsonValue]] }
731+
const result = redactAsyncStorageData(data, rules) as any
732+
const parsed = JSON.parse(result.pairs[0][1])
733+
expect(parsed.password).toBe(REDACTED)
734+
expect(parsed.api_key).toBe(REDACTED)
735+
expect(parsed.note).toBe(REDACTED) // Bearer pattern match
736+
})
737+
738+
test("preserves non-pair fields on the wrapper", () => {
739+
const data = { pairs: [["auth:password", "hunter2"]], extra: "preserved" }
740+
const result = redactAsyncStorageData(data, rules) as any
741+
expect(result.extra).toBe("preserved")
742+
expect(result.pairs[0][1]).toBe(REDACTED)
743+
})
730744
})
731745

732746
test("handles null and undefined", () => {
@@ -736,9 +750,9 @@ describe("redactAsyncStorageData()", () => {
736750

737751
test("returns data unchanged when no sensitiveKeys configured", () => {
738752
const emptyRules: McpRedactionRules = { sensitiveKeys: [], valuePatterns: [] }
739-
const data = [{ "0": "auth:password", "1": "hunter2" }]
740-
const result = redactAsyncStorageData(data, emptyRules) as any[]
741-
expect(result[0]["1"]).toBe("hunter2")
753+
const data = { pairs: [["auth:password", "hunter2"]] }
754+
const result = redactAsyncStorageData(data, emptyRules) as any
755+
expect(result.pairs[0][1]).toBe("hunter2")
742756
})
743757
})
744758

@@ -797,9 +811,9 @@ describe("createRedactor()", () => {
797811

798812
test("redactAsyncStorage applies storage-key heuristic", () => {
799813
const redactor = createRedactor(mockServer([]), DEFAULT_SERVER_CONFIG)
800-
const data = [{ "0": "auth:password", "1": "hunter2" }]
801-
const result = redactor.redactAsyncStorage(data) as any[]
802-
expect(result[0]["1"]).toBe(REDACTED)
814+
const data = { pairs: [["auth:password", "hunter2"]] }
815+
const result = redactor.redactAsyncStorage(data) as any
816+
expect(result.pairs[0][1]).toBe(REDACTED)
803817
})
804818

805819
test("returns data unchanged when client has fully disabled redaction", () => {

0 commit comments

Comments
 (0)