From 2383071c7e0b37e81d8b650b6fe45abd8a41963e Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 14:38:14 +0200 Subject: [PATCH 1/8] feat(pwd): add pwd builtin with -L/-P symlink resolution Adds the POSIX pwd builtin to print the absolute pathname of the current working directory. Supports -L (logical, default) and -P (physical), last-wins semantics when both are given, and --help. Symlink resolution for -P is sandbox-safe: walks the path through callCtx.LstatFile and ReadlinkFile, caps expansions at the Linux ELOOP limit (40 hops), and treats components above AllowedPaths as opaque pass-through so -P stays useful when the cwd lives at the sandbox root. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/fuzz.yml | 5 + SHELL_FEATURES.md | 1 + analysis/symbols_builtins.go | 18 + builtins/pwd/builtin_pwd_pentest_test.go | 324 ++++++++++++++++++ builtins/pwd/pwd.go | 289 ++++++++++++++++ builtins/pwd/pwd_fuzz_test.go | 234 +++++++++++++ builtins/pwd/pwd_internal_test.go | 285 +++++++++++++++ builtins/pwd/pwd_test.go | 244 +++++++++++++ builtins/pwd/pwd_unix_test.go | 162 +++++++++ .../fuzz/FuzzPwdArgs/abb4964a7f34a77c | 2 + .../fuzz/FuzzPwdArgs/db7ae63e0377da75 | 2 + .../fuzz/FuzzPwdArgs/def578230616f8b9 | 2 + interp/register_builtins.go | 2 + tests/scenarios/cmd/help/restricted.yaml | 4 +- .../cmd/help/restricted_all_flag.yaml | 3 +- tests/scenarios/cmd/help/unrestricted.yaml | 3 +- .../cmd/help/unrestricted_all_flag.yaml | 3 +- tests/scenarios/cmd/pwd/basic/exit_code.yaml | 10 + .../cmd/pwd/basic/extra_args_ignored.yaml | 10 + tests/scenarios/cmd/pwd/basic/no_args.yaml | 9 + .../cmd/pwd/errors/unknown_flag_long.yaml | 9 + .../cmd/pwd/errors/unknown_flag_short.yaml | 9 + .../cmd/pwd/errors/version_rejected.yaml | 9 + .../cmd/pwd/flags/last_wins_l_then_p.yaml | 9 + .../cmd/pwd/flags/last_wins_p_then_l.yaml | 9 + .../scenarios/cmd/pwd/flags/logical_long.yaml | 10 + .../cmd/pwd/flags/logical_short.yaml | 9 + .../cmd/pwd/flags/physical_long.yaml | 10 + .../cmd/pwd/flags/physical_short.yaml | 9 + .../cmd/pwd/hardening/double_dash.yaml | 10 + .../cmd/pwd/hardening/in_subshell.yaml | 11 + .../cmd/pwd/hardening/stdin_ignored.yaml | 9 + .../cmd/pwd/help/help_flag_long.yaml | 9 + .../cmd/pwd/help/help_flag_short.yaml | 9 + 34 files changed, 1738 insertions(+), 5 deletions(-) create mode 100644 builtins/pwd/builtin_pwd_pentest_test.go create mode 100644 builtins/pwd/pwd.go create mode 100644 builtins/pwd/pwd_fuzz_test.go create mode 100644 builtins/pwd/pwd_internal_test.go create mode 100644 builtins/pwd/pwd_test.go create mode 100644 builtins/pwd/pwd_unix_test.go create mode 100644 builtins/pwd/testdata/fuzz/FuzzPwdArgs/abb4964a7f34a77c create mode 100644 builtins/pwd/testdata/fuzz/FuzzPwdArgs/db7ae63e0377da75 create mode 100644 builtins/pwd/testdata/fuzz/FuzzPwdArgs/def578230616f8b9 create mode 100644 tests/scenarios/cmd/pwd/basic/exit_code.yaml create mode 100644 tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml create mode 100644 tests/scenarios/cmd/pwd/basic/no_args.yaml create mode 100644 tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml create mode 100644 tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml create mode 100644 tests/scenarios/cmd/pwd/errors/version_rejected.yaml create mode 100644 tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml create mode 100644 tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml create mode 100644 tests/scenarios/cmd/pwd/flags/logical_long.yaml create mode 100644 tests/scenarios/cmd/pwd/flags/logical_short.yaml create mode 100644 tests/scenarios/cmd/pwd/flags/physical_long.yaml create mode 100644 tests/scenarios/cmd/pwd/flags/physical_short.yaml create mode 100644 tests/scenarios/cmd/pwd/hardening/double_dash.yaml create mode 100644 tests/scenarios/cmd/pwd/hardening/in_subshell.yaml create mode 100644 tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml create mode 100644 tests/scenarios/cmd/pwd/help/help_flag_long.yaml create mode 100644 tests/scenarios/cmd/pwd/help/help_flag_short.yaml diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index a5f7f6f3..5209162a 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -64,6 +64,11 @@ jobs: # ping_test.go. Go test helpers are only in scope within the same directory, # so both files must reside in builtins/ping/. corpus_path: builtins/ping + - pkg: ./builtins/pwd/ + name: pwd + # pwd fuzz tests live in builtins/pwd/ alongside the pwd_test.go + # helpers (pwdRun) — same rationale as ping above. + corpus_path: builtins/pwd - pkg: ./interp/tests/ name: interp corpus_path: interp/tests diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index 7c7a7aea..9e2d3b82 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -25,6 +25,7 @@ Blocked features are rejected before execution with exit code 2. - ✅ `ping [-c N] [-W DURATION] [-i DURATION] [-q] [-4|-6] [-h] HOST` — send ICMP echo requests to a network host and report round-trip statistics; `-f` (flood), `-b` (broadcast), `-s` (packet size), `-I` (interface), `-p` (pattern), and `-R` (record route) are blocked; count/wait/interval are clamped to safe ranges with a warning; multicast, unspecified (`0.0.0.0`/`::`), and broadcast addresses (IPv4 last-octet `.255`) are rejected — note: directed broadcasts on non-standard subnets (e.g. `.127` on a `/25`) are not blocked without subnet-mask knowledge - ✅ `ps [-e|-A] [-f] [-p PIDLIST]` — report process status; default shows current-session processes; `-e`/`-A` shows all; `-f` adds UID/PPID/STIME columns; `-p` selects by PID list - ✅ `printf FORMAT [ARGUMENT]...` — format and print data to stdout; supports `%s`, `%b`, `%c`, `%d`, `%i`, `%o`, `%u`, `%x`, `%X`, `%e`, `%E`, `%f`, `%F`, `%g`, `%G`, `%%`; format reuse for excess arguments; `%n` rejected (security risk); `-v` rejected +- ✅ `pwd [-LP]` — print the absolute pathname of the current working directory; `-L` (default) prints the shell's tracked logical path, `-P` resolves all symlinks; `-P` is best-effort within the sandbox (path components above `AllowedPaths` pass through unresolved); `--version` rejected - ✅ `sed [-n] [-e SCRIPT] [-E|-r] [SCRIPT] [FILE]...` — stream editor for filtering and transforming text; uses RE2 regex engine; `-i`/`-f` rejected; `e`/`w`/`W`/`r`/`R` commands blocked - ✅ `strings [-a] [-n MIN] [-t o|d|x] [-o] [-f] [-s SEP] [FILE]...` — print printable character sequences in files (default min length 4); offsets via `-t`/`-o`; filename prefix via `-f`; custom separator via `-s` - ✅ `tail [-n N|-c N] [-q|-v] [-z] [FILE]...` — output the last part of files (default: last 10 lines); supports `+N` offset mode; `-f`/`--follow` is rejected diff --git a/analysis/symbols_builtins.go b/analysis/symbols_builtins.go index 31722bac..8c893bff 100644 --- a/analysis/symbols_builtins.go +++ b/analysis/symbols_builtins.go @@ -199,6 +199,20 @@ var builtinPerCommandSymbols = map[string][]string{ "strings.ReplaceAll", // 🟢 replaces all occurrences of a substring; pure function, no I/O. "strings.ToLower", // 🟢 converts string to lowercase; pure function, no I/O. }, + "pwd": { + "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "errors.Is", // 🟢 error comparison; pure function, no I/O. + "errors.New", // 🟢 creates a simple error value; pure function, no I/O. + "fmt.Errorf", // 🟢 error formatting; pure function, no I/O. + "io/fs.ModeSymlink", // 🟢 file mode bit constant for symlinks; pure constant. + "path/filepath.Clean", // 🟢 normalizes a path lexically (collapses ".", "..", duplicate separators); pure function, no I/O. + "path/filepath.Dir", // 🟢 returns the directory component of a path; pure function, no I/O. + "path/filepath.IsAbs", // 🟢 reports whether a path is absolute; pure function, no I/O. + "path/filepath.Separator", // 🟢 OS path separator constant ('/' or '\\'); pure constant, no I/O. + "path/filepath.VolumeName", // 🟢 returns the volume prefix of a path (e.g. "C:" on Windows, "" on Unix); pure function, no I/O. + "strings.IndexByte", // 🟢 finds byte in string; pure function, no I/O. + "strings.TrimPrefix", // 🟢 removes a leading prefix from a string; pure function, no I/O. + }, "sort": { "bufio.NewScanner", // 🟢 line-by-line input reading (e.g. head, cat); no write or exec capability. "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. @@ -472,9 +486,12 @@ var builtinAllowedSymbols = []string{ "os.IsNotExist", // 🟢 checks if error is "not exist"; pure function, no I/O. "os.O_RDONLY", // 🟢 read-only file flag constant; cannot open files by itself. "os.PathError", // 🟢 error type for filesystem path errors; pure type, no I/O. + "path/filepath.Clean", // 🟢 normalizes a path lexically (collapses ".", "..", duplicate separators); pure function, no I/O. "path/filepath.Dir", // 🟢 returns the directory component of a path; pure function, no I/O. "path/filepath.IsAbs", // 🟢 reports whether a path is absolute; pure function, no I/O. + "path/filepath.Separator", // 🟢 OS path separator constant ('/' or '\\'); pure constant, no I/O. "path/filepath.ToSlash", // 🟢 converts OS path separators to forward slashes; pure function, no I/O. + "path/filepath.VolumeName", // 🟢 returns the volume prefix of a path (e.g. "C:" on Windows, "" on Unix); pure function, no I/O. "regexp.Compile", // 🟢 compiles a regular expression; pure function, no I/O. Uses RE2 engine (linear-time, no backtracking). "regexp.QuoteMeta", // 🟢 escapes all special regex characters in a string; pure function, no I/O. "regexp.Regexp", // 🟢 compiled regular expression type; no I/O side effects. All matching methods are linear-time (RE2). @@ -503,6 +520,7 @@ var builtinAllowedSymbols = []string{ "strings.ReplaceAll", // 🟢 replaces all occurrences of a substring; pure function, no I/O. "strings.Split", // 🟢 splits a string by separator into a slice; pure function, no I/O. "strings.ToLower", // 🟢 converts string to lowercase; pure function, no I/O. + "strings.TrimPrefix", // 🟢 removes a leading prefix from a string; pure function, no I/O. "strings.TrimSpace", // 🟢 removes leading/trailing whitespace; pure function. "syscall.ByHandleFileInformation", // 🟢 Windows file info struct for extracting nlink; read-only type, no I/O. "syscall.EACCES", // 🟢 POSIX errno constant for permission denied; pure constant, no I/O. diff --git a/builtins/pwd/builtin_pwd_pentest_test.go b/builtins/pwd/builtin_pwd_pentest_test.go new file mode 100644 index 00000000..d848aa0f --- /dev/null +++ b/builtins/pwd/builtin_pwd_pentest_test.go @@ -0,0 +1,324 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package pwd_test + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/DataDog/rshell/builtins/testutil" + "github.com/DataDog/rshell/interp" +) + +const pentestTimeout = 5 * time.Second + +// pwdRunCtx is the timeout-aware variant of pwdRun. +func pwdRunCtx(ctx context.Context, t *testing.T, script, dir string) (string, string, int) { + t.Helper() + return testutil.RunScriptCtx(ctx, t, script, dir, interp.AllowedPaths([]string{dir})) +} + +// withTimeout wraps fn in a per-test deadline so a hang is reported as a +// failure instead of the full Go test timeout. +func withTimeout(t *testing.T, fn func(ctx context.Context)) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), pentestTimeout) + defer cancel() + done := make(chan struct{}) + go func() { + fn(ctx) + close(done) + }() + select { + case <-done: + case <-ctx.Done(): + t.Fatal("operation did not complete within pentestTimeout") + } +} + +// --- Many positional arguments are silently ignored, no FD leak --- + +func TestPwdPentestManyPositionalArgs(t *testing.T) { + dir := t.TempDir() + args := strings.Repeat("x ", 1000) + withTimeout(t, func(ctx context.Context) { + stdout, stderr, code := pwdRunCtx(ctx, t, "pwd "+args, dir) + assert.Equal(t, 0, code) + assert.Equal(t, "", stderr) + assert.True(t, filepath.IsAbs(strings.TrimRight(stdout, "\n"))) + }) +} + +// --- Many calls in a tight loop don't leak resources or slow over time --- + +func TestPwdPentestTightLoop(t *testing.T) { + dir := t.TempDir() + // 200 invocations via brace expansion (mvdan/sh supports {1..N}). + // The runner has a 30-second executor budget; this should finish + // well under a second. + withTimeout(t, func(ctx context.Context) { + stdout, _, code := pwdRunCtx(ctx, t, "for i in {1..200}; do pwd; done", dir) + assert.Equal(t, 0, code) + assert.Equal(t, 200, strings.Count(stdout, "\n")) + }) +} + +// --- Flag injection via word splitting/expansion is handled safely --- + +func TestPwdPentestFlagInjectionViaExpansion(t *testing.T) { + dir := t.TempDir() + // $bad expands to "-x". rshell must reject the expanded flag the same + // way as a literal -x — i.e. exit 1, not crash, not exec anything. + withTimeout(t, func(ctx context.Context) { + _, stderr, code := pwdRunCtx(ctx, t, "bad=-x; pwd $bad", dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "pwd:") + }) +} + +func TestPwdPentestFlagInjectionViaUnquotedArrayLikeArgs(t *testing.T) { + dir := t.TempDir() + withTimeout(t, func(ctx context.Context) { + _, stderr, code := pwdRunCtx(ctx, t, `for f in "--no-such"; do pwd $f; done`, dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "pwd:") + }) +} + +// --- pwd output never contains stdin content (stdin must be ignored) --- + +func TestPwdPentestStdinNotEchoed(t *testing.T) { + dir := t.TempDir() + withTimeout(t, func(ctx context.Context) { + stdout, _, code := pwdRunCtx(ctx, t, `printf "abc\ndef\n" | pwd`, dir) + assert.Equal(t, 0, code) + assert.False(t, strings.Contains(stdout, "abc")) + assert.False(t, strings.Contains(stdout, "def")) + }) +} + +// --- pwd does not write to filesystem (no creation, no modification) --- + +func TestPwdPentestNoFilesystemWrite(t *testing.T) { + dir := t.TempDir() + // Snapshot the directory contents. + beforeEntries, err := os.ReadDir(dir) + require.NoError(t, err) + + withTimeout(t, func(ctx context.Context) { + _, _, code := pwdRunCtx(ctx, t, "pwd; pwd -L; pwd -P; pwd --help", dir) + assert.Equal(t, 0, code) + }) + + afterEntries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Equal(t, len(beforeEntries), len(afterEntries), + "pwd must not create or remove files") +} + +// --- Symlink cycle is reported on -P, not infinite loop --- + +func TestPwdPentestSymlinkCycleTerminates(t *testing.T) { + if filepath.Separator == '\\' { + t.Skip("symlink cycles are exercised on Unix; Windows has separate test") + } + root := t.TempDir() + a := filepath.Join(root, "a") + b := filepath.Join(root, "b") + require.NoError(t, os.Symlink(b, a)) + require.NoError(t, os.Symlink(a, b)) + + withTimeout(t, func(ctx context.Context) { + stdout, stderr, code := testutil.RunScriptCtx(ctx, t, "pwd -P", a, interp.AllowedPaths([]string{root})) + assert.Equal(t, 1, code) + assert.Equal(t, "", stdout, "no stdout when -P aborts on cycle") + assert.Contains(t, stderr, "too many levels of symbolic links") + }) +} + +// --- Self-referential symlink (a → a) is detected via the hop cap --- + +func TestPwdPentestSelfReferentialSymlink(t *testing.T) { + if filepath.Separator == '\\' { + t.Skip("symlinks differ on Windows") + } + root := t.TempDir() + self := filepath.Join(root, "self") + require.NoError(t, os.Symlink("self", self)) + + withTimeout(t, func(ctx context.Context) { + _, stderr, code := testutil.RunScriptCtx(ctx, t, "pwd -P", self, interp.AllowedPaths([]string{root})) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "too many levels of symbolic links") + }) +} + +// --- Long path through many real (non-symlink) components is fine --- + +func TestPwdPentestDeepRealDirectoryTree(t *testing.T) { + root := t.TempDir() + // Build 50 nested real directories (not symlinks). + deepest := root + for range 50 { + deepest = filepath.Join(deepest, "d") + require.NoError(t, os.Mkdir(deepest, 0755)) + } + withTimeout(t, func(ctx context.Context) { + stdout, stderr, code := testutil.RunScriptCtx(ctx, t, "pwd -P", deepest, interp.AllowedPaths([]string{root})) + assert.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, deepest+"\n", stdout) + }) +} + +// --- 39 chained symlinks (one below the 40-hop cap) succeeds --- + +func TestPwdPentestSymlinkChainAtCapMinusOne(t *testing.T) { + if filepath.Separator == '\\' { + t.Skip("symlinks differ on Windows") + } + root := t.TempDir() + target := filepath.Join(root, "target") + require.NoError(t, os.Mkdir(target, 0755)) + prev := target + for i := range 39 { + next := filepath.Join(root, "lnk"+string('a'+byte(i%26))+strings.Repeat("x", i)) + require.NoError(t, os.Symlink(prev, next)) + prev = next + } + withTimeout(t, func(ctx context.Context) { + stdout, stderr, code := testutil.RunScriptCtx(ctx, t, "pwd -P", prev, interp.AllowedPaths([]string{root})) + assert.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, target+"\n", stdout) + }) +} + +// --- Path with embedded special characters is preserved verbatim --- + +func TestPwdPentestPathWithSpaces(t *testing.T) { + root := t.TempDir() + weird := filepath.Join(root, "with spaces and 'quotes'") + require.NoError(t, os.Mkdir(weird, 0755)) + + withTimeout(t, func(ctx context.Context) { + stdout, stderr, code := testutil.RunScriptCtx(ctx, t, "pwd", weird, interp.AllowedPaths([]string{root})) + assert.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, weird+"\n", stdout) + }) +} + +func TestPwdPentestPathWithUnicode(t *testing.T) { + root := t.TempDir() + weird := filepath.Join(root, "ünïcödé-測試-🦝") + require.NoError(t, os.Mkdir(weird, 0755)) + + withTimeout(t, func(ctx context.Context) { + stdout, stderr, code := testutil.RunScriptCtx(ctx, t, "pwd", weird, interp.AllowedPaths([]string{root})) + assert.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, weird+"\n", stdout) + }) +} + +// --- Output stays bounded: pwd never produces more than one line --- + +func TestPwdPentestOutputIsSingleLine(t *testing.T) { + dir := t.TempDir() + withTimeout(t, func(ctx context.Context) { + stdout, _, code := pwdRunCtx(ctx, t, "pwd", dir) + assert.Equal(t, 0, code) + assert.Equal(t, 1, strings.Count(stdout, "\n")) + }) +} + +// --- Help output stays bounded (a small fixed payload) --- + +func TestPwdPentestHelpOutputBounded(t *testing.T) { + dir := t.TempDir() + withTimeout(t, func(ctx context.Context) { + stdout, _, code := pwdRunCtx(ctx, t, "pwd --help", dir) + assert.Equal(t, 0, code) + assert.Less(t, len(stdout), 4096, "help should be a small fixed payload") + }) +} + +// --- Context cancellation aborts an in-flight -P walk promptly --- + +func TestPwdPentestContextCancelDuringPhysicalWalk(t *testing.T) { + if filepath.Separator == '\\' { + t.Skip("symlinks differ on Windows") + } + root := t.TempDir() + target := filepath.Join(root, "target") + require.NoError(t, os.Mkdir(target, 0755)) + prev := target + // Build a valid 39-link chain (well under the cap, and won't error). + for i := range 39 { + next := filepath.Join(root, "lnk"+string('a'+byte(i%26))+strings.Repeat("x", i)) + require.NoError(t, os.Symlink(prev, next)) + prev = next + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() // pre-cancel: the resolver must give up at the first iteration + stdout, stderr, code := testutil.RunScriptCtx(ctx, t, "pwd -P", prev, interp.AllowedPaths([]string{root})) + // The interpreter may report a context-cancelled error or simply + // return early. Either way, we must not hang and must not write + // stdout (since the script never completed). + _ = stdout + _ = stderr + _ = code + // no assertion on exact code; the goal is "did not hang" +} + +// --- pwd does not run any external command --- +// +// The runner is configured to allow all commands but the *only* allowed +// path is `dir`. If pwd were to exec/spawn anything, the interpreter +// would refuse it (because the binary lives outside dir). No assertion +// needed beyond "does not hang and does not error." + +func TestPwdPentestDoesNotInvokeExternalCommand(t *testing.T) { + dir := t.TempDir() + withTimeout(t, func(ctx context.Context) { + _, stderr, code := pwdRunCtx(ctx, t, "pwd", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "", stderr) + }) +} + +// --- Concurrent shell runs of pwd in different goroutines must be safe --- + +func TestPwdPentestConcurrentRuns(t *testing.T) { + dir := t.TempDir() + const N = 16 + errCh := make(chan error, N) + withTimeout(t, func(ctx context.Context) { + for range N { + go func() { + _, _, code := pwdRunCtx(ctx, t, "pwd", dir) + if code != 0 { + errCh <- assertErr("non-zero exit") + return + } + errCh <- nil + }() + } + for range N { + require.NoError(t, <-errCh) + } + }) +} + +// assertErr is a tiny helper used by TestPwdPentestConcurrentRuns to +// avoid pulling in a fmt.Errorf in pentest code. +type assertErr string + +func (e assertErr) Error() string { return string(e) } diff --git a/builtins/pwd/pwd.go b/builtins/pwd/pwd.go new file mode 100644 index 00000000..2d6a7943 --- /dev/null +++ b/builtins/pwd/pwd.go @@ -0,0 +1,289 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package pwd implements the pwd builtin command. +// +// pwd — print the absolute pathname of the current working directory +// +// Usage: pwd [-LP] [--help] +// +// Print the absolute pathname of the shell's current working directory +// to standard output. Stdin is not used. Extra positional arguments are +// ignored to match GNU coreutils and bash behavior. +// +// Flags: +// +// -L, --logical Use the shell's tracked logical path (default). +// The path may contain symbolic links, exactly as the +// user reached the directory. This matches the POSIX +// default and the bash builtin default. +// -P, --physical Resolve all symbolic links so the printed path contains +// no symlinks and no "." or ".." components. +// -h, --help Print usage to stdout and exit 0. +// +// If both -L and -P are given on the same invocation, the last one wins +// (matches POSIX and bash, implemented via pflag's declaration-order +// Visit traversal). +// +// Symlink resolution for -P is sandbox-safe: it walks the absolute path +// component-by-component using callCtx.LstatFile and callCtx.ReadlinkFile, +// never calling os.Lstat / os.Readlink directly. The total number of +// symlink expansions is capped at maxSymlinkHops (40) to defeat cycles, +// matching the Linux ELOOP limit. +// +// Sandbox best-effort for -P: if a path component lies outside the +// AllowedPaths sandbox (a common case when the working directory is +// itself the sandbox root), the resolver cannot walk that component and +// gives up gracefully — falling back to the logical path. This means +// "pwd -P" never produces a hard error from the sandbox; it returns the +// best canonical path it can compute. +// +// Exit codes: +// +// 0 Success — a working directory was written (logical or physical). +// 1 Error — invalid flag, or the runner exposes no working directory. +package pwd + +import ( + "context" + "errors" + "fmt" + iofs "io/fs" + "path/filepath" + "strings" + + "github.com/DataDog/rshell/builtins" +) + +// Cmd is the pwd builtin command descriptor. Help is intentionally not +// set: pwd registers flags, so the `help` builtin invokes `pwd --help` +// to produce its description, keeping `help pwd` and `pwd --help` +// identical. +var Cmd = builtins.Command{ + Name: "pwd", + Description: "print working directory", + MakeFlags: makeFlags, +} + +// maxSymlinkHops caps the number of symlink expansions during -P +// resolution. Matches the Linux ELOOP limit (40) so that pathological +// or cyclic chains terminate with a clear error. +const maxSymlinkHops = 40 + +// errSymlinkLoop is returned when -P resolution exceeds maxSymlinkHops. +var errSymlinkLoop = errors.New("too many levels of symbolic links") + +func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { + helpFlag := fs.BoolP("help", "h", false, "print usage and exit") + // -L and -P are deliberately not bound to local pointers: the + // command-line ordering matters (last-wins per POSIX), so the + // handler consults pflag.FlagSet.Visit in pickPhysical(fs) + // rather than reading the bool values directly. + fs.BoolP("logical", "L", false, "use the shell's working directory, even if it contains symlinks (default)") + fs.BoolP("physical", "P", false, "resolve all symlinks before printing the path") + + return func(ctx context.Context, callCtx *builtins.CallContext, _ []string) builtins.Result { + if *helpFlag { + printHelp(callCtx) + return builtins.Result{} + } + + if callCtx.WorkDir == nil { + callCtx.Errf("pwd: no working directory available\n") + return builtins.Result{Code: 1} + } + + cwd := callCtx.WorkDir() + if cwd == "" { + callCtx.Errf("pwd: working directory is empty\n") + return builtins.Result{Code: 1} + } + + if pickPhysical(fs) { + // Best-effort: if symlink resolution fails (typically because + // the cwd is the sandbox root and we cannot walk above it), + // fall back to the logical path silently. Cycles still error + // because they indicate corrupt input, not a sandbox limit. + if resolved, err := resolveSymlinks(ctx, callCtx, cwd); err == nil { + cwd = resolved + } else if errors.Is(err, errSymlinkLoop) { + callCtx.Errf("pwd: %s\n", err) + return builtins.Result{Code: 1} + } + } + + callCtx.Outf("%s\n", cwd) + return builtins.Result{} + } +} + +// pickPhysical reports whether to resolve symlinks (-P). Walks fs.Visit +// in declaration order so that when both -L and -P appear, the last one +// wins (POSIX). When neither is set, returns false (logical default). +func pickPhysical(fs *builtins.FlagSet) bool { + usePhysical := false + fs.Visit(func(f *builtins.Flag) { + switch f.Name { + case "logical": + usePhysical = false + case "physical": + usePhysical = true + } + }) + return usePhysical +} + +func printHelp(callCtx *builtins.CallContext) { + callCtx.Out("Usage: pwd [-LP] [--help]\n") + callCtx.Out("Print the absolute pathname of the current working directory.\n\n") + callCtx.Out(" -L, --logical use the shell's tracked working directory, even if it\n") + callCtx.Out(" contains symlinks (default)\n") + callCtx.Out(" -P, --physical resolve all symlinks before printing the path\n") + callCtx.Out(" -h, --help display this help and exit\n") +} + +// resolveSymlinks returns the canonical (no-symlinks, no "." or "..") +// absolute form of path. It walks path component-by-component using the +// sandbox-safe LstatFile and ReadlinkFile from callCtx, so it never +// touches the filesystem directly. The number of symlink expansions is +// capped at maxSymlinkHops to defeat cycles. +// +// Algorithm: maintain `out` (the already-resolved prefix) and `rest` +// (the remaining unresolved suffix, always relative). Pop one component +// off rest at a time. If the component is a symlink, prepend the link +// target back onto rest (and reset out to the absolute root if the +// target is absolute). Continue until rest is empty. +func resolveSymlinks(ctx context.Context, callCtx *builtins.CallContext, path string) (string, error) { + if !filepath.IsAbs(path) { + return "", fmt.Errorf("not an absolute path: %s", path) + } + if callCtx.LstatFile == nil || callCtx.ReadlinkFile == nil { + return "", errors.New("sandbox does not support symlink resolution") + } + + cleaned := filepath.Clean(path) + out := rootPrefix(cleaned) + rest := strings.TrimPrefix(cleaned, out) + + hops := 0 + for rest != "" { + if err := ctx.Err(); err != nil { + return "", err + } + + // Consume any leading separator(s) before extracting the next + // component. filepath.Clean collapses runs of separators, but + // expanded symlink targets (which we splice back into rest) + // can reintroduce them. + for len(rest) > 0 && rest[0] == filepath.Separator { + rest = rest[1:] + } + if rest == "" { + break + } + + var comp string + if i := strings.IndexByte(rest, filepath.Separator); i >= 0 { + comp = rest[:i] + rest = rest[i:] + } else { + comp = rest + rest = "" + } + + switch comp { + case ".": + continue + case "..": + out = parentDir(out) + continue + } + + candidate := joinPath(out, comp) + info, err := callCtx.LstatFile(ctx, candidate) + if err != nil { + // Cannot stat through the sandbox — typically because the + // path is above the AllowedPaths root. Treat the component + // as opaque (not a symlink) and continue. This is what makes + // -P resolution work when the cwd is somewhere under the + // sandbox root: components above the root pass through, and + // only the symlinks we can actually inspect get resolved. + out = candidate + continue + } + + if info.Mode()&iofs.ModeSymlink == 0 { + out = candidate + continue + } + + hops++ + if hops > maxSymlinkHops { + return "", errSymlinkLoop + } + + target, err := callCtx.ReadlinkFile(ctx, candidate) + if err != nil { + // Lstat said it's a symlink but readlink failed. Treat the + // component as opaque rather than aborting resolution. + out = candidate + continue + } + + if filepath.IsAbs(target) { + cleanedTarget := filepath.Clean(target) + out = rootPrefix(cleanedTarget) + rest = strings.TrimPrefix(cleanedTarget, out) + rest + } else { + rest = string(filepath.Separator) + target + rest + } + } + + if out == "" { + out = string(filepath.Separator) + } + return out, nil +} + +// rootPrefix returns the leading absolute-path root for path. On Unix +// this is always "/". On Windows it preserves the drive letter or UNC +// volume so subsequent component walking does not mistake "C:" for a +// path component. +func rootPrefix(path string) string { + vol := filepath.VolumeName(path) + if vol != "" { + if len(path) > len(vol) && path[len(vol)] == filepath.Separator { + return vol + string(filepath.Separator) + } + return vol + } + return string(filepath.Separator) +} + +// parentDir returns the directory containing dir, preserving the volume +// root. If dir is already at the root, parentDir returns dir unchanged. +func parentDir(dir string) string { + root := rootPrefix(dir) + if dir == root { + return dir + } + parent := filepath.Dir(dir) + if parent == "." { + return root + } + return parent +} + +// joinPath joins dir and a single non-empty component without using +// filepath.Join (which collapses ".." and may erase volume roots). +func joinPath(dir, comp string) string { + if dir == "" { + return comp + } + if dir[len(dir)-1] == filepath.Separator { + return dir + comp + } + return dir + string(filepath.Separator) + comp +} diff --git a/builtins/pwd/pwd_fuzz_test.go b/builtins/pwd/pwd_fuzz_test.go new file mode 100644 index 00000000..92786564 --- /dev/null +++ b/builtins/pwd/pwd_fuzz_test.go @@ -0,0 +1,234 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package pwd_test + +import ( + "context" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + "unicode/utf8" + + "github.com/DataDog/rshell/builtins/testutil" + "github.com/DataDog/rshell/interp" +) + +// fuzzCounter generates unique per-iteration subdir names. +var fuzzCounter atomic.Int64 + +const fuzzTimeout = 5 * time.Second + +// pwdRunCtxFuzz is the fuzz-friendly script runner. It uses a per-iteration +// subdirectory under base, with AllowedPaths scoped to that subdirectory. +func pwdRunCtxFuzz(ctx context.Context, t *testing.T, script, base string) (string, string, int) { + t.Helper() + dir, cleanup := testutil.FuzzIterDir(t, base, &fuzzCounter) + defer cleanup() + return testutil.RunScriptCtx(ctx, t, script, dir, interp.AllowedPaths([]string{dir})) +} + +// shellSafe rejects bytes that would change shell tokenization or +// expansion in a way the fuzzer cannot recover from. Parse errors, +// glob-regex compile failures, and tilde expansion all surface as +// non-ExitStatus errors that fail the test before pwd is even invoked. +// The fuzzer cares about pwd's behavior, not the shell parser's, so we +// filter aggressively. Inputs are also capped at 1 KiB and required to +// be valid UTF-8 (the parser rejects invalid encodings). +func shellSafe(s string) bool { + if len(s) > 1024 { + return false + } + if !utf8.ValidString(s) { + return false + } + for i := 0; i < len(s); i++ { + // Reject all ASCII control characters (0x00–0x1F and 0x7F). + // Shell tokenization differs from Go's strings.Fields on + // characters like \v and \f: the shell treats them as part of + // a token while Fields splits on them, which confuses + // flag-token-detection in this test. + if s[i] < 0x20 || s[i] == 0x7F { + return false + } + switch s[i] { + // Control / quoting / redirection / substitution. + case '`', '$', '\\', '"', '\'', '|', '&', ';', '<', '>', '(', ')': + return false + // Glob/brace/range expansion — these reach mvdan/sh's expander + // which can hit upstream bugs on weird inputs (e.g. "怎*" + // triggers a regex-compile failure on an invalid-UTF-8 prefix). + case '*', '?', '[', ']', '{', '}': + return false + // Tilde expansion is unsupported by the runner and rejected at + // the interpreter layer with a fixed exit code 2 — already + // covered by `validExit`, but excluding it keeps the corpus + // focused on real pwd-arg shapes. + case '~': + return false + // Comments — the rest of the line after `#` is dropped by the + // shell parser, so a `-h` after `#` never reaches pwd. + case '#': + return false + } + } + return true +} + +// FuzzPwdArgs fuzzes the pwd command's argument parser. The pwd builtin +// accepts -L, -P, -h, --logical, --physical, --help, and ignores any +// positional arguments. The fuzzer tries arbitrary single-token args: +// the command must always exit cleanly (0 for success, 1 for invalid +// flag) — never panic, never block, never produce a different code. +func FuzzPwdArgs(f *testing.F) { + // Implementation edge cases — cover every flag and obvious adversarial + // shapes for pflag. + for _, seed := range []string{ + "", // no args + "-L", // logical short + "-P", // physical short + "-h", // help short + "--logical", "--physical", "--help", + "-LP", // combined short flags + "-PL", // combined short flags, swapped + "-Lh", // logical + help + "--", // end-of-flags only + "-", // bare dash + "---", // triple dash + "--LL", // bogus long + "-x", // unknown short + "--no-flag", // unknown long + "--version", // GNU-but-not-supported flag + "--logical=", // long with empty value (boolean rejects) + "x", // bare positional + "-", // bare dash + "hello world", // spaces + strings.Repeat("a", 256), // long arg + } { + f.Add(seed) + } + + // Existing-test inputs: every concrete invocation pattern from the + // black-box test file should be in the corpus baseline so a regression + // stays caught by fuzz coverage. + for _, seed := range []string{ + "-L -P", "-P -L", "-L --physical", "--logical -P", + "-- foo", "-- --not-a-flag", "extra args", + } { + f.Add(seed) + } + + base := f.TempDir() + f.Fuzz(func(t *testing.T, args string) { + if !shellSafe(args) { + return + } + ctx, cancel := context.WithTimeout(context.Background(), fuzzTimeout) + defer cancel() + _, _, code := pwdRunCtxFuzz(ctx, t, "pwd "+args, base) + // pwd itself produces 0 (success) or 1 (flag error). The shell + // runner can return 2 for sandbox rejections (e.g. tilde + // expansion) before pwd ever runs — that's an interpreter + // concern, not pwd's. Anything else indicates a defect. + if !validExit(code) { + t.Errorf("unexpected exit code %d for args %q", code, args) + } + }) +} + +// validExit accepts the exit codes pwd or the interpreter's sandbox +// layer can legitimately produce. 130 is SIGINT-like (context cancel +// during expansion), 137 is SIGKILL — neither should escape, but they +// indicate runtime termination, not a defect in pwd. +func validExit(code int) bool { + return code == 0 || code == 1 || code == 2 +} + +// FuzzPwdFlagsCombo fuzzes combinations of recognized flags and +// positional arguments. The fuzzer feeds arbitrary tokens to pwd and +// verifies that pwd never panics, never hangs, and always returns a +// known exit code. +func FuzzPwdFlagsCombo(f *testing.F) { + for _, seed := range []string{ + "-L -P", "-P -L", "-L --physical", "--logical -P", + "-LP", "-PL", "-Lh", + "-h foo bar", "--help --no-such", + "-- foo", "-- --logical", + "--help -P", "-P --help", + "-L=true", "-P=false", + "--logical=true", "--physical=true", + } { + f.Add(seed) + } + + base := f.TempDir() + f.Fuzz(func(t *testing.T, args string) { + if !shellSafe(args) { + return + } + ctx, cancel := context.WithTimeout(context.Background(), fuzzTimeout) + defer cancel() + _, _, code := pwdRunCtxFuzz(ctx, t, "pwd "+args, base) + if !validExit(code) { + t.Errorf("unexpected exit code %d for args %q", code, args) + } + }) +} + +// FuzzPwdSymlinkTargets fuzzes -P resolution against symlink chains +// whose targets are arbitrary user-provided strings. The resolver must +// terminate (within the hop cap) for any input — never panic, never +// hang. +func FuzzPwdSymlinkTargets(f *testing.F) { + // Implementation edge cases + CVE-style adversarial inputs + existing + // test inputs all share the same fuzz target shape (single string). + seeds := []string{ + // Implementation edge cases. + "target", // simple relative + "./target", // dot prefix + "../target", // dot-dot prefix + "../../target", // multiple dot-dots + "/abs/target", // absolute target + "/", // root + "./.", // dot only + "sub/lnk", // multi-component + "a/b/c/d/e/f/g/h", // deep + string(filepath.Separator), // bare separator + "", // empty (invalid) + strings.Repeat("a", 200), // long name + "target/with spaces", // spaces + // CVE / weird-input class. + "target\x00null", // embedded NUL — Symlink rejects + "target/with/many/slashes", // benign multi-component + "a/" + strings.Repeat("b/", 30) + "z", // 60-component path + } + for _, seed := range seeds { + f.Add(seed) + } + + base := f.TempDir() + f.Fuzz(func(t *testing.T, target string) { + // Reject obviously-illegal symlink targets up front — Symlink will + // fail and we'd just skip the iteration anyway. + if len(target) == 0 || len(target) > 200 || strings.ContainsRune(target, 0) { + return + } + dir, cleanup := testutil.FuzzIterDir(t, base, &fuzzCounter) + defer cleanup() + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + return + } + ctx, cancel := context.WithTimeout(context.Background(), fuzzTimeout) + defer cancel() + _, _, code := testutil.RunScriptCtx(ctx, t, "pwd -P", link, interp.AllowedPaths([]string{dir})) + if !validExit(code) { + t.Errorf("unexpected exit code %d for symlink target %q", code, target) + } + }) +} diff --git a/builtins/pwd/pwd_internal_test.go b/builtins/pwd/pwd_internal_test.go new file mode 100644 index 00000000..c9e8d702 --- /dev/null +++ b/builtins/pwd/pwd_internal_test.go @@ -0,0 +1,285 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package pwd + +import ( + "bytes" + "context" + "errors" + "io" + iofs "io/fs" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/DataDog/rshell/builtins" +) + +// fakeFileInfo is a minimal io/fs.FileInfo implementation for tests. +type fakeFileInfo struct { + mode iofs.FileMode +} + +func (f *fakeFileInfo) Name() string { return "" } +func (f *fakeFileInfo) Size() int64 { return 0 } +func (f *fakeFileInfo) Mode() iofs.FileMode { return f.mode } +func (f *fakeFileInfo) ModTime() time.Time { return time.Time{} } +func (f *fakeFileInfo) IsDir() bool { return f.mode.IsDir() } +func (f *fakeFileInfo) Sys() any { return nil } + +// invokePwd runs the pwd handler against a synthetic CallContext and +// returns stdout, stderr, and the result. +func invokePwd(t *testing.T, callCtx *builtins.CallContext, args []string) (stdout, stderr string, res builtins.Result) { + t.Helper() + var sout, serr bytes.Buffer + callCtx.Stdout = &sout + callCtx.Stderr = &serr + + fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) + fs.SetOutput(io.Discard) + handler := makeFlags(fs) + require.NoError(t, fs.Parse(args)) + res = handler(context.Background(), callCtx, fs.Args()) + return sout.String(), serr.String(), res +} + +// --- WorkDir nil / empty paths --- + +func TestPwdNilWorkDir(t *testing.T) { + cc := &builtins.CallContext{} // WorkDir == nil + _, stderr, res := invokePwd(t, cc, []string{}) + assert.Equal(t, uint8(1), res.Code) + assert.Contains(t, stderr, "no working directory available") +} + +func TestPwdEmptyWorkDir(t *testing.T) { + cc := &builtins.CallContext{WorkDir: func() string { return "" }} + _, stderr, res := invokePwd(t, cc, []string{}) + assert.Equal(t, uint8(1), res.Code) + assert.Contains(t, stderr, "working directory is empty") +} + +// --- resolveSymlinks error paths reachable directly --- + +func TestResolveSymlinksRejectsRelativePath(t *testing.T) { + cc := &builtins.CallContext{ + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { return nil, errors.New("nope") }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { return "", errors.New("nope") }, + } + _, err := resolveSymlinks(context.Background(), cc, "relative/path") + require.Error(t, err) + assert.Contains(t, err.Error(), "not an absolute path") +} + +func TestResolveSymlinksMissingCallbacksError(t *testing.T) { + // Missing LstatFile callback. + cc := &builtins.CallContext{ReadlinkFile: func(_ context.Context, _ string) (string, error) { return "", nil }} + _, err := resolveSymlinks(context.Background(), cc, string(filepath.Separator)+"foo") + require.Error(t, err) + assert.Contains(t, err.Error(), "sandbox does not support symlink resolution") + + // Missing ReadlinkFile callback. + cc2 := &builtins.CallContext{LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { return nil, nil }} + _, err = resolveSymlinks(context.Background(), cc2, string(filepath.Separator)+"foo") + require.Error(t, err) + assert.Contains(t, err.Error(), "sandbox does not support symlink resolution") +} + +func TestResolveSymlinksContextCancelled(t *testing.T) { + cc := &builtins.CallContext{ + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { return &fakeFileInfo{}, nil }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { return "", nil }, + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := resolveSymlinks(ctx, cc, string(filepath.Separator)+"foo"+string(filepath.Separator)+"bar") + require.Error(t, err) + assert.True(t, errors.Is(err, context.Canceled)) +} + +// --- resolveSymlinks: dot and dot-dot components are collapsed even +// when the lstat result is non-symlink. --- + +func TestResolveSymlinksHandlesDotAndDotDot(t *testing.T) { + // Build a virtual filesystem with no symlinks; every lstat says + // "regular dir". Path includes . and .. — they must be collapsed. + cc := &builtins.CallContext{ + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { + return &fakeFileInfo{mode: iofs.ModeDir}, nil + }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { + return "", errors.New("not a symlink") + }, + } + sep := string(filepath.Separator) + // Use raw input with explicit . and .. to reach those branches before + // filepath.Clean collapses them. Note: filepath.Clean inside the + // resolver normalizes the input before walking, so the dot/dot-dot + // branches are reached by symlink targets containing those segments, + // which TestPwdPhysicalDotDotResolvesAcrossDepth already exercises + // from end-to-end. Here we stick to a simple absolute path. + out, err := resolveSymlinks(context.Background(), cc, sep+"a"+sep+"b") + require.NoError(t, err) + assert.Equal(t, sep+"a"+sep+"b", out) +} + +// --- Symlink loop detection at the maxSymlinkHops cap --- + +func TestResolveSymlinksLoopDetected(t *testing.T) { + cc := &builtins.CallContext{ + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { + return &fakeFileInfo{mode: iofs.ModeSymlink}, nil + }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { + // Always point back at the same target so the resolver loops + // until it hits the cap. + return "self", nil + }, + } + sep := string(filepath.Separator) + _, err := resolveSymlinks(context.Background(), cc, sep+"a") + require.Error(t, err) + assert.True(t, errors.Is(err, errSymlinkLoop)) +} + +// --- pickPhysical with no flags set defaults to logical --- + +func TestPickPhysicalNoFlagsDefaultsLogical(t *testing.T) { + fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) + makeFlags(fs) + require.NoError(t, fs.Parse([]string{})) + assert.False(t, pickPhysical(fs)) +} + +// --- joinPath / parentDir / rootPrefix unit cases --- + +func TestJoinPathEmptyDir(t *testing.T) { + assert.Equal(t, "comp", joinPath("", "comp")) +} + +func TestJoinPathDirEndingInSeparator(t *testing.T) { + sep := string(filepath.Separator) + assert.Equal(t, sep+"comp", joinPath(sep, "comp")) +} + +func TestJoinPathDirNoTrailingSeparator(t *testing.T) { + sep := string(filepath.Separator) + assert.Equal(t, sep+"foo"+sep+"bar", joinPath(sep+"foo", "bar")) +} + +func TestParentDirAtRoot(t *testing.T) { + root := string(filepath.Separator) + assert.Equal(t, root, parentDir(root)) +} + +func TestParentDirOneLevel(t *testing.T) { + sep := string(filepath.Separator) + assert.Equal(t, sep+"foo", parentDir(sep+"foo"+sep+"bar")) +} + +func TestRootPrefixUnixPath(t *testing.T) { + if filepath.Separator != '/' { + t.Skip("Unix-only path semantics") + } + assert.Equal(t, "/", rootPrefix("/foo/bar")) +} + +// --- Help / pickPhysical interaction --- + +func TestHelpFlagShortCircuits(t *testing.T) { + cc := &builtins.CallContext{WorkDir: func() string { return "/should/not/print" }} + stdout, stderr, res := invokePwd(t, cc, []string{"--help"}) + assert.Equal(t, uint8(0), res.Code) + assert.Contains(t, stdout, "Usage: pwd") + assert.Equal(t, "", stderr) + // Working directory must not appear in stdout when --help is passed. + assert.False(t, strings.Contains(stdout, "/should/not/print")) +} + +// --- ReadlinkFile error: lstat says symlink, readlink fails. --- + +func TestResolveSymlinksReadlinkFails(t *testing.T) { + cc := &builtins.CallContext{ + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { + return &fakeFileInfo{mode: iofs.ModeSymlink}, nil + }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { + return "", errors.New("readlink failed") + }, + } + sep := string(filepath.Separator) + out, err := resolveSymlinks(context.Background(), cc, sep+"a"+sep+"b") + require.NoError(t, err) + // Both components got passed through. + assert.Equal(t, sep+"a"+sep+"b", out) +} + +// --- A symlink target containing "." segments exercises the dot branch. + +func TestResolveSymlinksDotInTarget(t *testing.T) { + calls := 0 + cc := &builtins.CallContext{ + LstatFile: func(_ context.Context, p string) (iofs.FileInfo, error) { + calls++ + // First call: /lnk is a symlink. After splicing "./real", the + // next component "." is short-circuited before any new lstat. + if calls == 1 { + return &fakeFileInfo{mode: iofs.ModeSymlink}, nil + } + // Anything else is a regular dir. + return &fakeFileInfo{mode: iofs.ModeDir}, nil + }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { return "./real", nil }, + } + sep := string(filepath.Separator) + out, err := resolveSymlinks(context.Background(), cc, sep+"lnk") + require.NoError(t, err) + assert.Equal(t, sep+"real", out) +} + +// --- Symlink target of "/" leaves rest empty after the leading-sep strip. --- + +func TestResolveSymlinksTargetIsRoot(t *testing.T) { + cc := &builtins.CallContext{ + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { + return &fakeFileInfo{mode: iofs.ModeSymlink}, nil + }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { + return string(filepath.Separator), nil // target is just "/" + }, + } + sep := string(filepath.Separator) + out, err := resolveSymlinks(context.Background(), cc, sep+"x") + require.NoError(t, err) + assert.Equal(t, sep, out) +} + +// --- Logical path is returned unchanged for -L (no filesystem touched) --- + +func TestLogicalPathNoFilesystemAccess(t *testing.T) { + calls := 0 + cc := &builtins.CallContext{ + WorkDir: func() string { return "/some/path" }, + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { + calls++ + return nil, errors.New("must not be called for -L") + }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { + calls++ + return "", errors.New("must not be called for -L") + }, + } + stdout, stderr, res := invokePwd(t, cc, []string{"-L"}) + assert.Equal(t, uint8(0), res.Code) + assert.Equal(t, "/some/path\n", stdout) + assert.Equal(t, "", stderr) + assert.Equal(t, 0, calls, "-L must not stat or readlink") +} diff --git a/builtins/pwd/pwd_test.go b/builtins/pwd/pwd_test.go new file mode 100644 index 00000000..4677da9b --- /dev/null +++ b/builtins/pwd/pwd_test.go @@ -0,0 +1,244 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package pwd_test + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/DataDog/rshell/builtins/testutil" + "github.com/DataDog/rshell/interp" +) + +// pwdRun runs a script in `dir` with AllowedPaths set to that dir, mirroring +// the runtime sandbox configuration. +func pwdRun(t *testing.T, script, dir string) (string, string, int) { + t.Helper() + return testutil.RunScript(t, script, dir, interp.AllowedPaths([]string{dir})) +} + +// --- Basic invocation --- + +func TestPwdNoArgsPrintsAbsolutePath(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "", stderr) + assert.True(t, filepath.IsAbs(strings.TrimRight(stdout, "\n"))) + assert.True(t, strings.HasSuffix(stdout, "\n")) +} + +func TestPwdNoArgsTrailingNewline(t *testing.T) { + dir := t.TempDir() + stdout, _, _ := pwdRun(t, "pwd", dir) + assert.True(t, strings.HasSuffix(stdout, "\n")) + // There must be exactly one trailing newline. + assert.False(t, strings.HasSuffix(stdout, "\n\n")) +} + +func TestPwdExtraArgsIgnored(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd extra args here", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "", stderr) + assert.True(t, filepath.IsAbs(strings.TrimRight(stdout, "\n"))) +} + +func TestPwdNoArgsExitCodeZero(t *testing.T) { + dir := t.TempDir() + stdout, _, code := pwdRun(t, "pwd; echo $?", dir) + assert.Equal(t, 0, code) + lines := strings.Split(strings.TrimRight(stdout, "\n"), "\n") + assert.Equal(t, "0", lines[len(lines)-1]) +} + +// --- -L / --logical --- + +func TestPwdLogicalShort(t *testing.T) { + dir := t.TempDir() + stdoutPlain, _, _ := pwdRun(t, "pwd", dir) + stdoutL, _, code := pwdRun(t, "pwd -L", dir) + assert.Equal(t, 0, code) + assert.Equal(t, stdoutPlain, stdoutL, "pwd and pwd -L must agree") +} + +func TestPwdLogicalLong(t *testing.T) { + dir := t.TempDir() + stdoutPlain, _, _ := pwdRun(t, "pwd", dir) + stdoutL, _, code := pwdRun(t, "pwd --logical", dir) + assert.Equal(t, 0, code) + assert.Equal(t, stdoutPlain, stdoutL) +} + +// --- -P / --physical --- + +func TestPwdPhysicalShort(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd -P", dir) + assert.Equal(t, 0, code, "stderr=%q", stderr) + assert.True(t, filepath.IsAbs(strings.TrimRight(stdout, "\n"))) +} + +func TestPwdPhysicalLong(t *testing.T) { + dir := t.TempDir() + stdoutShort, _, _ := pwdRun(t, "pwd -P", dir) + stdoutLong, _, code := pwdRun(t, "pwd --physical", dir) + assert.Equal(t, 0, code) + assert.Equal(t, stdoutShort, stdoutLong) +} + +// --- Last-wins semantics --- + +func TestPwdLastWinsLthenP(t *testing.T) { + dir := t.TempDir() + stdoutBoth, _, code := pwdRun(t, "pwd -L -P", dir) + stdoutP, _, _ := pwdRun(t, "pwd -P", dir) + assert.Equal(t, 0, code) + assert.Equal(t, stdoutP, stdoutBoth) +} + +func TestPwdLastWinsPthenL(t *testing.T) { + dir := t.TempDir() + stdoutBoth, _, code := pwdRun(t, "pwd -P -L", dir) + stdoutL, _, _ := pwdRun(t, "pwd -L", dir) + assert.Equal(t, 0, code) + assert.Equal(t, stdoutL, stdoutBoth) +} + +func TestPwdLastWinsMixedLongShort(t *testing.T) { + dir := t.TempDir() + mixed, _, code := pwdRun(t, "pwd --physical -L", dir) + logical, _, _ := pwdRun(t, "pwd -L", dir) + assert.Equal(t, 0, code) + assert.Equal(t, logical, mixed) +} + +// --- --help --- + +func TestPwdHelpLongPrintsToStdout(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd --help", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "", stderr) + assert.Contains(t, stdout, "Usage: pwd") + assert.Contains(t, stdout, "logical") + assert.Contains(t, stdout, "physical") +} + +func TestPwdHelpShortPrintsToStdout(t *testing.T) { + dir := t.TempDir() + stdout, _, code := pwdRun(t, "pwd -h", dir) + assert.Equal(t, 0, code) + assert.Contains(t, stdout, "Usage: pwd") +} + +// --- Rejected flags / arguments --- + +func TestPwdUnknownLongFlagRejected(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd --no-such-flag", dir) + assert.Equal(t, 1, code) + assert.Equal(t, "", stdout) + assert.Contains(t, stderr, "pwd:") + assert.Contains(t, stderr, "unknown flag") +} + +func TestPwdUnknownShortFlagRejected(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd -x", dir) + assert.Equal(t, 1, code) + assert.Equal(t, "", stdout) + assert.Contains(t, stderr, "pwd:") +} + +func TestPwdVersionRejected(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd --version", dir) + assert.Equal(t, 1, code) + assert.Equal(t, "", stdout) + assert.Contains(t, stderr, "pwd:") +} + +// --- Stdin --- + +func TestPwdStdinIgnored(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "echo unread | pwd", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "", stderr) + assert.True(t, filepath.IsAbs(strings.TrimRight(stdout, "\n"))) + // pwd's output should be only one line — it should not have echoed + // stdin back. + lines := strings.Split(strings.TrimRight(stdout, "\n"), "\n") + assert.Len(t, lines, 1) +} + +// --- End-of-flags --- --- + +func TestPwdDoubleDashEndsFlags(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := pwdRun(t, "pwd -- --not-a-flag", dir) + assert.Equal(t, 0, code, "stderr=%q", stderr) + assert.True(t, filepath.IsAbs(strings.TrimRight(stdout, "\n"))) +} + +// --- Idempotence / determinism --- + +func TestPwdMultipleInvocationsConsistent(t *testing.T) { + dir := t.TempDir() + stdout, _, code := pwdRun(t, "pwd; pwd; pwd", dir) + assert.Equal(t, 0, code) + lines := strings.Split(strings.TrimRight(stdout, "\n"), "\n") + assert.Len(t, lines, 3) + assert.Equal(t, lines[0], lines[1]) + assert.Equal(t, lines[1], lines[2]) +} + +// --- Sandbox path-not-allowed for -P --- + +// TestPwdPhysicalFallsBackOutsideSandbox verifies that requesting -P when +// the working directory cannot be walked through the sandbox does not +// produce a hard error. Resolution is best-effort and falls back to the +// logical path silently. +func TestPwdPhysicalFallsBackOutsideSandbox(t *testing.T) { + dir := t.TempDir() + otherDir := t.TempDir() + stdout, stderr, code := testutil.RunScript(t, "pwd -P", dir, interp.AllowedPaths([]string{otherDir})) + assert.Equal(t, 0, code) + assert.Equal(t, "", stderr) + assert.True(t, filepath.IsAbs(strings.TrimRight(stdout, "\n"))) +} + +// --- pwd inside command substitution / shell features --- + +func TestPwdInCommandSubstitution(t *testing.T) { + dir := t.TempDir() + stdout, _, code := pwdRun(t, `here=$(pwd); echo "[$here]"`, dir) + assert.Equal(t, 0, code) + assert.Contains(t, stdout, "[") + assert.Contains(t, stdout, "]") +} + +func TestPwdInIfCondition(t *testing.T) { + dir := t.TempDir() + stdout, _, code := pwdRun(t, `if pwd > /dev/null; then echo ok; else echo fail; fi`, dir) + assert.Equal(t, 0, code) + assert.Contains(t, stdout, "ok") +} + +func TestPwdInForLoop(t *testing.T) { + dir := t.TempDir() + stdout, _, code := pwdRun(t, `for i in 1 2 3; do pwd; done`, dir) + assert.Equal(t, 0, code) + lines := strings.Split(strings.TrimRight(stdout, "\n"), "\n") + assert.Len(t, lines, 3) + for i := 1; i < len(lines); i++ { + assert.Equal(t, lines[0], lines[i], "all iterations must agree") + } +} diff --git a/builtins/pwd/pwd_unix_test.go b/builtins/pwd/pwd_unix_test.go new file mode 100644 index 00000000..3d82041a --- /dev/null +++ b/builtins/pwd/pwd_unix_test.go @@ -0,0 +1,162 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +//go:build unix + +package pwd_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/DataDog/rshell/builtins/testutil" + "github.com/DataDog/rshell/interp" +) + +// pwdRunDirAllowed runs a script with `dir` as the working directory and +// `allowedRoot` as the AllowedPaths root. Useful when the cwd is *inside* +// a larger sandbox (so the resolver can walk above the cwd, exercising +// real -P logic) — unlike pwdRun in pwd_test.go which scopes AllowedPaths +// to the cwd itself. +func pwdRunDirAllowed(t *testing.T, script, dir, allowedRoot string) (string, string, int) { + t.Helper() + return testutil.RunScript(t, script, dir, interp.AllowedPaths([]string{allowedRoot})) +} + +// TestPwdPhysicalResolvesSymlink: when the cwd is reached via a symlink +// inside the sandbox, "pwd -P" must print the canonical (target) path. +func TestPwdPhysicalResolvesSymlink(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "real") + link := filepath.Join(root, "lnk") + require.NoError(t, os.Mkdir(target, 0755)) + require.NoError(t, os.Symlink(target, link)) + + stdoutP, stderr, code := pwdRunDirAllowed(t, "pwd -P", link, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, target+"\n", stdoutP) +} + +// TestPwdLogicalKeepsSymlink: when the cwd is reached via a symlink, +// "pwd -L" must print the logical (symlink) path, not the canonical one. +func TestPwdLogicalKeepsSymlink(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "real") + link := filepath.Join(root, "lnk") + require.NoError(t, os.Mkdir(target, 0755)) + require.NoError(t, os.Symlink(target, link)) + + stdoutL, _, code := pwdRunDirAllowed(t, "pwd -L", link, root) + require.Equal(t, 0, code) + assert.Equal(t, link+"\n", stdoutL) +} + +// TestPwdPhysicalChainedSymlinks: A -> B -> C resolves to C. +func TestPwdPhysicalChainedSymlinks(t *testing.T) { + root := t.TempDir() + c := filepath.Join(root, "c") + require.NoError(t, os.Mkdir(c, 0755)) + b := filepath.Join(root, "b") + require.NoError(t, os.Symlink(c, b)) + a := filepath.Join(root, "a") + require.NoError(t, os.Symlink(b, a)) + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -P", a, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, c+"\n", stdout) +} + +// TestPwdPhysicalRelativeSymlink: relative symlink targets are resolved +// against the link's directory, not the cwd. +func TestPwdPhysicalRelativeSymlink(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(root, "real"), 0755)) + // "lnk" → "real" (relative). When at $root/lnk, -P must yield $root/real. + require.NoError(t, os.Symlink("real", filepath.Join(root, "lnk"))) + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -P", filepath.Join(root, "lnk"), root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, filepath.Join(root, "real")+"\n", stdout) +} + +// TestPwdPhysicalSymlinkCycle: A -> B -> A loops; -P must error with a +// loop diagnostic and exit 1, not hang or recurse forever. +func TestPwdPhysicalSymlinkCycle(t *testing.T) { + root := t.TempDir() + a := filepath.Join(root, "a") + b := filepath.Join(root, "b") + require.NoError(t, os.Symlink(b, a)) + require.NoError(t, os.Symlink(a, b)) + + _, stderr, code := pwdRunDirAllowed(t, "pwd -P", a, root) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "pwd:") + assert.Contains(t, stderr, "too many levels of symbolic links") +} + +// TestPwdPhysicalNestedSymlinks: link inside a real dir, with components +// after the link. $root/real/lnk → $root/real/sub. +func TestPwdPhysicalNestedSymlinks(t *testing.T) { + root := t.TempDir() + realDir := filepath.Join(root, "real") + sub := filepath.Join(realDir, "sub") + require.NoError(t, os.MkdirAll(sub, 0755)) + link := filepath.Join(realDir, "lnk") + require.NoError(t, os.Symlink("sub", link)) + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -P", link, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, sub+"\n", stdout) +} + +// TestPwdPhysicalWithDotDotInTarget: a symlink whose target contains +// ".." resolves correctly. $root/sibling/lnk → "../target". +func TestPwdPhysicalWithDotDotInTarget(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "target") + sibling := filepath.Join(root, "sibling") + require.NoError(t, os.Mkdir(target, 0755)) + require.NoError(t, os.Mkdir(sibling, 0755)) + link := filepath.Join(sibling, "lnk") + require.NoError(t, os.Symlink("../target", link)) + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -P", link, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, target+"\n", stdout) +} + +// TestPwdPhysicalAbsoluteSymlinkTargetInsideSandbox: an absolute symlink +// whose target is inside the sandbox resolves correctly (target is +// absolute, so we reset the resolved prefix to the absolute root). +func TestPwdPhysicalAbsoluteSymlinkTargetInsideSandbox(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "real") + link := filepath.Join(root, "abslnk") + require.NoError(t, os.Mkdir(target, 0755)) + require.NoError(t, os.Symlink(target, link)) // absolute path + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -P", link, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, target+"\n", stdout) +} + +// TestPwdPhysicalDotDotComponents: symlink target with explicit ".." +// segments is canonicalized. $root/d1/d2/lnk → "../../d3", which lands +// at $root/d3. +func TestPwdPhysicalDotDotResolvesAcrossDepth(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(root, "d1", "d2"), 0755)) + require.NoError(t, os.Mkdir(filepath.Join(root, "d3"), 0755)) + link := filepath.Join(root, "d1", "d2", "lnk") + require.NoError(t, os.Symlink("../../d3", link)) + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -P", link, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, filepath.Join(root, "d3")+"\n", stdout) +} diff --git a/builtins/pwd/testdata/fuzz/FuzzPwdArgs/abb4964a7f34a77c b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/abb4964a7f34a77c new file mode 100644 index 00000000..777ee48d --- /dev/null +++ b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/abb4964a7f34a77c @@ -0,0 +1,2 @@ +go test fuzz v1 +string("怎*") diff --git a/builtins/pwd/testdata/fuzz/FuzzPwdArgs/db7ae63e0377da75 b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/db7ae63e0377da75 new file mode 100644 index 00000000..b426c1c1 --- /dev/null +++ b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/db7ae63e0377da75 @@ -0,0 +1,2 @@ +go test fuzz v1 +string("~") diff --git a/builtins/pwd/testdata/fuzz/FuzzPwdArgs/def578230616f8b9 b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/def578230616f8b9 new file mode 100644 index 00000000..4054d5fb --- /dev/null +++ b/builtins/pwd/testdata/fuzz/FuzzPwdArgs/def578230616f8b9 @@ -0,0 +1,2 @@ +go test fuzz v1 +string("\xff") diff --git a/interp/register_builtins.go b/interp/register_builtins.go index d16f1b69..504aaa74 100644 --- a/interp/register_builtins.go +++ b/interp/register_builtins.go @@ -25,6 +25,7 @@ import ( "github.com/DataDog/rshell/builtins/ping" printfcmd "github.com/DataDog/rshell/builtins/printf" pscmd "github.com/DataDog/rshell/builtins/ps" + "github.com/DataDog/rshell/builtins/pwd" "github.com/DataDog/rshell/builtins/sed" sortcmd "github.com/DataDog/rshell/builtins/sort" "github.com/DataDog/rshell/builtins/ss" @@ -60,6 +61,7 @@ func registerBuiltins() { sortcmd.Cmd, printfcmd.Cmd, pscmd.Cmd, + pwd.Cmd, sed.Cmd, ss.Cmd, strings_cmd.Cmd, diff --git a/tests/scenarios/cmd/help/restricted.yaml b/tests/scenarios/cmd/help/restricted.yaml index bc2e34ce..fcbb2c04 100644 --- a/tests/scenarios/cmd/help/restricted.yaml +++ b/tests/scenarios/cmd/help/restricted.yaml @@ -6,13 +6,13 @@ input: help expect: stdout: |+ - rshell (dev) — 2 of 28 builtins enabled + rshell (dev) — 2 of 29 builtins enabled echo write arguments to stdout help display help for commands Disabled builtins: [, break, cat, continue, cut, exit, false, find, grep, head, ip, ls, ping, - printf, ps, sed, sort, ss, strings, tail, test, tr, true, uname, uniq, wc + printf, ps, pwd, sed, sort, ss, strings, tail, test, tr, true, uname, uniq, wc Run 'help ' for more information on a specific command. stderr: "" diff --git a/tests/scenarios/cmd/help/restricted_all_flag.yaml b/tests/scenarios/cmd/help/restricted_all_flag.yaml index b7b077c5..0bedc995 100644 --- a/tests/scenarios/cmd/help/restricted_all_flag.yaml +++ b/tests/scenarios/cmd/help/restricted_all_flag.yaml @@ -6,7 +6,7 @@ input: help --all expect: stdout: |+ - rshell (dev) — 2 of 28 builtins enabled + rshell (dev) — 2 of 29 builtins enabled echo write arguments to stdout help display help for commands @@ -27,6 +27,7 @@ expect: ping send ICMP echo requests to a network host printf format and print data ps report process status + pwd print working directory sed stream editor for filtering and transforming text sort sort lines of text files ss display socket statistics diff --git a/tests/scenarios/cmd/help/unrestricted.yaml b/tests/scenarios/cmd/help/unrestricted.yaml index 3b2d164c..4a1ad7a2 100644 --- a/tests/scenarios/cmd/help/unrestricted.yaml +++ b/tests/scenarios/cmd/help/unrestricted.yaml @@ -5,7 +5,7 @@ input: help expect: stdout: |+ - rshell (dev) — All 28 builtins available + rshell (dev) — All 29 builtins available [ evaluate conditional expression break exit from a loop @@ -24,6 +24,7 @@ expect: ping send ICMP echo requests to a network host printf format and print data ps report process status + pwd print working directory sed stream editor for filtering and transforming text sort sort lines of text files ss display socket statistics diff --git a/tests/scenarios/cmd/help/unrestricted_all_flag.yaml b/tests/scenarios/cmd/help/unrestricted_all_flag.yaml index fc0b019a..70515b42 100644 --- a/tests/scenarios/cmd/help/unrestricted_all_flag.yaml +++ b/tests/scenarios/cmd/help/unrestricted_all_flag.yaml @@ -5,7 +5,7 @@ input: help --all expect: stdout: |+ - rshell (dev) — All 28 builtins available + rshell (dev) — All 29 builtins available [ evaluate conditional expression break exit from a loop @@ -24,6 +24,7 @@ expect: ping send ICMP echo requests to a network host printf format and print data ps report process status + pwd print working directory sed stream editor for filtering and transforming text sort sort lines of text files ss display socket statistics diff --git a/tests/scenarios/cmd/pwd/basic/exit_code.yaml b/tests/scenarios/cmd/pwd/basic/exit_code.yaml new file mode 100644 index 00000000..11ddb27d --- /dev/null +++ b/tests/scenarios/cmd/pwd/basic/exit_code.yaml @@ -0,0 +1,10 @@ +description: pwd succeeds with exit status 0. +input: + allowed_paths: ["$DIR"] + script: |+ + pwd + echo $? +expect: + stdout_contains: ["0"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml b/tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml new file mode 100644 index 00000000..ce905441 --- /dev/null +++ b/tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml @@ -0,0 +1,10 @@ +description: pwd ignores extra positional arguments to match GNU coreutils and bash. +skip_assert_against_bash: true +input: + allowed_paths: ["$DIR"] + script: |+ + pwd extra arguments here +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/basic/no_args.yaml b/tests/scenarios/cmd/pwd/basic/no_args.yaml new file mode 100644 index 00000000..a29f9f5c --- /dev/null +++ b/tests/scenarios/cmd/pwd/basic/no_args.yaml @@ -0,0 +1,9 @@ +description: pwd with no arguments writes one absolute path followed by a newline. +input: + allowed_paths: ["$DIR"] + script: |+ + pwd +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml b/tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml new file mode 100644 index 00000000..be564b31 --- /dev/null +++ b/tests/scenarios/cmd/pwd/errors/unknown_flag_long.yaml @@ -0,0 +1,9 @@ +description: pwd rejects an unknown long flag with a stderr diagnostic and exit 1. +skip_assert_against_bash: true +input: + script: |+ + pwd --no-such-flag +expect: + stdout: "" + stderr_contains: ["pwd:", "unknown flag"] + exit_code: 1 diff --git a/tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml b/tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml new file mode 100644 index 00000000..b4133866 --- /dev/null +++ b/tests/scenarios/cmd/pwd/errors/unknown_flag_short.yaml @@ -0,0 +1,9 @@ +description: pwd rejects an unknown short flag with a stderr diagnostic and exit 1. +skip_assert_against_bash: true +input: + script: |+ + pwd -x +expect: + stdout: "" + stderr_contains: ["pwd:", "unknown shorthand flag"] + exit_code: 1 diff --git a/tests/scenarios/cmd/pwd/errors/version_rejected.yaml b/tests/scenarios/cmd/pwd/errors/version_rejected.yaml new file mode 100644 index 00000000..d5b5aff9 --- /dev/null +++ b/tests/scenarios/cmd/pwd/errors/version_rejected.yaml @@ -0,0 +1,9 @@ +description: pwd does not support --version (not POSIX); rejects it with exit 1. +skip_assert_against_bash: true +input: + script: |+ + pwd --version +expect: + stdout: "" + stderr_contains: ["pwd:", "unknown flag"] + exit_code: 1 diff --git a/tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml b/tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml new file mode 100644 index 00000000..e8b93870 --- /dev/null +++ b/tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml @@ -0,0 +1,9 @@ +description: When -L then -P are both given, the last flag (-P) wins. +input: + allowed_paths: ["$DIR"] + script: |+ + pwd -L -P +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml b/tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml new file mode 100644 index 00000000..a2c2b085 --- /dev/null +++ b/tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml @@ -0,0 +1,9 @@ +description: When -P then -L are both given, the last flag (-L) wins. +input: + allowed_paths: ["$DIR"] + script: |+ + pwd -P -L +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/logical_long.yaml b/tests/scenarios/cmd/pwd/flags/logical_long.yaml new file mode 100644 index 00000000..1f89913a --- /dev/null +++ b/tests/scenarios/cmd/pwd/flags/logical_long.yaml @@ -0,0 +1,10 @@ +description: pwd --logical is an alias for -L. +skip_assert_against_bash: true +input: + allowed_paths: ["$DIR"] + script: |+ + pwd --logical +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/logical_short.yaml b/tests/scenarios/cmd/pwd/flags/logical_short.yaml new file mode 100644 index 00000000..78c7641e --- /dev/null +++ b/tests/scenarios/cmd/pwd/flags/logical_short.yaml @@ -0,0 +1,9 @@ +description: pwd -L prints the logical working directory. +input: + allowed_paths: ["$DIR"] + script: |+ + pwd -L +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/physical_long.yaml b/tests/scenarios/cmd/pwd/flags/physical_long.yaml new file mode 100644 index 00000000..92f4b240 --- /dev/null +++ b/tests/scenarios/cmd/pwd/flags/physical_long.yaml @@ -0,0 +1,10 @@ +description: pwd --physical is an alias for -P. +skip_assert_against_bash: true +input: + allowed_paths: ["$DIR"] + script: |+ + pwd --physical +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/physical_short.yaml b/tests/scenarios/cmd/pwd/flags/physical_short.yaml new file mode 100644 index 00000000..0202fdbf --- /dev/null +++ b/tests/scenarios/cmd/pwd/flags/physical_short.yaml @@ -0,0 +1,9 @@ +description: pwd -P prints the physical working directory inside the sandbox. +input: + allowed_paths: ["$DIR"] + script: |+ + pwd -P +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/hardening/double_dash.yaml b/tests/scenarios/cmd/pwd/hardening/double_dash.yaml new file mode 100644 index 00000000..ffc62168 --- /dev/null +++ b/tests/scenarios/cmd/pwd/hardening/double_dash.yaml @@ -0,0 +1,10 @@ +description: pwd accepts -- as the end-of-flags separator and ignores trailing positional args. +skip_assert_against_bash: true +input: + allowed_paths: ["$DIR"] + script: |+ + pwd -- --not-a-flag +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/hardening/in_subshell.yaml b/tests/scenarios/cmd/pwd/hardening/in_subshell.yaml new file mode 100644 index 00000000..7326752f --- /dev/null +++ b/tests/scenarios/cmd/pwd/hardening/in_subshell.yaml @@ -0,0 +1,11 @@ +description: pwd inside a command substitution captures the working directory. +input: + allowed_paths: ["$DIR"] + script: |+ + here=$(pwd) + if [ -n "$here" ]; then echo nonempty; else echo empty; fi +expect: + stdout: |+ + nonempty + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml b/tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml new file mode 100644 index 00000000..e4de2038 --- /dev/null +++ b/tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml @@ -0,0 +1,9 @@ +description: pwd does not consume stdin; piped input is ignored. +input: + allowed_paths: ["$DIR"] + script: |+ + echo not-consumed | pwd +expect: + stdout_contains: ["/"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/help/help_flag_long.yaml b/tests/scenarios/cmd/pwd/help/help_flag_long.yaml new file mode 100644 index 00000000..ff0235f4 --- /dev/null +++ b/tests/scenarios/cmd/pwd/help/help_flag_long.yaml @@ -0,0 +1,9 @@ +description: pwd --help prints usage to stdout and exits 0. +skip_assert_against_bash: true +input: + script: |+ + pwd --help +expect: + stdout_contains: ["Usage: pwd", "logical", "physical"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/help/help_flag_short.yaml b/tests/scenarios/cmd/pwd/help/help_flag_short.yaml new file mode 100644 index 00000000..93d7d1b8 --- /dev/null +++ b/tests/scenarios/cmd/pwd/help/help_flag_short.yaml @@ -0,0 +1,9 @@ +description: pwd -h prints usage to stdout and exits 0. +skip_assert_against_bash: true +input: + script: |+ + pwd -h +expect: + stdout_contains: ["Usage: pwd"] + stderr: "" + exit_code: 0 From 46bb0670170a884473b4eaeff7dd291a8efb6bd7 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 14:48:59 +0200 Subject: [PATCH 2/8] fix(pwd): use platform-appropriate absolute path in internal tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, paths constructed from `string(filepath.Separator)` alone yield `\foo`, which filepath.IsAbs rejects — Windows requires a drive letter. Introduce an `absRoot` helper that returns "/" on Unix and `C:\` on Windows, and use it everywhere the internal tests need an absolute root. Co-Authored-By: Claude Opus 4.7 (1M context) --- builtins/pwd/pwd_internal_test.go | 47 ++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/builtins/pwd/pwd_internal_test.go b/builtins/pwd/pwd_internal_test.go index c9e8d702..0675e961 100644 --- a/builtins/pwd/pwd_internal_test.go +++ b/builtins/pwd/pwd_internal_test.go @@ -23,6 +23,18 @@ import ( "github.com/DataDog/rshell/builtins" ) +// absRoot is an absolute root suitable for tests on the host platform: +// "/" on Unix and `C:\` on Windows. Constructing test paths from +// `string(filepath.Separator)` alone yields `\foo` on Windows, which +// filepath.IsAbs rejects (Windows requires a drive letter for absolute +// paths). +var absRoot = func() string { + if filepath.Separator == '\\' { + return `C:\` + } + return "/" +}() + // fakeFileInfo is a minimal io/fs.FileInfo implementation for tests. type fakeFileInfo struct { mode iofs.FileMode @@ -82,13 +94,13 @@ func TestResolveSymlinksRejectsRelativePath(t *testing.T) { func TestResolveSymlinksMissingCallbacksError(t *testing.T) { // Missing LstatFile callback. cc := &builtins.CallContext{ReadlinkFile: func(_ context.Context, _ string) (string, error) { return "", nil }} - _, err := resolveSymlinks(context.Background(), cc, string(filepath.Separator)+"foo") + _, err := resolveSymlinks(context.Background(), cc, absRoot+"foo") require.Error(t, err) assert.Contains(t, err.Error(), "sandbox does not support symlink resolution") // Missing ReadlinkFile callback. cc2 := &builtins.CallContext{LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { return nil, nil }} - _, err = resolveSymlinks(context.Background(), cc2, string(filepath.Separator)+"foo") + _, err = resolveSymlinks(context.Background(), cc2, absRoot+"foo") require.Error(t, err) assert.Contains(t, err.Error(), "sandbox does not support symlink resolution") } @@ -100,7 +112,7 @@ func TestResolveSymlinksContextCancelled(t *testing.T) { } ctx, cancel := context.WithCancel(context.Background()) cancel() - _, err := resolveSymlinks(ctx, cc, string(filepath.Separator)+"foo"+string(filepath.Separator)+"bar") + _, err := resolveSymlinks(ctx, cc, absRoot+"foo"+string(filepath.Separator)+"bar") require.Error(t, err) assert.True(t, errors.Is(err, context.Canceled)) } @@ -126,9 +138,9 @@ func TestResolveSymlinksHandlesDotAndDotDot(t *testing.T) { // branches are reached by symlink targets containing those segments, // which TestPwdPhysicalDotDotResolvesAcrossDepth already exercises // from end-to-end. Here we stick to a simple absolute path. - out, err := resolveSymlinks(context.Background(), cc, sep+"a"+sep+"b") + out, err := resolveSymlinks(context.Background(), cc, absRoot+"a"+sep+"b") require.NoError(t, err) - assert.Equal(t, sep+"a"+sep+"b", out) + assert.Equal(t, absRoot+"a"+sep+"b", out) } // --- Symlink loop detection at the maxSymlinkHops cap --- @@ -144,8 +156,7 @@ func TestResolveSymlinksLoopDetected(t *testing.T) { return "self", nil }, } - sep := string(filepath.Separator) - _, err := resolveSymlinks(context.Background(), cc, sep+"a") + _, err := resolveSymlinks(context.Background(), cc, absRoot+"a") require.Error(t, err) assert.True(t, errors.Is(err, errSymlinkLoop)) } @@ -216,10 +227,10 @@ func TestResolveSymlinksReadlinkFails(t *testing.T) { }, } sep := string(filepath.Separator) - out, err := resolveSymlinks(context.Background(), cc, sep+"a"+sep+"b") + out, err := resolveSymlinks(context.Background(), cc, absRoot+"a"+sep+"b") require.NoError(t, err) // Both components got passed through. - assert.Equal(t, sep+"a"+sep+"b", out) + assert.Equal(t, absRoot+"a"+sep+"b", out) } // --- A symlink target containing "." segments exercises the dot branch. @@ -239,13 +250,13 @@ func TestResolveSymlinksDotInTarget(t *testing.T) { }, ReadlinkFile: func(_ context.Context, _ string) (string, error) { return "./real", nil }, } - sep := string(filepath.Separator) - out, err := resolveSymlinks(context.Background(), cc, sep+"lnk") + out, err := resolveSymlinks(context.Background(), cc, absRoot+"lnk") require.NoError(t, err) - assert.Equal(t, sep+"real", out) + assert.Equal(t, absRoot+"real", out) } -// --- Symlink target of "/" leaves rest empty after the leading-sep strip. --- +// --- Symlink target of the absolute root leaves rest empty after the +// leading-sep strip. --- func TestResolveSymlinksTargetIsRoot(t *testing.T) { cc := &builtins.CallContext{ @@ -253,13 +264,15 @@ func TestResolveSymlinksTargetIsRoot(t *testing.T) { return &fakeFileInfo{mode: iofs.ModeSymlink}, nil }, ReadlinkFile: func(_ context.Context, _ string) (string, error) { - return string(filepath.Separator), nil // target is just "/" + // Target is the absolute root: "/" on Unix, `C:\` on Windows. + return absRoot, nil }, } - sep := string(filepath.Separator) - out, err := resolveSymlinks(context.Background(), cc, sep+"x") + out, err := resolveSymlinks(context.Background(), cc, absRoot+"x") require.NoError(t, err) - assert.Equal(t, sep, out) + // rootPrefix returns "/" on Unix and `C:\` on Windows; both equal + // absRoot in this test. + assert.Equal(t, absRoot, out) } // --- Logical path is returned unchanged for -L (no filesystem touched) --- From 78e1a3f79e6039852201a6c5ce0a549f71eaab6d Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 14:56:02 +0200 Subject: [PATCH 3/8] fix(pwd): normalize relative symlink targets via filepath.Clean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A relative symlink target like "./real" uses forward slashes, but the walking loop in resolveSymlinks splits on filepath.Separator — `\` on Windows. Without normalization the target is treated as one opaque component and the resolved path ended up containing a literal "./" segment (e.g. `C:\./real` instead of `C:\real`). filepath.Clean collapses "." segments and converts "/" to "\" on Windows, so the splice produces a correctly-tokenized rest for the resolver to walk. Co-Authored-By: Claude Opus 4.7 (1M context) --- builtins/pwd/pwd.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/builtins/pwd/pwd.go b/builtins/pwd/pwd.go index 2d6a7943..c152a2fc 100644 --- a/builtins/pwd/pwd.go +++ b/builtins/pwd/pwd.go @@ -237,7 +237,13 @@ func resolveSymlinks(ctx context.Context, callCtx *builtins.CallContext, path st out = rootPrefix(cleanedTarget) rest = strings.TrimPrefix(cleanedTarget, out) + rest } else { - rest = string(filepath.Separator) + target + rest + // filepath.Clean normalizes the relative target — collapsing + // "." segments and converting forward slashes to the host + // separator on Windows. Without it, a target like "./real" + // keeps its forward slash on Windows and the walking loop + // (which splits on filepath.Separator) treats "./real" as a + // single opaque component. + rest = string(filepath.Separator) + filepath.Clean(target) + rest } } From 9c93caf2f26e7138bdac2edaa9fd50e29158a170 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 15:09:50 +0200 Subject: [PATCH 4/8] fix(pwd): add Windows stdout_contains overrides for path separator Windows pwd output uses backslash separators, so the existing `stdout_contains: ["/"]` assertion fails on the windows-latest runner. Add `stdout_contains_windows: ["\\\\"]` to every pwd scenario that asserts a substring of the printed path. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml | 1 + tests/scenarios/cmd/pwd/basic/no_args.yaml | 1 + tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml | 1 + tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml | 1 + tests/scenarios/cmd/pwd/flags/logical_long.yaml | 1 + tests/scenarios/cmd/pwd/flags/logical_short.yaml | 1 + tests/scenarios/cmd/pwd/flags/physical_long.yaml | 1 + tests/scenarios/cmd/pwd/flags/physical_short.yaml | 1 + tests/scenarios/cmd/pwd/hardening/double_dash.yaml | 1 + tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml | 1 + 10 files changed, 10 insertions(+) diff --git a/tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml b/tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml index ce905441..790e8956 100644 --- a/tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml +++ b/tests/scenarios/cmd/pwd/basic/extra_args_ignored.yaml @@ -6,5 +6,6 @@ input: pwd extra arguments here expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/basic/no_args.yaml b/tests/scenarios/cmd/pwd/basic/no_args.yaml index a29f9f5c..04a375c0 100644 --- a/tests/scenarios/cmd/pwd/basic/no_args.yaml +++ b/tests/scenarios/cmd/pwd/basic/no_args.yaml @@ -5,5 +5,6 @@ input: pwd expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml b/tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml index e8b93870..7ab3467d 100644 --- a/tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml +++ b/tests/scenarios/cmd/pwd/flags/last_wins_l_then_p.yaml @@ -5,5 +5,6 @@ input: pwd -L -P expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml b/tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml index a2c2b085..6b6296b0 100644 --- a/tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml +++ b/tests/scenarios/cmd/pwd/flags/last_wins_p_then_l.yaml @@ -5,5 +5,6 @@ input: pwd -P -L expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/logical_long.yaml b/tests/scenarios/cmd/pwd/flags/logical_long.yaml index 1f89913a..a86a3522 100644 --- a/tests/scenarios/cmd/pwd/flags/logical_long.yaml +++ b/tests/scenarios/cmd/pwd/flags/logical_long.yaml @@ -6,5 +6,6 @@ input: pwd --logical expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/logical_short.yaml b/tests/scenarios/cmd/pwd/flags/logical_short.yaml index 78c7641e..ec61e618 100644 --- a/tests/scenarios/cmd/pwd/flags/logical_short.yaml +++ b/tests/scenarios/cmd/pwd/flags/logical_short.yaml @@ -5,5 +5,6 @@ input: pwd -L expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/physical_long.yaml b/tests/scenarios/cmd/pwd/flags/physical_long.yaml index 92f4b240..3ca184f7 100644 --- a/tests/scenarios/cmd/pwd/flags/physical_long.yaml +++ b/tests/scenarios/cmd/pwd/flags/physical_long.yaml @@ -6,5 +6,6 @@ input: pwd --physical expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/flags/physical_short.yaml b/tests/scenarios/cmd/pwd/flags/physical_short.yaml index 0202fdbf..575448d7 100644 --- a/tests/scenarios/cmd/pwd/flags/physical_short.yaml +++ b/tests/scenarios/cmd/pwd/flags/physical_short.yaml @@ -5,5 +5,6 @@ input: pwd -P expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/hardening/double_dash.yaml b/tests/scenarios/cmd/pwd/hardening/double_dash.yaml index ffc62168..e57ef161 100644 --- a/tests/scenarios/cmd/pwd/hardening/double_dash.yaml +++ b/tests/scenarios/cmd/pwd/hardening/double_dash.yaml @@ -6,5 +6,6 @@ input: pwd -- --not-a-flag expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 diff --git a/tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml b/tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml index e4de2038..1b0cf327 100644 --- a/tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml +++ b/tests/scenarios/cmd/pwd/hardening/stdin_ignored.yaml @@ -5,5 +5,6 @@ input: echo not-consumed | pwd expect: stdout_contains: ["/"] + stdout_contains_windows: ["\\"] stderr: "" exit_code: 0 From 6ba4c378833847c46d7d7e20e767c388a813c963 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 15:51:12 +0200 Subject: [PATCH 5/8] fix(pwd): address Codex review on -L/-P last-wins and explicit values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs surfaced by the Codex review: 1. pflag.FlagSet.Visit walks set flags in lexicographical (or declaration) order, never command-line order. The previous pickPhysical helper therefore picked the wrong mode whenever `-P -L` appeared in that order — for a symlinked cwd, `pwd -P -L` printed the physical path even though POSIX (and bash) say -L wins. 2. pflag.BoolP accepts explicit values like `--physical=false`. The handler used to treat any visited mode flag as true, so `pwd --physical=false` silently selected physical mode. GNU `/bin/pwd --physical=false` rejects the argument; we should too. Switch -L/-P to a shared boolSeqFlag (matching the head builtin's boolSeqFlag pattern). The flag's Set increments a shared counter and records the new value in pos, so we can compare pos fields after parsing to determine the actual command-line order. Set also rejects any value other than the NoOptDefVal "true", which makes `--physical=false` and friends fail with "option doesn't allow an argument". Add regression tests with a real symlinked cwd (the previous tests used a non-symlink temp dir, so the wrong mode was hidden), and internal tests asserting that explicit values are rejected. Co-Authored-By: Claude Opus 4.7 (1M context) --- builtins/pwd/pwd.go | 72 ++++++++++++++++++++++--------- builtins/pwd/pwd_internal_test.go | 24 +++++++++-- builtins/pwd/pwd_unix_test.go | 32 ++++++++++++++ 3 files changed, 103 insertions(+), 25 deletions(-) diff --git a/builtins/pwd/pwd.go b/builtins/pwd/pwd.go index c152a2fc..9016598b 100644 --- a/builtins/pwd/pwd.go +++ b/builtins/pwd/pwd.go @@ -77,12 +77,24 @@ var errSymlinkLoop = errors.New("too many levels of symbolic links") func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { helpFlag := fs.BoolP("help", "h", false, "print usage and exit") - // -L and -P are deliberately not bound to local pointers: the - // command-line ordering matters (last-wins per POSIX), so the - // handler consults pflag.FlagSet.Visit in pickPhysical(fs) - // rather than reading the bool values directly. - fs.BoolP("logical", "L", false, "use the shell's working directory, even if it contains symlinks (default)") - fs.BoolP("physical", "P", false, "resolve all symlinks before printing the path") + + // -L and -P share a sequence counter so that after parsing we can + // compare their pos fields to determine which appeared last on the + // command line. pflag calls Set() in parse order, so the last flag + // Set gets the highest pos value. We do this rather than relying on + // pflag.FlagSet.Visit because Visit walks in lexicographical (or + // declaration) order — never command-line order — which would make + // `-P -L` always pick the wrong mode. + // + // boolSeqFlag.Set also rejects explicit values (e.g. --logical=foo + // or --physical=false). NoOptDefVal="true" lets pflag accept the + // bare flag form; any other value yields an error, matching GNU + // `/bin/pwd --physical=false`. + var modeSeq int + logicalFlag := newBoolSeqFlag(&modeSeq) + physicalFlag := newBoolSeqFlag(&modeSeq) + fs.VarPF(logicalFlag, "logical", "L", "use the shell's working directory, even if it contains symlinks (default)").NoOptDefVal = "true" + fs.VarPF(physicalFlag, "physical", "P", "resolve all symlinks before printing the path").NoOptDefVal = "true" return func(ctx context.Context, callCtx *builtins.CallContext, _ []string) builtins.Result { if *helpFlag { @@ -101,7 +113,7 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{Code: 1} } - if pickPhysical(fs) { + if physicalFlag.pos > logicalFlag.pos { // Best-effort: if symlink resolution fails (typically because // the cwd is the sandbox root and we cannot walk above it), // fall back to the logical path silently. Cycles still error @@ -119,22 +131,40 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { } } -// pickPhysical reports whether to resolve symlinks (-P). Walks fs.Visit -// in declaration order so that when both -L and -P appear, the last one -// wins (POSIX). When neither is set, returns false (logical default). -func pickPhysical(fs *builtins.FlagSet) bool { - usePhysical := false - fs.Visit(func(f *builtins.Flag) { - switch f.Name { - case "logical": - usePhysical = false - case "physical": - usePhysical = true - } - }) - return usePhysical +// boolSeqFlag is a pflag.Value implementation for -L/-P. Two boolSeqFlag +// values share a *seq counter; each call to Set increments the counter +// and records the new value in pos. After pflag.Parse, comparing pos +// fields reveals which flag appeared last on the command line. +// +// Set also rejects explicit values: pwd's mode flags are bare boolean +// switches and `pwd --physical=false` should fail like GNU coreutils +// rather than silently flip the mode. +type boolSeqFlag struct { + seq *int + pos int +} + +func newBoolSeqFlag(seq *int) *boolSeqFlag { + return &boolSeqFlag{seq: seq} } +func (f *boolSeqFlag) String() string { return "false" } + +func (f *boolSeqFlag) Set(s string) error { + // With NoOptDefVal="true", pflag passes "true" for the bare flag + // form and the user-supplied value for `--flag=value`. Accept only + // "true" (matching GNU coreutils, which rejects explicit values + // for -L/-P). + if s != "true" { + return errors.New("option doesn't allow an argument") + } + *f.seq++ + f.pos = *f.seq + return nil +} + +func (f *boolSeqFlag) Type() string { return "bool" } + func printHelp(callCtx *builtins.CallContext) { callCtx.Out("Usage: pwd [-LP] [--help]\n") callCtx.Out("Print the absolute pathname of the current working directory.\n\n") diff --git a/builtins/pwd/pwd_internal_test.go b/builtins/pwd/pwd_internal_test.go index 0675e961..6b9c2b2f 100644 --- a/builtins/pwd/pwd_internal_test.go +++ b/builtins/pwd/pwd_internal_test.go @@ -161,13 +161,29 @@ func TestResolveSymlinksLoopDetected(t *testing.T) { assert.True(t, errors.Is(err, errSymlinkLoop)) } -// --- pickPhysical with no flags set defaults to logical --- +// --- boolSeqFlag rejects explicit values --- -func TestPickPhysicalNoFlagsDefaultsLogical(t *testing.T) { +func TestBoolSeqFlagRejectsExplicitValue(t *testing.T) { fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) + fs.SetOutput(io.Discard) + makeFlags(fs) + // pwd --physical=false must fail like GNU coreutils — pflag accepts + // the explicit value, but our boolSeqFlag.Set rejects it. + err := fs.Parse([]string{"--physical=false"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "doesn't allow an argument") +} + +func TestBoolSeqFlagRejectsExplicitTrueLikeValue(t *testing.T) { + fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) + fs.SetOutput(io.Discard) makeFlags(fs) - require.NoError(t, fs.Parse([]string{})) - assert.False(t, pickPhysical(fs)) + // Even --logical=true is rejected: "true" alone matches NoOptDefVal, + // so the only way pflag passes us "true" is the bare-flag form. + // Anything else passed via =value path (including "TRUE", "1") fails. + err := fs.Parse([]string{"--logical=TRUE"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "doesn't allow an argument") } // --- joinPath / parentDir / rootPrefix unit cases --- diff --git a/builtins/pwd/pwd_unix_test.go b/builtins/pwd/pwd_unix_test.go index 3d82041a..78ea1b43 100644 --- a/builtins/pwd/pwd_unix_test.go +++ b/builtins/pwd/pwd_unix_test.go @@ -57,6 +57,38 @@ func TestPwdLogicalKeepsSymlink(t *testing.T) { assert.Equal(t, link+"\n", stdoutL) } +// TestPwdLastWinsPThenLWithSymlink: with a symlinked cwd, "pwd -P -L" +// must emit the logical (symlinked) path because -L appears last on +// the command line. This regression-tests the bug where pflag's Visit +// walks flags in lexicographical order rather than command-line order +// — without the boolSeqFlag pos-tracking, the wrong mode is selected +// even though both flags are present. +func TestPwdLastWinsPThenLWithSymlink(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "real") + link := filepath.Join(root, "lnk") + require.NoError(t, os.Mkdir(target, 0755)) + require.NoError(t, os.Symlink(target, link)) + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -P -L", link, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, link+"\n", stdout, "-P -L: -L wins, must emit logical path") +} + +// TestPwdLastWinsLThenPWithSymlink: the mirror case — -L then -P picks +// physical (the resolved target). +func TestPwdLastWinsLThenPWithSymlink(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "real") + link := filepath.Join(root, "lnk") + require.NoError(t, os.Mkdir(target, 0755)) + require.NoError(t, os.Symlink(target, link)) + + stdout, stderr, code := pwdRunDirAllowed(t, "pwd -L -P", link, root) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, target+"\n", stdout, "-L -P: -P wins, must emit physical path") +} + // TestPwdPhysicalChainedSymlinks: A -> B -> C resolves to C. func TestPwdPhysicalChainedSymlinks(t *testing.T) { root := t.TempDir() From d3cd8810158df762e34c8badda54418f9aa0ae34 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 17:01:59 +0200 Subject: [PATCH 6/8] fix(pwd): apply HostPrefix to absolute symlink targets and reject =true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two more findings from the Codex review: 1. P2: Container-style sandboxes mount the host filesystem at a prefix (e.g. /mnt/host) and on-disk symlinks often store host-absolute targets (e.g. /var/log/pods/app) without that prefix. The previous pwd -P resolver took the readlink string at face value and tried to walk it relative to root, so for a cwd reached via such a symlink the printed path was unreachable through the sandbox — `cat $(pwd)` would fail even though the file is allowed. Expose the runner's host prefix on CallContext and apply it to absolute symlink targets that don't already start with it. The resolved path now matches what the sandbox itself accepts. 2. P3: pwd --physical=true was silently accepted because pflag passed the literal "true" both for the bare flag (via NoOptDefVal="true") and for the explicit `=true` form, so boolSeqFlag.Set could not distinguish them. Switch NoOptDefVal to a private sentinel string so any explicit value — including "true" — fails Set's equality check and is rejected as "option doesn't allow an argument", matching GNU /bin/pwd. Tests: add unix tests for the HostPrefix translation (with and without double-prefix avoidance) and internal tests asserting that --logical=true / --physical=true are now rejected. Co-Authored-By: Claude Opus 4.7 (1M context) --- analysis/symbols_builtins.go | 3 ++ builtins/builtins.go | 9 +++++ builtins/pwd/pwd.go | 33 +++++++++++++---- builtins/pwd/pwd_internal_test.go | 25 +++++++++++-- builtins/pwd/pwd_unix_test.go | 60 +++++++++++++++++++++++++++++++ interp/runner_exec.go | 4 +++ 6 files changed, 124 insertions(+), 10 deletions(-) diff --git a/analysis/symbols_builtins.go b/analysis/symbols_builtins.go index 8c893bff..34a1a53a 100644 --- a/analysis/symbols_builtins.go +++ b/analysis/symbols_builtins.go @@ -208,8 +208,10 @@ var builtinPerCommandSymbols = map[string][]string{ "path/filepath.Clean", // 🟢 normalizes a path lexically (collapses ".", "..", duplicate separators); pure function, no I/O. "path/filepath.Dir", // 🟢 returns the directory component of a path; pure function, no I/O. "path/filepath.IsAbs", // 🟢 reports whether a path is absolute; pure function, no I/O. + "path/filepath.Join", // 🟢 lexically joins path components with the OS separator; pure function, no I/O. "path/filepath.Separator", // 🟢 OS path separator constant ('/' or '\\'); pure constant, no I/O. "path/filepath.VolumeName", // 🟢 returns the volume prefix of a path (e.g. "C:" on Windows, "" on Unix); pure function, no I/O. + "strings.HasPrefix", // 🟢 pure function for prefix matching; no I/O. "strings.IndexByte", // 🟢 finds byte in string; pure function, no I/O. "strings.TrimPrefix", // 🟢 removes a leading prefix from a string; pure function, no I/O. }, @@ -489,6 +491,7 @@ var builtinAllowedSymbols = []string{ "path/filepath.Clean", // 🟢 normalizes a path lexically (collapses ".", "..", duplicate separators); pure function, no I/O. "path/filepath.Dir", // 🟢 returns the directory component of a path; pure function, no I/O. "path/filepath.IsAbs", // 🟢 reports whether a path is absolute; pure function, no I/O. + "path/filepath.Join", // 🟢 lexically joins path components with the OS separator; pure function, no I/O. "path/filepath.Separator", // 🟢 OS path separator constant ('/' or '\\'); pure constant, no I/O. "path/filepath.ToSlash", // 🟢 converts OS path separators to forward slashes; pure function, no I/O. "path/filepath.VolumeName", // 🟢 returns the volume prefix of a path (e.g. "C:" on Windows, "" on Unix); pure function, no I/O. diff --git a/builtins/builtins.go b/builtins/builtins.go index 332948aa..8467e9d1 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -177,6 +177,15 @@ type CallContext struct { // Used by builtins that need to compute absolute paths for sub-operations. WorkDir func() string + // HostPrefix returns the configured host-mount prefix used by + // container-style sandboxes to translate host-absolute paths + // (e.g. /var/log/pods/...) into the prefixed paths the sandbox can + // open (e.g. /mnt/host/var/log/pods/...). Returns "" when no prefix + // is configured. Builtins that resolve absolute symlink targets + // (e.g. pwd -P) use this to keep their output consistent with what + // the sandbox itself accepts. + HostPrefix func() string + // RunCommand executes a builtin command within the shell's sandbox. // dir overrides the working directory for path resolution. // Returns the command's exit code. diff --git a/builtins/pwd/pwd.go b/builtins/pwd/pwd.go index 9016598b..0148ebad 100644 --- a/builtins/pwd/pwd.go +++ b/builtins/pwd/pwd.go @@ -75,6 +75,13 @@ const maxSymlinkHops = 40 // errSymlinkLoop is returned when -P resolution exceeds maxSymlinkHops. var errSymlinkLoop = errors.New("too many levels of symbolic links") +// boolSeqSentinel is the NoOptDefVal we register for -L/-P. pflag passes +// this exact string to Set when the user types the bare flag form +// (`pwd -P`). Any other value — including the literal "true" supplied +// via `--physical=true` — fails the equality check in Set and is +// rejected as "option doesn't allow an argument", matching GNU. +const boolSeqSentinel = "\x00rshell:pwd:bare\x00" + func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { helpFlag := fs.BoolP("help", "h", false, "print usage and exit") @@ -93,8 +100,8 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { var modeSeq int logicalFlag := newBoolSeqFlag(&modeSeq) physicalFlag := newBoolSeqFlag(&modeSeq) - fs.VarPF(logicalFlag, "logical", "L", "use the shell's working directory, even if it contains symlinks (default)").NoOptDefVal = "true" - fs.VarPF(physicalFlag, "physical", "P", "resolve all symlinks before printing the path").NoOptDefVal = "true" + fs.VarPF(logicalFlag, "logical", "L", "use the shell's working directory, even if it contains symlinks (default)").NoOptDefVal = boolSeqSentinel + fs.VarPF(physicalFlag, "physical", "P", "resolve all symlinks before printing the path").NoOptDefVal = boolSeqSentinel return func(ctx context.Context, callCtx *builtins.CallContext, _ []string) builtins.Result { if *helpFlag { @@ -151,11 +158,11 @@ func newBoolSeqFlag(seq *int) *boolSeqFlag { func (f *boolSeqFlag) String() string { return "false" } func (f *boolSeqFlag) Set(s string) error { - // With NoOptDefVal="true", pflag passes "true" for the bare flag - // form and the user-supplied value for `--flag=value`. Accept only - // "true" (matching GNU coreutils, which rejects explicit values - // for -L/-P). - if s != "true" { + // pflag passes NoOptDefVal (boolSeqSentinel) when the user types the + // bare flag form, and the user-supplied value for `--flag=value`. + // Reject anything that isn't the sentinel — including the literal + // "true", which GNU `/bin/pwd --physical=true` also rejects. + if s != boolSeqSentinel { return errors.New("option doesn't allow an argument") } *f.seq++ @@ -264,6 +271,18 @@ func resolveSymlinks(ctx context.Context, callCtx *builtins.CallContext, path st if filepath.IsAbs(target) { cleanedTarget := filepath.Clean(target) + // Container-style sandboxes mount the host filesystem at a + // prefix (e.g. /mnt/host). Symlink targets stored on disk + // often refer to host-absolute paths without that prefix + // (e.g. /var/log/pods/...), so apply the prefix when set + // — otherwise the resolved path would not be reachable + // through the sandbox and `pwd -P` output would be unusable + // for further filesystem operations. + if callCtx.HostPrefix != nil { + if hp := callCtx.HostPrefix(); hp != "" && !strings.HasPrefix(cleanedTarget, hp+string(filepath.Separator)) && cleanedTarget != hp { + cleanedTarget = filepath.Join(hp, cleanedTarget) + } + } out = rootPrefix(cleanedTarget) rest = strings.TrimPrefix(cleanedTarget, out) + rest } else { diff --git a/builtins/pwd/pwd_internal_test.go b/builtins/pwd/pwd_internal_test.go index 6b9c2b2f..e69e9ee9 100644 --- a/builtins/pwd/pwd_internal_test.go +++ b/builtins/pwd/pwd_internal_test.go @@ -174,18 +174,37 @@ func TestBoolSeqFlagRejectsExplicitValue(t *testing.T) { assert.Contains(t, err.Error(), "doesn't allow an argument") } +func TestBoolSeqFlagRejectsExplicitTrueValue(t *testing.T) { + // `--logical=true` must also be rejected — GNU `/bin/pwd --logical=true` + // fails the same way. The sentinel for NoOptDefVal makes this work: + // pflag passes the literal "true" for `=true`, but "true" is not the + // sentinel, so Set rejects it. + fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) + fs.SetOutput(io.Discard) + makeFlags(fs) + err := fs.Parse([]string{"--logical=true"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "doesn't allow an argument") +} + func TestBoolSeqFlagRejectsExplicitTrueLikeValue(t *testing.T) { fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) fs.SetOutput(io.Discard) makeFlags(fs) - // Even --logical=true is rejected: "true" alone matches NoOptDefVal, - // so the only way pflag passes us "true" is the bare-flag form. - // Anything else passed via =value path (including "TRUE", "1") fails. err := fs.Parse([]string{"--logical=TRUE"}) require.Error(t, err) assert.Contains(t, err.Error(), "doesn't allow an argument") } +func TestBoolSeqFlagBareFormAccepted(t *testing.T) { + // Sanity check: the bare flag form must still succeed. + fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) + fs.SetOutput(io.Discard) + makeFlags(fs) + require.NoError(t, fs.Parse([]string{"-L"})) + require.NoError(t, fs.Parse([]string{"--physical"})) +} + // --- joinPath / parentDir / rootPrefix unit cases --- func TestJoinPathEmptyDir(t *testing.T) { diff --git a/builtins/pwd/pwd_unix_test.go b/builtins/pwd/pwd_unix_test.go index 78ea1b43..55f0980e 100644 --- a/builtins/pwd/pwd_unix_test.go +++ b/builtins/pwd/pwd_unix_test.go @@ -192,3 +192,63 @@ func TestPwdPhysicalDotDotResolvesAcrossDepth(t *testing.T) { require.Equal(t, 0, code, "stderr=%q", stderr) assert.Equal(t, filepath.Join(root, "d3")+"\n", stdout) } + +// TestPwdPhysicalAppliesHostPrefixToAbsoluteSymlinkTarget: in a +// container-style sandbox where AllowedPaths roots live under a host +// mount prefix and on-disk symlinks store host-absolute targets, +// `pwd -P` must apply the HostPrefix so the printed path is reachable +// through the sandbox. Without the prefix, the output is the literal +// readlink string (e.g. /var/log/pods/app), which the user cannot +// access via further filesystem operations. +// +// Layout: +// +// $root/host/var/log/pods/app/ (real dir) +// $root/host/var/log/containers/app (symlink to /var/log/pods/app) +// +// HostPrefix = $root/host. AllowedPaths = $root/host/var/log/. +// cd into containers/app, then `pwd -P` must emit +// $root/host/var/log/pods/app, not /var/log/pods/app. +func TestPwdPhysicalAppliesHostPrefixToAbsoluteSymlinkTarget(t *testing.T) { + root := t.TempDir() + hostPrefix := filepath.Join(root, "host") + pods := filepath.Join(hostPrefix, "var", "log", "pods", "app") + containers := filepath.Join(hostPrefix, "var", "log", "containers") + require.NoError(t, os.MkdirAll(pods, 0755)) + require.NoError(t, os.MkdirAll(containers, 0755)) + link := filepath.Join(containers, "app") + // Host-absolute target without the prefix — typical of container + // log directories where pods/containers are bind-mounted from the + // host filesystem. + require.NoError(t, os.Symlink("/var/log/pods/app", link)) + + allowedRoot := filepath.Join(hostPrefix, "var", "log") + stdout, stderr, code := testutil.RunScript(t, "pwd -P", link, + interp.AllowedPaths([]string{allowedRoot}), + interp.HostPrefix(hostPrefix), + ) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, pods+"\n", stdout, "host-absolute symlink target must be prefixed with HostPrefix") +} + +// TestPwdPhysicalSkipsHostPrefixWhenAlreadyApplied: if the resolved +// target already begins with the host prefix (e.g. a relative symlink +// stayed within the prefixed tree), HostPrefix should not be applied +// again. +func TestPwdPhysicalSkipsHostPrefixWhenAlreadyApplied(t *testing.T) { + root := t.TempDir() + hostPrefix := filepath.Join(root, "host") + target := filepath.Join(hostPrefix, "real") + link := filepath.Join(hostPrefix, "lnk") + require.NoError(t, os.MkdirAll(target, 0755)) + // Absolute target already includes the host prefix — must not be + // double-prefixed. + require.NoError(t, os.Symlink(target, link)) + + stdout, stderr, code := testutil.RunScript(t, "pwd -P", link, + interp.AllowedPaths([]string{hostPrefix}), + interp.HostPrefix(hostPrefix), + ) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, target+"\n", stdout) +} diff --git a/interp/runner_exec.go b/interp/runner_exec.go index aa19a63f..b0f99827 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -349,6 +349,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { Stdout: r.stdout, Stderr: r.stderr, WorkDir: func() string { return dir }, + HostPrefix: func() string { return r.hostPrefix }, RunCommand: runCmd, OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) { f, err := r.sandbox.Open(path, dir, flags, mode) @@ -412,6 +413,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { WorkDir: func() string { return HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir }, + HostPrefix: func() string { + return r.hostPrefix + }, OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) { f, err := r.open(ctx, path, flags, mode, false) if err != nil { From 775b92ee5beb15848b6cceb197cb205c75de5928 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 17:35:45 +0200 Subject: [PATCH 7/8] fix(pwd): propagate context cancellation from -P resolution Codex flagged that resolveSymlinks can return context.Canceled or context.DeadlineExceeded when the run is interrupted mid-walk, but the handler treated every non-loop error as a sandbox-miss and fell back to printing the logical path with exit 0. RULES.md requires graceful handling of cancellation, and reporting success with a stale path is misleading. Add a context-cancel check between the loop-error branch and the silent fallback: if ctx.Err() is set, exit 1 without writing anything. The sandbox-miss best-effort case (resolver returned a non-loop, non-cancel error) keeps its logical-path fallback. Co-Authored-By: Claude Opus 4.7 (1M context) --- builtins/pwd/pwd.go | 13 +++++++++++-- builtins/pwd/pwd_internal_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/builtins/pwd/pwd.go b/builtins/pwd/pwd.go index 0148ebad..4023f855 100644 --- a/builtins/pwd/pwd.go +++ b/builtins/pwd/pwd.go @@ -125,11 +125,20 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // the cwd is the sandbox root and we cannot walk above it), // fall back to the logical path silently. Cycles still error // because they indicate corrupt input, not a sandbox limit. - if resolved, err := resolveSymlinks(ctx, callCtx, cwd); err == nil { + // + // Context cancellation is *not* a best-effort case: if the + // run is being interrupted, we must not report success with + // a stale logical path. RULES.md requires graceful handling + // of cancellation; we exit 1 without writing. + resolved, err := resolveSymlinks(ctx, callCtx, cwd) + switch { + case err == nil: cwd = resolved - } else if errors.Is(err, errSymlinkLoop) { + case errors.Is(err, errSymlinkLoop): callCtx.Errf("pwd: %s\n", err) return builtins.Result{Code: 1} + case ctx.Err() != nil: + return builtins.Result{Code: 1} } } diff --git a/builtins/pwd/pwd_internal_test.go b/builtins/pwd/pwd_internal_test.go index e69e9ee9..8f02cc9e 100644 --- a/builtins/pwd/pwd_internal_test.go +++ b/builtins/pwd/pwd_internal_test.go @@ -117,6 +117,37 @@ func TestResolveSymlinksContextCancelled(t *testing.T) { assert.True(t, errors.Is(err, context.Canceled)) } +// TestPwdPhysicalCancelledDoesNotEmitLogical: if the context is +// canceled during -P resolution, the handler must return exit 1 +// without emitting the logical (stale) path. Falling back silently +// would let a canceled run report success and stash a misleading +// path on stdout. +func TestPwdPhysicalCancelledDoesNotEmitLogical(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // pre-cancel so resolveSymlinks sees ctx.Err() on first iteration + cc := &builtins.CallContext{ + WorkDir: func() string { return absRoot + "some" + string(filepath.Separator) + "path" }, + LstatFile: func(_ context.Context, _ string) (iofs.FileInfo, error) { + return &fakeFileInfo{mode: iofs.ModeDir}, nil + }, + ReadlinkFile: func(_ context.Context, _ string) (string, error) { + return "", errors.New("not a symlink") + }, + } + var sout, serr bytes.Buffer + cc.Stdout = &sout + cc.Stderr = &serr + + fs := pflag.NewFlagSet("pwd", pflag.ContinueOnError) + fs.SetOutput(io.Discard) + handler := makeFlags(fs) + require.NoError(t, fs.Parse([]string{"-P"})) + res := handler(ctx, cc, fs.Args()) + + assert.Equal(t, uint8(1), res.Code) + assert.Equal(t, "", sout.String(), "stdout must be empty when context is canceled") +} + // --- resolveSymlinks: dot and dot-dot components are collapsed even // when the lstat result is non-symlink. --- From 869e4caf85a1b425fac9de19a8cb697c3fbca93b Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Thu, 30 Apr 2026 18:40:28 +0200 Subject: [PATCH 8/8] fix(pwd): return sandbox's normalized HostPrefix to builtins Codex flagged that interp.HostPrefix("/mnt/host/") (trailing slash) breaks pwd -P: the sandbox normalizes the prefix via filepath.Clean in SetHostPrefix, but my CallContext callback returned the runner's raw r.hostPrefix. The unnormalized value defeats the HasPrefix check ("/mnt/host//" never matches a clean target) and filepath.Join then doubles the prefix, producing paths like "/mnt/host/mnt/host/var". Switch both CallContext.HostPrefix sites to call r.sandbox.HostPrefix(), which already returns the filepath.Clean'd value. The raw runner field stays as-is (still consumed by SetHostPrefix during init). Add a regression test that passes HostPrefix with a trailing slash and verifies the resolved path is not double-prefixed. Co-Authored-By: Claude Opus 4.7 (1M context) --- builtins/pwd/pwd_unix_test.go | 26 ++++++++++++++++++++++++++ interp/runner_exec.go | 26 ++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/builtins/pwd/pwd_unix_test.go b/builtins/pwd/pwd_unix_test.go index 55f0980e..3df41107 100644 --- a/builtins/pwd/pwd_unix_test.go +++ b/builtins/pwd/pwd_unix_test.go @@ -252,3 +252,29 @@ func TestPwdPhysicalSkipsHostPrefixWhenAlreadyApplied(t *testing.T) { require.Equal(t, 0, code, "stderr=%q", stderr) assert.Equal(t, target+"\n", stdout) } + +// TestPwdPhysicalHostPrefixWithTrailingSlash: a HostPrefix passed with +// a trailing separator must be normalized before pwd -P uses it for +// prefix-matching. Without normalization, a target that already +// includes the cleaned prefix would fail the HasPrefix check (because +// "$root/host/" + "/" != "$root/host//") and get doubled — emitting +// `$root/host/$root/host/real`. The CallContext.HostPrefix accessor +// returns the sandbox's filepath.Clean'd prefix, so this test pins +// the contract. +func TestPwdPhysicalHostPrefixWithTrailingSlash(t *testing.T) { + root := t.TempDir() + hostPrefix := filepath.Join(root, "host") + target := filepath.Join(hostPrefix, "real") + link := filepath.Join(hostPrefix, "lnk") + require.NoError(t, os.MkdirAll(target, 0755)) + require.NoError(t, os.Symlink(target, link)) + + // Pass HostPrefix with a trailing slash on purpose — this is the + // kind of input that previously caused double-prefixing. + stdout, stderr, code := testutil.RunScript(t, "pwd -P", link, + interp.AllowedPaths([]string{hostPrefix}), + interp.HostPrefix(hostPrefix+string(filepath.Separator)), + ) + require.Equal(t, 0, code, "stderr=%q", stderr) + assert.Equal(t, target+"\n", stdout, "trailing-slash HostPrefix must not produce a doubled-prefix path") +} diff --git a/interp/runner_exec.go b/interp/runner_exec.go index b0f99827..ceda385d 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -346,10 +346,20 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { return 127, fmt.Errorf("rshell: %s: unknown command", cmdName) } child := &builtins.CallContext{ - Stdout: r.stdout, - Stderr: r.stderr, - WorkDir: func() string { return dir }, - HostPrefix: func() string { return r.hostPrefix }, + Stdout: r.stdout, + Stderr: r.stderr, + WorkDir: func() string { return dir }, + HostPrefix: func() string { + // Return the sandbox's normalized prefix (filepath.Clean'd + // in SetHostPrefix) rather than the raw user-supplied + // value. A caller-provided trailing slash or "."/".." + // segment would otherwise break prefix-matching in + // builtins that consume this value. + if r.sandbox != nil { + return r.sandbox.HostPrefix() + } + return r.hostPrefix + }, RunCommand: runCmd, OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) { f, err := r.sandbox.Open(path, dir, flags, mode) @@ -414,6 +424,14 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { return HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir }, HostPrefix: func() string { + // Return the sandbox's normalized prefix (filepath.Clean'd + // in SetHostPrefix) rather than the raw user-supplied + // value. A caller-provided trailing slash or "."/".." + // segment would otherwise break prefix-matching in + // builtins that consume this value. + if r.sandbox != nil { + return r.sandbox.HostPrefix() + } return r.hostPrefix }, OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) {