From 7b1e927bec64cbe7519c12dd4a0afc5b80a87335 Mon Sep 17 00:00:00 2001 From: rcooney-sh Date: Fri, 11 Sep 2026 20:46:15 +0000 Subject: [PATCH 1/4] post: run findmnt, df, and umount with argv arrays instead of shell strings Co-authored-by: Codesmith Staging --- src/__tests__/mount.test.ts | 81 ++++++++++++++++++++++++++++++++++ src/mount.ts | 88 +++++++++++++++++++++++++++++++++++++ src/post.ts | 63 ++++---------------------- 3 files changed, 178 insertions(+), 54 deletions(-) create mode 100644 src/__tests__/mount.test.ts create mode 100644 src/mount.ts diff --git a/src/__tests__/mount.test.ts b/src/__tests__/mount.test.ts new file mode 100644 index 0000000..ac4aa6c --- /dev/null +++ b/src/__tests__/mount.test.ts @@ -0,0 +1,81 @@ +import { tmpdir } from "os"; +import { + CommandRunner, + findMountedDevice, + getFilesystemUsedField, + parseMountSource, + unmount, +} from "../mount"; + +// A path a workflow could feed through the `path` input; every character is +// meaningful to a shell and must reach the child process as plain filename +// bytes. +const hostilePath = "/$(id>&2).*;`touch /tmp/pwned`|\"x'y"; + +function recordingRunner( + results: Record = {}, +): { run: CommandRunner; calls: { file: string; args: string[] }[] } { + const calls: { file: string; args: string[] }[] = []; + const run: CommandRunner = async (file, args) => { + calls.push({ file, args }); + const result = results[file]; + if (result instanceof Error) throw result; + return { stdout: result?.stdout ?? "", stderr: "" }; + }; + return { run, calls }; +} + +describe("mount helpers", () => { + it("passes the mount path to findmnt, df, and umount as one argv element", async () => { + const { run, calls } = recordingRunner({ + findmnt: { stdout: "/dev/vdb\n" }, + df: { stdout: "Used\n123456\n" }, + }); + + expect(await findMountedDevice(hostilePath, run)).toBe("/dev/vdb"); + expect(await getFilesystemUsedField(hostilePath, run)).toBe("123456"); + await unmount(hostilePath, run); + + expect(calls).toEqual([ + { + file: "findmnt", + args: ["-n", "-o", "SOURCE", "--mountpoint", hostilePath], + }, + { file: "df", args: ["-B1", "--output=used", "--", hostilePath] }, + { file: "sudo", args: ["umount", "--", hostilePath] }, + ]); + }); + + it("falls back to the mount listing and matches the target exactly", async () => { + const listing = [ + "/dev/vda1 on / type ext4 (rw,relatime)", + "/dev/vdb on /mnt/cache type ext4 (rw,relatime)", + "/dev/vdc on /mnt/cache2 type ext4 (rw,relatime)", + ].join("\n"); + + expect(parseMountSource(listing, "/mnt/cache")).toBe("/dev/vdb"); + expect(parseMountSource(listing, "/mnt/cache2")).toBe("/dev/vdc"); + expect(parseMountSource(listing, "/mnt/cach")).toBeNull(); + expect(parseMountSource(listing, "/mnt")).toBeNull(); + + const { run } = recordingRunner({ + findmnt: new Error("findmnt: not found"), + mount: { stdout: listing }, + }); + expect(await findMountedDevice("/mnt/cache", run)).toBe("/dev/vdb"); + expect(await findMountedDevice("/mnt/cache/nested", run)).toBeNull(); + }); + + const itOnLinux = process.platform === "linux" ? it : it.skip; + + itOnLinux("runs the real binaries without a shell", async () => { + // With a shell in the way this path would be a glob plus a substitution; + // through execFile it is just a filename df cannot find. + await expect(getFilesystemUsedField(hostilePath)).rejects.toThrow(); + expect(await findMountedDevice(hostilePath)).toBeNull(); + + const used = parseInt(await getFilesystemUsedField(tmpdir()), 10); + expect(used).toBeGreaterThan(0); + expect(await findMountedDevice("/")).not.toBeNull(); + }); +}); diff --git a/src/mount.ts b/src/mount.ts new file mode 100644 index 0000000..7a1c86a --- /dev/null +++ b/src/mount.ts @@ -0,0 +1,88 @@ +import { execFile } from "child_process"; +import { promisify } from "util"; + +const execFileAsync = promisify(execFile); + +/** + * Runs a binary with an argv array and no shell, so a mount path is only ever + * a filename to the child process. `path` is an action input and can carry + * `$(...)`, backticks, quotes, or `;` from a workflow's untrusted data. + */ +export type CommandRunner = ( + file: string, + args: string[], +) => Promise<{ stdout: string; stderr: string }>; + +const runCommand: CommandRunner = async (file, args) => { + const { stdout, stderr } = await execFileAsync(file, args); + return { stdout: String(stdout), stderr: String(stderr) }; +}; + +/** + * Source device of a `mount` listing line whose target is exactly + * `mountPoint`, e.g. `/dev/vdb on /mnt/cache type ext4 (rw)` → `/dev/vdb`. + */ +export function parseMountSource( + mountOutput: string, + mountPoint: string, +): string | null { + for (const line of mountOutput.split("\n")) { + const match = line.match(/^(\S+) on (.+) type \S+ /); + if (match && match[2] === mountPoint) { + return match[1]; + } + } + return null; +} + +/** Device mounted at `mountPoint`, or null when nothing is mounted there. */ +export async function findMountedDevice( + mountPoint: string, + run: CommandRunner = runCommand, +): Promise { + try { + const { stdout } = await run("findmnt", [ + "-n", + "-o", + "SOURCE", + "--mountpoint", + mountPoint, + ]); + const device = stdout.trim(); + if (device) { + return device; + } + } catch { + // findmnt exits non-zero when the path is not a mount point; fall through + // to the `mount` listing in case findmnt itself is unavailable. + } + + try { + const { stdout } = await run("mount", []); + return parseMountSource(stdout, mountPoint); + } catch { + return null; + } +} + +/** The `used` column of `df -B1` for `mountPoint`, as printed. */ +export async function getFilesystemUsedField( + mountPoint: string, + run: CommandRunner = runCommand, +): Promise { + const { stdout } = await run("df", [ + "-B1", + "--output=used", + "--", + mountPoint, + ]); + const lines = stdout.trim().split("\n"); + return lines[lines.length - 1].trim(); +} + +export async function unmount( + mountPoint: string, + run: CommandRunner = runCommand, +): Promise { + await run("sudo", ["umount", "--", mountPoint]); +} diff --git a/src/post.ts b/src/post.ts index 8d8b2cf..eb46731 100644 --- a/src/post.ts +++ b/src/post.ts @@ -6,6 +6,7 @@ import { createStickyDiskClient } from "./utils"; import { CommitIntent, commitIntentFromMode } from "./commit-intent"; import { evaluateOnChangeCommit, formatBytes } from "./on-change"; import { checkPreviousStepFailures } from "./step-checker"; +import { findMountedDevice, getFilesystemUsedField, unmount } from "./mount"; const execAsync = promisify(exec); @@ -101,30 +102,6 @@ async function cleanupStickyDiskWithoutCommit( } } -async function getDeviceFromMount(mountPoint: string): Promise { - try { - const { stdout } = await execAsync(`findmnt -n -o SOURCE "${mountPoint}"`); - const device = stdout.trim(); - if (device) { - return device; - } - } catch { - core.info(`findmnt failed for ${mountPoint}, trying mount command`); - } - - try { - const { stdout } = await execAsync(`mount | grep " ${mountPoint} "`); - const match = stdout.match(/^(\/dev\/\S+)/); - if (match) { - return match[1]; - } - } catch { - core.info(`mount grep failed for ${mountPoint}`); - } - - return null; -} - const FLUSH_TIMEOUT_SECS = 10; const TIMEOUT_EXIT_CODE = 124; @@ -219,26 +196,12 @@ async function run(): Promise { try { // Check if path is mounted and get the device name for later flush - let devicePath: string | null = null; - try { - const { stdout: mountOutput } = await execAsync( - `mount | grep "${stickyDiskPath}"`, - ); - if (!mountOutput) { - logNotMounted(); - return; - } - devicePath = await getDeviceFromMount(stickyDiskPath); - if (devicePath) { - core.info( - `Found device ${devicePath} for mount point ${stickyDiskPath}`, - ); - } - } catch { - // grep returns non-zero if no match found + const devicePath = await findMountedDevice(stickyDiskPath); + if (!devicePath) { logNotMounted(); return; } + core.info(`Found device ${devicePath} for mount point ${stickyDiskPath}`); // Ensure all pending writes are flushed to disk before collecting usage. await execAsync("sync"); @@ -246,14 +209,12 @@ async function run(): Promise { // Get filesystem usage BEFORE unmounting (critical timing) let fsDiskUsageBytes: number | null = null; try { - const { stdout } = await execAsync( - `df -B1 --output=used "${stickyDiskPath}" | tail -n1`, - ); - const parsedValue = parseInt(stdout.trim(), 10); + const usedField = await getFilesystemUsedField(stickyDiskPath); + const parsedValue = parseInt(usedField, 10); if (isNaN(parsedValue) || parsedValue <= 0) { core.warning( - `Invalid filesystem usage value from df: "${stdout.trim()}". Will not report fs usage.`, + `Invalid filesystem usage value from df: "${usedField}". Will not report fs usage.`, ); } else { fsDiskUsageBytes = parsedValue; @@ -273,7 +234,7 @@ async function run(): Promise { // Unmount with retries. for (let attempt = 1; attempt <= 10; attempt++) { try { - await execAsync(`sudo umount "${stickyDiskPath}"`); + await unmount(stickyDiskPath); core.info(`Successfully unmounted ${stickyDiskPath}`); break; } catch (error) { @@ -287,13 +248,7 @@ async function run(): Promise { // Flush block device buffers after unmount to ensure data durability // before the Ceph RBD snapshot is taken. The device is still mapped even though unmounted. - if (devicePath) { - await flushBlockDevice(devicePath); - } else { - core.info( - "Skipping durability flush: device path not found for mount point", - ); - } + await flushBlockDevice(devicePath); // Determine whether to commit based on commit mode if (commitIntent === CommitIntent.NEVER) { From edfe12e6100710a63a6ef5f64d529868fa26d422 Mon Sep 17 00:00:00 2001 From: rcooney-sh Date: Fri, 11 Sep 2026 20:56:15 +0000 Subject: [PATCH 2/4] ci: build with Node 24 so ncc's module concatenation does not hit the 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 --- .nvmrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nvmrc b/.nvmrc index 0f576a1..166aae1 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -v23.2.0 +v24.21.0 From a12293de81091bc8f84cd6380888dcfcfe2b7ef4 Mon Sep 17 00:00:00 2001 From: rcooney-sh Date: Fri, 11 Sep 2026 21:00:56 +0000 Subject: [PATCH 3/4] ci: upload the built dist as an artifact when the drift check fails 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 --- .github/workflows/build.yaml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 1b3da79..53d7ea4 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -43,11 +43,23 @@ jobs: run: npm run lint - name: Build Action + id: build run: | npm run build # Check if any files were changed during build if ! git diff --quiet; then - echo "Error: Build generated changes that aren't committed. Please run 'npm run build' locally and commit the changes." + echo "Error: Build generated changes that aren't committed. Please run 'npm run build' locally and commit the changes," + echo "or download the 'dist' artifact from this run (gh run download ${{ github.run_id }} -n dist -D dist) and commit it." git diff exit 1 fi + + # Building needs BUF_TOKEN for the private @buf packages, which contributors + # without registry access (and automated fixers) do not have. Publish the + # bundle CI just built so it can be committed without a local build. + - name: Upload built dist + if: failure() && steps.build.outcome == 'failure' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: dist + path: dist/ From 01bf42c71295a7f0152897558f1e700dc48532f6 Mon Sep 17 00:00:00 2001 From: rcooney-sh Date: Fri, 11 Sep 2026 21:02:16 +0000 Subject: [PATCH 4/4] dist: rebuild post bundle with the argv-based mount helpers Co-authored-by: Codesmith Staging --- dist/post/index.js | 119 +++++++++++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/dist/post/index.js b/dist/post/index.js index ca57971..8a7b31f 100644 --- a/dist/post/index.js +++ b/dist/post/index.js @@ -37024,6 +37024,69 @@ async function hasAnyStepFailed(runnerBasePath) { return result.hasFailures; } +;// CONCATENATED MODULE: ./src/mount.ts + + +const execFileAsync = (0,external_util_.promisify)(external_child_process_.execFile); +const runCommand = async (file, args) => { + const { stdout, stderr } = await execFileAsync(file, args); + return { stdout: String(stdout), stderr: String(stderr) }; +}; +/** + * Source device of a `mount` listing line whose target is exactly + * `mountPoint`, e.g. `/dev/vdb on /mnt/cache type ext4 (rw)` → `/dev/vdb`. + */ +function parseMountSource(mountOutput, mountPoint) { + for (const line of mountOutput.split("\n")) { + const match = line.match(/^(\S+) on (.+) type \S+ /); + if (match && match[2] === mountPoint) { + return match[1]; + } + } + return null; +} +/** Device mounted at `mountPoint`, or null when nothing is mounted there. */ +async function findMountedDevice(mountPoint, run = runCommand) { + try { + const { stdout } = await run("findmnt", [ + "-n", + "-o", + "SOURCE", + "--mountpoint", + mountPoint, + ]); + const device = stdout.trim(); + if (device) { + return device; + } + } + catch { + // findmnt exits non-zero when the path is not a mount point; fall through + // to the `mount` listing in case findmnt itself is unavailable. + } + try { + const { stdout } = await run("mount", []); + return parseMountSource(stdout, mountPoint); + } + catch { + return null; + } +} +/** The `used` column of `df -B1` for `mountPoint`, as printed. */ +async function getFilesystemUsedField(mountPoint, run = runCommand) { + const { stdout } = await run("df", [ + "-B1", + "--output=used", + "--", + mountPoint, + ]); + const lines = stdout.trim().split("\n"); + return lines[lines.length - 1].trim(); +} +async function unmount(mountPoint, run = runCommand) { + await run("sudo", ["umount", "--", mountPoint]); +} + ;// CONCATENATED MODULE: ./src/post.ts @@ -37033,6 +37096,7 @@ async function hasAnyStepFailed(runnerBasePath) { + const execAsync = (0,external_util_.promisify)(external_child_process_.exec); async function commitStickydisk(exposeId, stickyDiskKey, fsDiskUsageBytes) { core.info(`Requesting commit of sticky disk ${stickyDiskKey} with expose ID ${exposeId}`); @@ -37095,29 +37159,6 @@ async function cleanupStickyDiskWithoutCommit(exposeId, stickyDiskKey, reason) { // We don't want to fail the build if this fails so we swallow the error. } } -async function getDeviceFromMount(mountPoint) { - try { - const { stdout } = await execAsync(`findmnt -n -o SOURCE "${mountPoint}"`); - const device = stdout.trim(); - if (device) { - return device; - } - } - catch { - core.info(`findmnt failed for ${mountPoint}, trying mount command`); - } - try { - const { stdout } = await execAsync(`mount | grep " ${mountPoint} "`); - const match = stdout.match(/^(\/dev\/\S+)/); - if (match) { - return match[1]; - } - } - catch { - core.info(`mount grep failed for ${mountPoint}`); - } - return null; -} const FLUSH_TIMEOUT_SECS = 10; const TIMEOUT_EXIT_CODE = 124; async function flushBlockDevice(devicePath) { @@ -37189,32 +37230,21 @@ async function run() { }; try { // Check if path is mounted and get the device name for later flush - let devicePath = null; - try { - const { stdout: mountOutput } = await execAsync(`mount | grep "${stickyDiskPath}"`); - if (!mountOutput) { - logNotMounted(); - return; - } - devicePath = await getDeviceFromMount(stickyDiskPath); - if (devicePath) { - core.info(`Found device ${devicePath} for mount point ${stickyDiskPath}`); - } - } - catch { - // grep returns non-zero if no match found + const devicePath = await findMountedDevice(stickyDiskPath); + if (!devicePath) { logNotMounted(); return; } + core.info(`Found device ${devicePath} for mount point ${stickyDiskPath}`); // Ensure all pending writes are flushed to disk before collecting usage. await execAsync("sync"); // Get filesystem usage BEFORE unmounting (critical timing) let fsDiskUsageBytes = null; try { - const { stdout } = await execAsync(`df -B1 --output=used "${stickyDiskPath}" | tail -n1`); - const parsedValue = parseInt(stdout.trim(), 10); + const usedField = await getFilesystemUsedField(stickyDiskPath); + const parsedValue = parseInt(usedField, 10); if (isNaN(parsedValue) || parsedValue <= 0) { - core.warning(`Invalid filesystem usage value from df: "${stdout.trim()}". Will not report fs usage.`); + core.warning(`Invalid filesystem usage value from df: "${usedField}". Will not report fs usage.`); } else { fsDiskUsageBytes = parsedValue; @@ -37231,7 +37261,7 @@ async function run() { // Unmount with retries. for (let attempt = 1; attempt <= 10; attempt++) { try { - await execAsync(`sudo umount "${stickyDiskPath}"`); + await unmount(stickyDiskPath); core.info(`Successfully unmounted ${stickyDiskPath}`); break; } @@ -37245,12 +37275,7 @@ async function run() { } // Flush block device buffers after unmount to ensure data durability // before the Ceph RBD snapshot is taken. The device is still mapped even though unmounted. - if (devicePath) { - await flushBlockDevice(devicePath); - } - else { - core.info("Skipping durability flush: device path not found for mount point"); - } + await flushBlockDevice(devicePath); // Determine whether to commit based on commit mode if (commitIntent === stickydisk_pb_CommitIntent.NEVER) { await cleanupStickyDiskWithoutCommit(exposeId, stickyDiskKey, "commit mode is 'false' (read-only consumer)");