Repository navigation
test(runtime): create temporary roots that are canonical and removable - #1019
Conversation
0cff206 to
593dc2f
Compare
`os.tmpdir()` is `/var/folders/...` on macOS and `/var` is a symlink to `/private/var`, so a root built directly on it is not canonical. Ultrafuzz deliberately rejects non-canonical roots -- that check exists to stop a path component being swapped underneath a resolved path -- so the code failed against its own temporary directories. Linux is unaffected because `/tmp` is already canonical, which is why CI never saw this. #1019 fixes this for `packages/runtime/test`. The same defect was present in 92 files across ten packages, the release-validation script, and the CI scripts, including seven production files. Those are worth fixing regardless of platform: they encode an assumption that the system temporary directory is canonical, and no platform guarantees that. Measured on macOS, `main` versus this change, per package: @ultrafuzz/modal 440 -> 492 passing (52 fixed) @ultrafuzz/dashboard 16 -> 34 passing (18 fixed) @ultrafuzz/artifacts 312 -> 324 passing (12 fixed) @ultrafuzz/cli 111 -> 116 passing (5 fixed) @ultrafuzz/references 26 -> 31 passing (5 fixed) 92 tests fixed, no new failures. `@ultrafuzz/config`, `@ultrafuzz/prompts`, `@ultrafuzz/topology`, `@ultrafuzz/evals`, `@ultrafuzz/evmbench` and `@ultrafuzz/security` passed before and still pass. The failures named the cause directly, for example: Error [ArtifactPathError]: generated test file root must be canonical: /var/folders/... Error: prompt schema destination crosses a symlink: /var `scripts/validate-packed-install.mjs` built its consumer and initialized project under the same root, so `pnpm validate:pack` failed on macOS with `SYMLINK_PATH: Topology path contains a symlink component: /var` before it validated anything. `packages/runtime/test/**` and the two runtime source files are left to #1019 so the two changes stay independently mergeable.
The external static-analysis gate added in #1005 fails on pre-existing fixtures. `VALIDATE_ALL_CODEBASE` is false, so gitleaks scans changed *files* rather than changed lines: touching any part of one of these files fails the gate on content the change never edited, and `release-gates` fails with it. The fixtures are deliberate. `packages/artifacts/test/artifacts.test.ts` carries a catalogue of fifteen fake credentials, one per format, asserting that each is rejected; `packages/security/test/sensitive-redaction.test.ts` carries another covering vendor formats. The `references` tests use spelled-out placeholders such as `ghs_examplereferencetokenvalue0123456789`. These are the inputs the redaction tests exist to prove. One site already carried a `// secretlint-disable` for the same reason. Forty-nine lines across eight files are marked `gitleaks:allow` with a reason: artifacts/test/artifacts.test.ts 22 security/test/sensitive-redaction.test.ts 17 runtime/test/runtime.test.ts 4 runtime/test/generated-workflow-verifier.test.ts 2 runtime/test/verified-output.test.ts 1 references/test/git-credential.test.ts 1 references/test/references.test.ts 1 references/test/vulnerability-database.test.ts 1 An inline marker travels with the line. A line-numbered fingerprint in `.gitleaksignore` silently stops matching the moment anything is inserted above it. Excluding `packages/**/test/**` would also clear the failure, but that disables the gate for the files most likely to carry a pasted credential by accident. The one finding in production source is fixed by rewording rather than suppressed. `sensitive-redaction.ts` explained the supplemental PEM pattern by quoting a BEGIN/END pair on one line, which reads as a private key; the prose now names the markers instead. Nothing is suppressed outside test fixtures. Verified by running gitleaks 8.28.0 over the tree: no tracked file reports a finding. Remaining hits are all in gitignored `dist/` and `dist-test/` output. `@ultrafuzz/security` reports 23/23 and `@ultrafuzz/references` 26/31, both identical to main; comments and one comment rewording only.
`os.tmpdir()` is `/var/folders/...` on macOS and `/var` is a symlink to `/private/var`, so a root built directly on it is not canonical. Ultrafuzz deliberately rejects non-canonical roots -- that check exists to stop a path component being swapped underneath a resolved path -- so the code failed against its own temporary directories. Linux is unaffected because `/tmp` is already canonical, which is why CI never saw this. #1019 fixes this for `packages/runtime/test`. The same defect was present in 92 files across ten packages, the release-validation script, and the CI scripts, including seven production files. Those are worth fixing regardless of platform: they encode an assumption that the system temporary directory is canonical, and no platform guarantees that. Measured on macOS, `main` versus this change, per package: @ultrafuzz/modal 440 -> 492 passing (52 fixed) @ultrafuzz/dashboard 16 -> 34 passing (18 fixed) @ultrafuzz/artifacts 312 -> 324 passing (12 fixed) @ultrafuzz/cli 111 -> 116 passing (5 fixed) @ultrafuzz/references 26 -> 31 passing (5 fixed) 92 tests fixed, no new failures. `@ultrafuzz/config`, `@ultrafuzz/prompts`, `@ultrafuzz/topology`, `@ultrafuzz/evals`, `@ultrafuzz/evmbench` and `@ultrafuzz/security` passed before and still pass. The failures named the cause directly, for example: Error [ArtifactPathError]: generated test file root must be canonical: /var/folders/... Error: prompt schema destination crosses a symlink: /var `scripts/validate-packed-install.mjs` built its consumer and initialized project under the same root, so `pnpm validate:pack` failed on macOS with `SYMLINK_PATH: Topology path contains a symlink component: /var` before it validated anything. `packages/runtime/test/**` and the two runtime source files are left to #1019 so the two changes stay independently mergeable.
Every fixture built its temporary root with `mkdtempSync(path.join(os.tmpdir(), ...))`, which has two problems that each call site had to solve and none did. The root was not canonical. On macOS `os.tmpdir()` is `/var/folders/...` and `/var` is a symlink to `/private/var`, so the runtime rejected the fixture's own root -- correctly, since that check exists to stop a component being swapped underneath a resolved path. The suite therefore failed against itself rather than against anything under test: 362 distinct failures on macOS before any change, 44 of them literally `must be canonical`. Linux is unaffected because `/tmp` is already canonical, which is why CI never saw it. The root was not removable. Sealed execution snapshots are left `dr-x------`, and unlinking an entry needs the write bit on its parent rather than on the entry itself, so `rmSync` could not remove a tree containing one. Of 39 files creating temporary roots, 25 attempted removal and 11 restored permissions first. One run left 1,232 directories and 567,065 files behind; repeated runs reached 11 GB, at which point the suite slowed from 2m30s to over 90 minutes and individual tests from tens of milliseconds to 55-85 seconds. `temporaryRoot` does both: it builds on a canonicalized tmpdir and registers the root for removal, restoring owner write permission on directories on the way down -- the same treatment `restoreRemovableDirectoryPermissions` applies in `clean.ts` (#985). `registerTemporaryPath` covers directories a fixture places beside its root rather than inside it, which root removal cannot reach. The two production sites are canonicalized for the same reason the tests are: the runtime requires canonical roots, so it should create them. Measured on macOS, `main` versus this change, default tmpdir throughout: distinct failures 362 -> 219 (no new failures; the remaining 219 all fail on main, 159 of them on the darwin /proc limitation tracked in #1011) `must be canonical` 44 -> 0 leaked directories 1,232 -> 0 leaked files 567,065 -> 0 The one directory that survives is a deliberately retained native-continuation controller root, which production removes through its own exit hook.
593dc2f to
9b89361
Compare
`os.tmpdir()` is `/var/folders/...` on macOS and `/var` is a symlink to `/private/var`, so a root built directly on it is not canonical. Ultrafuzz deliberately rejects non-canonical roots -- that check exists to stop a path component being swapped underneath a resolved path -- so the code failed against its own temporary directories. Linux is unaffected because `/tmp` is already canonical, which is why CI never saw this. #1019 fixes this for `packages/runtime/test`. The same defect was present in 92 files across ten packages, the release-validation script, and the CI scripts, including seven production files. Those are worth fixing regardless of platform: they encode an assumption that the system temporary directory is canonical, and no platform guarantees that. Measured on macOS, `main` versus this change, per package: @ultrafuzz/modal 440 -> 492 passing (52 fixed) @ultrafuzz/dashboard 16 -> 34 passing (18 fixed) @ultrafuzz/artifacts 312 -> 324 passing (12 fixed) @ultrafuzz/cli 111 -> 116 passing (5 fixed) @ultrafuzz/references 26 -> 31 passing (5 fixed) 92 tests fixed, no new failures. `@ultrafuzz/config`, `@ultrafuzz/prompts`, `@ultrafuzz/topology`, `@ultrafuzz/evals`, `@ultrafuzz/evmbench` and `@ultrafuzz/security` passed before and still pass. The failures named the cause directly, for example: Error [ArtifactPathError]: generated test file root must be canonical: /var/folders/... Error: prompt schema destination crosses a symlink: /var `scripts/validate-packed-install.mjs` built its consumer and initialized project under the same root, so `pnpm validate:pack` failed on macOS with `SYMLINK_PATH: Topology path contains a symlink component: /var` before it validated anything. `packages/runtime/test/**` and the two runtime source files are left to #1019 so the two changes stay independently mergeable.
|
Rebased onto This PR was failing Stacking rather than duplicating the markers keeps this diff to its own delta (unchanged at +292/-211) and makes the dependency explicit instead of implicit. It should be retargeted back to To be explicit, since the failure name invites the wrong conclusion: no credential is added by this branch. Scanning only the lines it adds reports no findings, and the flagged fixtures trace to #865. |
# Conflicts: # packages/runtime/test/generated-workflow-verifier.test.ts # packages/runtime/test/runtime.test.ts
…1025) * test: mark the fake credential fixtures the secret scanner flags The external static-analysis gate added in #1005 fails on pre-existing fixtures. `VALIDATE_ALL_CODEBASE` is false, so gitleaks scans changed *files* rather than changed lines: touching any part of one of these files fails the gate on content the change never edited, and `release-gates` fails with it. The fixtures are deliberate. `packages/artifacts/test/artifacts.test.ts` carries a catalogue of fifteen fake credentials, one per format, asserting that each is rejected; `packages/security/test/sensitive-redaction.test.ts` carries another covering vendor formats. The `references` tests use spelled-out placeholders such as `ghs_examplereferencetokenvalue0123456789`. These are the inputs the redaction tests exist to prove. One site already carried a `// secretlint-disable` for the same reason. Forty-nine lines across eight files are marked `gitleaks:allow` with a reason: artifacts/test/artifacts.test.ts 22 security/test/sensitive-redaction.test.ts 17 runtime/test/runtime.test.ts 4 runtime/test/generated-workflow-verifier.test.ts 2 runtime/test/verified-output.test.ts 1 references/test/git-credential.test.ts 1 references/test/references.test.ts 1 references/test/vulnerability-database.test.ts 1 An inline marker travels with the line. A line-numbered fingerprint in `.gitleaksignore` silently stops matching the moment anything is inserted above it. Excluding `packages/**/test/**` would also clear the failure, but that disables the gate for the files most likely to carry a pasted credential by accident. The one finding in production source is fixed by rewording rather than suppressed. `sensitive-redaction.ts` explained the supplemental PEM pattern by quoting a BEGIN/END pair on one line, which reads as a private key; the prose now names the markers instead. Nothing is suppressed outside test fixtures. Verified by running gitleaks 8.28.0 over the tree: no tracked file reports a finding. Remaining hits are all in gitignored `dist/` and `dist-test/` output. `@ultrafuzz/security` reports 23/23 and `@ultrafuzz/references` 26/31, both identical to main; comments and one comment rewording only. * fix: build temporary roots on a canonical system temporary directory `os.tmpdir()` is `/var/folders/...` on macOS and `/var` is a symlink to `/private/var`, so a root built directly on it is not canonical. Ultrafuzz deliberately rejects non-canonical roots -- that check exists to stop a path component being swapped underneath a resolved path -- so the code failed against its own temporary directories. Linux is unaffected because `/tmp` is already canonical, which is why CI never saw this. #1019 fixes this for `packages/runtime/test`. The same defect was present in 92 files across ten packages, the release-validation script, and the CI scripts, including seven production files. Those are worth fixing regardless of platform: they encode an assumption that the system temporary directory is canonical, and no platform guarantees that. Measured on macOS, `main` versus this change, per package: @ultrafuzz/modal 440 -> 492 passing (52 fixed) @ultrafuzz/dashboard 16 -> 34 passing (18 fixed) @ultrafuzz/artifacts 312 -> 324 passing (12 fixed) @ultrafuzz/cli 111 -> 116 passing (5 fixed) @ultrafuzz/references 26 -> 31 passing (5 fixed) 92 tests fixed, no new failures. `@ultrafuzz/config`, `@ultrafuzz/prompts`, `@ultrafuzz/topology`, `@ultrafuzz/evals`, `@ultrafuzz/evmbench` and `@ultrafuzz/security` passed before and still pass. The failures named the cause directly, for example: Error [ArtifactPathError]: generated test file root must be canonical: /var/folders/... Error: prompt schema destination crosses a symlink: /var `scripts/validate-packed-install.mjs` built its consumer and initialized project under the same root, so `pnpm validate:pack` failed on macOS with `SYMLINK_PATH: Topology path contains a symlink component: /var` before it validated anything. `packages/runtime/test/**` and the two runtime source files are left to #1019 so the two changes stay independently mergeable. --------- Co-authored-by: thankyou <> Co-authored-by: Antonio Viggiano <agfviggiano@gmail.com>
Fixes #1017. Fixes #1018.
Both issues come from the same line, repeated 136 times:
It is not canonical. On macOS
os.tmpdir()is/var/folders/...and/varis a symlink to
/private/var, so the runtime rejects the fixture's own root --correctly, since that check exists to stop a component being swapped underneath
a resolved path. The suite failed against itself rather than against anything
under test. Linux is unaffected (
/tmpis already canonical), which is why CInever saw this.
It is not removable. Sealed execution snapshots are left
dr-x------, andunlinking an entry needs the write bit on its parent rather than on the entry
itself, so
rmSynccannot remove a tree that contains one. Of 39 files creatingtemporary roots, 25 attempted removal and 11 restored permissions first.
Measured on macOS,
mainvs this branch, default tmpdir throughoutmainmust be canonicalfailuresThe remaining 219 all fail on
maintoo. 159 are the darwin/proclimitationtracked in #1011 and fixed by #1016; the rest are unrelated to temporary roots.
The one directory that still survives a run is a deliberately retained
native-continuation controller root, which production removes through its own
exit hook -- not a leak, and left alone.
What changed
temporaryRoot(prefix)builds on a canonicalized tmpdir and registers the rootfor removal, restoring owner write permission on directories on the way down --
the same treatment
restoreRemovableDirectoryPermissionsalready applies inclean.ts(#985).registerTemporaryPath(dir)covers paths a fixture places beside its rootrather than inside it, which root removal cannot reach. These were the last
survivors:
${project}-fake-bin,-fake-ps-bin,-redaction-bin,-sealed-runner-interpreterand friends.Two production sites are canonicalized for the same reason the tests are -- the
runtime requires canonical roots, so it should create them:
smithers.tsultrafuzz-controller-workspace-handoff.tsultrafuzz-workspace-index-Incidental cleanups the rewrite forced:
trusted-cli.test.tshad a localtemporaryRoot()wrapper that the renamemade self-recursive; removed, with its prefix inlined at its 16 call sites.
workflow-execution-snapshot-capability.test.tshad a local const shadowingthe import; renamed to
fixtureRoot.os, and 3 no longer needmkdtempSync.Why this is worth more than the failure count suggests
A macOS contributor currently cannot tell a real regression from the noise: the
suite is red before they touch anything, and the failures name canonicality
rather than the temporary directory, so the cause is not evident from the
output. The accumulation problem compounds it -- the slowdown is silent and
reads as a hang or as a regression in whatever you happen to be editing.
It also means darwin-specific defects could not be caught by the suite at all,
because it never got far enough to exercise one. #1016 is a concrete instance:
the tests that would have caught it were skipping.
Verification
tsc --noEmit(bothtsconfig.jsonandtsconfig.test.json),eslint,prettier --check: cleanpnpm build: cleanGreptile Summary
The PR centralizes runtime-test temporary-root creation and cleanup while canonicalizing the two production temporary roots that feed trusted runtime paths.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Test or runtime requests temporary root] --> B[Resolve canonical system tmpdir] B --> C[Create uniquely prefixed directory] C --> D{Test fixture?} D -->|Yes| E[Register root and sibling paths] D -->|No| F[Use runtime-specific cleanup ownership] E --> G[Process exit] G --> H[Restore owner permissions recursively] H --> I[Remove registered paths]Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
Context used: