Skip to content

post: run findmnt, df, and umount with argv arrays instead of shell strings - #79

Merged
rcooney-sh merged 4 commits into
mainfrom
post/argv-mount-commands
Sep 11, 2026
Merged

rcooney-sh merged 4 commits into
mainfrom
post/argv-mount-commands

Conversation

@rcooney-sh

@rcooney-sh rcooney-sh commented Sep 11, 2026

Copy link
Copy Markdown

Security patch for vulnagent problem prb_ef0452cf3a0d8e30 (Medium, remediation group stickydisk-shell-quoting): command injection in the post-step cleanup via the path input. Aikido finding 654097526, Linear SEC-747.

The main step already quotes the path (shellQuote), but src/post.ts reads it back from state and interpolated it into five exec shell strings inside double quotes, where $(...) and backticks still expand. The pentest used path: /$(id>&2).* and got id output in the post-step log from sudo umount "...".

The fix removes the shell from every command that takes the path. src/mount.ts runs findmnt, df, and umount through execFile with argv arrays, and post.ts calls those helpers:

run("findmnt", ["-n", "-o", "SOURCE", "--mountpoint", mountPoint])
run("df", ["-B1", "--output=used", "--", mountPoint])
run("sudo", ["umount", "--", mountPoint])

The two mount | grep pipelines are gone: the mounted check and the device lookup collapse into findMountedDevice, which asks findmnt --mountpoint and falls back to parsing the mount listing in-process with an exact target match. This is also stricter than before: mount | grep "/mnt/cache" treated /mnt/cache2 as a hit and would attempt an unmount of a path that was never mounted. If nothing is mounted at the path the post step logs and returns as it did. Commands whose arguments come from the system (sync, drop_caches, blockdev --flushbufs on the device findmnt reported) are unchanged.

No path validation or charset restriction was added: with argv execution the input is only ever a filename, and rejecting characters would break legitimate paths (spaces, unicode) for no security gain.

Tests: src/__tests__/mount.test.ts checks a hostile path (/$(id>&2).*;\touch /tmp/pwned`|"x'y) reaches findmnt, df, and umountas one argv element, checks themountlisting parser matches the target exactly, and on Linux runs the real binaries:dffails on the hostile path instead of expanding it, and succeeds onos.tmpdir()`.

Draft because dist/ is not rebuilt. The build needs the private @buf registry (BUF_TOKEN), which this environment does not have, so the Build workflow will fail until someone runs npm ci && npm run build on this branch and commits dist/index.js and dist/post/index.js. Only dist/post/index.js should change materially; dist/index.js is untouched by the source change.


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled. (Staging)


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.

rcooney-sh and others added 2 commits September 11, 2026 20:46
…trings

Co-authored-by: Codesmith Staging <codesmith-bot@users.noreply.github.com>
… 23.2.0 JSON.parse regression

Node 23.2.0 ships a V8 regression (nodejs/node#55826) that makes webpack's
ConcatenationScope.matchModuleReference throw "Unexpected end of JSON
input" for some concatenated module graphs. Adding src/mount.ts to the
post bundle produced such a graph, so `ncc build src/post.ts` failed in
CI while the main bundle still built. Node 24 also matches the
`node24` runtime declared in action.yml.

Co-authored-by: Codesmith Staging <codesmith-bot@users.noreply.github.com>
@rcooney-sh
rcooney-sh marked this pull request as ready for review September 11, 2026 20:56
rcooney-sh and others added 2 commits September 11, 2026 21:00
The Build job rebuilds dist/ and fails if it differs from what is committed,
but reproducing that build locally needs BUF_TOKEN for the private @buf
packages, which most contributors and automated fixers do not have. Upload
the bundle CI just built so it can be downloaded and committed directly.

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith Staging <codesmith-bot@users.noreply.github.com>
@rcooney-sh
rcooney-sh merged commit 94697d4 into main Sep 11, 2026
17 checks passed
@rcooney-sh
rcooney-sh deleted the post/argv-mount-commands branch September 11, 2026 22:36
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