From 8230d422268ac859907c1d2c75151f2df408f88d Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Sun, 12 Jul 2026 10:10:48 -0400 Subject: [PATCH] security: use fs.mkdtempSync for all scratch dirs (fixes 9 CodeQL js/insecure-temporary-file) Every demo/test/MCP scratch path was built from a predictable name (os.tmpdir() + a fixed prefix + process.pid), which a local attacker can pre-create or symlink ahead of the write. Switch all of them to fs.mkdtempSync(path.join(os.tmpdir(), '-')), which creates an unpredictable 0700 directory atomically. - demo.mjs, test/stack.test.mjs, demo/mcp-demo.mjs, test/mcp.test.mjs: HOME/KH now mkdtempSync; the redundant mkdirSync is dropped. - audit-trail.mjs runTrilogy(): default home is mkdtempSync; the explicit mkdirSync(home) stays so caller-supplied homes are still created. - test/audit.test.mjs: the four caller-supplied homes are mkdtempSync too, so the interprocedural write in runTrilogy has no predictable source. - mcp.mjs canon_scan: write the manifest into a mkdtempSync dir and rmSync the whole dir in finally; the predictable pid+hash filename (and the now unused hashStr helper) are removed. Also normalizes demo/mcp-demo.mjs and test/mcp.test.mjs, which used the same pattern but were not yet flagged, so the class can't resurface. Tests: 16/16 pass; demo.mjs and demo/mcp-demo.mjs run clean. --- audit-trail.mjs | 2 +- demo.mjs | 3 +-- demo/mcp-demo.mjs | 3 +-- mcp.mjs | 12 +++--------- test/audit.test.mjs | 8 ++++---- test/mcp.test.mjs | 3 +-- test/stack.test.mjs | 3 +-- 7 files changed, 12 insertions(+), 22 deletions(-) diff --git a/audit-trail.mjs b/audit-trail.mjs index ebb6391..56ffde4 100644 --- a/audit-trail.mjs +++ b/audit-trail.mjs @@ -66,7 +66,7 @@ export function guardedCall({ manifest, name, action, leaseId, host, lock, polic // so the demo and the assertions exercise the EXACT same run. Sets up an isolated // strongroom vault + a clean (pinned) and a poisoned (never-pinned) tool, then drives // the four beats through the audited gate. Returns the trail + per-beat results. -export function runTrilogy({ home = path.join(os.tmpdir(), 'oys-audit-' + process.pid) } = {}) { +export function runTrilogy({ home = fs.mkdtempSync(path.join(os.tmpdir(), 'oys-audit-')) } = {}) { process.env.KEEPER_HOME = home; fs.mkdirSync(home, { recursive: true }); const tmp = (n) => path.join(home, n); diff --git a/demo.mjs b/demo.mjs index 7494284..a40ecce 100644 --- a/demo.mjs +++ b/demo.mjs @@ -8,9 +8,8 @@ import { check } from '@askalf/redstamp'; import { scan, pin, diff } from '@askalf/truecopy'; import { addSecret, grant, redeem } from '@askalf/strongroom'; -const HOME = path.join(os.tmpdir(), 'ass-demo-' + process.pid); +const HOME = fs.mkdtempSync(path.join(os.tmpdir(), 'ass-demo-')); process.env.KEEPER_HOME = HOME; -fs.mkdirSync(HOME, { recursive: true }); const tmp = (n) => path.join(HOME, n); const lock = tmp('truecopy.lock'); const policy = { egressAllow: ['api.example.com'] }; diff --git a/demo/mcp-demo.mjs b/demo/mcp-demo.mjs index 93ef8ff..cc30295 100644 --- a/demo/mcp-demo.mjs +++ b/demo/mcp-demo.mjs @@ -8,9 +8,8 @@ import fs from 'node:fs'; import path from 'node:path'; // strongroom reads KEEPER_HOME lazily, so seeding a temp vault here is enough. -const KH = path.join(os.tmpdir(), 'oys-mcp-demo-' + process.pid); +const KH = fs.mkdtempSync(path.join(os.tmpdir(), 'oys-mcp-demo-')); process.env.KEEPER_HOME = KH; -fs.mkdirSync(KH, { recursive: true }); const { Client } = await import('@modelcontextprotocol/sdk/client/index.js'); const { InMemoryTransport } = await import('@modelcontextprotocol/sdk/inMemory.js'); diff --git a/mcp.mjs b/mcp.mjs index 90cce48..df677a5 100644 --- a/mcp.mjs +++ b/mcp.mjs @@ -67,7 +67,8 @@ export function createOysServer(opts = {}) { }, async ({ manifest }) => { let parsed; try { parsed = JSON.parse(manifest); } catch (e) { return err(`manifest is not valid JSON: ${e.message}`); } - const tmp = path.join(os.tmpdir(), `oys-truecopy-${process.pid}-${Math.abs(hashStr(manifest))}.json`); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'oys-truecopy-')); + const tmp = path.join(dir, 'manifest.json'); try { fs.writeFileSync(tmp, JSON.stringify(parsed)); const r = truecopyScan(tmp); @@ -75,7 +76,7 @@ export function createOysServer(opts = {}) { } catch (e) { return err(`truecopy scan failed: ${e.message}`); } finally { - try { fs.unlinkSync(tmp); } catch { /* noop */ } + fs.rmSync(dir, { recursive: true, force: true }); } }); @@ -103,10 +104,3 @@ export function createOysServer(opts = {}) { return { server }; } - -/** Tiny stable string hash for temp filenames (not security-sensitive). */ -function hashStr(s) { - let h = 0; - for (let i = 0; i < s.length; i++) { h = (h << 5) - h + s.charCodeAt(i); h |= 0; } - return h; -} diff --git a/test/audit.test.mjs b/test/audit.test.mjs index c271922..9300daa 100644 --- a/test/audit.test.mjs +++ b/test/audit.test.mjs @@ -16,7 +16,7 @@ import path from 'node:path'; import { runTrilogy, verifyAuditFile, forgeEntry, findEntry, AuditLog } from '../audit-trail.mjs'; test('the gate records every layer decision into one chain, in order', () => { - const { audit, results } = runTrilogy({ home: path.join(os.tmpdir(), 'oys-audit-order-' + process.pid) }); + const { audit, results } = runTrilogy({ home: fs.mkdtempSync(path.join(os.tmpdir(), 'oys-audit-order-')) }); // the four beats produced the expected verdicts (same as stack.test.mjs) assert.ok(results.proceed.ok, 'beat 0: clean call proceeds'); assert.equal(results.truecopy.by, 'truecopy', 'beat 1: truecopy stops the poisoned tool'); @@ -32,7 +32,7 @@ test('the gate records every layer decision into one chain, in order', () => { }); test('the untouched chain verifies, on disk', () => { - const { audit, trailPath } = runTrilogy({ home: path.join(os.tmpdir(), 'oys-audit-intact-' + process.pid) }); + const { audit, trailPath } = runTrilogy({ home: fs.mkdtempSync(path.join(os.tmpdir(), 'oys-audit-intact-')) }); // in-memory verify assert.equal(audit.verify(), true); // durable verify (the daemon-grade path) @@ -43,7 +43,7 @@ test('the untouched chain verifies, on disk', () => { }); test('editing a past verdict breaks verification and pinpoints the entry', () => { - const { audit, trailPath } = runTrilogy({ home: path.join(os.tmpdir(), 'oys-audit-edit-' + process.pid) }); + const { audit, trailPath } = runTrilogy({ home: fs.mkdtempSync(path.join(os.tmpdir(), 'oys-audit-edit-')) }); audit.flush(trailPath); assert.equal(verifyAuditFile(trailPath).ok, true); @@ -58,7 +58,7 @@ test('editing a past verdict breaks verification and pinpoints the entry', () => }); test('deleting an entry (truncating the chain mid-log) is also caught', () => { - const { audit, trailPath } = runTrilogy({ home: path.join(os.tmpdir(), 'oys-audit-del-' + process.pid) }); + const { audit, trailPath } = runTrilogy({ home: fs.mkdtempSync(path.join(os.tmpdir(), 'oys-audit-del-')) }); audit.flush(trailPath); const lines = fs.readFileSync(trailPath, 'utf8').split('\n').filter((l) => l.trim()); // drop a middle entry: the NEXT entry's `prev` no longer matches the new predecessor's hash diff --git a/test/mcp.test.mjs b/test/mcp.test.mjs index 79f4895..aab04c9 100644 --- a/test/mcp.test.mjs +++ b/test/mcp.test.mjs @@ -11,9 +11,8 @@ import path from 'node:path'; // strongroom reads KEEPER_HOME lazily (at call time), so setting it here — after the // hoisted imports — is fine; seed a secret to lease. -const KH = path.join(os.tmpdir(), 'oys-mcp-test-' + process.pid); +const KH = fs.mkdtempSync(path.join(os.tmpdir(), 'oys-mcp-test-')); process.env.KEEPER_HOME = KH; -fs.mkdirSync(KH, { recursive: true }); import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; diff --git a/test/stack.test.mjs b/test/stack.test.mjs index 32ab75e..3bc7ae1 100644 --- a/test/stack.test.mjs +++ b/test/stack.test.mjs @@ -12,9 +12,8 @@ import { check } from '@askalf/redstamp'; import { scan, pin, diff } from '@askalf/truecopy'; import { addSecret, grant, redeem } from '@askalf/strongroom'; -const HOME = path.join(os.tmpdir(), 'trifecta-' + process.pid); +const HOME = fs.mkdtempSync(path.join(os.tmpdir(), 'trifecta-')); process.env.KEEPER_HOME = HOME; -fs.mkdirSync(HOME, { recursive: true }); const tmp = (n) => path.join(HOME, n); const lock = tmp('truecopy.lock'); const policy = { egressAllow: ['api.example.com'] };