From c9c9b2d46ae09f93ed3a6afee2c6099a9d3a9126 Mon Sep 17 00:00:00 2001 From: josephbajor Date: Thu, 10 Sep 2026 19:17:30 -0400 Subject: [PATCH] fix(interpreter): restore cwd after pipeline subshell stages --- .changeset/tidy-pipelines-return.md | 5 ++ .../pipeline-cwd.comparison.fixtures.json | 72 +++++++++++++++ .../pipeline-cwd.comparison.test.ts | 39 +++++++++ .../src/interpreter/pipeline-cwd.test.ts | 87 +++++++++++++++++++ .../src/interpreter/pipeline-execution.ts | 22 ++--- 5 files changed, 212 insertions(+), 13 deletions(-) create mode 100644 .changeset/tidy-pipelines-return.md create mode 100644 packages/just-bash/src/comparison-tests/fixtures/pipeline-cwd.comparison.fixtures.json create mode 100644 packages/just-bash/src/comparison-tests/pipeline-cwd.comparison.test.ts create mode 100644 packages/just-bash/src/interpreter/pipeline-cwd.test.ts diff --git a/.changeset/tidy-pipelines-return.md b/.changeset/tidy-pipelines-return.md new file mode 100644 index 000000000..d2541001f --- /dev/null +++ b/.changeset/tidy-pipelines-return.md @@ -0,0 +1,5 @@ +--- +"just-bash": patch +--- + +Restore the working directory as well as PWD after pipeline stages that run in subshell contexts, so cd cannot redirect later stages or subsequent relative file operations. Preserve current-shell behavior for single commands and the final stage with lastpipe enabled. diff --git a/packages/just-bash/src/comparison-tests/fixtures/pipeline-cwd.comparison.fixtures.json b/packages/just-bash/src/comparison-tests/fixtures/pipeline-cwd.comparison.fixtures.json new file mode 100644 index 000000000..3372c1fc5 --- /dev/null +++ b/packages/just-bash/src/comparison-tests/fixtures/pipeline-cwd.comparison.fixtures.json @@ -0,0 +1,72 @@ +{ + "0056df1a76b37de5": { + "command": "cd child; if [ \"$PWD\" = \"$(pwd)\" ]; then echo synced; else echo desynced; fi; cat marker.txt", + "files": { + "marker.txt": "parent\n", + "child/marker.txt": "child\n" + }, + "stdout": "synced\nchild\n", + "stderr": "", + "exitCode": 0 + }, + "6852d30f05a0baa7": { + "command": "true | cd child; if [ \"$PWD\" = \"$(pwd)\" ]; then echo synced; else echo desynced; fi; cat marker.txt", + "files": { + "marker.txt": "parent\n", + "child/marker.txt": "child\n" + }, + "stdout": "synced\nparent\n", + "stderr": "", + "exitCode": 0 + }, + "78474a21f718cd09": { + "command": "{ cd child; exit 7; } | cat; if [ \"$PWD\" = \"$(pwd)\" ]; then echo synced; else echo desynced; fi; cat marker.txt", + "files": { + "marker.txt": "parent\n", + "child/marker.txt": "child\n" + }, + "stdout": "synced\nparent\n", + "stderr": "", + "exitCode": 0 + }, + "815399436f3396df": { + "command": "{ cd child; }; if [ \"$PWD\" = \"$(pwd)\" ]; then echo synced; else echo desynced; fi; cat marker.txt", + "files": { + "marker.txt": "parent\n", + "child/marker.txt": "child\n" + }, + "stdout": "synced\nchild\n", + "stderr": "", + "exitCode": 0 + }, + "ade8f36ca956a439": { + "command": "cd child | cat; if [ \"$PWD\" = \"$(pwd)\" ]; then echo synced; else echo desynced; fi; cat marker.txt", + "files": { + "marker.txt": "parent\n", + "child/marker.txt": "child\n" + }, + "stdout": "synced\nparent\n", + "stderr": "", + "exitCode": 0 + }, + "af3447f9c5fc038f": { + "command": "cd child | cat marker.txt; if [ \"$PWD\" = \"$(pwd)\" ]; then echo synced; else echo desynced; fi; cat marker.txt", + "files": { + "marker.txt": "parent\n", + "child/marker.txt": "child\n" + }, + "stdout": "parent\nsynced\nparent\n", + "stderr": "", + "exitCode": 0 + }, + "f797979d6579e416": { + "command": "true | cd child | cat; if [ \"$PWD\" = \"$(pwd)\" ]; then echo synced; else echo desynced; fi; cat marker.txt", + "files": { + "marker.txt": "parent\n", + "child/marker.txt": "child\n" + }, + "stdout": "synced\nparent\n", + "stderr": "", + "exitCode": 0 + } +} diff --git a/packages/just-bash/src/comparison-tests/pipeline-cwd.comparison.test.ts b/packages/just-bash/src/comparison-tests/pipeline-cwd.comparison.test.ts new file mode 100644 index 000000000..33d0fd96b --- /dev/null +++ b/packages/just-bash/src/comparison-tests/pipeline-cwd.comparison.test.ts @@ -0,0 +1,39 @@ +import { afterEach, beforeEach, describe, it } from "vitest"; +import { + cleanupTestDir, + compareOutputs, + createTestDir, + setupFiles, +} from "./fixture-runner.js"; + +describe("Pipeline cwd - Real Bash Comparison", () => { + let testDir: string; + + beforeEach(async () => { + testDir = await createTestDir(); + }); + + afterEach(async () => { + await cleanupTestDir(testDir); + }); + + it.each([ + "cd child | cat", + "true | cd child", + "true | cd child | cat", + "{ cd child; exit 7; } | cat", + "cd child | cat marker.txt", + "cd child", + "{ cd child; }", + ])("preserves bash cwd semantics: %s", async (command) => { + const bash = await setupFiles(testDir, { + "marker.txt": "parent\n", + "child/marker.txt": "child\n", + }); + await compareOutputs( + bash, + testDir, + `${command}; if [ "$PWD" = "$(pwd)" ]; then echo synced; else echo desynced; fi; cat marker.txt`, + ); + }); +}); diff --git a/packages/just-bash/src/interpreter/pipeline-cwd.test.ts b/packages/just-bash/src/interpreter/pipeline-cwd.test.ts new file mode 100644 index 000000000..dded0b382 --- /dev/null +++ b/packages/just-bash/src/interpreter/pipeline-cwd.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from "vitest"; +import { Bash } from "../Bash.js"; +import { parse } from "../parser/parser.js"; +import { executePipeline } from "./pipeline-execution.js"; +import type { InterpreterContext } from "./types.js"; + +function createBash() { + return new Bash({ + cwd: "/work", + files: { + "/work/marker.txt": "parent\n", + "/work/child/marker.txt": "child\n", + }, + }); +} + +describe("pipeline working directory isolation", () => { + it.each([ + "cd child | cat", + "echo input | cd child", + "echo input | cd child | cat", + "{ cd child; } | cat", + "{ cd child; exit 7; } | cat", + "{ cd child; echo inner | cat; } | cat", + "shopt -s lastpipe; cd child | cat", + ])("restores cwd and PWD after %s", async (pipeline) => { + const bash = createBash(); + const result = await bash.exec( + `${pipeline}; printf 'PWD=%s\n' "$PWD"; pwd; cat marker.txt; echo saved > result.txt`, + ); + expect(result).toMatchObject({ + stdout: `${pipeline.includes("echo inner") ? "inner\n" : ""}PWD=/work\n/work\nparent\n`, + stderr: "", + exitCode: 0, + }); + expect(await bash.fs.readFile("/work/result.txt")).toBe("saved\n"); + expect(await bash.fs.exists("/work/child/result.txt")).toBe(false); + }); + + it("starts later pipeline stages in the parent's cwd", async () => { + const result = await createBash().exec("cd child | cat marker.txt"); + expect(result).toMatchObject({ + stdout: "parent\n", + stderr: "", + exitCode: 0, + }); + }); + + it.each([ + "cd child", + "{ cd child; }", + "shopt -s lastpipe; true | cd child", + ])("retains cwd changes in the current shell: %s", async (command) => { + const result = await createBash().exec( + `${command}; printf 'PWD=%s\n' "$PWD"; pwd; cat marker.txt`, + ); + expect(result).toMatchObject({ + stdout: "PWD=/work/child\n/work/child\nchild\n", + stderr: "", + exitCode: 0, + }); + }); + + it("restores cwd when a pipeline stage throws an unhandled error", async () => { + const ctx = { + state: { + cwd: "/work", + env: new Map([["PWD", "/work"]]), + arrays: new Map(), + shoptOptions: { lastpipe: false }, + }, + executionScope: { outputBytesUsed: 0, chargeCommand: () => 1 }, + } as unknown as InterpreterContext; + const pipeline = parse("cd child | cat").statements[0].pipelines[0]; + const failure = new Error("stage failed"); + + await expect( + executePipeline(ctx, pipeline, async () => { + ctx.state.cwd = "/work/child"; + ctx.state.env.set("PWD", "/work/child"); + throw failure; + }), + ).rejects.toBe(failure); + expect(ctx.state.cwd).toBe("/work"); + expect(ctx.state.env.get("PWD")).toBe("/work"); + }); +}); diff --git a/packages/just-bash/src/interpreter/pipeline-execution.ts b/packages/just-bash/src/interpreter/pipeline-execution.ts index 8c6ff555d..34de1a970 100644 --- a/packages/just-bash/src/interpreter/pipeline-execution.ts +++ b/packages/just-bash/src/interpreter/pipeline-execution.ts @@ -85,10 +85,11 @@ export async function executePipeline( const runsInSubshell = isMultiCommandPipeline && (!isLast || !ctx.state.shoptOptions.lastpipe); - // Save environment for commands running in subshell context - // This prevents variable assignments (e.g., ${cmd=echo}) from leaking to parent + // Save environment and cwd for commands running in subshell context. + // Restoring PWD alone does not restore relative path resolution after cd. const savedEnv = runsInSubshell ? new Map(ctx.state.env) : null; const savedArrays = runsInSubshell ? cloneArrays(ctx.state.arrays) : null; + const savedCwd = ctx.state.cwd; let result: ExecResult; const outputCheckpoint = ctx.executionScope.outputBytesUsed; @@ -122,11 +123,6 @@ export async function executePipeline( exitCode: error.exitCode, }; } else { - // Restore environment before re-throwing - if (savedEnv) { - ctx.state.env = savedEnv; - ctx.state.arrays = savedArrays ?? new Map(); - } throw error; } } finally { @@ -143,12 +139,12 @@ export async function executePipeline( ctx.state.groupStdin = sharedStdin; ctx.state.groupStdinSourceFd = sharedStdinSourceFd; } - } - - // Restore environment for subshell commands to prevent variable assignment leakage - if (savedEnv) { - ctx.state.env = savedEnv; - ctx.state.arrays = savedArrays ?? new Map(); + // Restore on normal completion and on errors before the next stage runs. + if (savedEnv) { + ctx.state.env = savedEnv; + ctx.state.arrays = savedArrays ?? new Map(); + ctx.state.cwd = savedCwd; + } } // Charge every stage before it can become a retained pipeline