fix(security): replace predictable tempfile names with crypto.randomBytes (#1387)

gitnexus/src/core/group/bridge-db.ts

@@ -1,6 +1,6 @@
 import fsp from 'node:fs/promises';
 import path from 'node:path';
-import { createHash } from 'node:crypto';
+import { createHash, randomBytes } from 'node:crypto';
 import lbug from '@ladybugdb/core';
 import type { LbugValue } from '@ladybugdb/core';
 import type { BridgeHandle, BridgeMeta, StoredContract, CrossLink, RepoSnapshot } from './types.js';
@@ -24,7 +24,7 @@ import { dedupeContracts, dedupeCrossLinks } from './normalization.js';
  * - `.shadow` — non-blocking concurrent checkpoint sidecar (added in
  *   LadybugDB 0.15.4); same pairing constraint as `.wal`.
  *
- * `bridge-db` writes to a `bridge.lbug.tmp` file and then atomically renames
+ * `bridge-db` writes to a `bridge.lbug.tmp.<random>` file and then atomically renames
  * it into place. The rename only moves the main file; sidecars must be
  * cleaned up explicitly or the next writer trips the database-id check.
  */
@@ -41,6 +41,26 @@ async function removeLbugFile(basePath: string): Promise<void> {
   }
 }
 
+/**
+ * Remove all stale `bridge.lbug.tmp.*` files (and their sidecars) from a
+ * group directory.  With randomBytes-based temp names, a crashed writeBridge
+ * leaves behind a uniquely-named tmp file that no future run will target by
+ * name — so we glob for the prefix and clean up everything matching.
+ */
+async function cleanStaleBridgeTmpFiles(groupDir: string): Promise<void> {
+  try {
+    const entries = await fsp.readdir(groupDir);
+    const staleBases = entries.filter(
+      (e) => e.startsWith('bridge.lbug.tmp.') && !LBUG_SIDECAR_SUFFIXES.some((s) => e.endsWith(s)),
+    );
+    for (const name of staleBases) {
+      await removeLbugFile(path.join(groupDir, name));
+    }
+  } catch {
+    /* best-effort: directory may not exist yet */
+  }
+}
+
 export function contractNodeId(
   repo: string,
   contractId: string,
@@ -276,7 +296,7 @@ export async function retryRename(src: string, dst: string, attempts = 3): Promi
 
 export async function writeBridgeMeta(groupDir: string, meta: BridgeMeta): Promise<void> {
   const target = path.join(groupDir, 'meta.json');
-  const tmp = `${target}.tmp.${Date.now()}`;
+  const tmp = `${target}.tmp.${randomBytes(8).toString('hex')}`;
   await fsp.writeFile(tmp, JSON.stringify(meta, null, 2), 'utf-8');
   // Use retryRename for consistency with writeBridge's atomic swap — on
   // Windows a concurrent reader can cause EBUSY/EPERM even on a tiny
@@ -346,7 +366,7 @@ export async function writeBridge(
   const crossLinks = dedupeCrossLinks(input.crossLinks);
 
   const finalPath = path.join(groupDir, 'bridge.lbug');
-  const tmpPath = path.join(groupDir, 'bridge.lbug.tmp');
+  const tmpPath = path.join(groupDir, `bridge.lbug.tmp.${randomBytes(8).toString('hex')}`);
   const bakPath = path.join(groupDir, 'bridge.lbug.bak');
 
   const report: WriteBridgeReport = {
@@ -366,11 +386,12 @@ export async function writeBridge(
     }
   };
 
-  // Clean up any leftover tmp main file AND its `.wal` / `.shadow` sidecars.
-  // LadybugDB 0.16.0 rejects opening a database whose sidecars belong to a
-  // different database instance (database-id check), so any stale sidecar
-  // from a crashed previous run will fail the next writeBridge.
-  await removeLbugFile(tmpPath);
+  // Clean up stale tmp files left behind by previously crashed writeBridge
+  // runs.  With randomBytes-based names each run picks a unique path, so
+  // the old fixed-name `removeLbugFile(tmpPath)` was a no-op — stale
+  // artifacts accumulated.  The glob-based helper finds *all* leftover
+  // `bridge.lbug.tmp.*` entries and removes them (including sidecars).
+  await cleanStaleBridgeTmpFiles(groupDir);
 
   // 1. Create temp DB, insert all data.
   //

gitnexus/src/core/group/storage.ts

@@ -2,6 +2,7 @@ import * as fs from 'node:fs';
 import * as fsp from 'node:fs/promises';
 import * as path from 'node:path';
 import * as os from 'node:os';
+import { randomBytes } from 'node:crypto';
 import type { ContractRegistry } from './types.js';
 
 const CONTRACTS_FILE = 'contracts.json';
@@ -34,7 +35,7 @@ export async function writeContractRegistry(
   registry: ContractRegistry,
 ): Promise<void> {
   const targetPath = path.join(groupDir, CONTRACTS_FILE);
-  const tmpPath = `${targetPath}.tmp.${Date.now()}`;
+  const tmpPath = `${targetPath}.tmp.${randomBytes(8).toString('hex')}`;
 
   await fsp.writeFile(tmpPath, JSON.stringify(registry, null, 2), 'utf-8');
   await fsp.rename(tmpPath, targetPath);

gitnexus/test/unit/group/insecure-tempfile.test.ts

@@ -0,0 +1,192 @@
+/**
+ * Security tests for insecure tempfile remediation (#1318 U6).
+ *
+ * CodeQL js/insecure-temporary-file flags predictable temp filenames
+ * (e.g. Date.now() suffix) because an attacker with write access to
+ * the same directory can win a symlink race. The fix replaces all
+ * predictable suffixes with crypto.randomBytes(8).
+ *
+ * Two layers:
+ *   1. Structural — source-grep confirms randomBytes, not Date.now().
+ *   2. Behavioural — writeContractRegistry produces no leftover tmp files
+ *      and the final file is correctly written.
+ */
+import { beforeAll, describe, expect, it, beforeEach, afterEach } from 'vitest';
+import fs from 'node:fs';
+import fsp from 'node:fs/promises';
+import path from 'node:path';
+import os from 'node:os';
+import { writeContractRegistry, readContractRegistry } from '../../../src/core/group/storage.js';
+import { writeBridgeMeta, readBridgeMeta } from '../../../src/core/group/bridge-db.js';
+import type { ContractRegistry, BridgeMeta } from '../../../src/core/group/types.js';
+
+// ---------------------------------------------------------------------------
+// Structural: source files use randomBytes, not Date.now(), for temp paths
+// ---------------------------------------------------------------------------
+
+describe('insecure tempfile — structural guards (#1318 U6)', () => {
+  let bridgeSource: string;
+  let storageSource: string;
+
+  beforeAll(async () => {
+    bridgeSource = await fsp.readFile(
+      path.join(__dirname, '..', '..', '..', 'src', 'core', 'group', 'bridge-db.ts'),
+      'utf-8',
+    );
+    storageSource = await fsp.readFile(
+      path.join(__dirname, '..', '..', '..', 'src', 'core', 'group', 'storage.ts'),
+      'utf-8',
+    );
+  });
+
+  it('bridge-db.ts imports randomBytes from node:crypto', () => {
+    expect(bridgeSource).toMatch(/import\s*\{[^}]*randomBytes[^}]*\}\s*from\s*'node:crypto'/);
+  });
+
+  it('bridge-db.ts uses randomBytes for bridge.lbug temp path', () => {
+    expect(bridgeSource).toMatch(/bridge\.lbug\.tmp\.\$\{randomBytes/);
+  });
+
+  it('bridge-db.ts uses randomBytes for meta.json temp path', () => {
+    expect(bridgeSource).toMatch(/\.tmp\.\$\{randomBytes\(8\)\.toString\('hex'\)\}/);
+  });
+
+  it('bridge-db.ts does not use Date.now() in any temp path', () => {
+    // Match Date.now() specifically in tmp-path contexts — not in unrelated code.
+    const tmpDateNow = bridgeSource.match(/\.tmp\.\$\{Date\.now\(\)\}/g) ?? [];
+    expect(tmpDateNow.length).toBe(0);
+  });
+
+  it('bridge-db.ts uses readdir-based cleanup for stale bridge tmp files', () => {
+    expect(bridgeSource).toMatch(/cleanStaleBridgeTmpFiles/);
+    expect(bridgeSource).toMatch(/readdir\(groupDir\)/);
+    expect(bridgeSource).toMatch(/startsWith\('bridge\.lbug\.tmp\.'\)/);
+  });
+
+  it('bridge-db.ts calls cleanStaleBridgeTmpFiles before openBridgeDb in writeBridge', () => {
+    // Ensure cleanup happens before the DB is opened with the new random path.
+    const cleanIdx = bridgeSource.indexOf('cleanStaleBridgeTmpFiles(groupDir)');
+    const openIdx = bridgeSource.indexOf('openBridgeDb(tmpPath)');
+    expect(cleanIdx).toBeGreaterThan(-1);
+    expect(openIdx).toBeGreaterThan(-1);
+    expect(cleanIdx).toBeLessThan(openIdx);
+  });
+
+  it('storage.ts imports randomBytes from node:crypto', () => {
+    expect(storageSource).toMatch(/import\s*\{[^}]*randomBytes[^}]*\}\s*from\s*'node:crypto'/);
+  });
+
+  it('storage.ts uses randomBytes for contracts.json temp path', () => {
+    expect(storageSource).toMatch(/\.tmp\.\$\{randomBytes\(8\)\.toString\('hex'\)\}/);
+  });
+
+  it('storage.ts does not use Date.now() in any temp path', () => {
+    const tmpDateNow = storageSource.match(/\.tmp\.\$\{Date\.now\(\)\}/g) ?? [];
+    expect(tmpDateNow.length).toBe(0);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Behavioural: writeContractRegistry atomic write leaves no tmp files
+// ---------------------------------------------------------------------------
+
+describe('insecure tempfile — behavioural (#1318 U6)', () => {
+  let tmpDir: string;
+
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-u6-'));
+  });
+
+  afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  const sampleRegistry: ContractRegistry = {
+    version: 1,
+    generatedAt: '2026-05-06T00:00:00Z',
+    repoSnapshots: {},
+    missingRepos: [],
+    contracts: [],
+    crossLinks: [],
+  };
+
+  it('writeContractRegistry leaves no .tmp files after completion', async () => {
+    await writeContractRegistry(tmpDir, sampleRegistry);
+
+    const files = await fsp.readdir(tmpDir);
+    const tmpFiles = files.filter((f) => f.includes('.tmp.'));
+    expect(tmpFiles).toEqual([]);
+  });
+
+  it('writeContractRegistry writes correct data to final path', async () => {
+    await writeContractRegistry(tmpDir, sampleRegistry);
+
+    const loaded = await readContractRegistry(tmpDir);
+    expect(loaded).not.toBeNull();
+    expect(loaded!.version).toBe(1);
+    expect(loaded!.generatedAt).toBe('2026-05-06T00:00:00Z');
+  });
+
+  it('concurrent writes do not collide (randomBytes prevents same-ms race)', async () => {
+    // Fire two writes simultaneously — with Date.now() these could collide
+    // if they land in the same millisecond. With randomBytes they can't.
+    await Promise.all([
+      writeContractRegistry(tmpDir, { ...sampleRegistry, generatedAt: 'A' }),
+      writeContractRegistry(tmpDir, { ...sampleRegistry, generatedAt: 'B' }),
+    ]);
+
+    const loaded = await readContractRegistry(tmpDir);
+    expect(loaded).not.toBeNull();
+    // One of the two writes wins the rename — we just verify no crash.
+    expect(['A', 'B']).toContain(loaded!.generatedAt);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Behavioural: writeBridgeMeta atomic write leaves no tmp files
+// ---------------------------------------------------------------------------
+
+describe('insecure tempfile — writeBridgeMeta behavioural (#1318 U6)', () => {
+  let tmpDir: string;
+
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-u6-meta-'));
+  });
+
+  afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  const sampleMeta: BridgeMeta = {
+    version: 1,
+    generatedAt: '2026-05-06T00:00:00Z',
+    missingRepos: ['repo-x'],
+  };
+
+  it('writeBridgeMeta leaves no .tmp files after completion', async () => {
+    await writeBridgeMeta(tmpDir, sampleMeta);
+
+    const files = await fsp.readdir(tmpDir);
+    const tmpFiles = files.filter((f) => f.includes('.tmp.'));
+    expect(tmpFiles).toEqual([]);
+  });
+
+  it('writeBridgeMeta writes correct data to meta.json', async () => {
+    await writeBridgeMeta(tmpDir, sampleMeta);
+
+    const loaded = await readBridgeMeta(tmpDir);
+    expect(loaded.version).toBe(1);
+    expect(loaded.generatedAt).toBe('2026-05-06T00:00:00Z');
+    expect(loaded.missingRepos).toEqual(['repo-x']);
+  });
+
+  it('concurrent writeBridgeMeta calls do not collide', async () => {
+    await Promise.all([
+      writeBridgeMeta(tmpDir, { ...sampleMeta, generatedAt: 'A' }),
+      writeBridgeMeta(tmpDir, { ...sampleMeta, generatedAt: 'B' }),
+    ]);
+
+    const loaded = await readBridgeMeta(tmpDir);
+    expect(['A', 'B']).toContain(loaded.generatedAt);
+  });
+});