fix(security): close secret-scrubbing bypass paths in all MCP write routes

[Srinath279]

Problem

stripPrivateData was wired into the observe pipeline only (hooks/prompt-submit.ts).
Seven MCP write tools that accept content directly bypassed that pipeline entirely,
meaning secrets submitted through those routes were stored in plaintext:

Tool	Bypass reason
memory_save (mem::remember)	Explicit save, no hook
memory_lesson_save	Crystallize output + manual saves
memory_slot_create / append / replace	Slot writes never touch the hook
memory_sketch_create	Sketch/action title+description
memory_team_share	Re-shares already-stored rows
mem::compress (internal)	LLM can echo secrets into its summary
mem::import	Imported dumps skip capture entirely

Solution

Apply stripPrivateData (or the new scrubRecord for untyped payloads) at each
write site before data touches KV storage.

New patterns in privacy.ts

• PEM private key blocks — -----BEGIN * PRIVATE KEY-----
• DB connection URLs with credentials — postgres://, mysql://, mongodb+srv://, redis://, amqp://, mssql://

New utility: scrubRecord

A recursive T→T walker (objects, arrays, strings). Lets callers scrub arbitrarily-shaped
payloads (LLM output, import blobs, team-share content) without knowing their schema.

Tests

New test/scrubbing-bypass.test.ts covers every bypass path.

Bonus: pnpm-workspace.yaml

Adds allowBuilds for esbuild, onnxruntime-node, protobufjs, sharp — fixes
ERR_PNPM_IGNORED_BUILDS on fresh checkouts.

Test plan

• pnpm test passes
• Each tool stores [REDACTED_SECRET] instead of plaintext secrets
• Import with plaintext secrets → stored rows are scrubbed
• Clean content passes through unchanged (no false positives)

Summary by CodeRabbit

• Documentation

  • Added comprehensive end-to-end architecture documentation covering system design, data flow, operational flows, security, observability, and deployment.

• Bug Fixes

  • Strengthened privacy: introduced recursive scrubbing and applied automated secret/credential redaction across memory operations, imports, sharing, compression, slots, sketches, lessons, and team boundaries.

• Tests

  • Added end-to-end tests validating scrubbing behavior across storage and memory workflows.

• Chores

  • Updated workspace build configuration.

[vercel]

Someone is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 53a52420-e0e6-4f51-b304-a549ebcf933c

📥 Commits

Reviewing files that changed from the base of the PR and between d89342c and 915c526.

📒 Files selected for processing (2)

• docs/E2E_ARCHITECTURE.md
• src/functions/slots.ts

✅ Files skipped from review due to trivial changes (1)

• docs/E2E_ARCHITECTURE.md

🚧 Files skipped from review as they are similar to previous changes (1)

• src/functions/slots.ts

---

📝 Walkthrough

Walkthrough

Adds recursive record scrubbing and new secret-detection patterns, applies scrubbing across memory operations and import/export boundaries, updates architecture docs and workspace build config, and adds comprehensive tests validating redaction across workflows.

Changes

Privacy Scrubbing Enhancement

Layer / File(s)	Summary
Privacy module core extension src/functions/privacy.ts	SECRET_PATTERN_SOURCES expanded with PEM private key and DB-URI credential patterns; new exported scrubRecord<T> recursively scrubs nested structures while preserving non-string values.
Privacy scrubbing across memory operations src/functions/remember.ts, src/functions/lessons.ts, src/functions/sketches.ts, src/functions/slots.ts, src/functions/team.ts, src/functions/export-import.ts, src/functions/compress.ts	Scrubbing integrated at persistence and boundary points: remember (dedup/input), lesson-save (content/context fingerprinting), sketch create/add (title/description), slot create/append/replace (content), team-share (shared payload), mem::import (all imported collections/observations), and compress (LLM output fields).
Test suite for privacy scrubbing test/scrubbing-bypass.test.ts	Vitest suite with unit tests for stripPrivateData/scrubRecord (PEM/DB patterns, recursion, idempotence) and integration tests verifying scrubbing across remember, lessons, slots, sketches, team-share, import, and compress.
Architecture documentation and build configuration docs/E2E_ARCHITECTURE.md, pnpm-workspace.yaml	Adds full E2E architecture blueprint (versioned) including security model and scrubbing details; enables native build permissions for specified packages in pnpm workspace.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• rohitg00/agentmemory#654: Overlaps changes around mem::remember and memory persistence paths.

Suggested reviewers

• rohitg00

Poem

🐰 In burrows deep the secrets hide,

I hop with scrubbing brush in stride—
PEM and URIs I gently sweep,
Through nested fields I softly creep,
Now memories sleep safe and tidy.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main objective: closing secret-scrubbing bypass paths in all MCP write routes, which is the core change across multiple function files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

src/functions/slots.ts (1)

322-322: 💤 Low value

Consider assigning to a local variable instead of mutating the input parameter.

While the current approach works correctly, mutating data.content directly is stylistically inconsistent with the patterns used elsewhere in this file (slot-create and slot-append assign to local constants). Using a local variable would make the code more consistent and easier to reason about.

♻️ Optional refactor for consistency

     const label = validateLabel(data?.label);
     if (!label) return { success: false, error: "label required" };
     if (typeof data?.content !== "string") return { success: false, error: "content required (string)" };
-    data.content = stripPrivateData(data.content);
+    const content = stripPrivateData(data.content);
     return withKeyedLock(`slot:${label}`, async () => {
       const { slot, scope } = await readSlot(kv, label);
       if (!slot) return { success: false, error: "slot not found (use mem::slot-create first)" };
       if (slot.readOnly) return { success: false, error: "slot is read-only" };
-      if (data.content.length > slot.sizeLimit) {
+      if (content.length > slot.sizeLimit) {
         return {
           success: false,
-          error: `content exceeds sizeLimit (${data.content.length} > ${slot.sizeLimit})`,
+          error: `content exceeds sizeLimit (${content.length} > ${slot.sizeLimit})`,
           sizeLimit: slot.sizeLimit,
         };
       }
-      const updated: MemorySlot = { ...slot, content: data.content, updatedAt: nowIso() };
+      const updated: MemorySlot = { ...slot, content, updatedAt: nowIso() };
       await kv.set(scopeKv(scope), label, updated);
       await recordAudit(kv, "slot_replace", "mem::slot-replace", [label], {
         scope,
         before: slot.content.length,
-        after: data.content.length,
+        after: content.length,
       });
-      return { success: true, slot: updated, size: data.content.length };
+      return { success: true, slot: updated, size: content.length };
     });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/functions/slots.ts` at line 322, Replace the in-place mutation of the
input parameter by assigning the sanitized value to a local variable: instead of
mutating data.content with stripPrivateData, create a local const (e.g., const
content = stripPrivateData(data.content)) and use that local variable for
subsequent logic; update any references in the surrounding function (slots.ts,
functions that follow this line) to read from the new local variable rather than
data.content to keep the style consistent with slot-create/slot-append.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/E2E_ARCHITECTURE.md`:
- Around line 686-699: The Session model lacks the parentSessionId used by the
subagent flow; add an optional parentSessionId?: string (UUID v4) to the Session
interface to reflect that subagents link back to a parent session, and update
any other occurrences of the Session declaration (the later duplicate) and any
documentation/examples that reference Session to include this optional field so
the contract in Section 7 and Section 5 stays consistent.
- Around line 455-459: The table row for Scope Key `mem:obs:{sessionId}`
contains the cell text RawObservation | CompressedObservation[] which breaks the
markdown table; update that cell to escape the pipe (e.g., replace the vertical
bar with \| or the HTML entity &`#124`;) so it stays a single column (locate the
row with `mem:obs:{sessionId}` and adjust the cell content).
- Around line 100-106: The diagram contains a truncated reference
"index-persist.."—replace it with the full filename used in Section 4.4, e.g.
"index-persistence.ts", so the entry reads "agent-sdk   │   │  IndexPersistence
(disk)      ←─── index-persistence.ts" and ensure the spelling matches the
actual source file name referenced elsewhere.

---

Nitpick comments:
In `@src/functions/slots.ts`:
- Line 322: Replace the in-place mutation of the input parameter by assigning
the sanitized value to a local variable: instead of mutating data.content with
stripPrivateData, create a local const (e.g., const content =
stripPrivateData(data.content)) and use that local variable for subsequent
logic; update any references in the surrounding function (slots.ts, functions
that follow this line) to read from the new local variable rather than
data.content to keep the style consistent with slot-create/slot-append.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ff725c5-7bd4-49de-821c-4549c61dc0cf

📥 Commits

Reviewing files that changed from the base of the PR and between f6f9e3c and d89342c.

📒 Files selected for processing (11)

• docs/E2E_ARCHITECTURE.md
• pnpm-workspace.yaml
• src/functions/compress.ts
• src/functions/export-import.ts
• src/functions/lessons.ts
• src/functions/privacy.ts
• src/functions/remember.ts
• src/functions/sketches.ts
• src/functions/slots.ts
• src/functions/team.ts
• test/scrubbing-bypass.test.ts