fix: shorten CAFS temp directory names#12327
Merged
zkochan merged 1 commit intoJun 16, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR addresses Unix domain socket path length violations when pnpm runs as root by shortening CAFS temporary directory names. The path-temp dependency is replaced with direct fs.mkdtemp under storeDir/tmp, and a test validates the shortened naming pattern. ChangesShortened CAFS temporary directories
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review by Qodo
Context used 1. CAFS tmpdir test too narrow
|
PR Summary by QodoFix: shorten CAFS temp directory names WalkthroughsDescription• Generate CAFS temp dirs via fs.mkdtemp() with short _tmp_ prefix. • Remove path-temp dependency from @pnpm/store.create-cafs-store. • Add regression test covering short temp dirs during git package preparation. Diagramgraph TD
A["Git fetcher tests"] --> B["createCafsStore.tempDir()"] --> C[("storeDir/tmp")]
B --> D["node:fs mkdtemp"]
E["Git fetcher"] --> B
High-Level AssessmentThe following are alternative approaches to this PR: 1. Custom short random suffix + fs.mkdir
2. Make temp prefix/length configurable
Recommendation: Current approach (use File ChangesBug fix (1)
Tests (1)
Documentation (1)
Other (2)
|
Contributor
Integrated-Benchmark Report (Linux)Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.08180669906,
"stddev": 0.1653513131514235,
"median": 4.02862475026,
"user": 3.9326239599999995,
"system": 3.48139066,
"min": 3.9176072572600003,
"max": 4.47408084826,
"times": [
4.115356398259999,
3.9968011212600003,
4.23826134526,
4.47408084826,
4.03000832526,
4.06759294726,
3.9176072572600003,
4.0272411752599995,
3.9310631542600003,
4.02005441826
]
},
{
"command": "pacquet@main",
"mean": 4.22580853936,
"stddev": 0.17997262195543334,
"median": 4.23840520776,
"user": 4.010822760000001,
"system": 3.4888501599999997,
"min": 3.94620010726,
"max": 4.50707519026,
"times": [
4.3287879692599995,
4.50707519026,
4.15093156026,
4.35710668926,
4.17718996926,
4.04296699926,
3.94620010726,
4.39661433326,
4.0515921292599995,
4.29962044626
]
},
{
"command": "pnpr@HEAD",
"mean": 2.1428855379600007,
"stddev": 0.11083272090378257,
"median": 2.1010237827600005,
"user": 2.7106796599999994,
"system": 3.0304449599999996,
"min": 2.05098536926,
"max": 2.36173212026,
"times": [
2.09169626726,
2.1457342122600003,
2.05418895326,
2.1365017442600003,
2.3254434662600003,
2.05098536926,
2.0953708632600003,
2.06052568126,
2.36173212026,
2.10667670226
]
},
{
"command": "pnpr@main",
"mean": 2.1400536806600003,
"stddev": 0.11786480976087328,
"median": 2.1325446682600004,
"user": 2.7069799599999995,
"system": 3.02190636,
"min": 1.9584658912600001,
"max": 2.31573770526,
"times": [
2.15863165826,
2.3036867952600004,
2.03675233026,
2.31573770526,
2.03362918526,
2.1644965162600003,
2.2300396732600003,
1.9584658912600001,
2.1064576782600004,
2.0926393732600004
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6318537173200001,
"stddev": 0.010979494871669543,
"median": 0.63110591212,
"user": 0.37517696,
"system": 1.32561168,
"min": 0.61550196612,
"max": 0.64821928212,
"times": [
0.61808611112,
0.64648567912,
0.61550196612,
0.63489623212,
0.64037008012,
0.63095304212,
0.64821928212,
0.62510179012,
0.63125878212,
0.62766420812
]
},
{
"command": "pacquet@main",
"mean": 0.64143087142,
"stddev": 0.016586348945338433,
"median": 0.64000065712,
"user": 0.37670426,
"system": 1.3115391799999998,
"min": 0.62207425912,
"max": 0.66878124712,
"times": [
0.66878124712,
0.62613390412,
0.66376222712,
0.62207425912,
0.63167392512,
0.6458846271199999,
0.64526105912,
0.62346373212,
0.65253347812,
0.63474025512
]
},
{
"command": "pnpr@HEAD",
"mean": 0.68259529242,
"stddev": 0.015176770351765456,
"median": 0.68055915412,
"user": 0.38230536000000004,
"system": 1.36412058,
"min": 0.66324851412,
"max": 0.70678746012,
"times": [
0.67441227712,
0.66446964412,
0.67036408212,
0.68180933512,
0.66324851412,
0.69507391612,
0.70678746012,
0.70162169712,
0.68885702512,
0.6793089731199999
]
},
{
"command": "pnpr@main",
"mean": 0.6761023698199999,
"stddev": 0.020107210589841287,
"median": 0.68313435012,
"user": 0.37386506,
"system": 1.3500867799999998,
"min": 0.64019498712,
"max": 0.69961755712,
"times": [
0.68923419512,
0.68828969312,
0.69220703712,
0.66752285012,
0.67797900712,
0.69961755712,
0.64019498712,
0.64656784212,
0.66873532512,
0.6906752041199999
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.2083581401000005,
"stddev": 0.04407802545208617,
"median": 4.194311255600001,
"user": 3.764365819999999,
"system": 3.3404644600000006,
"min": 4.1625217246,
"max": 4.2834036996,
"times": [
4.2197599836,
4.2278092026000005,
4.2017663686,
4.1868561426,
4.1813664846,
4.162984481600001,
4.2834036996,
4.1625217246,
4.2792302506,
4.1778830626
]
},
{
"command": "pacquet@main",
"mean": 4.2206151925999995,
"stddev": 0.08925608206533911,
"median": 4.1836348201,
"user": 3.7890516199999995,
"system": 3.34032616,
"min": 4.1161395406,
"max": 4.3961720936,
"times": [
4.3492995636,
4.3961720936,
4.1818260776,
4.2716824156,
4.1730423876,
4.1655138216,
4.1850683296,
4.1161395406,
4.1852063856,
4.1822013106
]
},
{
"command": "pnpr@HEAD",
"mean": 2.1514622583999996,
"stddev": 0.10913976509998213,
"median": 2.1459676616000003,
"user": 2.5007577199999997,
"system": 2.8681727600000007,
"min": 1.9957455736,
"max": 2.3822116526,
"times": [
2.1787549056,
2.2293431236,
1.9957455736,
2.0628847736,
2.0628396716,
2.0961953996,
2.2147121606,
2.3822116526,
2.1524798666000002,
2.1394554566000004
]
},
{
"command": "pnpr@main",
"mean": 2.1915211821000002,
"stddev": 0.11537774484393201,
"median": 2.1743880541000005,
"user": 2.5205407199999996,
"system": 2.89964996,
"min": 2.0367507786,
"max": 2.3880198396,
"times": [
2.3623952356,
2.2500245216000003,
2.3880198396,
2.0812763886,
2.0367507786,
2.1933687076000004,
2.1158472156,
2.1554074006,
2.1205773786,
2.2115443546
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.34447293388,
"stddev": 0.03098791952681914,
"median": 1.3430835219800001,
"user": 1.3450211200000002,
"system": 1.70776426,
"min": 1.3001847209800002,
"max": 1.3892047599800001,
"times": [
1.3770508779800001,
1.32042438898,
1.30999923998,
1.32930213498,
1.3389319749800002,
1.3001847209800002,
1.34831878798,
1.38407738398,
1.3472350689800001,
1.3892047599800001
]
},
{
"command": "pacquet@main",
"mean": 1.3237684765800002,
"stddev": 0.020790849386291183,
"median": 1.3207967549800002,
"user": 1.3099624200000002,
"system": 1.7139221599999999,
"min": 1.28930156098,
"max": 1.35665516798,
"times": [
1.3332582459800002,
1.35455723198,
1.35665516798,
1.28930156098,
1.3301705319800001,
1.32288491698,
1.31065696598,
1.30926823898,
1.31222331198,
1.3187085929800002
]
},
{
"command": "pnpr@HEAD",
"mean": 0.67090703848,
"stddev": 0.0807161054200225,
"median": 0.65028016098,
"user": 0.32482372000000004,
"system": 1.2900561600000002,
"min": 0.62695816198,
"max": 0.89881550098,
"times": [
0.62695816198,
0.64977754398,
0.63515460398,
0.65078277798,
0.89881550098,
0.64685019298,
0.65293631898,
0.66012623698,
0.6352144399799999,
0.65245460698
]
},
{
"command": "pnpr@main",
"mean": 0.63201239438,
"stddev": 0.0075829010155113495,
"median": 0.63160946648,
"user": 0.32204271999999995,
"system": 1.24772756,
"min": 0.62217036598,
"max": 0.64609586298,
"times": [
0.62217036598,
0.63321174898,
0.64609586298,
0.63161496998,
0.62631867998,
0.63160396298,
0.63090121698,
0.62248452798,
0.63414123398,
0.64158137398
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.00567046972,
"stddev": 0.06922773311194935,
"median": 2.98154389912,
"user": 1.7517634199999996,
"system": 1.9449801199999999,
"min": 2.91881552012,
"max": 3.1741893231200002,
"times": [
3.05305252012,
2.96842141112,
2.98141427412,
2.97405275712,
2.91881552012,
2.97227410712,
3.02128791712,
3.01152334312,
3.1741893231200002,
2.98167352412
]
},
{
"command": "pacquet@main",
"mean": 2.9843242399200003,
"stddev": 0.04747482056272787,
"median": 2.96757068362,
"user": 1.7675999199999999,
"system": 1.95360652,
"min": 2.91756918612,
"max": 3.06592952612,
"times": [
2.94419023512,
2.9637464491200003,
3.06592952612,
3.04641895712,
2.99068263312,
2.97139491812,
2.91756918612,
2.9571785411200002,
3.02609481412,
2.96003713912
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6585417846200001,
"stddev": 0.014389335598381143,
"median": 0.65610944662,
"user": 0.32924892000000006,
"system": 1.30676962,
"min": 0.63950563912,
"max": 0.6888614081200001,
"times": [
0.6623162141200001,
0.65817519512,
0.64991407812,
0.6701642051200001,
0.6888614081200001,
0.6540436981200001,
0.64523712712,
0.66756312612,
0.63950563912,
0.6496371551200001
]
},
{
"command": "pnpr@main",
"mean": 0.64219949182,
"stddev": 0.008480588685803064,
"median": 0.64104370562,
"user": 0.32049062000000006,
"system": 1.27558592,
"min": 0.62622884912,
"max": 0.65614882212,
"times": [
0.64165564412,
0.64664567312,
0.6493377461200001,
0.64937861212,
0.63668501012,
0.62622884912,
0.64043176712,
0.63679379512,
0.65614882212,
0.63868899912
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/12327 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,208.36 ms(+0.85%)Baseline: 4,172.95 ms | 5,007.54 ms (84.04%) |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot 🚷 view threshold | 3,005.67 ms(+0.43%)Baseline: 2,992.85 ms | 3,591.42 ms (83.69%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,344.47 ms(+2.20%)Baseline: 1,315.54 ms | 1,578.65 ms (85.17%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,081.81 ms(+2.78%)Baseline: 3,971.44 ms | 4,765.72 ms (85.65%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 631.85 ms(+1.69%)Baseline: 621.36 ms | 745.63 ms (84.74%) |
Contributor
|
| Branch | pr/12327 |
| Testbed | pnpr |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot | 2,151.46 ms |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot | 658.54 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot | 670.91 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot | 2,142.89 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot | 682.60 ms |
|
Code review by qodo was updated up to the latest commit a980b73 |
a980b73 to
0e25b7d
Compare
|
Code review by qodo was updated up to the latest commit 0e25b7d |
Comment on lines
+50
to
+57
| test('uses short CAFS temp directories for git package preparation', async () => { | ||
| const storeDir = temporaryDirectory() | ||
| const tmpDir = await createCafsStore(storeDir).tempDir() | ||
|
|
||
| expect(path.dirname(tmpDir)).toBe(path.join(storeDir, 'tmp')) | ||
| expect(path.basename(tmpDir)).toMatch(/^_tmp_[a-zA-Z0-9]{6}$/) | ||
| expect(path.basename(tmpDir).length).toBeLessThanOrEqual(11) | ||
| }) |
There was a problem hiding this comment.
1. Cafs tmpdir test too narrow 📎 Requirement gap ☼ Reliability
The added regression test only asserts the CAFS temp directory basename pattern/length and does not validate the root-context pnpm install reproduction with a git-hosted dependency prepare script that creates IPC sockets under TMPDIR. This leaves the rule’s required end-to-end root TMPDIR/socket-path behavior unverified.
Agent Prompt
## Issue description
PR Compliance requires validating the root-context reproduction scenario (git-hosted dependency with a `prepare` script that creates IPC sockets/pipes under `TMPDIR`). The current test only checks the CAFS temp directory name/length and does not exercise root-context lifecycle execution or verify that resulting IPC paths stay within the 108-byte `sockaddr_un` limit.
## Issue Context
The compliance objective is specifically about `pnpm install` as root (`unsafePerm=false`) and TMPDIR-based IPC artifacts created during git dependency preparation.
## Fix Focus Areas
- fetching/git-fetcher/test/index.ts[50-57]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Member
There was a problem hiding this comment.
Accurate observation, but this is the right level for the test, so I'm leaving it as-is.
A faithful end-to-end reproduction of the original bug requires running as root on Linux, a real git-hosted dependency whose prepare script opens an AF_UNIX IPC socket, and hitting the 108-byte sockaddr_un limit — none of which is portable or feasible in a Jest unit test. The added assertions (_tmp_ + 6 chars, length ≤ 11) lock in the exact invariant that prevents the regression: the CAFS temp directory name stays short and bounded, which is what reclaims the socket-path budget for lifecycle tooling.
Written by an agent (Claude Code, claude-opus-4-8).
Comment on lines
+142
to
+143
| await fs.mkdir(baseTempDir, { recursive: true }) | ||
| return fs.mkdtemp(path.join(baseTempDir, '_tmp_')) |
There was a problem hiding this comment.
2. Temp dir mode too strict 🐞 Bug ☼ Reliability
createCafsStore().tempDir() now uses fs.mkdtemp(), which creates the temp directory with restrictive permissions (0700 by default on Unix), unlike the prior fs.mkdir() flow whose permissions followed the process umask. Because fetchers run git clone and preparePackage() inside this temp directory, any lifecycle execution that drops privileges (the unsafePerm=false path) will fail to write into the temp working directory with EACCES.
Agent Prompt
### Issue description
`createCafsStore().tempDir()` now returns a directory created by `fs.mkdtemp()`. In this repo, `mkdtemp()` is documented as creating temp directories with restrictive permissions (0700 by default), which differs from the previous `fs.mkdir()` behavior (umask-derived). This can make CAFS temp working directories unwritable when lifecycle tooling executes under different credentials (e.g. `unsafePerm=false` root flows).
### Issue Context
The returned `tempLocation` is used as the working directory for `git` operations and for `preparePackage()` (which runs package managers and lifecycle hooks), so it must be writable by the effective user that runs those steps.
### Fix Focus Areas
- store/create-cafs-store/src/index.ts[131-145]
### Suggested fix
After `mkdtemp`, explicitly align permissions with `mkdir` defaults, e.g.:
- `const tmpDir = await fs.mkdtemp(path.join(baseTempDir, '_tmp_'))`
- `await fs.chmod(tmpDir, 0o777 & ~process.umask())`
- `return tmpDir`
Alternatively, avoid `mkdtemp` and generate a short random suffix yourself, then `fs.mkdir(tmpDir)` so the mode naturally follows umask.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Member
There was a problem hiding this comment.
Thanks for the catch, but I believe this is a false positive — keeping fs.mkdtemp (0700) as-is.
unsafePerm defaults to false only when pnpm runs as root (config/reader/src/npmDefaults.ts:9-10), which is exactly this scenario. The temp dir is created by the root pnpm process, so it's root-owned under both the old and new code. The git clone and preparePackage() steps are driven by that root process, and writing into a root-owned 0700 dir as root succeeds.
The only case described here — a privilege-dropped non-owner user writing into the dir — already failed under the previous 0755: 0755 grants "others" read+execute but no write, so a non-owner could not write into the root-owned dir either way. So 0700 introduces no new failing scenario; the set of working cases is unchanged. 0700 is also the more appropriate mode for a temp dir holding unpacked package contents, so reverting to 0755 would be a small security regression with no functional gain.
Written by an agent (Claude Code, claude-opus-4-8).
zkochan
approved these changes
Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fs.mkdtemp()with a short_tmp_prefix for CAFS temporary package directoriespath-tempfrom@pnpm/store.create-cafs-store@pnpm/store.create-cafs-storeandpnpmWhy
This leaves more Unix socket path budget for lifecycle tools that create IPC sockets under
TMPDIRduring git-hosted dependency preparation.Closes #12222.
Pacquet
No pacquet change is included because the Rust git fetcher already uses
tempfile::tempdir()instead of the TypeScript CAFSpath-tempsuffix changed here.Tests
pnpm --filter pnpm compilepnpm --filter @pnpm/store.create-cafs-store testpnpm --filter @pnpm/exec.prepare-package testpnpm --filter @pnpm/fetching.git-fetcher testcargo-dylintandtaplowere not installed and were skipped by the hookSummary by CodeRabbit
Release Notes
Bug Fixes
TMPDIR.Tests
Chores