Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Mrtenz
marked this pull request as ready for review
September 24, 2026 11:35
Writing `lavamoat/scripts.*.json` by hand means guessing which permissions a script needs, then loosening the config whenever the guess turns out to be too strict. `yarn audit <script>` instead runs the script under `--permission-audit`, collects every permission it actually exercises, and prints a config granting exactly those. The collector is embedded in the script itself and injected as a data URL through `NODE_OPTIONS`, so it reaches child processes at any depth and reports back over an append-only log that every process shares. Paths are generalised so the result is portable rather than a description of one machine: anything inside the project becomes `./`, known locations become `$TMPDIR`, `$HOME`, or a `$GITHUB_*` variable, and the upward directory walk that module and config resolution performs is recognised as such. Any grant that still reaches outside the project is reported with the path that caused it, so a widened config is never silent about why it was widened. Node.js is pinned to 26.9.0 because audit mode is only usable from that release onwards. Earlier versions still enforce `lstat`, `symlink`, and addon loading despite documenting audit mode as non-blocking, so any script touching those dies part-way through the audit and the resulting config is incomplete (nodejs/node#65419).
Mrtenz
force-pushed
the
mrtenz/permission-audit-script
branch
from
September 24, 2026 12:52
5ed2f83 to
7155726
Compare
Module and workspace resolution walks from the project directory up to the file system root, and the paths it probes for describe the machine rather than the script, so they are generalised to `/` rather than named. That classification was applied to writes as well, which it should never be: resolution only ever reads. The effect was worse than an overly broad entry. A write to an ancestor directory, or to a file such as `~/.npmrc`, was read as a probe and granted `/`, and collapsing the grants then short-circuits on `/` and discards everything else. A single misclassified write therefore turned the whole write list into unrestricted file system access, silently. Recursive `mkdir` checks write access on ancestor directories, so this was reachable in ordinary use. `generalise` now takes the action and only consults the probe rules for reads. Reads are unaffected. Also repairs two breakages from the surrounding work: `toSorted` is ES2023 and the target is now ES2022, and Oxlint reports the `any` that `JSON.parse` returns as an unsafe return.
The permissions a script needs on a CI runner are not the ones it needs on a developer's machine: the paths, the environment variables, and the tool cache all differ. Generating a config locally and hoping it holds in CI is guesswork. Re-running a workflow with debug logging enabled now runs each command through `yarn audit` instead of directly, so the job reports what it actually needed where it actually runs. `RUN` defaults to `yarn` and is overridden to `yarn audit` only for those runs, which keeps one copy of each command rather than a pair behind opposing conditions. `runner.debug` is not available to a job-level `if`, so the override is a step. It is also limited to Node 26.x: Node 22 rejects `--permission-audit` outright, and auditing every version in the matrix would produce several configs for no gain. This depends on the audit passing the script's exit code on, which it previously swallowed. Wrapping a command in something that always succeeds would have turned every job green regardless of whether the tests passed. The changelog steps keep calling `yarn` directly, since they pass arguments and the audit does not forward those to the audited script.
These steps had no name, so GitHub labelled them with the command itself. That read fine as `yarn build`, but now that the command is `$RUN build` the label shows the variable rather than what the step does, and it is identical in both of the jobs that run tests.
The audit is silent by default, so routing commands through it meant that turning debug logging on removed the build, lint, and test output a normal run shows. Enabling debug gave strictly less to look at, which is backwards. `--verbose` restores that output and adds the report of which grants reached outside the project and the path that earned each one, which is the part worth reading on a runner.
A CI runner reads standard output and standard error as two separate pipes and merges them into one log by arrival, with no ordering guarantee between them. The report was written to standard error immediately after the config went to standard output, so the runner was free to place it anywhere — in practice, in the middle of the JSON. Flushing cannot fix that: ordering is only guaranteed within a single descriptor. Writing both to standard output is what makes the sequence deterministic, since the kernel serialises writes to one descriptor and no reader can reorder them afterwards. That makes standard output human-facing under `--verbose`, which is what the flag is for; `--out` remains the way to get the config on its own. Simulating the runner — separate pipes merged by arrival — the JSON stayed contiguous across 23 runs of two different scripts.
Mrtenz
commented
Sep 24, 2026
| - name: Enable permission audit for debug runs | ||
| if: ${{ runner.debug == '1' && matrix.node-version == '26.x' }} | ||
| run: echo 'RUN=yarn audit --verbose' >> "$GITHUB_ENV" | ||
| - name: Test |
Unrecognised arguments are now treated as the audited script's and handed straight to it. With that, the changelog steps run through `RUN` like everything else, and every command in the workflow is audited on a debug run.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e53dec9. Configure here.
This branch has not been deployed
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
1 participant
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.
Now that scripts run under the Node.js permission model, every script needs a
lavamoat/scripts.*.jsongranting it the permissions it actually uses. Writing those by hand means guessing, then loosening the config each time the guess turns out to be too strict — and a guess that is too loose is invisible, because nothing fails.This adds
yarn audit <script>, which runs a script under--permission-audit, collects every permission it really exercises, and prints a config granting exactly those.Pass
--out lavamoat/scripts.test.jsonto write it to a file instead, and--verboseto see the audited script's own output along with an explanation of how each grant was chosen.How it works
A collector subscribes to the
node:permission-model:*diagnostics channels and appends every reported check to a log. SinceNODE_OPTIONSis inherited by every descendant, the collector is injected with--importas a percent-encodeddata:URL, which means it loads in child processes at any depth without needing a file on disk. Each process appends to one shared log; writes are synchronous and belowPIPE_BUF, so they interleave atomically without locking and survive a process that dies before itsexithandlers run.Raw paths are useless in a committed config, so they are generalised: anything inside the project becomes
./, known locations become$TMPDIR,$HOME, or a$GITHUB_*variable, and the upward directory walk that module and config resolution performs (package.json,node_modules,browserslist, and friends, probed all the way to the file system root) is recognised as such rather than baked in as this machine's directory layout. Path comparison is case-folded on macOS and Windows, because tools such as TypeScript probe with inconsistent casing and would otherwise leak absolute paths into the output.Any grant that still reaches outside the project is reported under
--verbose, alongside the path that caused it, so a widened config is never silent about why it was widened:Node.js 26.9.0
.nvmrcis pinned to 26.9.0 because audit mode is only usable from that release onwards. Earlier versions still enforcelstat,symlink, and addon loading despite documenting audit mode as non-blocking, so any script touching those dies part-way through the audit and the resulting config is silently incomplete. That was reported as nodejs/node#65419 and fixed in 26.9.0.Examples
Generated configs match the hand-written ones already in this repository. For
test:vitestthe script independently produces"--allow-fs-read": ["/"],"--allow-fs-write": ["./"],--allow-child-processand--allow-addonsenabled and everything else disabled, which islavamoat/scripts.test.jsonas committed today. Forlint:miscit produces the tighter"--allow-fs-read": ["./"]with no writes at all.The generated config for
test:vitestwas also verified under real enforcement (--permission, not audit mode): the suite passes with exactly those grants.See: nodejs/node#65419
Note
Low Risk
Dev-only tooling and lockfile/type bumps; normal CI still runs yarn directly unless workflow debug is enabled.
Overview
Adds
yarn audit <script>, which runs a package script under Node--permission-audit, records permission checks via diagnostics channels, generalises paths into portable LavaMoatnodeOptions, and prints or writes a JSON config (with--verbose/--out). The implementation lives in newscripts/audit.ts(collector injected through--import, resolution-probe handling, escape reporting)..nvmrcis pinned tov26.9.0so audit mode is reliable (per Node 26.9 fix).package.jsonwires the script under LavaMoat, bumps@types/nodeto 26, and addsyargs.CI
build-lint-test.ymlintroduces a sharedRUN=yarnenv var; when debug logging is on for Node 26.x,RUNbecomesyarn audit --verboseso build/lint/test steps exercise the auditor without duplicating steps.Reviewed by Cursor Bugbot for commit e53dec9. Bugbot is set up for automated code reviews on this repo. Configure here.