---
id: 11652
title: 'Substrate-layer guard: refuse canonical Chroma collection delete without UNIT_TEST_MODE'
state: CLOSED
labels:
  - bug
  - ai
  - testing
  - architecture
assignees:
  - neo-opus-ada
createdAt: '2026-05-19T15:35:10Z'
updatedAt: '2026-05-20T00:48:17Z'
githubUrl: 'https://github.com/neomjs/neo/issues/11652'
author: neo-opus-ada
commentsCount: 0
parentIssue: null
subIssues: []
subIssuesCompleted: 0
subIssuesTotal: 0
blockedBy: []
blocking: []
closedAt: '2026-05-20T00:48:17Z'
---
# Substrate-layer guard: refuse canonical Chroma collection delete without UNIT_TEST_MODE

## Context

A second Memory Core wipe incident has occurred: live MC went from **11,286 memories → 1,102 memories** between the `2026-05-16T13-08-06.565Z` and `2026-05-17T...` backup snapshots. Summaries went from 980 → 81 over the same window. Restored from the 5/16 bundle this turn (12,266 records back via `npm run ai:restore -- ... --mode merge --only-substrate=mc`).

Forensic context: prior wipe (#10867) was solved by adding a hardcoded safety check at the **test-helper layer** (`test/playwright/unit/ai/services/memory-core/util.mjs:16-18` throws on canonical-collection-name match). That guard fires only when the test routes through `cleanupChromaManager()`. The current recurrence happened **despite** that guard — most plausible attack surface: an agent invoking `npx playwright <spec>` directly (no `playwright.config.unit.mjs` loaded), so `process.env.UNIT_TEST_MODE` is undefined, `ai/mcp/server/memory-core/config.mjs:228-229` falls through to canonical names, and any test (or service-startup path) that touches Chroma destructive APIs operates on the live collection.

Operator framing (2026-05-19 ~15:30Z): *"gemini does frequently use npx playwright without our config files. so our tests must get hardened to still never delete live dbs."*

## The Problem

Defense-in-depth is currently asymmetric:

- **Config layer** (`config.mjs:228-229`) — toggles collection names on `UNIT_TEST_MODE`. Correct under `npm run test-unit`. Bypassed under `npx playwright` (config file not loaded).
- **Test-helper layer** (`util.mjs:16-18`) — guards `cleanupChromaManager()`. Bypassed if any code path calls `ChromaManager.client.deleteCollection()` directly, or if the test forgets the helper.
- **Substrate layer** (`ai/services/memory-core/managers/ChromaManager.mjs`) — **NO GUARD**. The underlying chromadb-js client is exposed unwrapped (`ChromaManager.client.deleteCollection({name})`). Any caller — production code, test, daemon, ingest path — can drop any collection by name, including the canonical ones.

The pattern works for the happy path but has zero defense against a caller who acquires `ChromaManager.client` and operates on it without going through the cleanup helper.

## The Architectural Reality

| Layer | File | Current behavior | Gap |
|---|---|---|---|
| Config | `ai/mcp/server/memory-core/config.mjs:228-229` | `process.env.UNIT_TEST_MODE === 'true' ? 'test-...' : 'neo-agent-memory'` | Only triggers when env var loaded; `npx playwright` bypasses |
| Test helper | `test/playwright/unit/ai/services/memory-core/util.mjs:16-18` | Throws if `collectionsConfig.memory === 'neo-agent-memory'` | Only fires inside the helper; direct `client.deleteCollection` skips |
| ChromaManager | `ai/services/memory-core/managers/ChromaManager.mjs` | No `deleteCollection` method; client exposed directly | **No substrate-level invariant** |
| Existing destructive-op guard | `Shared_DestructiveOperationGuard` (#10845) | Wraps known destructive paths (`restore.mjs`, etc.) | Not wired to chroma collection delete |

KB side has the same shape: `ai/services/knowledge-base/DatabaseService.mjs:372 truncateDatabase` *does* route through `DestructiveOperationGuard` for the wipe-for-restore path, but `ChromaManager.client.deleteCollection` is still reachable bare.

## The Fix

Add a **substrate-layer invariant** at `ChromaManager`: refuse `deleteCollection` against canonical collection names UNLESS `process.env.UNIT_TEST_MODE === 'true'` is set in the environment of the calling process.

Concretely:

1. **`ai/services/memory-core/managers/ChromaManager.mjs`** — add a guarded `async deleteCollection({name})` method that:
   - Throws `CANONICAL_COLLECTION_GUARDED` if `name` matches the canonical set (`neo-agent-memory`, `neo-agent-sessions`, `neo-agent-knowledge-base`, plus any other canonical names sourced from `config.mjs`).
   - Bypass only if `process.env.UNIT_TEST_MODE === 'true'` (test path).
   - Bypass only via explicit operator-acknowledged token if the call originates from a documented destructive recovery flow (e.g. `restore.mjs --mode replace` already gates via `DestructiveOperationGuard`).
2. **Forbid direct `ChromaManager.client.deleteCollection` access** — either wrap the client behind a Proxy in the manager so the underlying `deleteCollection` is intercepted at the JS level, OR add a CI lint rule (`grep` test) refusing the pattern `ChromaManager.client.deleteCollection` outside the manager itself.
3. **Mirror at the KB side** (`ai/services/knowledge-base/managers/ChromaManager.mjs` if it has one — verify during impl) so the invariant applies uniformly across memory-core + knowledge-base subsystems.
4. **Add UNIT_TEST_MODE-required check at `cleanupChromaManager`** — current `util.mjs:16-18` only checks the collection name; also check `process.env.UNIT_TEST_MODE === 'true'`. If not set, refuse even non-canonical names (covers the `npx playwright`-without-config scenario where the collection name falls through to canonical and the test author thought they were on a test-prefixed name).
5. **Spec coverage** — new test `test/playwright/unit/ai/services/memory-core/managers/ChromaManager.canonicalGuard.spec.mjs` that:
   - Calls the guarded delete against a canonical name without `UNIT_TEST_MODE` → expects throw
   - Calls against a canonical name with `UNIT_TEST_MODE=true` → expects pass
   - Calls against a test-prefixed name without `UNIT_TEST_MODE` → expects throw (defensive: the prefix-rule already gives test isolation, but if env var is missing the caller shouldn't be in a test path)

## Acceptance Criteria

- [ ] `ChromaManager.deleteCollection({name})` method exists and routes all destructive collection-delete operations through a single guarded path.
- [ ] Guard throws `CANONICAL_COLLECTION_GUARDED` when `name` matches the canonical set AND `process.env.UNIT_TEST_MODE !== 'true'`, regardless of caller.
- [ ] `ChromaManager.client` either Proxy-wrapped or `client.deleteCollection` made unreachable from outside the manager (lint rule + CI grep gate accepted as alternative if Proxy adds too much overhead).
- [ ] KB-side ChromaManager (if exists) mirrors the same invariant.
- [ ] `cleanupChromaManager()` adds `process.env.UNIT_TEST_MODE === 'true'` precondition; throws if absent.
- [ ] New spec `ChromaManager.canonicalGuard.spec.mjs` covers 3 scenarios above.
- [ ] Manual verification: `npx playwright test/playwright/unit/ai/services/memory-core/<any-spec>.spec.mjs` without `npm run test-unit` wrapper → tests refuse to touch canonical collections (the guard fires).
- [ ] Memory Core MCP daemon startup is unaffected (it reads canonical names directly from `config.mjs` and never invokes `deleteCollection`; verify production path).

## Out of Scope

- Forensic recovery of the 2026-05-17 wipe (separate operator-orchestrated investigation; restore already executed this turn).
- Backup retention policy adjustments (#11628 Phase 4 covers retention framing).
- Cross-substrate destructive-op guard unification beyond memory-core + KB Chroma surfaces (e.g., SQLite-layer guard already covered by #10845 `DestructiveOperationGuard`).
- Test-skill triggering hardening (#11529 closed; this ticket is one layer below).

## Avoided Traps

| Trap | Why rejected |
|---|---|
| Only extending `util.mjs` test-helper check | Same shape as #10867 fix that just failed in production. Doesn't defend against direct `ChromaManager.client.deleteCollection` callers. Substrate-layer invariant is required. |
| Removing `ChromaManager.client` accessor entirely | Production read paths legitimately need the client for non-destructive ops. Proxy-wrap or method-level guard is the right granularity. |
| Lock canonical names behind a `--force` CLI flag instead of env var | `--force` requires CLI wiring; tests don't have it. `process.env.UNIT_TEST_MODE === 'true'` is the existing convention and applies symmetrically. |
| Add a runtime "are we in a test process?" heuristic | Heuristics drift. Explicit env var is the invariant — if it's not set, you're not in a test. |
| Skip the lint/CI gate and rely on developer discipline | Empirically failed twice (#10867 + this incident). The guard must be mechanically enforced, not advisory. |

## Related

- **Prior incident:** [#10867](https://github.com/neomjs/neo/issues/10867) (closed; insufficient depth, only addressed test-helper layer)
- **Skill-trigger hardening:** [#11529](https://github.com/neomjs/neo/issues/11529) (closed; meta layer above this)
- **Existing destructive-op guard:** [#10845](https://github.com/neomjs/neo/issues/10845) (`DestructiveOperationGuard` infrastructure to extend)
- **Restore tooling that fires the existing guard correctly:** `buildScripts/ai/restore.mjs` (#10129 / #11141 / #11144)

## Origin Session ID

`7360e917-1733-4cdd-a6f3-5ac51c34b838`

## Handoff Retrieval Hints

- `query_raw_memories({query: 'memory core wipe npx playwright canonical collection guard'})`
- `ask_knowledge_base({query: 'ChromaManager deleteCollection guard canonical names UNIT_TEST_MODE', type: 'src'})`
- Empirical anchor: live MC count 1,102 → 12,842 post-restore on 2026-05-19; backup `backup-2026-05-16T13-08-06.565Z` contained 11,286+980 records
- Existing guard precedent: `DestructiveOperationGuard.assertDestructiveTargetAllowed` in `ai/services/shared/DestructiveOperationGuard.mjs`

## Timeline

- 2026-05-19T15:35:11Z @neo-opus-ada added the `bug` label
- 2026-05-19T15:35:11Z @neo-opus-ada added the `ai` label
- 2026-05-19T15:35:12Z @neo-opus-ada added the `testing` label
- 2026-05-19T15:35:12Z @neo-opus-ada added the `architecture` label
- 2026-05-19T15:35:19Z @neo-opus-ada assigned to @neo-opus-ada
- 2026-05-19T15:37:10Z @neo-opus-ada cross-referenced by #11653
- 2026-05-19T15:58:10Z @neo-gpt cross-referenced by PR #11655
- 2026-05-19T15:59:20Z @neo-opus-ada cross-referenced by PR #11656
- 2026-05-19T16:30:39Z @neo-opus-ada referenced in commit `257cb94` - "fix(ai): tighten canonical guard + drop bare-client fallback + KB wrapper coverage (#11652)

Addresses 3 Required Actions from @neo-gpt's Cycle 1 review on PR #11656
(commentId PRR_kwDODSospM8AAAABAYwPjg):

**RA1 — Uniform gate (tighten missing-UNIT_TEST_MODE behavior):**
`assertCanonicalCollectionDeleteAllowed` now requires `UNIT_TEST_MODE === 'true'`
OR a valid `confirmation` token for EVERY destructive collection-delete attempt,
regardless of whether the name is in the canonical set. Non-canonical names are
no longer a free bypass — a production caller invoking
`deleteCollection({name: 'arbitrary'})` without env or token now fails closed.
The canonical-collection-name set remains as a diagnostic indicator (the
`CanonicalCollectionGuardError.isCanonical` field + error-message phrasing
distinguish canonical-target refusals from non-canonical-target refusals).

**RA2 — Drop bare-client fallback in CollectionProxy.drop:**
The `else if (manager.client?.deleteCollection)` fallback in `CollectionProxy.mjs:120-127`
was the very bypass surface this PR exists to close. Replaced with fail-closed
throw when a manager lacks the guarded `deleteCollection({name, confirmation})`
wrapper. Future vector managers must implement the wrapper or be rejected by
the proxy; no silent regression to raw client deletion possible.

**RA3 — Direct KB-side wrapper coverage:**
Added 3 new tests under
`Neo.ai.services.knowledge-base.ChromaManager#deleteCollection — guard integration`
mirroring the MC-side wrapper tests (refuse-without-bypass, UNIT_TEST_MODE forward,
confirmation-token forward + non-leak). Closes the MC-only-tested asymmetry —
both wrappers now have direct integration coverage proving the canonical guard
fires before chromadb-client and that confirmation token doesn't leak into the
forwarded args.

**Test coverage:**
- 15 canonical-guard tests (was 10; +5 new for uniform-gate behavior, non-canonical
  refusal, isCanonical diagnostic field, and KB wrapper integration)
- 13 pre-existing DestructiveOperationGuard tests still pass — no regression
- Spec assertions inverted where the prior test encoded the now-deprecated
  canonical-set-only behavior (`Allows non-canonical (test-prefixed) collection
  name regardless of env state` → `Refuses non-canonical (test-prefixed) name
  without UNIT_TEST_MODE — uniform-gate (#11656 RA1)`)

28 tests passed in 783ms. (#11652)"
- 2026-05-20T00:48:17Z @tobiu referenced in commit `7022a1a` - "fix(ai): substrate guard refusing canonical Chroma collection deletes (#11652) (#11656)

* fix(ai): substrate guard refusing canonical Chroma collection deletes (#11652)

Empirical anchor: 2026-05-17 Memory Core wipe took live memories from 11,286 → 1,102
because `npx playwright` bypassed `playwright.config.unit.mjs` (UNIT_TEST_MODE absent
in env), making `aiConfig.collections.*` fall through to canonical names. The prior
test-helper-only guard (#10867 util.mjs:16-18) was insufficient — any caller invoking
`ChromaManager.client.deleteCollection` directly bypassed it.

Substrate-level invariant added at the chroma-collection-name boundary:

- New shared helper `assertCanonicalCollectionDeleteAllowed` in
  `ai/mcp/server/shared/services/DestructiveOperationGuard.mjs` plus typed
  `CanonicalCollectionGuardError` carrying stable `CANONICAL_COLLECTION_GUARDED` code.
- New guarded `deleteCollection({name, confirmation})` on both
  `ai/services/memory-core/managers/ChromaManager.mjs` and
  `ai/services/knowledge-base/ChromaManager.mjs`. Refuses canonical names
  (`neo-agent-memory`, `neo-agent-sessions`, `neo-agent-graph`, `neo-knowledge-base`)
  unless UNIT_TEST_MODE=true or the explicit production confirmation token is supplied.
- All four bare-`client.deleteCollection` callers rerouted through the guarded method:
  KB DatabaseService.truncateDatabase, KB VectorService.deleteCollection, MC
  CollectionProxy.drop, and the test-helper util.mjs cleanup.
- util.mjs also adds a `UNIT_TEST_MODE !== 'true'` precondition so bare-`npx playwright`
  invocations fail fast with a clear diagnostic before any chroma call (defense-in-depth
  ahead of the substrate guard).

New spec ChromaManager.canonicalGuard.spec.mjs covers 10 scenarios including the
attack-surface refusal (canonical name without UNIT_TEST_MODE) and both bypass paths.
All 13 pre-existing DestructiveOperationGuard tests still pass alongside the new 10;
KB DatabaseService.backup + VectorService.WorkVolumeBranching specs unchanged. (#11652)

* fix(ai): tighten canonical guard + drop bare-client fallback + KB wrapper coverage (#11652)

Addresses 3 Required Actions from @neo-gpt's Cycle 1 review on PR #11656
(commentId PRR_kwDODSospM8AAAABAYwPjg):

**RA1 — Uniform gate (tighten missing-UNIT_TEST_MODE behavior):**
`assertCanonicalCollectionDeleteAllowed` now requires `UNIT_TEST_MODE === 'true'`
OR a valid `confirmation` token for EVERY destructive collection-delete attempt,
regardless of whether the name is in the canonical set. Non-canonical names are
no longer a free bypass — a production caller invoking
`deleteCollection({name: 'arbitrary'})` without env or token now fails closed.
The canonical-collection-name set remains as a diagnostic indicator (the
`CanonicalCollectionGuardError.isCanonical` field + error-message phrasing
distinguish canonical-target refusals from non-canonical-target refusals).

**RA2 — Drop bare-client fallback in CollectionProxy.drop:**
The `else if (manager.client?.deleteCollection)` fallback in `CollectionProxy.mjs:120-127`
was the very bypass surface this PR exists to close. Replaced with fail-closed
throw when a manager lacks the guarded `deleteCollection({name, confirmation})`
wrapper. Future vector managers must implement the wrapper or be rejected by
the proxy; no silent regression to raw client deletion possible.

**RA3 — Direct KB-side wrapper coverage:**
Added 3 new tests under
`Neo.ai.services.knowledge-base.ChromaManager#deleteCollection — guard integration`
mirroring the MC-side wrapper tests (refuse-without-bypass, UNIT_TEST_MODE forward,
confirmation-token forward + non-leak). Closes the MC-only-tested asymmetry —
both wrappers now have direct integration coverage proving the canonical guard
fires before chromadb-client and that confirmation token doesn't leak into the
forwarded args.

**Test coverage:**
- 15 canonical-guard tests (was 10; +5 new for uniform-gate behavior, non-canonical
  refusal, isCanonical diagnostic field, and KB wrapper integration)
- 13 pre-existing DestructiveOperationGuard tests still pass — no regression
- Spec assertions inverted where the prior test encoded the now-deprecated
  canonical-set-only behavior (`Allows non-canonical (test-prefixed) collection
  name regardless of env state` → `Refuses non-canonical (test-prefixed) name
  without UNIT_TEST_MODE — uniform-gate (#11656 RA1)`)

28 tests passed in 783ms. (#11652)

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"
- 2026-05-20T00:48:17Z @tobiu closed this issue

