fix(storage): guard collectActiveMemoryPaths against symlink escape (info leak) (#1565)

* fix(storage): guard collectActiveMemoryPaths against symlink escape (info leak)

collectActiveMemoryPaths() (#1497) walked the RECALL_FALLBACK_DIRS memory
category dirs with a raw readdir and no symlink/containment guard. It feeds the
QMD-unavailable filesystem recall fallback, so a category dir symlinked outside
memoryDir (e.g. decisions/ -> an external dir) would be followed, pulling
out-of-store files into recall results. This is the same class of issue PR #1563
(issue #1546) hardened across the other memory-store walkers;
collectActiveMemoryPaths predates that PR and was the one remaining straggler.

Adopt the now-standard containment pattern: resolve the memory root once via
realpath; per directory lstat + skip symlinked/non-dir entries and assert the
realpath stays inside the root before reading; per entry skip symlinks and assert
the file's realpath is inside the root before including it. The shared
assertPathInsideRoot/pathIsInside check is extracted to
utils/path-containment.ts rather than forking a new one.

- Add symlinked-category-dir and symlinked-nested-entry containment tests
  (win32-guarded like the existing symlink tests).
- Bump the storage.ts structural-ratchet baseline (#1520) for the added guards.

* fix(storage): isolate per-entry failures in collectActiveMemoryPaths walk

Cursor Bugbot (medium): a containment/realpath failure on a single .md entry
was caught by the directory-wide try/catch, so the deferred subdir recursion
loop never ran — valid nested in-store memories under that dir could be dropped
from QMD fallback recall because one sibling entry was poisoned.

Split the directory-level guard (lstat/realpath/readdir → skip subtree on
failure) from per-entry handling, and wrap the per-.md containment assert in its
own try/catch so a single bad entry only skips itself. Subdir recursion now runs
unconditionally. Mirrors the per-file try/catch in document-scanner.ts scanDir
and consolidation-provenance-check.ts walkMarkdownFiles.

Add a regression test asserting a nested in-store memory is still returned when
its category dir also holds a symlinked-out sibling (win32-guarded). Bump the
storage.ts structural-ratchet baseline for the added lines.

Author: joshuaswarren

packages/remnic-core/src/storage-fallback-category-dirs.test.ts

@@ -27,7 +27,7 @@
  */
 
 import assert from "node:assert/strict";
-import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
+import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import test from "node:test";
@@ -356,6 +356,155 @@ test("question-queue items written by writeQuestion() do NOT leak into fallback
  * directly with a config object that explicitly disables QMD, proving the disk
  * scan a QMD-disabled deployment relies on returns every category.
  */
+/**
+ * Symlink containment (same walker-hardening class as PR #1563 / issue #1546).
+ *
+ * `collectActiveMemoryPaths()` walks the RECALL_FALLBACK_DIRS category dirs for
+ * the QMD-unavailable filesystem recall fallback. A category dir symlinked
+ * outside `memoryDir` (e.g. `decisions/` -> an external directory) must NOT be
+ * followed — otherwise out-of-store files leak into recall results. The walker
+ * now `lstat`s each dir (skipping symlinks/non-dirs), realpaths it, and asserts
+ * it stays inside the memory root; per entry it skips symlinks and asserts the
+ * file resolves inside the root before including it.
+ */
+test("category dir symlinked outside memoryDir does NOT leak files into fallback recall", async (t) => {
+  if (process.platform === "win32") {
+    t.skip("directory symlink setup is platform-specific");
+    return;
+  }
+  const baseDir = await mkdtemp(path.join(os.tmpdir(), "engram-1497-symlink-dir-"));
+  const outsideDir = await mkdtemp(path.join(os.tmpdir(), "engram-1497-symlink-out-"));
+  const storage = new StorageManager(baseDir);
+  StorageManager.clearAllStaticCaches();
+  storage.invalidateAllMemoriesCacheForDir();
+  try {
+    // A genuine in-store memory that MUST still be recalled.
+    await seedMemory(baseDir, "facts", "real-fact", "fact", "A genuine in-store fact.");
+
+    // An out-of-store memory reachable only via a symlinked category dir.
+    await writeFile(
+      path.join(outsideDir, "leaked.md"),
+      memoryFile("leaked-secret", "decision", "Out-of-store file must never be recalled."),
+      "utf-8",
+    );
+    // `decisions/` is a RECALL_FALLBACK_DIRS category dir; point it outside the store.
+    await symlink(outsideDir, path.join(baseDir, "decisions"), "dir");
+
+    storage.invalidateAllMemoriesCacheForDir();
+    const memories = await storage.readAllMemories();
+    const foundIds = new Set(memories.map((m) => m.frontmatter.id));
+
+    assert.ok(foundIds.has("real-fact"), "the genuine in-store fact must still be recalled");
+    assert.ok(
+      !foundIds.has("leaked-secret"),
+      "a symlinked-out category dir must not leak files into recall",
+    );
+    assert.equal(memories.length, 1, "only the in-store memory should be returned");
+  } finally {
+    StorageManager.clearAllStaticCaches();
+    await rm(baseDir, { recursive: true, force: true });
+    await rm(outsideDir, { recursive: true, force: true });
+  }
+});
+
+test("symlinked entry nested inside a category dir does NOT leak into fallback recall", async (t) => {
+  if (process.platform === "win32") {
+    t.skip("directory symlink setup is platform-specific");
+    return;
+  }
+  const baseDir = await mkdtemp(path.join(os.tmpdir(), "engram-1497-symlink-entry-"));
+  const outsideDir = await mkdtemp(path.join(os.tmpdir(), "engram-1497-symlink-entry-out-"));
+  const storage = new StorageManager(baseDir);
+  StorageManager.clearAllStaticCaches();
+  storage.invalidateAllMemoriesCacheForDir();
+  try {
+    // A genuine in-store memory in the same category dir that holds the symlink.
+    await seedMemory(baseDir, "facts", "real-fact", "fact", "A genuine in-store fact.");
+
+    await writeFile(
+      path.join(outsideDir, "leaked.md"),
+      memoryFile("nested-leak", "fact", "Out-of-store nested file must never be recalled."),
+      "utf-8",
+    );
+    // A symlinked *entry* inside facts/ that escapes the store.
+    await symlink(outsideDir, path.join(baseDir, "facts", "escape"), "dir");
+
+    storage.invalidateAllMemoriesCacheForDir();
+    const memories = await storage.readAllMemories();
+    const foundIds = new Set(memories.map((m) => m.frontmatter.id));
+
+    assert.ok(foundIds.has("real-fact"), "the genuine in-store fact must still be recalled");
+    assert.ok(
+      !foundIds.has("nested-leak"),
+      "a symlinked entry escaping the store must not leak files into recall",
+    );
+    assert.equal(memories.length, 1, "only the in-store memory should be returned");
+  } finally {
+    StorageManager.clearAllStaticCaches();
+    await rm(baseDir, { recursive: true, force: true });
+    await rm(outsideDir, { recursive: true, force: true });
+  }
+});
+
+/**
+ * Regression for Cursor Bugbot "Poisoned md skips sibling subdirs".
+ *
+ * Per-entry containment/realpath failures must be isolated so they cannot drop
+ * sibling `.md` files or, crucially, the nested-subdir recursion. This asserts
+ * that a category dir which also contains a symlinked-out entry still recurses
+ * into its real nested subdirectory and returns every in-store memory, while
+ * the out-of-store target stays excluded.
+ */
+test("a guarded/skipped sibling entry does NOT drop nested in-store memories", async (t) => {
+  if (process.platform === "win32") {
+    t.skip("directory symlink setup is platform-specific");
+    return;
+  }
+  const baseDir = await mkdtemp(path.join(os.tmpdir(), "engram-1497-sibling-"));
+  const outsideDir = await mkdtemp(path.join(os.tmpdir(), "engram-1497-sibling-out-"));
+  const storage = new StorageManager(baseDir);
+  StorageManager.clearAllStaticCaches();
+  storage.invalidateAllMemoriesCacheForDir();
+  try {
+    const factsDir = path.join(baseDir, "facts");
+    await mkdir(factsDir, { recursive: true });
+    // A top-level in-store memory and a nested in-store memory in the SAME
+    // category dir that also holds the symlinked-out sibling.
+    await writeFile(
+      path.join(factsDir, "top.md"),
+      memoryFile("top-fact", "fact", "Top-level in-store fact."),
+      "utf-8",
+    );
+    await seedMemory(baseDir, "facts", "deep-fact", "fact", "Deeply nested in-store fact.", {
+      nested: true,
+    });
+
+    await writeFile(
+      path.join(outsideDir, "leaked.md"),
+      memoryFile("leaked-secret", "fact", "Out-of-store file must never be recalled."),
+      "utf-8",
+    );
+    // A symlinked-out sibling entry alongside the real files/subdir.
+    await symlink(outsideDir, path.join(factsDir, "escape"), "dir");
+
+    storage.invalidateAllMemoriesCacheForDir();
+    const memories = await storage.readAllMemories();
+    const foundIds = new Set(memories.map((m) => m.frontmatter.id));
+
+    assert.ok(foundIds.has("top-fact"), "the top-level in-store fact must be recalled");
+    assert.ok(
+      foundIds.has("deep-fact"),
+      "the nested in-store fact must still be recalled past a skipped sibling",
+    );
+    assert.ok(!foundIds.has("leaked-secret"), "the symlinked-out file must not leak");
+    assert.equal(memories.length, 2, "exactly the two in-store memories should be returned");
+  } finally {
+    StorageManager.clearAllStaticCaches();
+    await rm(baseDir, { recursive: true, force: true });
+    await rm(outsideDir, { recursive: true, force: true });
+  }
+});
+
 test("QMD-disabled deployment: disk-scan collector returns all categories", async () => {
   const baseDir = await mkdtemp(path.join(os.tmpdir(), "engram-1497-qmd-off-"));
   // entitySchemas is the only other constructor arg; QMD is never constructed by

packages/remnic-core/src/storage.ts

@@ -1,10 +1,11 @@
-import { access, readdir, readFile, stat, writeFile, mkdir, unlink, rename, appendFile, open } from "node:fs/promises";
-import { appendFileSync, createReadStream, mkdirSync, readFileSync, statSync } from "node:fs";
+import { access, lstat, readdir, readFile, realpath, stat, writeFile, mkdir, unlink, rename, appendFile, open } from "node:fs/promises";
+import { appendFileSync, createReadStream, mkdirSync, readFileSync, statSync, type Dirent } from "node:fs";
 import { createHash } from "node:crypto";
 import path from "node:path";
 import { log } from "./logger.js";
 import { isErrnoCode } from "./utils/errno.js";
 import { RECALL_FALLBACK_DIRS, getCategoryDir, categoryDirName } from "./utils/category-dir.js";
+import { assertPathInsideRoot } from "./utils/path-containment.js";
 import { getCachedEntities, invalidateAllForDir, setCachedEntities } from "./memory-cache.js";
 import { rotateMarkdownFileToArchive } from "./hygiene.js";
 import { sanitizeMemoryContent } from "./sanitize.js";
@@ -4082,23 +4083,59 @@ export class StorageManager {
   private async collectActiveMemoryPaths(): Promise<string[]> {
     const filePaths: string[] = [];
 
+    // Resolve the memory root once for containment checks below. A category dir
+    // symlinked outside memoryDir (e.g. decisions/ -> an external dir) must NOT
+    // pull out-of-store files into the QMD-unavailable recall fallback (info
+    // leak). Same walker-hardening pattern as document-scanner.ts / cli.ts /
+    // consolidation-provenance-check.ts; reuses the shared containment helper.
+    let memoryRootReal: string;
+    try {
+      memoryRootReal = await realpath(this.baseDir);
+    } catch {
+      return filePaths;
+    }
+
     const collectPaths = async (dir: string) => {
+      // Directory-level guard, isolated from per-entry handling: skip symlinked
+      // or non-directory category dirs and assert the resolved dir stays inside
+      // the memory root before reading. A failure here means the whole subtree
+      // does not exist or escaped the store — fail closed by skipping it.
+      let entries: Dirent[];
       try {
-        const entries = await readdir(dir, { withFileTypes: true });
-        const subdirs: string[] = [];
-        for (const entry of entries) {
-          const fullPath = path.join(dir, entry.name);
-          if (entry.isDirectory()) {
-            subdirs.push(fullPath);
-          } else if (entry.name.endsWith(".md")) {
+        const dirStat = await lstat(dir);
+        if (dirStat.isSymbolicLink() || !dirStat.isDirectory()) return;
+        assertPathInsideRoot(memoryRootReal, await realpath(dir), dir);
+        entries = await readdir(dir, { withFileTypes: true });
+      } catch {
+        return;
+      }
+
+      const subdirs: string[] = [];
+      for (const entry of entries) {
+        // Never follow symlinked entries out of the store.
+        if (entry.isSymbolicLink()) continue;
+        const fullPath = path.join(dir, entry.name);
+        if (entry.isDirectory()) {
+          subdirs.push(fullPath);
+        } else if (entry.name.endsWith(".md")) {
+          // Isolate per-entry failures in their own try/catch: a containment or
+          // realpath failure on ONE .md entry must not drop sibling files or,
+          // crucially, the deferred subdir recursion below (Cursor Bugbot:
+          // "Poisoned md skips sibling subdirs"). Mirrors the per-file try/catch
+          // in search/document-scanner.ts scanDir and
+          // consolidation-provenance-check.ts walkMarkdownFiles.
+          try {
+            assertPathInsideRoot(memoryRootReal, await realpath(fullPath), fullPath);
             filePaths.push(fullPath);
+          } catch {
+            // Skip just this entry (symlink/containment/realpath failure).
           }
         }
-        for (const subdir of subdirs) {
-          await collectPaths(subdir);
-        }
-      } catch {
-        // Directory does not exist yet.
+      }
+      // Recurse into real subdirectories regardless of any single poisoned entry
+      // above, so valid nested in-store memories are never dropped.
+      for (const subdir of subdirs) {
+        await collectPaths(subdir);
       }
     };
 

scripts/ratchet-baseline.json

@@ -7,7 +7,7 @@
       "packages/remnic-core/src/orchestrator.ts": 20050,
       "packages/remnic-core/src/cli.ts": 9851,
       "packages/remnic-core/src/access-service.ts": 8277,
-      "packages/remnic-core/src/storage.ts": 7283,
+      "packages/remnic-core/src/storage.ts": 7320,
       "packages/remnic-core/src/config.ts": 4548
     },
     "oversizedFileCount": 10,