You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix: create temp dirs with mkdtemp, not a name built from Date.now() (#3241)
* 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.
* fix: track the wav temp dir for cleanup and finish the mkdtemp sweep
The wav helper pushed the file path into `dirs`, so `afterEach` removed
`tone.wav` and left the directory it had just made — four per suite run.
Push the directory and derive the file path from it. Measured: the old code
leaks 4 directories per run, the new code leaks 0.
Three sites still built a predictable name and then created it. CodeQL never
flagged them — its dataflow reaches the template preview writes through a
`readdir` walk and does not connect them back to the `tmpdir()` root — so the
alert list was narrower than the pattern, and closing only the alerts would
turn the rule green while the shape survived where nothing would re-flag it.
`generate-template-previews.ts` is the near-twin of the file this change
started from, and the other two are producer dev entry points. All three use
the path only through the variable, so the random suffix changes nothing.
Catalog previews now call the existing `createCatalogPreviewTempDir` instead
of repeating its body. That test was in no runner, so it pinned uniqueness and
mode 0700 on a function nothing called; adding it to `test:scripts` alongside
a real caller makes it load-bearing. The rationale for the primitive moves to
the helper, which is now the only place it lives.
* ci: re-run catalog previews when the temp-dir module changes
Routing the renderer through `createCatalogPreviewTempDir` made that module
part of its runtime path, and the workflow already states the rule for the
sibling case: a module the renderer imports has to appear in the trigger, or a
change to it alone never re-runs the job that exercises it. Add it to the
`paths:` filter and to the renderer canary, so a PR touching only the temp-dir
allocation still renders both shape canaries.
Verified against this branch's own range: the previous argument list does not
report the file, so a helper-only PR was invisible to both checks.
0 commit comments