fix(isolation): route remaining raw fs writers into data/ through guarded primitives

server/lib/README.md

@@ -233,7 +233,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub
 | `dataRoot.js` | Data-root resolution + worktree-checkout detection (#1947). `resolveInstallRoot(fallbackRoot)` prefers the `PORTOS_DATA_ROOT` env var (pinned at real launch in `ecosystem.config.cjs`) over an `import.meta.url`-derived fallback, so a process booted from inside a CoS agent git worktree still resolves `data/`/`data.reference/` to the real install instead of the worktree's empty tree. `isWorktreeRoot(rootDir)` is the boot-migration backstop — true when `rootDir` lives under `data/cos/worktrees/` (keyed on the path segment only, so a fresh install's empty `data/` isn't a false positive). `resolveCodeRootForModule(moduleUrl)` is the single source of truth for the "two directories above this file" depth assumption — `paths.js`'s `CODE_ROOT` and `services/userActions.js`'s data-root guard both derive through it so they cannot silently drift apart. `DATA_ROOT_ENV` is the env-var name constant. Consumed by `fileUtils.js` (`PATHS`), `server/index.js`, and `scripts/run-migrations.js`. |
 | `downloadPreflight.js` | Free-disk preflight + resumable weight download. `assessDownloadPreflight({ destPath, expectedBytes })` → `{ freeBytes, requiredBytes, headroomBytes, verdict: ok\|tight\|insufficient }`; `assertDownloadFits` throws typed `DISK_INSUFFICIENT` (507) when the volume cannot hold the payload. `streamResumableDownload` Range-resumes a `.partial` on transport failure, discards it on user cancel, and verifies a published sha256 before rename. `sweepOrphanedPartials(dirs, { maxAgeMs })` unlinks leftover `.partial` (+ `.partial.etag`) files older than 7 days (default) and skips recent/in-flight dests; missing dirs are a no-op. `probeRemoteSize` / `siblingDownloadMeta` / `verifyDownloadHash` are the size+digest helpers the four weight-download entry points share. `createDownloadSlot({ codePrefix, idleStallEnvVar, exclusive, keepPartialOnCancel })` is the one transfer-slot registry a weight download claims before its first await — in-flight map, abort controller carrying a cancel/stall reason, typed `<PREFIX>_DOWNLOAD_IN_FLIGHT`/`_STALLED`/`_CANCELLED` errors, a per-chunk progress throttle, and an `isInFlight(path)` predicate covering the dest, its `.partial`, and shards under a claimed directory. Call it at module scope only. `isAnyDownloadInFlight` answers for every registered slot at once, which is how the orphaned-partial GC protects live transfers. |
 | `agentInstructionsFile.js` | The `AGENTS.md` + bridge `CLAUDE.md` pair a repo carries (#4852). `writeAgentInstructions(repoPath, content)` writes the body to `AGENTS.md` and the one-line `@AGENTS.md` import beside it — use it in scaffolders instead of a bare `writeFile(join(repoPath, 'CLAUDE.md'), …)`, since a generated repo carrying only one name is unreadable to half the CLIs PortOS can point at it. Constants: `AGENT_INSTRUCTIONS_FILENAME`, `CLAUDE_BRIDGE_FILENAME`, `AGENT_INSTRUCTIONS_IMPORT`. |
-| `fileCore.js` | Cross-cutting filesystem primitives (`atomicWrite`, directory helpers, bounded tail reads/watchers), time/format helpers, directory sizing, and SHA-256 helpers. |
+| `fileCore.js` | Cross-cutting filesystem primitives (`atomicWrite`, `writeFileGuarded`, `appendFileGuarded`, `copyFileGuarded`, `rmGuarded`, `unlinkGuarded`, `createWriteStreamGuarded`, directory helpers, bounded tail reads/watchers), time/format helpers, directory sizing, and SHA-256 helpers. |
 | `fileUtils.js` | Backward-compatible facade re-exporting the focused file utility modules so existing deep imports need no caller changes. |
 | `portosEnv.js` | Single server-side helper for PortOS's own `.env` (`PORTOS_ENV_PATH` anchored to `installRoot` per #1947, `parseEnvContents`, `readPortosEnvValue`, `upsertPortosEnvLine` + `upsertEnvLine`), with replacer-function guard for `$`-patterns and `process.env` wins precedence; `scripts/lib/envFile.js` stays as the zero-dependency boundary copy. |
 | `secretText.js` | `scrubSecretTokens(text)` — replace credential-SHAPED substrings (prefixed API keys, GitHub/Slack tokens, JWTs, AWS key ids, pasted Bearer headers, 48+-char hex) with `[REDACTED]` in free text bound for an LLM provider or a world-readable artifact; `scrubSecretTokensDeep(value)` walks arrays/plain objects and scrubs every string value. Value-side counterpart to the operator-action ledger's key-based `redactPayload` and `commandSecurity.js#redactOutput`'s JSON patterns; conservative so prose, 40-hex git SHAs, and short ids survive. |
@@ -532,5 +532,5 @@ pm` default, `NPM_CONFIG_PREFIX`, nvm/Volta) installed `codex` successfully and
 | `mockPathsDataRoot.js` | Shared Vitest helpers for `PATHS.data → temp dir` and no-peer record creation guards. |
 | `settingsTestUtil.js` | `bindSettingsFile(dataRoot)` → `writeSettingsFile`/`mergeSettingsFile`: direct settings.json disk writes that also drop the `getSettings()` read cache (dynamic-import reset) so a stale cache can't survive a bypass-`save()` write. |
 | `runtimeEnv.js` | `isTestRunner()` — NODE_ENV=test **or** the VITEST env var, so a run that dropped NODE_ENV is still armed. Dependency-free and deliberately apart from `db.js`, which used to own it: the lowest-level file primitives need the same answer and must not pull in `pg`, and the many suites spelling `vi.mock('../lib/db.js', () => ({ query }))` used to strip it out of the graph for every other consumer. |
-| `testDataIsolation.js` | Runtime backstop against a test WRITING into the install's real `data/` tree — the filesystem analogue of `db.js`'s row-write guard. `isInsideRealDataRoot(path)` canonicalizes a target through symlinked ancestors (it need not exist yet) and tests containment with `pathSafety.js#isPathAtOrInsideDir`, so a `..` climb, a relative path, a `data-archive` sibling, or a differently-cased spelling on a case-insensitive filesystem can't walk in sideways; the real root is re-derived through `dataRoot.js` rather than read from `PATHS`, so a suite that redirects `PATHS.data` to a temp root can't also redirect the guard. `assertNotRealDataWrite(path, operation)` throws under the test runner from `atomicWrite`, the `writeFile`/`appendFile`/`copyFile` wrappers in `fileCore.js`, and `collectionStore`'s record delete; `assertNotNewRealDataDir(dir)` is the create-only variant `ensureDir` uses, since `mkdir -p` on an existing directory mutates nothing. Inert outside the runner — the `isTestRunner()` check precedes every syscall. NOT yet universal: ~40 services still write to `PATHS.*` with raw `fs`, tracked as follow-up. Closes the write half of the bug class `testDataIsolation.guards.test.js` covers statically and the two-run probe covers for reads (#6176, after #6171). |
+| `testDataIsolation.js` | Runtime backstop against a test WRITING into the install's real `data/` tree — the filesystem analogue of `db.js`'s row-write guard. `isInsideRealDataRoot(path)` canonicalizes a target through symlinked ancestors (it need not exist yet) and tests containment with `pathSafety.js#isPathAtOrInsideDir`, so a `..` climb, a relative path, a `data-archive` sibling, or a differently-cased spelling on a case-insensitive filesystem can't walk in sideways; the real root is re-derived through `dataRoot.js` rather than read from `PATHS`, so a suite that redirects `PATHS.data` to a temp root can't also redirect the guard. `assertNotRealDataWrite(path, operation)` throws under the test runner from `atomicWrite`, the guarded wrappers in `fileCore.js` (`writeFileGuarded`, `appendFileGuarded`, `copyFileGuarded`, `rmGuarded`, `unlinkGuarded`, `createWriteStreamGuarded`), and `collectionStore`'s record delete; `assertNotNewRealDataDir(dir)` is the create-only variant `ensureDir` uses, since `mkdir -p` on an existing directory mutates nothing. Inert outside the runner — the `isTestRunner()` check precedes every syscall. Services mutating files under `data/` route through these guarded wrappers. Closes the write half of the bug class `testDataIsolation.guards.test.js` covers statically and the two-run probe covers for reads (#6176, after #6171, #6203). |
 | `testHelper.js` | Test helpers: `request()` (supertest-style HTTP) + `mockJsonResponse`/`mockTextResponse` (fetch `Response` mocks with `.text()`, `.json()`, and a `headers.get` content-type), `startLoopbackServer(app)`/`closeLoopbackServer(server)`/`waitForAbort(signal)` for tests that need a real socket (raw disconnects, SSE streaming) that `request()`'s run-to-completion fetch harness can't model, plus the source-scan pair `collectServerSources()` / `readServerSource(rel)` (and `SERVER_DIR`) used by the whole-tree guard suites — `spawnCwd.test.js` (#3193) and `cliChildEnv.test.js` (#3194). Those guards overlap deliberately, so they share one definition of "a source file"; change the ignore rules here and both move together. Cross-platform trio: `posixPath(v)` normalizes a RECEIVED path before comparing it to a POSIX-spelled literal (no-op on POSIX — never normalize the expectation, which would hide a genuinely wrong path), and `resolveTestPython()` returns an interpreter that actually runs, probing by execution because Windows ships a `python` Store-alias stub that exists but fails; `null` when there is none, for `describe.skipIf`; `pinPlatform(value)` pins `process.platform` and returns a restore that reinstates the ORIGINAL descriptor (deleting the pin when there was none) — it carries the one hazard every hand-rolled pin had to rediscover: never pin above an import that loads a native addon, which picks its prebuilt binary off the platform at load time (#4085). Python-shelling suites also take their two nested budgets from here: `PY_TEST_TIMEOUT_MS` (vitest per-test, passed as `it()`'s third argument — a real interpreter's wall time tracks machine load, not the assertion, so a ~4s case crosses the tight global 10s `testTimeout` on a contended full-suite worker) and the strictly smaller `PY_SUBPROCESS_TIMEOUT_MS` (every `execFileSync` spawn's own `timeout`, so a hung interpreter trips the spawn guard first and names the command instead of producing a bare vitest timeout; a subprocess allowance ABOVE the vitest budget is dead intent — vitest always wins). |

server/lib/fileCore.js

@@ -1,6 +1,6 @@
 /** Cross-cutting filesystem, time, formatting, and hashing helpers. */
-import { access, appendFile, chmod, copyFile, mkdir, readFile, readdir, stat, writeFile, rename, unlink } from 'fs/promises';
-import { createReadStream, existsSync, statSync, watch as watchFileSystem } from 'fs';
+import { access, appendFile, chmod, copyFile, mkdir, readFile, readdir, stat, writeFile, rename, unlink, rm } from 'fs/promises';
+import { createReadStream, createWriteStream, existsSync, statSync, watch as watchFileSystem } from 'fs';
 import { createHash, randomUUID } from 'crypto';
 import { basename, dirname, extname, join } from 'path';
 import { promisify } from 'util';
@@ -279,6 +279,24 @@ export async function copyFileGuarded(src, dest, mode) {
   return copyFile(src, dest, mode);
 }
 
+/** Guarded `fs/promises.rm`. Same signature. */
+export async function rmGuarded(target, options) {
+  if (isVitestRunner()) (await loadGuard()).assertNotRealDataWrite(target, 'rm');
+  return rm(target, options);
+}
+
+/** Guarded `fs/promises.unlink`. Same signature. */
+export async function unlinkGuarded(target) {
+  if (isVitestRunner()) (await loadGuard()).assertNotRealDataWrite(target, 'unlink');
+  return unlink(target);
+}
+
+/** Guarded `fs.createWriteStream`. Returns a Promise resolving to the WriteStream. */
+export async function createWriteStreamGuarded(filePath, options) {
+  if (isVitestRunner()) (await loadGuard()).assertNotRealDataWrite(filePath, 'createWriteStream');
+  return createWriteStream(filePath, options);
+}
+
 export const MINUTE = 60 * 1000;
 export const HOUR = 60 * MINUTE;
 export const DAY = 24 * HOUR;

server/lib/testDataIsolation.js

@@ -41,14 +41,10 @@
  *
  * Under the runner it fires from the shared primitives in `fileCore.js` /
  * `jsonIo.js` (`atomicWrite`, `ensureDir`'s create path, `writeFileGuarded`,
- * `appendFileGuarded`, `copyFileGuarded`, `appendJSONLine`) and from
- * `collectionStore`'s record delete. It is NOT yet universal: roughly forty
- * services still reach `PATHS.*` with raw `fs` calls of their own (see
- * `sharing/importer.js`, `videoUpload.js`, `catalogMedia.js`, `genome.js`),
- * and on a populated install their target directories already exist, so
- * `ensureDir`'s create-path check is a no-op for them. Routing those onto the
- * guarded wrappers is tracked as follow-up work — do not read this module as
- * proof that every write is covered.
+ * `appendFileGuarded`, `copyFileGuarded`, `rmGuarded`, `unlinkGuarded`,
+ * `createWriteStreamGuarded`, `appendJSONLine`) and from
+ * `collectionStore`'s record delete. All services mutating files under `data/`
+ * route through these guarded wrappers (#6203).
  *
  * A suite that genuinely needs a data root redirects it with
  * `createTempDataRoot()` + `makePathsProxy()` from `lib/mockPathsDataRoot.js`;

server/lib/testDataIsolation.test.js

@@ -18,7 +18,7 @@ import { describe, it, expect, vi, afterEach, afterAll } from 'vitest';
 import { existsSync, mkdtempSync, readFileSync, rmSync, symlinkSync } from 'fs';
 import { tmpdir } from 'os';
 import { isAbsolute, join, relative, sep } from 'path';
-import { appendFileGuarded, atomicWrite, copyFileGuarded, ensureDir, writeFileGuarded } from './fileCore.js';
+import { appendFileGuarded, atomicWrite, copyFileGuarded, createWriteStreamGuarded, ensureDir, rmGuarded, unlinkGuarded, writeFileGuarded } from './fileCore.js';
 import { appendJSONLine } from './jsonIo.js';
 import { assertNotNewRealDataDir, assertNotRealDataWrite, isInsideRealDataRoot } from './testDataIsolation.js';
 import { isPathAtOrInsideDir } from './pathContainment.js';
@@ -175,6 +175,32 @@ describe('the guarded raw-fs wrappers', () => {
     await copyFileGuarded(src, join(tempRoot, 'dest.txt'));
     expect(readFileSync(join(tempRoot, 'dest.txt'), 'utf8')).toBe('payload');
   });
+
+  it('rmGuarded refuses the real tree and allows a temp root', async () => {
+    await expect(rmGuarded(join(REAL_DATA, 'probe.txt'))).rejects.toThrow(/rm refused/);
+    const target = join(tempRoot, 'to-rm.txt');
+    await writeFileGuarded(target, 'x');
+    await rmGuarded(target);
+    expect(existsSync(target)).toBe(false);
+  });
+
+  it('unlinkGuarded refuses the real tree and allows a temp root', async () => {
+    await expect(unlinkGuarded(join(REAL_DATA, 'probe.txt'))).rejects.toThrow(/unlink refused/);
+    const target = join(tempRoot, 'to-unlink.txt');
+    await writeFileGuarded(target, 'x');
+    await unlinkGuarded(target);
+    expect(existsSync(target)).toBe(false);
+  });
+
+  it('createWriteStreamGuarded refuses the real tree and allows a temp root', async () => {
+    await expect(createWriteStreamGuarded(join(REAL_DATA, 'probe.txt'))).rejects.toThrow(/createWriteStream refused/);
+    const target = join(tempRoot, 'streamed.txt');
+    const ws = await createWriteStreamGuarded(target);
+    await new Promise((res, rej) => {
+      ws.write('streamed content', (err) => (err ? rej(err) : ws.end(res)));
+    });
+    expect(readFileSync(target, 'utf8')).toBe('streamed content');
+  });
 });
 
 describe('appendJSONLine', () => {

server/routes/attachments.js

@@ -4,12 +4,13 @@
  */
 
 import { Router } from 'express';
-import { unlink, readdir, stat } from 'fs/promises';
+import { readdir, stat } from 'fs/promises';
 import { join, resolve } from 'path';
 import { asyncHandler, ServerError } from '../lib/errorHandler.js';
 import {
   pathExists, PATHS, sanitizeFilename, getFileExtension, getMimeType,
   ATTACHMENT_ALLOWED_EXTENSIONS, isPathInsideDir, saveBase64Upload, serveLocalFile,
+  unlinkGuarded,
 } from '../lib/fileUtils.js';
 import { MAX_BASE64_UPLOAD_BYTES } from '../lib/uploadLimits.js';
 import { validateRequest, attachmentUploadRequestSchema } from '../lib/validation.js';
@@ -65,7 +66,7 @@ router.delete('/:filename', asyncHandler(async (req, res) => {
     throw new ServerError('Attachment not found', { status: 404, code: 'NOT_FOUND' });
   }
 
-  await unlink(filepath);
+  await unlinkGuarded(filepath);
 
   console.log(`🗑️ Attachment deleted: ${safeFilename}`);
 

server/routes/brainSongbook.js

@@ -19,7 +19,6 @@
  */
 
 import { Router } from 'express';
-import { unlink } from 'fs/promises';
 import { join, resolve } from 'path';
 import { createHash } from 'crypto';
 import { asyncHandler, ServerError } from '../lib/errorHandler.js';
@@ -38,6 +37,7 @@ import { MAX_BASE64_UPLOAD_BYTES } from '../lib/uploadLimits.js';
 import {
   pathExists, PATHS, sanitizeFilename, isPathInsideDir,
   SONGBOOK_ATTACHMENT_EXTENSIONS, saveBase64Upload, serveLocalFile,
+  unlinkGuarded,
 } from '../lib/fileUtils.js';
 
 const router = Router();
@@ -231,7 +231,7 @@ router.delete('/:id/attachments/:filename', asyncHandler(async (req, res) => {
 
   // Bytes may legitimately be absent on this machine (meta synced from a peer).
   if (await pathExists(filepath)) {
-    await unlink(filepath);
+    await unlinkGuarded(filepath);
   }
 
   console.log(`🗑️ Song attachment deleted: ${safeFilename}`);

server/routes/imageGen.js

@@ -11,7 +11,6 @@
 
 import { Router } from 'express';
 import { z } from 'zod';
-import { unlink, copyFile } from 'fs/promises';
 import { asyncHandler, ServerError, failValidation } from '../lib/errorHandler.js';
 import {
   validateRequest, imageEdgeSchema, refineImagePixelCap, PIXEL_CAP_MESSAGE,
@@ -26,7 +25,7 @@ import { recordUserAction } from '../services/userActions.js';
 import { getImageModels, requiredReposForModel } from '../lib/mediaModels.js';
 import { inspectModelCache, verifyModelCache, repairModelCache, aggregateVerifies } from '../lib/hfCache.js';
 import { startHfDownloadStream } from '../services/hfDownloadStream.js';
-import { PATHS, ensureDir, resolveGalleryImage } from '../lib/fileUtils.js';
+import { PATHS, ensureDir, resolveGalleryImage, unlinkGuarded, copyFileGuarded } from '../lib/fileUtils.js';
 import { prepareGenerateParams, resolveLocalImageModel, selectLocalImageModel } from '../services/imageGen/prepareParams.js';
 import { applyImageClean, applyWatermarkRemoval, applyLightRegenVariant } from '../services/imageGen/variants.js';
 import { join, basename } from 'node:path';
@@ -535,7 +534,7 @@ router.post('/generate', imageGenUploads, asyncHandler(async (req, res) => {
   // the client drops the connection mid-flight.
   if (uploadedTempPaths.length) {
     res.on('close', () => {
-      for (const p of uploadedTempPaths) unlink(p).catch(() => {});
+      for (const p of uploadedTempPaths) unlinkGuarded(p).catch(() => {});
     });
   }
   // Local + codex both go through mediaJobQueue (separate lanes — codex
@@ -970,7 +969,7 @@ router.post('/:filename/regenerate', asyncHandler(async (req, res) => {
   if (annotatedSketchPath) {
     await ensureDir(PATHS.imageRefs);
     initImageAbsPath = join(PATHS.imageRefs, `init-${randomUUID()}.png`);
-    await copyFile(annotatedSketchPath, initImageAbsPath);
+    await copyFileGuarded(annotatedSketchPath, initImageAbsPath);
   }
 
   // Provider-aware default (issue #912): SynthID-bearing sources keep the