Skip to content

fix(producer): isolate font cache temporary writes - #3669

Merged
jrusso1020 merged 1 commit into
mainfrom
fix/security-temp-files-batch-1
Sep 4, 2026
Merged

fix(producer): isolate font cache temporary writes#3669
jrusso1020 merged 1 commit into
mainfrom
fix/security-temp-files-batch-1

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

Font cache writes use a predictable PID/timestamp filename and unconditionally remove it in finally. If another file already occupies that name, the exclusive write fails but cleanup deletes the other writer's file. This addresses code-scanning alert 768.

Create a private temporary directory atomically inside the cache filesystem, then publish the compressed font with the existing atomic rename. Cleanup removes only the directory created by this invocation. Cache keys, font output, cache reuse, and best-effort behavior when the cache is unavailable remain unchanged.

Validation: reproduced the pre-existing-file deletion with a failing regression test before the fix; all 18 font compression and deterministic font tests now pass. Added coverage for failed cache publication and an unavailable cache directory. Core build, producer typecheck, changed-file lint/format, and all pre-commit checks pass. CI and CodeQL must pass before landing.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 939bb170e46f95224790c7a7aeb23beea390192b. No actionable findings.

fontCompression.ts:67-84 now ties cleanup to the directory created by this invocation while retaining same-filesystem atomic publication and best-effort cache behavior. Both font embedding callers continue through this shared helper.

Validation: all 11 exact-head font-compression tests passed on Linux, including real TTF compression. The collision regression fails against the base implementation. An eight-process same-key publication probe also passed with complete cache contents and no leftover staging directories. Windows behavior was inspected, not executed locally. CI is still running; no failures observed at review time.

— Magi

Verdict: APPROVE
Reasoning: The fix establishes cleanup ownership without changing cache keys or font output, supported by targeted regression/error/concurrency checks. Outstanding CI remains a landing gate.

@jrusso1020
jrusso1020 merged commit 6605225 into main Sep 4, 2026
55 checks passed
@jrusso1020
jrusso1020 deleted the fix/security-temp-files-batch-1 branch September 4, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants