fix(security): a memory write cannot target another project's domain (GHSA-7848-qh48-mx2w) - #940

Claude finished @cdeust's task in 5m 24s —— View job

Review 1 of 3: PR #940. Verdict: block

• Read the diff
• Run five reviews: security, code quality, tests, docs, performance
• Check the findings myself
• Post the summary: full review

The domain guard itself is sound, but the attack still works without naming a domain. That is one Blocker:

• remember(is_global=True) bypasses the guard. The guard returns early when no domain is given, so the write is stored as a global memory. query_methodology then serves that text in every project's hot memories, which is the injection this advisory is meant to close.
• anchor has the same hole. It can rewrite any memory's content and set it global, with no domain check.

The full review also has 3 notes, which need no action before merge.

I could not run pytest, ruff or the craftsmanship gate here, because Python commands need approval in this job. The findings come from tracing the code.

Review 1 of 3: PR #940 (GHSA-7848-qh48-mx2w)

I ran five reviews (security, code quality, tests, docs, performance) and checked the main finding against the code myself. One Blocker.

The domain guard itself is sound:

• It sits in remember, so codebase_analyze, import_sessions, wiki_* and the other writers that go through remember are covered too.
• Layering is respected.
• The repo registry is cached, so the extra cost per call is negligible.

The Blocker is that the same attack still works without naming a domain.

Blocker: a write can still reach another project's query_methodology context through global scope

The advisory's attack ends with text landing in another project's hot memories. The guard checks only domain, but hot-memory reads also return every global row. Traced:

• remember accepts a caller-set is_global argument (mcp_server/handlers/remember_schema.py:303). An explicit value is always honoured, because resolve_global_scope returns True whenever explicit is set (mcp_server/core/global_detector.py:273). The row is then stored as global (remember.py:190-202).
• require_writable_domain("") returns at once (_write_scope.py:49), because an empty domain is allowed.
• query_methodology._get_hot_memories("alpha", …) calls get_memories_for_domain, which selects WHERE (domain = ? OR is_global = …) (sqlite_store_queries.py:46, pg_store_queries.py:34).

Scenario: in the test's setup, a session in beta calls remember({"content": MARKER, "is_global": True, "force": True}). The marker then shows up in _alpha_context(). That is the outcome test_a_directory_inside_another_project_is_refused asserts cannot happen.

anchor (mcp_server/handlers/anchor.py:121-150) has the same hole:

• It takes any memory_id, puts the caller's reason at the front of that memory's content, sets its heat to 1.0, and sets is_global from the arguments. It never checks the domain.
• So a session can rewrite another project's memory directly, or make its own memory global.

Fix:

• An explicit is_global=True, and anchor with is_global=True, should require CORTEX_CROSS_PROJECT_WRITES=*.
• anchor should refuse a memory_id whose domain fails require_writable_domain.
• Add a test for each to test_ghsa_7848_cross_project_write.py.

The security reviewer also traced, but I have not checked myself, three more paths that modify existing rows without checking their domain:

• merge candidates in try_curation, which searches with no domain filter (remember_helpers.py:450);
• supersedes_id targets (remember_helpers.py:845);
• the replica vpath: upsert (remember_helpers.py:373).

These are worth sweeping in the same fix.

Fix this →

Notes, no action before merge

• seed_project, ingest_prd and ingest_document check the raw domain without mapping it to its canonical name first, unlike remember.py:84. An alias passes the check. That is harmless today only because the read side matches domains exactly.
• project_domains() only knows repos under the dev roots, and it is cached for the life of the process. A repo outside those roots, or one cloned after the server starts, is unprotected.
• The guards in seed_project, ingest_prd and ingest_document have no tests. record_session_end_memory.py:95 catches the new ValidationError and logs it only at debug level.

I could not run pytest, ruff or the craftsmanship gate: running Python in this job needs approval it was not given. The findings above come from tracing the code.

Verdict: block

Claude finished @cdeust's task in 2m 51s —— View job

Review 2 of 3: PR #940. Verdict: block

• Check whether review 1's Blocker is fixed
• Run five reviews (security, code quality, tests, docs, performance) on the two commits pushed since 265ff1c
• Post the summary: full review

Review 1's Blocker is still open. The two commits since then (8614c26, edde263) only change tests, and nothing in remember.py, anchor.py or _write_scope.py has changed:

• remember(is_global=True) still gets past the guard. The guard returns early when no domain is given, and the row is stored as global. Another project's query_methodology then serves it in its hot memories.
• anchor has the same hole. It can still rewrite any memory and make it global without a domain check.

The new tests have no Blockers. I traced them against the code and they should pass in CI. The full review has 3 notes, which need no action before merge.

I didn't run pytest, ruff or the craftsmanship gate. The findings come from tracing the code.

Review 2 of 3: PR #940 (GHSA-7848-qh48-mx2w)

The last review covered 265ff1c. Since then the PR's own commits are 8614c26 and edde263, which change only tests_py/handlers/test_ghsa_7848_cross_project_write.py. fe6dd31 merges main and changes nothing in this fix. I ran five reviews on those two commits (security, code quality, tests, docs, performance), then checked the earlier Blocker against HEAD myself.

Blocker (from review 1, still open): global scope bypasses the guard

No commit since 265ff1c touches remember.py, anchor.py or _write_scope.py. At HEAD:

• remember.py:178 checks only the domain. require_writable_domain("") returns at _write_scope.py:49.
• remember.py:190-202 then stores is_global=True whenever the caller passes it, because resolve_global_scope returns True for an explicit flag (global_detector.py:273).
• Hot-memory reads in another project return every global row: WHERE (domain = ? OR is_global = 1) (sqlite_store_queries.py:46, pg_store_queries.py:34).

Scenario: a session in beta calls remember({"content": MARKER, "is_global": True, "force": True}). The marker then reaches alpha's query_methodology context.

anchor.py:121-149 is still open as well. For any memory_id and any domain, it puts the caller's reason at the front of the memory's content and sets is_global from the arguments. It never calls the guard.

Fix this →

The new tests have no Blockers. I traced their expected strings, the write_page monkeypatch target, the fixture's cache clearing and the asyncio markers against the code, and they should pass in CI. The file is 229 lines, under the craftsmanship limit.

Notes, no action before merge

• test_ingest_document_refuses_before_writing (:220) never checks that nothing was written, unlike the ingest_prd test, which asserts written == []. To match it, stub _persist.
• The name test_without_the_variable_no_project_is_allowed (:165) overstates the behaviour, since the session's own project stays writable. ..._no_other_project_is_allowed would be accurate.
• The Codex P2 is still open: seed_project with directory=<alpha> and domain="beta" passes _parse_args, purges beta's seeded memories, and then remember refuses. Validate the domain remember would resolve before the purge.

I didn't run pytest, ruff or the craftsmanship gate here. Everything above comes from reading and tracing the code.

Verdict: block

Claude finished @cdeust's task in 1m 5s —— View job

Review 3 of 3: PR #940 (GHSA-7848-qh48-mx2w)

• List commits since edde263: only 9457b60, which merges main (fix(security): ADR task records stay inside the wiki root (GHSA-5w3v-7v7h-q9x4) #941)
• Check whether earlier Blockers are fixed: not fixed
• Check the merge: it touches none of this PR's files
• Post the verdict

Review 2 covered edde263. The only commit since then is 9457b60, which merges main (#941, GHSA-5w3v-7v7h-q9x4). git diff origin/main...HEAD still shows the same 8 files and +307/-0, and the merge touches none of them. There was no new PR code to review, so I did not run the five reviewer agents again.

Blocker (from reviews 1 and 2, still open): global scope bypasses the guard

remember.py, anchor.py, _write_scope.py and seed_project.py are identical to edde263. At HEAD:

• remember.py:178 checks only domain, and require_writable_domain("") returns early.
• remember.py:190-191 then honours a caller's is_global=True (explicit=bool(args.get("is_global", False))).
• In another project, hot-memory reads return every global row: WHERE (domain = ? OR is_global = …).

So a session in beta that calls remember({"content": MARKER, "is_global": True, "force": True}) still gets the marker into alpha's query_methodology context.

anchor.py:140-148 still rewrites any memory_id's content and sets is_global from the arguments. It never calls the guard.

Fix this →

Accepted vs open

• Accepted: the domain guard as written. That is _write_scope.py, domain_mapping.py, the calls in remember.py, seed_project.py, ingest_prd.py and ingest_document.py, the tests, and the CHANGELOG (commits b42058e, 8614c26, edde263). It correctly refuses a write that names another project's domain, and it can merge as is.
• Open, for a refinement PR: the bypass through global scope in mcp_server/handlers/remember.py (explicit is_global, lines 190-191) and mcp_server/handlers/anchor.py (lines 140-148). No commit in this PR fixes either one. Until a fix lands, the advisory's attack still works by that route. If this part merges first, the CHANGELOG entry and the advisory should not claim GHSA-7848 is fully closed.

Notes, no action before merge

• seed_project.py:121 (Codex P2): with directory=<alpha>, domain="beta", beta's seeded memories are purged before remember refuses the write.
• seed_project, ingest_prd and ingest_document check the raw domain without mapping it to its canonical name first, unlike remember.py:84.
• test_ingest_document_refuses_before_writing never checks that nothing was written.

I couldn't run pytest, ruff or the craftsmanship gate, and I couldn't post a separate comment with gh pr comment, because this job needs approval for those commands and for writing a temp file. This comment is the review. The findings come from tracing the code at 9457b60.

Verdict: block

Diagnostic du check unit (run 42373733761, job 128407016638, 4 tests en échec) : la PR exige désormais CORTEX_CROSS_PROJECT_WRITES=* pour toute écriture is_global=true. Deux tests écrivaient encore une mémoire globale sans cette autorisation (test_global_memory_visible_across_domains, test_explicit_global_hooks_cross_projects[auto_recall|session_start]). Le troisième échec vient de l'UPDATE de anchor.py, passé de la ligne 145 à 154 : la liste ALLOWED_WRITERS de l'invariant I2 pointait sur l'ancienne ligne. merge est l'agrégat et échoue parce que unit échoue.

Commit poussé : 10b5afb (les tests posent l'autorisation via monkeypatch, comme test_remember/test_anchor dans cette PR ; I2 passe à anchor.py:154). Aucun code de production modifié.

Vérifié en local : les 4 tests passent ; tests_py/invariants, tests_py/hooks et tests_py/handlers donnent 2983 passés et 2 échecs (test_entry_auto_dispatch[False-*]), qui échouent aussi sur origin/main dans cet environnement et passent en CI.

Non vérifié : la CI de 10b5afb (en cours). La branche a aussi 1 commit de retard sur main : le rebase avec --force-with-lease a été refusé par le contrôle de permissions de cette session, il n'a donc pas été poussé. Le conflit CHANGELOG qu'il soulève est mécanique : garder l'entrée GHSA-7848 longue et supprimer la courte.

Generated by Claude Code

Claude finished @cdeust's task in 3m 19s —— View job

Review 4 of 3: PR #940 (GHSA-7848-qh48-mx2w)

• List commits since 9457b60: d9f160e (the fix) and 10b5afb (tests only)
• Check the earlier Blocker (global scope, anchor): fixed
• Run five reviews (security, code quality, tests, docs, performance) on the new commits
• Post the verdict

Earlier Blocker: fixed

• remember: an explicit is_global=true is now refused without CORTEX_CROSS_PROJECT_WRITES=*, through global_scope in _exact_record and _handler_impl (remember.py:196, :341). A global scope that the detector infers on its own is kept local and reported in global_reason.
• anchor: it now calls require_writable_memory on the target row and refuses an explicit is_global (anchor.py:122-126, :145).
• Sweep of existing rows: supersedes_id, curation merges and replica refreshes now skip or refuse rows the session may not change (remember.py:231, remember_helpers.py:403, :469).

Blocker: seed_project checks one domain and deletes from another

d9f160e changed the guard so it checks the domain remember would write under. That domain comes from the directory first. The purge still uses the raw domain argument:

• seed_project.py:121 sets domain = args.get("domain") or root.name.
• seed_project.py:124 checks _remember_domain(str(root), domain). For a directory inside a known repo, that is the repo's domain (remember.py:84-87).
• seed_project.py:199 runs delete_memories_by_tag("seeded", domain=domain) on the raw argument.

Scenario: a session in beta calls seed_project({"domain": "alpha"}). The directory defaults to the working directory, which is beta. The check sees beta and passes. Every seeded memory in alpha is then deleted, and the new discoveries are written under beta. Before this commit, the check ran on the argument itself, so this call was refused. This commit reopened the path. The Codex P1 on seed_project.py:124 reports the same thing.

The CHANGELOG line "seed_project checks the domain remember will use before it purges anything" (CHANGELOG.md:21-22) is true of the check but not of the purge.

Fix: resolve the domain once in _parse_args, then use that same value for the check, the purge, the writes and the response. Add a test: call seed_project with directory=<beta>, domain="alpha" and dry_run=False, and assert that alpha's seeded row survives.

Fix this →

Accepted vs open

• Accepted: everything else in the PR. That is the domain guard from the earlier commits, plus the global-scope, anchor, supersedes_id, curation and replica guards in d9f160e (_write_scope.py, anchor.py, remember.py, remember_helpers.py), and the tests in 10b5afb.
• Open: mcp_server/handlers/seed_project.py from d9f160e (lines 121-124 against line 199), and the matching CHANGELOG sentence. If this goes to a refinement PR, the simplest interim fix is to go back to checking the raw domain as well as the resolved one.

Notes, no action before merge

• remember_helpers.py:387-406 (Codex P2): the LIMIT 1 replica lookup can keep returning a row the session may not change. Each forced sync then adds a new replica row. Filter by writability in the SQL, or pick the first writable match.
• write_post_store.py:129-144: synaptic tagging still raises the heat and importance of hot memories from any project. This is unchanged by this PR and injects no content.
• The is_global descriptions (remember_schema.py:306, anchor.py:70) don't say that CORTEX_CROSS_PROJECT_WRITES=* is now required.

I didn't run pytest, ruff or the craftsmanship gate. I also couldn't post a separate comment with gh pr comment, because this job blocked writing the comment body to a temp file. This comment is the review. The findings come from tracing the code at 10b5afb.

Verdict: block

CI autopilot : diagnostic du check unit (run 42377328684, job 128417628663, runner Hosted Compute Agent 20261028.624)

Les 12328 tests passent. C'est le gate de mutation qui échoue : mutation: 8 file(s), 4 failed, avec 52 survivants authentiques (28 dans _write_scope, 4 dans anchor, 19 dans remember, 1 dans remember_helpers). merge échoue parce qu'il dépend de unit.

La cause racine tient en trois points :

• Les details des refus globaux et l'environnement passé à chaque contrôle n'étaient jamais vérifiés.
• memory_writable et anchor passaient des valeurs que l'appelé jetait, ce qui ne produisait que des mutants équivalents.
• remember calcule la portée deux fois (l'enregistrement de doublon exact, puis l'écriture), si bien que chaque refus masquait l'autre.

Commit poussé : a0cd381

• _refused_domain (un prédicat) et require_global_allowed remplacent les appels dont le résultat était jeté. Le comportement reste identique, et tous les tests existants passent.
• Nouveau fichier tests_py/handlers/test_ghsa_7848_scope_decisions.py (14 tests). Il tue chaque mutant listé :
  • les refus exacts et leurs details ;
  • l'environnement lu ;
  • la relecture globale explicite d'une note locale (refusée, pas traitée comme doublon) ;
  • les doublons globaux détectés par le contenu, par les tags, ou explicites ;
  • l'écriture supersedes_id globale ;
  • le résultat non stocké ;
  • la curation qui saute le candidat d'alpha et fusionne celui de beta.
• Dans test_I2_canonical_writer, l'écrivain d'anchor est passé en ligne 157.

Vérifié localement :

• mutmut 3.8.1, avec la sélection de tests de ci_mutation_diff.select_tests : 0 survivant sur _write_scope. Sur anchor, aucun survivant dans _require_anchor_allowed ; le seul survivant (_build_anchor_tags) est sur une ligne non touchée. Sur les mutants visés de remember et remember_helpers, aucun survivant.
• tests_py/handlers, tests_py/invariants et test_team_project_scope : 2077 réussis, 97 sautés.
• ruff sur les fichiers modifiés, et check_craftsmanship : OK.

Non vérifié :

• Le gate complet en CI (Python 3.12, PostgreSQL) : torch ne s'installe pas dans mon environnement, et le CI comptait des survivants que mon run local tuait déjà. Le prochain unit fait foi.
• La branche a 3 commits de retard sur main. J'ai fait le rebase localement : un seul conflit, mécanique, dans la section Security du CHANGELOG, où les deux entrées ont été gardées. Mais le force-push avec lease a été refusé par les permissions de cette session, donc il n'est pas poussé.

Generated by Claude Code

Blocage du review 4 corrigé dans a653394 : la purge de seed_project cible maintenant le domaine vérifié par _parse_args (celui sous lequel remember écrit, le répertoire prime), plus l'argument brut. Test de régression ajouté : une session dans beta qui appelle seed_project({"domain": "alpha"}) depuis son répertoire ne purge que beta ; il échoue sans le correctif. Phrase du CHANGELOG alignée. Les trois notes non bloquantes (recherche de réplique LIMIT 1, marquage synaptique, descriptions de is_global) restent pour une PR de suivi.

🤖 Generated with Claude Code

Generated by Claude Code

CI autopilot : diagnostic du check unit (run 42742461370, job 129596153622, image ubuntu-26.04)

Les tests passent ; c'est le gate de mutation qui échoue (mutation: 8 file(s), 2 failed), avec 3 survivants authentiques. merge échoue parce qu'il dépend de unit. Cause racine :

• remember._exact_record mutmut_14/15 retirent le contenu ou les tags de la détection globale du pré-contrôle de doublon. L'insertion sous verrou refait la recherche avec la vraie portée, donc un rejeu via le handler renvoyait quand même exact_duplicate : le test ne voyait pas le pré-contrôle.
• seed_project._parse_args mutmut_26 ne passe plus le domaine au résolveur de remember. Hors dépôt connu, le domaine explicite restait alors brut (non normalisé), et aucun test ne le vérifiait.

Commit poussé : 3b7f417 (tests seulement)

• test_a_detected_global_memory_is_its_own_exact_duplicate vérifie désormais que remember.prepare_write renvoie lui-même le doublon, avant toute préparation.
• Nouveau test_seed_project_reports_the_domain_remember_resolves : " Shared-Notes " depuis un répertoire non enregistré donne shared-notes.

Vérifié localement :

• Chacun des trois mutants a été appliqué à la main, et les tests mis à jour échouent : 2 échecs pour m14, 1 pour m15, 1 pour m26.
• Les trois fichiers test_ghsa_7848_* passent sans mutant : 45 tests OK.
• ruff (check et format) et check_craftsmanship sont OK.

Non vérifié :

• Le gate complet en CI (PostgreSQL, torch absent ici). Le prochain unit fait foi.
• La branche a 15 commits de retard sur main. Le rebase n'est pas poussé : dans cette session, le force-push avec lease est refusé par le classifieur de permissions.

Generated by Claude Code

Continued in #969 on a new branch, same commits (head 3b7f417). This PR used its review budget and its head was never reviewed: the last verdict (block, 2026-11-07) is on an earlier head. A new branch gives the current head its review (ADR-1119).

Generated by Claude Code