Skip to content

Commit 7fdf4db

Browse files
committed
chore(engine): suppress the temp-file alert with the reason it is safe
CodeQL flags `writeWav`'s `writeFileSync` as js/insecure-temporary-file (high) — the one new alert on #3021, and the reason its CodeQL check is red. It is a false positive, and the comment says why rather than just silencing it: `path` is always inside a directory made by `mkdtempSync`, never a name assembled directly under `tmpdir()`. Both callers are covered — the browser host page writes into `mkdtempSync(join(tmpdir(), "hf-fx-host-"))`, and the render output goes to the producer work dir, itself `mkdtempSync(join(tempRoot, "producer-project-"))`. mkdtemp picks the random suffix and creates the directory 0700 in one syscall, so the predictable filename inside it cannot be pre-created or symlinked by another user, which is the attack the rule is about. The analyzer sees the dataflow reach `tmpdir()` and not the mkdtemp in between. Suppressed inline rather than dismissed in the UI, so the justification lives next to the code and the rule stays live for anything added later in this file. Matches the repo's existing convention — `planV2.ts:222` carries an `lgtm[js/insecure-temporary-file]` for a different reason on the same rule. Correcting myself: I first reported this alert as not real, having intersected the PR's files against the default-branch alert list, which does not contain PR-ref alerts. Querying ?ref=refs/pull/3021/merge returns it straight away.
1 parent 384fdc4 commit 7fdf4db

1 file changed

Lines changed: 11 additions & 0 deletions

File tree

packages/engine/src/services/audioFxRender.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,17 @@ export function writeWav(
135135
const v = Math.max(-1, Math.min(1, samples[i] ?? 0));
136136
buf.writeInt16LE(Math.round(v * 32767), 44 + i * 2);
137137
}
138+
// lgtm[js/insecure-temporary-file] — `path` is always inside a directory the
139+
// caller made with `mkdtempSync`, never a name assembled directly under
140+
// `tmpdir()`. Both routes here are covered: the browser host page writes into
141+
// `mkdtempSync(join(tmpdir(), "hf-fx-host-"))` below, and the render output
142+
// goes to the producer's work dir, itself created as
143+
// `mkdtempSync(join(tempRoot, "producer-project-"))`. mkdtemp picks the random
144+
// suffix and creates the directory 0700 in one syscall, so the predictable
145+
// FILENAME inside it (`<elementId>-fx.wav`) cannot be pre-created or
146+
// symlinked by another user — which is the attack this rule is about. CodeQL
147+
// flags it because the dataflow reaches `tmpdir()` without seeing the mkdtemp
148+
// in between.
138149
writeFileSync(path, buf);
139150
}
140151

0 commit comments

Comments
 (0)