Skip to content

Commit d82fa5f

Browse files
vanceingallsvanceingalls
committed
fix: create temp dirs with mkdtemp, not a name built from Date.now()
Closes nine open `js/insecure-temporary-file` alerts — the technically correct ones. An audit of all 29 open alerts for that rule split them three ways: - 19 false positives: the write lands inside a directory the caller already made with `mkdtempSync`, and CodeQL's dataflow reaches `tmpdir()` without seeing the mkdtemp in between. - 1 mitigated: `fontCompression.ts` writes with `flag: "wx"` and only takes the tmpdir branch inside Lambda, where /tmp is single-tenant. - 9 real, and these are them. A name built from `Date.now()` under the shared temp dir, followed by `mkdirSync`, is guessable to the millisecond AND leaves a window between choosing the name and creating it, so on a shared machine another user can pre-create or symlink the path first. `mkdtempSync` closes both halves: it picks the random suffix and creates the directory 0700 in one syscall. Same shape, one line shorter, and the alerts go away rather than being dismissed. Six sites in `normalize.test.ts` (its `mkdirSync` import goes with them), one in `generate-catalog-previews.ts` — that single construction accounted for three alerts, since the other two were writes into the directory it made. No shared helper. `mkdtempSync` is already the stdlib primitive for exactly this, and the two callers live in different packages, so a wrapper would need a home in core to serve one CLI test and one build script — more indirection than the line it saves. Deliberately not touching the other 20: excluding the rule repo-wide would hide this class of bug from future code, which is the reason these are fixed rather than silenced.
1 parent 5752d22 commit d82fa5f

2 files changed

Lines changed: 12 additions & 14 deletions

File tree

‎packages/cli/src/whisper/normalize.test.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, afterEach } from "vitest";
2-
import { writeFileSync, readFileSync, mkdirSync, rmSync } from "node:fs";
2+
import { writeFileSync, readFileSync, mkdtempSync, rmSync } from "node:fs";
33
import { join } from "node:path";
44
import { tmpdir } from "node:os";
55
import {
@@ -14,8 +14,7 @@ import {
1414
import { detectSpeechOnset } from "./transcribe.js";
1515

1616
function tmpFile(name: string, content: string): string {
17-
const dir = join(tmpdir(), `hf-normalize-test-${Date.now()}`);
18-
mkdirSync(dir, { recursive: true });
17+
const dir = mkdtempSync(join(tmpdir(), "hf-normalize-test-"));
1918
dirs.push(dir);
2019
const path = join(dir, name);
2120
writeFileSync(path, content);
@@ -478,8 +477,7 @@ describe("whisper-cpp zero-duration interpolation", () => {
478477

479478
describe("patchCaptionHtml", () => {
480479
it("replaces const script = [] in HTML files", () => {
481-
const dir = join(tmpdir(), `hf-patch-test-${Date.now()}`);
482-
mkdirSync(dir, { recursive: true });
480+
const dir = mkdtempSync(join(tmpdir(), "hf-patch-test-"));
483481
dirs.push(dir);
484482

485483
const html = `<html><body><script>
@@ -501,8 +499,7 @@ describe("patchCaptionHtml", () => {
501499
});
502500

503501
it("replaces const TRANSCRIPT = [] variant", () => {
504-
const dir = join(tmpdir(), `hf-patch-test-${Date.now()}`);
505-
mkdirSync(dir, { recursive: true });
502+
const dir = mkdtempSync(join(tmpdir(), "hf-patch-test-"));
506503
dirs.push(dir);
507504

508505
const html = `<script>const TRANSCRIPT = [];</script>`;
@@ -516,8 +513,7 @@ describe("patchCaptionHtml", () => {
516513
});
517514

518515
it("does not modify HTML files without matching script patterns", () => {
519-
const dir = join(tmpdir(), `hf-patch-test-${Date.now()}`);
520-
mkdirSync(dir, { recursive: true });
516+
const dir = mkdtempSync(join(tmpdir(), "hf-patch-test-"));
521517
dirs.push(dir);
522518

523519
const html = `<html><body><script>console.log("hello");</script></body></html>`;
@@ -530,8 +526,7 @@ describe("patchCaptionHtml", () => {
530526
});
531527

532528
it("skips empty word arrays", () => {
533-
const dir = join(tmpdir(), `hf-patch-test-${Date.now()}`);
534-
mkdirSync(dir, { recursive: true });
529+
const dir = mkdtempSync(join(tmpdir(), "hf-patch-test-"));
535530
dirs.push(dir);
536531

537532
const html = `<script>const script = [];</script>`;
@@ -572,7 +567,7 @@ describe("detectSpeechOnset", () => {
572567
const amplitude = energyFn(t);
573568
buf.writeInt16LE(Math.round(amplitude * 32767), 44 + i * 2);
574569
}
575-
const path = join(tmpdir(), `hf-wav-test-${Date.now()}-${Math.floor(Math.random() * 1e6)}.wav`);
570+
const path = join(mkdtempSync(join(tmpdir(), "hf-wav-test-")), "tone.wav");
576571
writeFileSync(path, buf);
577572
dirs.push(path);
578573
return path;

‎scripts/generate-catalog-previews.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
readFileSync,
2525
existsSync,
2626
mkdirSync,
27+
mkdtempSync,
2728
cpSync,
2829
rmSync,
2930
writeFileSync,
@@ -170,8 +171,10 @@ export async function prepareProjectDir(
170171
item: CatalogItem,
171172
options: PrepareOptions = {},
172173
): Promise<string> {
173-
const tmpDir = join(tmpdir(), `hf-catalog-${item.name}-${Date.now()}`);
174-
mkdirSync(tmpDir, { recursive: true });
174+
// mkdtemp rather than a name built from `Date.now()`: it picks the random
175+
// suffix and creates the directory 0700 in one syscall, so nothing can
176+
// pre-create or symlink the path between choosing it and making it.
177+
const tmpDir = mkdtempSync(join(tmpdir(), `hf-catalog-${item.name}-`));
175178
cpSync(item.sourceDir, tmpDir, { recursive: true });
176179
mirrorRegistryTargets(tmpDir);
177180

0 commit comments

Comments
 (0)