test: guard user-action-events file-backend writes against leaking to real data/

server/lib/dataRoot.js

@@ -15,11 +15,28 @@
  * worktree-rooted process so callers (boot migrations) can skip work that
  * assumes the real install tree instead of crashing.
  */
-import { isAbsolute, resolve as resolvePath, sep } from 'path';
+import { dirname, isAbsolute, join, resolve as resolvePath, sep } from 'path';
+import { fileURLToPath } from 'url';
 
 /** Env var a real launch sets to pin the install root (see ecosystem.config.cjs). */
 export const DATA_ROOT_ENV = 'PORTOS_DATA_ROOT';
 
+/**
+ * The executing checkout's root, derived from a caller's `import.meta.url`
+ * (two directories up from wherever that module physically lives). Single
+ * source of truth for this depth assumption — `lib/paths.js`'s `CODE_ROOT`
+ * and any code that must independently re-derive the real (non-test-mocked)
+ * install root, such as `userActions.js`'s data-root guard, both resolve it
+ * through here so the two can never silently drift apart if this formula
+ * ever changes.
+ *
+ * @param {string} moduleUrl the caller's `import.meta.url`
+ * @returns {string} absolute path two directories above the caller's file
+ */
+export function resolveCodeRootForModule(moduleUrl) {
+  return join(dirname(fileURLToPath(moduleUrl)), '../..');
+}
+
 /**
  * Resolve the PortOS install root (the parent of `data/` and `data.reference/`).
  *

server/lib/paths.js

@@ -1,16 +1,12 @@
 /** Canonical install/source roots and path construction helpers. */
 import { homedir } from 'os';
-import { dirname, join } from 'path';
-import { fileURLToPath } from 'url';
-import { resolveInstallRoot } from './dataRoot.js';
-
-const __lib_filename = fileURLToPath(import.meta.url);
-const __lib_dirname = dirname(__lib_filename);
+import { join } from 'path';
+import { resolveCodeRootForModule, resolveInstallRoot } from './dataRoot.js';
 
 // The executing checkout's root (where THIS file physically lives). Code/source
 // paths — the repo root for git ops, `lib/slashdo` — must stay anchored here so
 // they always point at the checkout that loaded the code.
-const CODE_ROOT = join(__lib_dirname, '../..');
+const CODE_ROOT = resolveCodeRootForModule(import.meta.url);
 
 // The install root that holds the runtime `data/` tree. Prefer an explicit
 // PORTOS_DATA_ROOT env var over the executing-file location so a process booted

server/services/userActions.js

@@ -47,6 +47,7 @@ import { createFileWriteQueue } from '../lib/fileWriteQueue.js';
 import { isPlainObject } from '../lib/objects.js';
 import { createPgFileFacade, resolvePgBackend } from '../lib/pgFileFacade.js';
 import { isTestRunner } from '../lib/db.js';
+import { resolveCodeRootForModule, resolveInstallRoot } from '../lib/dataRoot.js';
 import { isUserActionActor, isUserActionType } from '../lib/userActionTypes.js';
 import { insertUserActionEvent, listUserActionEvents, pruneUserActionEvents } from './userActionsDb.js';
 
@@ -55,6 +56,38 @@ import { insertUserActionEvent, listUserActionEvents, pruneUserActionEvents } fr
 // pre-mock value and crash any suite whose fileUtils stub omits PATHS.data.
 const eventsFile = () => join(PATHS.data, 'user-action-events.json');
 
+// The REAL repo data/ dir, computed independently of the (possibly test-mocked)
+// `PATHS` import above via the same resolveCodeRootForModule/resolveInstallRoot
+// technique lib/paths.js itself uses (both go through the shared helper, so
+// they can't silently drift apart) — so a suite that redirects PATHS.data to a
+// temp root can't accidentally spoof this comparison too. `dataRoot.js` reads
+// its own env var directly rather than through anything a PATHS mock would touch.
+const REAL_REPO_DATA_DIR = join(resolveInstallRoot(resolveCodeRootForModule(import.meta.url)), 'data');
+
+/**
+ * Structural guard against the bug class in #3683/#3687/#5605: a suite that
+ * exercises a route wired to `recordUserAction` without redirecting
+ * PATHS.data to a temp root would otherwise silently write
+ * `user-action-events.json` into the developer's live `data/` tree the next
+ * time such a route gets exercised — #5594 patched three known offenders
+ * one at a time, which is a per-suite fix, not a guard against the next one.
+ * Fires only under the test runner, and only at the moment a write is
+ * actually attempted, so a suite that merely reads (or never triggers a
+ * `recordUserAction` call) is unaffected either way.
+ */
+function assertTestDataRootRedirected() {
+  if (!isTestRunner() || PATHS.data !== REAL_REPO_DATA_DIR) return;
+  throw new Error(
+    'recordUserAction attempted a file-backend write of user-action-events.json ' +
+      "into the repo's real data/ tree. This suite exercises a route wired to " +
+      'recordUserAction but never redirected PATHS.data to a temp root - mock ' +
+      "`../lib/fileUtils.js` with lib/mockPathsDataRoot.js's makePathsProxy/" +
+      'createTempDataRoot (the same fix #5594 applied to cos.test.js / ' +
+      'cosTaskRoutes.test.js / cosAgentFeedback.test.js) rather than letting the ' +
+      'write land here.',
+  );
+}
+
 /** LLM-readable one-liner cap — the ledger is read by a model, not paged through. */
 export const SUMMARY_MAX_CHARS = 240;
 /** Per-string payload cap; the containing object gets `truncated: true`. */
@@ -281,6 +314,13 @@ function makeFileBackend() {
   return {
     name: 'file',
     record: (event) => queueWrite(async () => {
+      // Hoisted above loadFileEvents()/the dedupe check on purpose (#5627
+      // review): an un-redirected suite replaying an existing
+      // (type, dedupeKey) used to hit the dedupe short-circuit's `return null`
+      // BEFORE this guard ever ran, silently no-op'ing past it with no throw —
+      // and by then loadFileEvents() had already read the real ledger into the
+      // test process regardless. Running the guard first closes both holes.
+      assertTestDataRootRedirected();
       const events = await loadFileEvents();
       if (events.some((row) => row.type === event.type && row.dedupeKey === event.dedupeKey)) return null;
       await ensureDir(PATHS.data);

server/services/userActionsDataRootGuard.test.js

@@ -0,0 +1,56 @@
+/**
+ * Regression coverage for the #5605 structural guard: a suite that exercises
+ * a route wired to `recordUserAction` without redirecting PATHS.data to a
+ * temp root must fail loudly instead of writing user-action-events.json into
+ * the repo's real data/ tree (the bug class #5594 patched per-suite for
+ * cos.test.js / cosTaskRoutes.test.js / cosAgentFeedback.test.js).
+ *
+ * Deliberately does NOT mock `../lib/fileUtils.js` — this is the one test in
+ * the suite that is SUPPOSED to run with PATHS.data unredirected, so it can
+ * prove the guard rejects that exact condition. If the guard regresses, this
+ * would otherwise be the test that writes a real file into the repo's data/
+ * tree, so it also asserts that never happens.
+ */
+import { describe, it, expect } from 'vitest';
+import { existsSync, readFileSync, rmSync } from 'node:fs';
+import { join } from 'node:path';
+import { PATHS } from '../lib/fileUtils.js';
+import { recordUserAction } from './userActions.js';
+
+const REAL_EVENTS_FILE = join(PATHS.data, 'user-action-events.json');
+
+describe('recordUserAction — data-root guard (#5605)', () => {
+  it('throws instead of writing user-action-events.json into the real data/ tree', async () => {
+    // Snapshot whatever is already there BEFORE exercising the guard. An
+    // install using the documented MEMORY_BACKEND=file escape hatch may
+    // legitimately already have this file as its real ledger — this test
+    // must prove the guard leaves it untouched, not assert it's absent (the
+    // previous version's `expect(existsSync(...)).toBe(false)` failed
+    // spuriously on exactly that install shape, and its unconditional
+    // afterEach `rmSync` then deleted the developer's real ledger).
+    const existedBefore = existsSync(REAL_EVENTS_FILE);
+    const contentBefore = existedBefore ? readFileSync(REAL_EVENTS_FILE, 'utf8') : null;
+
+    try {
+      await expect(recordUserAction({
+        type: 'cos.task.create',
+        summary: 'Guard regression probe',
+        dedupeKey: `guard-probe-${Math.random().toString(36).slice(2)}`,
+      })).rejects.toThrow(/real data\/ tree/);
+
+      // The guard must leave the real tree exactly as it found it — present
+      // and unchanged, or still absent — never newly created or modified.
+      expect(existsSync(REAL_EVENTS_FILE)).toBe(existedBefore);
+      if (existedBefore) {
+        expect(readFileSync(REAL_EVENTS_FILE, 'utf8')).toBe(contentBefore);
+      }
+    } finally {
+      // Only clean up a file THIS run's own guard regression created — never
+      // touch one that was already there before it ran (which could be a
+      // developer's real MEMORY_BACKEND=file ledger).
+      if (!existedBefore && existsSync(REAL_EVENTS_FILE)) {
+        rmSync(REAL_EVENTS_FILE, { force: true });
+      }
+    }
+  });
+});