Skip to content

DeriveDenyHashes never writes on a public install (stale skip predicate), and the write itself is not contained or atomic #2017

Description

@0bsolescence

What happens

On a fresh 7.40.4 install, LIFEOS/TOOLS/DeriveDenyHashes.ts exits early every DerivedSync run:

[DeriveDenyHashes] skills/_LIFEOS absent (public install) — skipping hash write

So USER/SECURITY/DENY_HASHES.json is never produced, and hooks/lib/system-file-guard-core.ts, which ships in every install and reads that file, fails open with nothing to check. The guard exists; the thing it guards on does not.

Why

The skip predicate (DeriveDenyHashes.ts ~L216) still gates on skills/_LIFEOS existing. That was correct before #1689 moved the filter's home to USER/SECURITY and put the consumer in hooks/lib. Since then a public install has both a home for the file and a consumer for it; only the predicate is pre-relocation.

Two smaller things in the same function, found while fixing the first:

  1. The write is a bare writeFileSync(OUT_PATH, …). If a restored or untrusted USER tree carries SECURITY/ or DENY_HASHES.json as a symlink, the write follows it outside the tree. The USER root itself is legitimately a symlink (SystemUserBoundary.md), so the check has to be "resolve both sides and require containment", not "no symlinks".
  2. No temp-then-rename, so a crash mid-write leaves a truncated JSON that the consumer then has to cope with.

Proposed fix (diff attached, against v7.40.4)

  • Skip only when the consumer is ALSO missing: !existsSync(skills/_LIFEOS) && !existsSync(hooks/lib/system-file-guard-core.ts).
  • realpathSync the USER root and the output dir; refuse (exit 1, message) if the output dir is not inside the root.
  • Write to a temp file created with O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW and a random suffix (a predictable .tmp-<pid> name can be pre-planted as a symlink), write through the fd, fsync, then renameSync onto DENY_HASHES.json in the resolved dir. Rename replaces the directory entry, so a planted symlink at the final path is replaced, never followed.

Verified on two Linux nodes: with the predicate fixed the file lands with N salted hashes and the guard starts matching; with a symlink pre-planted at the final path the write no longer escapes the tree; with one pre-planted at a guessed temp path O_EXCL refuses and the run halts with a clear message.

Reported from the 0bsolescence/LifeOS fork, where these are carried as patches on patches/v7.40.4. Drafted with AI assistance; the reproduction and the verification were run by hand.

Proposed diff (against v7.40.4)

diff --git a/LifeOS/install/LIFEOS/TOOLS/DeriveDenyHashes.ts b/LifeOS/install/LIFEOS/TOOLS/DeriveDenyHashes.ts
index f33176e..538fe35 100755
--- a/LifeOS/install/LIFEOS/TOOLS/DeriveDenyHashes.ts
+++ b/LifeOS/install/LIFEOS/TOOLS/DeriveDenyHashes.ts
@@ -28,9 +28,9 @@
  * token, so filtering here keeps the scan precise. Tune ALLOWLIST/STOPWORDS with
  * --show-tokens.
  */
-import { readFileSync, writeFileSync, existsSync, appendFileSync, readdirSync, mkdirSync } from "node:fs";
+import { readFileSync, existsSync, appendFileSync, readdirSync, mkdirSync, realpathSync, renameSync, openSync, writeSync, fsyncSync, closeSync, unlinkSync, constants } from "node:fs";
 import { homedir } from "node:os";
-import { join, dirname } from "node:path";
+import { join, dirname, sep } from "node:path";
 import { createHash, randomBytes } from "node:crypto";
 
 const HOME = process.env.HOME || homedir();
@@ -213,8 +213,13 @@ function main(): void {
   // of exiting 1 on every derivedsync run (public issue #1488). The consumer
   // (hooks/lib/system-file-guard-core.ts) already fails open on a missing
   // DENY_HASHES.json, so nothing is lost.
-  if (!existsSync(join(CLAUDE, "skills", "_LIFEOS"))) {
-    console.log("[DeriveDenyHashes] skills/_LIFEOS absent (public install) — skipping hash write");
+  // Fix 2026-08-23: since #1689 the filter's home is USER/SECURITY and the
+  // consumer ships in hooks/lib — a public install WITH that consumer present
+  // both has a home for the filter and a guard reading it, so only skip when
+  // the consumer is missing too (predicate was stale, pre-relocation).
+  const consumerPresent = existsSync(join(CLAUDE, "hooks", "lib", "system-file-guard-core.ts"));
+  if (!existsSync(join(CLAUDE, "skills", "_LIFEOS")) && !consumerPresent) {
+    console.log("[DeriveDenyHashes] skills/_LIFEOS absent and no hash consumer installed — skipping hash write");
     return;
   }
 
@@ -233,8 +238,68 @@ function main(): void {
   // Create the output dir first — absent on fresh installs, and a throw here
   // aborts the whole DerivedSync pass (public PR #1652, @elhoim).
   mkdirSync(dirname(OUT_PATH), { recursive: true });
-  writeFileSync(OUT_PATH, JSON.stringify(payload, null, 0) + "\n");
-  console.log(`[DeriveDenyHashes] wrote ${hashes.length} salted hashes -> ${OUT_PATH} (no plaintext)`);
+  // The USER root is legitimately a symlink (XDG mount, SystemUserBoundary.md),
+  // but nothing below it may be: a restored or untrusted USER tree carrying
+  // SECURITY/ or DENY_HASHES.json as a symlink would redirect this write
+  // outside the tree. Resolve both sides and refuse anything not contained.
+  const userRoot = realpathSync(join(CLAUDE, "LIFEOS", "USER"));
+  const outDir = realpathSync(dirname(OUT_PATH));
+  if (outDir !== userRoot && !outDir.startsWith(userRoot + sep)) {
+    console.error(`[DeriveDenyHashes] refusing write: ${OUT_PATH} resolves outside the USER root`);
+    process.exit(1);
+  }
+  // Write temp-then-rename ON THE RESOLVED DIR: rename replaces the directory
+  // entry, so a statically crafted DENY_HASHES.json that is a symlink or hard
+  // link to a file elsewhere is replaced, never followed/truncated.
+  //
+  // The TEMP file is the other half of that guarantee, and it used to be the
+  // weak half: `.tmp-<pid>` is predictable (PIDs are small and enumerable), so
+  // anyone able to write into this directory could PRE-PLANT a symlink at the
+  // temp path and capture the payload before the rename ever ran. Two changes
+  // close that: the suffix now carries 16 bytes of CSPRNG entropy, and the file
+  // is created O_EXCL|O_NOFOLLOW so an existing entry — symlink or regular file
+  // — makes the open REFUSE instead of following it. Mode 0600 keeps the
+  // intermediate unreadable. fsync before rename so the payload is durable on
+  // disk before it becomes the live file.
+  //
+  // Threat-model boundary, restated: this now covers a STATICALLY pre-created
+  // symlink at either the temp path or the final path. An attacker mutating the
+  // tree CONCURRENTLY mid-run already has code execution as this user and
+  // remains out of scope.
+  const finalPath = join(outDir, "DENY_HASHES.json");
+  const tmpPath = join(outDir, `.DENY_HASHES.json.tmp-${randomBytes(16).toString("hex")}`);
+  const body = Buffer.from(JSON.stringify(payload, null, 0) + "\n", "utf-8");
+  let fd: number;
+  try {
+    fd = openSync(tmpPath, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | constants.O_NOFOLLOW, 0o600);
+  } catch (e) {
+    const code = (e as NodeJS.ErrnoException)?.code ?? String(e);
+    console.error(`[DeriveDenyHashes] refusing write: could not create a fresh temp file at ${tmpPath} (${code})`);
+    process.exit(1);
+  }
+  let closed = false;
+  let renamed = false;
+  try {
+    // writeSync is permitted to short-write. Unlooped, a partial write would
+    // still be fsynced and renamed into place, installing a TRUNCATED deny
+    // list as the live one — a security control silently degraded rather than
+    // failing loudly. Loop until the whole buffer is down.
+    let off = 0;
+    while (off < body.length) {
+      const n = writeSync(fd, body, off, body.length - off);
+      if (n <= 0) throw new Error(`writeSync made no progress at offset ${off}/${body.length}`);
+      off += n;
+    }
+    fsyncSync(fd);
+    closeSync(fd); closed = true;
+    renameSync(tmpPath, finalPath); renamed = true;
+  } finally {
+    // Leave nothing behind on ANY failure path, including a throwing close or
+    // a failed rename: the temp carries the same payload as the real file.
+    if (!closed) { try { closeSync(fd); } catch { /* fd already unusable */ } }
+    if (!renamed) { try { unlinkSync(tmpPath); } catch { /* best effort cleanup */ } }
+  }
+  console.log(`[DeriveDenyHashes] wrote ${hashes.length} salted hashes -> ${finalPath} (no plaintext)`);
 }
 
 if (import.meta.main) main();

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions