Skip to content

worker: fix TOCTOU race in CWD caching - #61725

Closed
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix/worker-cwd-cache-toctou
Closed

worker: fix TOCTOU race in CWD caching#61725
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix/worker-cwd-cache-toctou

Conversation

@mcollina

Copy link
Copy Markdown
Member

The atomic counter used to signal CWD changes to worker threads was
being incremented before chdir() completed, creating a race window
where workers could cache stale directory paths with the new counter
value. This caused process.cwd() in workers to return incorrect values
until the next chdir() call.

Fix by reordering operations: call originalChdir() first, then
increment the counter. This ensures workers never cache stale data
while believing it is current.

A unit test for this fix is not feasible as it would be too flaky due to the timing-dependent nature of the race condition.

Reported-by: Giulio Comi
Reported-by: Caleb Everett

The atomic counter used to signal CWD changes to worker threads was
being incremented before chdir() completed, creating a race window
where workers could cache stale directory paths with the new counter
value. This caused process.cwd() in workers to return incorrect values
until the next chdir() call.
Fix by reordering operations: call originalChdir() first, then
increment the counter. This ensures workers never cache stale data
while believing it is current.
Reported-by: Giulio Comi
Reported-by: Caleb Everett
@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. worker Issues and PRs related to Worker support. labels Feb 7, 2026
@mcollina

Copy link
Copy Markdown
MemberAuthor

cc @nodejs/tsc

@mcollinamcollina added lts-watch-v20.x lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x labels Feb 7, 2026
@Renegade334

Copy link
Copy Markdown
Member

FYI, a PR was opened by the community reporter at #61664.

@anonriganonrig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice catch.

@codecov

codecovBot commented Feb 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.75%. Comparing base (81e05e1) to head (4b045af).
⚠️ Report is 357 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #61725 +/- ##
==========================================
+ Coverage 88.53% 89.75% +1.21% 
==========================================
Files 703 675 -28 Lines 208538 204525 -4013 Branches 40224 39306 -918 ==========================================
- Hits 184629 183565 -1064 + Misses 15912 13241 -2671 + Partials 7997 7719 -278 
Files with missing linesCoverage Δ
lib/internal/worker.js96.52% <100.00%> (ø)

... and 199 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollinamcollina closed this Feb 8, 2026
@mcollina
mcollina deleted the fix/worker-cwd-cache-toctou branch February 8, 2026 08:52
@aduh95aduh95 removed the lts-watch-v24.x PRs that may need to be released in v24.x label Feb 22, 2026
@richardlaurichardlau removed lts-watch-v20.x lts-watch-v22.x PRs that may need to be released in v22.x labels Feb 24, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.workerIssues and PRs related to Worker support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@mcollina@Renegade334@jasnell@anonrig@aduh95@richardlau@nodejs-github-bot