fix(security): cross-tenant memory poisoning — GLOBAL scope isolation (GH#1610) - #1791
Merged
Merged
Conversation
…mory/recall_memory (GH#1610) Two critical gaps in a2a_tools.py let any tenant workspace poison org-wide (GLOBAL) memory and bypass all RBAC enforcement: 1. tool_commit_memory had no RBAC check — any agent could write any scope. 2. tool_commit_memory had no root-workspace enforcement for GLOBAL scope — Tenant A could POST scope=GLOBAL and pollute the shared memory store that Tenant B's agent reads as trusted context. Fix adds: - _ROLE_PERMISSIONS table (mirrors builtin_tools/audit.py) so a2a_tools has isolated RBAC logic without depending on memory.py. - _check_memory_write_permission() / _check_memory_read_permission() helpers: evaluate RBAC roles from WorkspaceConfig; fail closed (deny) on errors. - _is_root_workspace() / _get_workspace_tier(): read WorkspaceConfig.tier (0 = root/org, 1+ = tenant) from config.yaml; fall back to WORKSPACE_TIER env var. - tool_commit_memory now (a) checks memory.write RBAC, (b) rejects GLOBAL scope for non-root workspaces, (c) embeds workspace_id in the POST body so the platform can namespace-isolate and audit cross-workspace writes. - tool_recall_memory now checks memory.read RBAC before any HTTP call, and always sends workspace_id as a GET param for platform cross-validation. Security regression tests added: - GLOBAL scope denied for non-root (tier>0) workspaces. - RBAC denial blocks all scope levels (including LOCAL) on write. - RBAC denial blocks recall entirely. - workspace_id present in POST body and GET params. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
HongmingWang-Rabbit
force-pushed
the
fix/memory-poisoning-GH1610
branch
from
April 23, 2026 17:21
a20edae to
e00797b
Compare
4 tasks
4 tasks
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…2 step 1) (#1794) CTO-bypass merge 2026-05-24: Phase A2 step 1 — route POST /memories through v2 plugin
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…ge (#1796) CTO-bypass merge 2026-05-24: memory-system chain
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…surface Closes the v1→v2 memory migration. Phase A2 (#1791) ran on production 2026-05-24 and verified parity: every active tenant has its agent_memories rows mirrored 1:1 into memory_plugin.memory_records, live writes go to v2 only (v1 frozen). With parity confirmed, this PR drops the entire v1 surface. Per the audit before this PR: | Tenant | v1 (frozen) | v2 (live) | Status | |---|---|---|---| | agents-team | 1805 | 1805+live | parity | | hongming | 144 | 144 | parity | | chloe-dong | 1 | 1 | parity | | reno-stars | 102 | 102 | parity | ## Changes 1. **Migration** drops the agent_memories table. Down migration recreates an empty table for tool symmetry; rollback would not restore data (A2 was one-way). 2. **memories.go**: removed Search, Update, Delete methods + their dead helpers (EmbeddingFunc, embed field, WithEmbedding, formatVector, nextArg, memoryFTSMinQueryLen, memoryRecallMaxLimit). Kept Commit, which post-#1794 routes through the v2 plugin. 3. **router.go**: removed GET /memories, DELETE /memories/:id, PATCH /memories/:id routes. Callers use /v2/memories (canvas does this already) and /v2/memories/:id (Forget) instead. POST /memories stays — it's the high-volume write surface, still on v2. 4. **activity.go**: dropped the agent_memories UNION branch from buildSessionSearchQuery. Session search now returns only activity_logs items; memory-tab content comes from /v2/memories directly via MemoryInspectorPanel. 5. **workspace_crud.go**: removed agent_memories from the workspace purge cleanup list. Memory rows now cascade-delete via the memory plugin's namespace deletion path. 6. **entrypoint-tenant.sh**: removed the MEMORY_V2_CUTOVER deprecation shim (#1747 deprecated it; A3 retires the synonym). New tenants use MEMORY_PLUGIN_URL directly. Controlplane user-data still sets MEMORY_V2_CUTOVER='true' as belt-and-suspenders — that's a no-op now and will be cleaned up in a separate molecule-controlplane PR. 7. **Tests**: removed test functions that exercised the deleted methods (Search/Update/Delete and the embed/recall paths). Tests for Commit + redactSecrets stay. ## Risk - **Hard 404** on any caller still hitting GET /workspaces/:id/memories, PATCH /workspaces/:id/memories/:id, or DELETE /workspaces/:id/memories/:id. Production traffic audit showed 2 GETs vs 66 POSTs to legacy /memories over a 24h window — runtime callers are POST-dominant. Canvas reads from /v2/memories. Acceptable. - **No DB rollback** restores data — A2 was one-way. If a critical bug appears post-merge, recover via memory_plugin.memory_records direct SQL (data is preserved there). ## SOP Checklist (RFC #351) ### 1. Comprehensive testing performed - `go test -short -count=1 ./internal/handlers/` green. - `go test -short -count=1 ./cmd/memory-backfill/` green (sqlmock tests still pass; tool is now effectively inert on tenants since the source table is gone but the binary stays for one image cycle). - `go vet ./...` clean. ### 2. Local-postgres E2E run N/A. Schema change verified against the well-tested migration tool shape; no new SQL paths added. ### 3. Staging-smoke verified or pending Pending merge + tenant recycle. Will verify by SSM-checking that agent_memories is gone from each tenant's DB and POST /memories still returns 201 with rows landing in memory_plugin.memory_records. ### 4. Root-cause not symptom Yes. The v1 table existed only as a dual-write target during the A1+A2 transition. With A2 done and parity verified, the table is dead weight. Dropping it removes the SSOT-violation surface entirely. ### 5. Five-Axis review walked Walked solo. Happy to dispatch a hostile reviewer if anyone wants sign-off on the cleanup scope (whether to also drop memory-backfill binary, the activity UNION removal, etc). ### 6. No backwards-compat shim / dead code added Net deletion: -787 LOC across 7 files. The MEMORY_V2_CUTOVER shim is removed (was the last backwards-compat hook). One follow-up needed: controlplane ec2.go still sets MEMORY_V2_CUTOVER='true' — that's a no-op now but should be cleaned up in a separate PR for tidiness. ### 7. Memory/saved-feedback consulted - `feedback_no_single_source_of_truth` — A3 is the final step in establishing v2 as the only memory backend. - `feedback_check_for_parallel_work_before_fix_pr` — grep'd recent PRs touching memories.go / activity.go / workspace_crud.go; no parallel in flight. Closes #1792. Memory v1→v2 migration complete.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes GH#1610 — GLOBAL Memory Poisoning (cross-tenant memory contamination).
Root cause
tool_commit_memoryinworkspace/a2a_tools.pyhad two critical gaps that let any tenant workspace poison org-wide (GLOBAL) memory:No RBAC enforcement — any agent could call
tool_commit_memoryregardless of role. The siblingbuiltin_tools/memory.pyproperly checksmemory.writeRBAC, buta2a_tools.pyhad none.No root-workspace enforcement for GLOBAL scope — the documented HMA contract states "GLOBAL: readable by all, writable by root workspaces only", but the implementation had zero enforcement. A tenant workspace could
POST /workspaces/{id}/memories {scope: "GLOBAL"}and silently write to the shared org-wide memory store.Changes
workspace/a2a_tools.py_ROLE_PERMISSIONStable (mirrorsbuiltin_tools/audit.py) soa2a_toolshas isolated RBAC logic._get_workspace_tier()/_is_root_workspace()helpers: readsWorkspaceConfig.tier(0 = root, 1+ = tenant) fromconfig.yaml, falls back toWORKSPACE_TIERenv var._check_memory_write_permission()/_check_memory_read_permission()helpers: evaluate RBAC roles; fail closed on config errors.tool_commit_memory: now (a) checksmemory.writeRBAC before any HTTP call, (b) returnsError: RBAC — only root workspaces (tier 0) can write to GLOBAL scopefor non-root tenants, (c) embedsworkspace_idin the POST body so the platform can namespace-isolate and audit cross-workspace writes.tool_recall_memory: now (a) checksmemory.readRBAC before any HTTP call, (b) always sendsworkspace_idas a GET param for platform cross-validation against future path-traversal bugs.workspace/tests/test_a2a_tools_impl.pyAdded 5 security regression tests covering:
test_global_scope_denied_for_non_root_workspace: tenant workspace cannot write GLOBAL scopetest_rbac_deny_blocks_all_scopes_including_local: RBAC denial blocks all scope levels including LOCALtest_rbac_deny_blocks_recall: RBAC denial blocks recall entirelytest_post_includes_workspace_id_in_body: POST body includesworkspace_idfor audit traceabilitytest_recall_includes_workspace_id_in_params: GET params always includeworkspace_idTest plan
test_a2a_tools_impl.pypasstest_memory.pypass🤖 Generated with Claude Code