PR #881 closed SAFE-T1201 (#838) on the HTTP path by wiring redactSecrets()
into MemoriesHandler.Commit — but the sibling code path on the MCP bridge
(MCPHandler.toolCommitMemory) was left with only the TODO comment. Agents
calling commit_memory via the MCP tool bridge are the PRIMARY attack vector
for #838 (confused / prompt-injected agent pipes raw tool-response text
containing plain-text credentials into agent_memories, leaking into shared
TEAM scope). The HTTP path is only exercised by canvas UI posts, so the MCP
gap was the hotter one.
Change:
workspace-server/internal/handlers/mcp.go:725
- TODO(#838): run _redactSecrets(content) before insert — plain-text
- API keys from tool responses must not land in the memories table.
+ SAFE-T1201 (#838): scrub known credential patterns before persistence…
+ content, _ = redactSecrets(workspaceID, content)
Reuses redactSecrets (same package) so there's no duplicated pattern list —
a future-added pattern in memories.go automatically covers the MCP path too.
Tests added in mcp_test.go:
- TestMCPHandler_CommitMemory_SecretInContent_IsRedactedBeforeInsert
Exercises three patterns (env-var assignment, Bearer token, sk-…)
and uses sqlmock's WithArgs to bind the exact REDACTED form — so a
regression (removing the redactSecrets call) fails with arg-mismatch
rather than silently persisting the secret.
- TestMCPHandler_CommitMemory_CleanContent_PassesThrough
Regression guard — benign content must NOT be altered by the redactor.
NOTE: unable to run `go test -race ./...` locally (this container has no Go
toolchain). The change is mechanical reuse of an already-shipped function in
the same package; CI must validate. The sqlmock patterns mirror the existing
TestMCPHandler_CommitMemory_LocalScope_Success test exactly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
818d5cde91