From 863a9b36f6f9b411ef9b8a70e72d736c5db45011 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 2 Apr 2026 16:14:50 -0400 Subject: [PATCH 01/14] feat(allowedpaths): prepend /host to symlink targets in containers In containerized environments (DOCKER_DD_AGENT set), host symlinks use host-absolute paths (e.g. /var/log/pods/...) that don't include the /host mount prefix. When resolving cross-root symlinks, prepend /host to the target path so it matches the container's allowed roots. Refs #166 Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 19 +++++++++++++++++-- analysis/symbols_allowedpaths.go | 1 + 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 705dbe85..75956f05 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -37,11 +37,17 @@ type root struct { root *os.Root } +// hostMountPrefix is prepended to symlink targets when running inside a +// container. Host filesystems are mounted under this prefix, but symlinks +// created on the host use paths without it. +const hostMountPrefix = "/host" + // Sandbox restricts filesystem access to a set of allowed directories. // The restriction is enforced using os.Root (Go 1.24+), which uses openat // syscalls for atomic path validation — immune to symlink and ".." traversal attacks. type Sandbox struct { - roots []root + roots []root + containerized bool // true when running inside a container (DOCKER_DD_AGENT set) } // New creates a sandbox from an allowlist of directory paths. Paths that do @@ -69,7 +75,10 @@ func New(paths []string) (sb *Sandbox, warnings []byte, err error) { } roots = append(roots, root{absPath: abs, root: r}) } - return &Sandbox{roots: roots}, buf.Bytes(), nil + return &Sandbox{ + roots: roots, + containerized: os.Getenv("DOCKER_DD_AGENT") != "", + }, buf.Bytes(), nil } // isPathEscapeError reports whether err is the unexported "path escapes @@ -171,6 +180,12 @@ func (s *Sandbox) resolveRootFollowingSymlinks(absPath string, preserveLast bool target = filepath.Join(target, remaining) } absPath = filepath.Clean(target) + // In containers, host symlinks use host-absolute paths + // (e.g. /var/log/pods/...) that don't include the /host + // mount prefix. Prepend it so the path matches our roots. + if s.containerized && !strings.HasPrefix(absPath, hostMountPrefix+string(filepath.Separator)) { + absPath = filepath.Join(hostMountPrefix, absPath) + } symlinkFound = true break } diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index 73cc7c7b..ee60baac 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -38,6 +38,7 @@ var allowedpathsAllowedSymbols = []string{ "os.ErrPermission", // 🟢 sentinel error for permission denied; pure constant. "os.File", // 🟠 file handle returned by os.Root.Open; needed for cross-root symlink fallback. "os.FileMode", // 🟢 file permission bits type; pure type. + "os.Getenv", // 🟠 reads environment variable; used once at construction to detect containerized environments. "os.Getgid", // 🟠 returns the numeric group id of the caller; read-only syscall. "os.Getgroups", // 🟠 returns supplementary group ids; read-only syscall. "os.Getuid", // 🟠 returns the numeric user id of the caller; read-only syscall. From c553417d82c012bd4956ade3554613e7e038b983 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 2 Apr 2026 16:29:41 -0400 Subject: [PATCH 02/14] feat(allowedpaths): prepend host mount prefix to symlink targets in containers In containerized environments (DOCKER_DD_AGENT set), host symlinks use host-absolute paths (e.g. /var/log/pods/...) that don't include the /host mount prefix. When resolving cross-root symlinks, prepend the host prefix so the path matches the container's allowed roots. The host prefix is stored as a field on Sandbox (defaulting to /host) so tests can override it with temp dirs. Refs #166 Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 16 ++-- allowedpaths/sandbox_unix_test.go | 136 ++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 7 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 75956f05..2262ca96 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -37,17 +37,18 @@ type root struct { root *os.Root } -// hostMountPrefix is prepended to symlink targets when running inside a -// container. Host filesystems are mounted under this prefix, but symlinks -// created on the host use paths without it. -const hostMountPrefix = "/host" +// defaultHostMountPrefix is prepended to symlink targets when running +// inside a container. Host filesystems are mounted under this prefix, +// but symlinks created on the host use paths without it. +const defaultHostMountPrefix = "/host" // Sandbox restricts filesystem access to a set of allowed directories. // The restriction is enforced using os.Root (Go 1.24+), which uses openat // syscalls for atomic path validation — immune to symlink and ".." traversal attacks. type Sandbox struct { roots []root - containerized bool // true when running inside a container (DOCKER_DD_AGENT set) + containerized bool // true when running inside a container (DOCKER_DD_AGENT set) + hostPrefix string // mount prefix for host paths inside the container } // New creates a sandbox from an allowlist of directory paths. Paths that do @@ -78,6 +79,7 @@ func New(paths []string) (sb *Sandbox, warnings []byte, err error) { return &Sandbox{ roots: roots, containerized: os.Getenv("DOCKER_DD_AGENT") != "", + hostPrefix: defaultHostMountPrefix, }, buf.Bytes(), nil } @@ -183,8 +185,8 @@ func (s *Sandbox) resolveRootFollowingSymlinks(absPath string, preserveLast bool // In containers, host symlinks use host-absolute paths // (e.g. /var/log/pods/...) that don't include the /host // mount prefix. Prepend it so the path matches our roots. - if s.containerized && !strings.HasPrefix(absPath, hostMountPrefix+string(filepath.Separator)) { - absPath = filepath.Join(hostMountPrefix, absPath) + if s.containerized && !strings.HasPrefix(absPath, s.hostPrefix+string(filepath.Separator)) { + absPath = filepath.Join(s.hostPrefix, absPath) } symlinkFound = true break diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index f9568edc..6a3851f1 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -696,3 +696,139 @@ func TestCrossRootSymlinkSiblingDirs(t *testing.T) { assert.NoError(t, sb.Access(filepath.Join(dir2, "sym.txt"), "/", modeRead)) } + +// --- Container /host prefix tests --- + +// newContainerSandbox creates a sandbox with containerized=true and a +// custom host prefix, simulating a containerized environment where host +// filesystems are mounted under prefix. +func newContainerSandbox(t *testing.T, paths []string, hostPrefix string) *Sandbox { + t.Helper() + sb, _, err := New(paths) + require.NoError(t, err) + sb.containerized = true + sb.hostPrefix = hostPrefix + return sb +} + +// setupContainerDirs creates a directory structure simulating a container +// where host paths are mounted under a prefix. The symlink target is a +// "host-absolute" path (e.g. /var/log/pods/app.log) that only resolves +// after the host prefix is prepended. The symlink is dangling on the test +// machine, but our code reads it via Readlink and constructs the correct +// path. +func setupContainerDirs(t *testing.T) (hostPrefix, pods, containers string) { + t.Helper() + root := t.TempDir() + hostPrefix = root + pods = filepath.Join(root, "var", "log", "pods") + containers = filepath.Join(root, "var", "log", "containers") + require.NoError(t, os.MkdirAll(pods, 0755)) + require.NoError(t, os.MkdirAll(containers, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(pods, "app.log"), []byte("log line"), 0644)) + // Symlink points to "/var/log/pods/app.log" — a host-absolute path. + // This is dangling on the test machine, but after our code prepends + // hostPrefix (root), it becomes root/var/log/pods/app.log which exists. + require.NoError(t, os.Symlink("/var/log/pods/app.log", filepath.Join(containers, "app.log"))) + return hostPrefix, pods, containers +} + +// TestContainerSymlinkHostPrefixOpen verifies that a symlink using a +// host-absolute path (without the mount prefix) can be opened when +// containerized. +func TestContainerSymlinkHostPrefixOpen(t *testing.T) { + hostPrefix, pods, containers := setupContainerDirs(t) + + sb := newContainerSandbox(t, []string{pods, containers}, hostPrefix) + defer sb.Close() + + f, err := sb.Open("app.log", containers, os.O_RDONLY, 0) + require.NoError(t, err, "container symlink with host prefix should be readable") + defer f.Close() + + buf := make([]byte, 64) + n, _ := f.Read(buf) + assert.Equal(t, "log line", string(buf[:n])) +} + +// TestContainerSymlinkHostPrefixStat verifies Stat through a container +// symlink with host-absolute target. +func TestContainerSymlinkHostPrefixStat(t *testing.T) { + hostPrefix, pods, containers := setupContainerDirs(t) + + sb := newContainerSandbox(t, []string{pods, containers}, hostPrefix) + defer sb.Close() + + info, err := sb.Stat("app.log", containers) + require.NoError(t, err) + assert.Equal(t, int64(8), info.Size()) // "log line" = 8 bytes +} + +// TestContainerSymlinkHostPrefixAccess verifies Access through a container +// symlink with host-absolute target. +func TestContainerSymlinkHostPrefixAccess(t *testing.T) { + hostPrefix, pods, containers := setupContainerDirs(t) + + sb := newContainerSandbox(t, []string{pods, containers}, hostPrefix) + defer sb.Close() + + assert.NoError(t, sb.Access("app.log", containers, modeRead)) +} + +// TestContainerSymlinkHostPrefixNotAppliedWhenNotContainerized verifies +// that the host prefix is NOT prepended when not in a container. +func TestContainerSymlinkHostPrefixNotAppliedWhenNotContainerized(t *testing.T) { + // Use the same setup but don't enable containerized mode. + _, pods, containers := setupContainerDirs(t) + + sb, _, err := New([]string{pods, containers}) + require.NoError(t, err) + defer sb.Close() + + _, err = sb.Open("app.log", containers, os.O_RDONLY, 0) + assert.Error(t, err, "without container mode, host-absolute symlinks should fail") +} + +// TestContainerSymlinkHostPrefixAlreadyPresent verifies that the prefix +// is not double-prepended when the symlink target already includes it. +func TestContainerSymlinkHostPrefixAlreadyPresent(t *testing.T) { + hostPrefix, pods, containers := setupContainerDirs(t) + + // Overwrite the symlink to use a target that already starts with the + // host prefix (i.e. a container-aware path, not a host-absolute one). + require.NoError(t, os.Remove(filepath.Join(containers, "app.log"))) + require.NoError(t, os.Symlink(filepath.Join(pods, "app.log"), filepath.Join(containers, "app.log"))) + + sb := newContainerSandbox(t, []string{pods, containers}, hostPrefix) + defer sb.Close() + + f, err := sb.Open("app.log", containers, os.O_RDONLY, 0) + require.NoError(t, err, "symlink with prefix already present should not be double-prefixed") + defer f.Close() + + buf := make([]byte, 64) + n, _ := f.Read(buf) + assert.Equal(t, "log line", string(buf[:n])) +} + +// TestContainerSymlinkHostPrefixOutsideAllRoots verifies that a container +// symlink pointing outside all allowed roots is still blocked even with +// the host prefix applied. +func TestContainerSymlinkHostPrefixOutsideAllRoots(t *testing.T) { + root := t.TempDir() + hostPrefix := root + containers := filepath.Join(root, "var", "log", "containers") + require.NoError(t, os.MkdirAll(containers, 0755)) + // /etc/secret exists under the host prefix, but is NOT in allowed roots. + outside := filepath.Join(root, "etc", "secret") + require.NoError(t, os.MkdirAll(outside, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(outside, "passwd"), []byte("secret"), 0644)) + // Symlink uses host-absolute path to /etc/secret/passwd. + require.NoError(t, os.Symlink("/etc/secret/passwd", filepath.Join(containers, "escape.log"))) + + sb := newContainerSandbox(t, []string{containers}, hostPrefix) + defer sb.Close() + + _, err := sb.Open("escape.log", containers, os.O_RDONLY, 0) + assert.Error(t, err, "container symlink to path outside allowed roots should be blocked") +} From d05ae760191ae1ddd31d8f2b5be2a7911334545f Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 2 Apr 2026 16:56:09 -0400 Subject: [PATCH 03/14] feat(allowedpaths): add HostPrefix option and scenario test for container symlinks - Add SetHostPrefix on Sandbox and public HostPrefix RunnerOption - Add containerized field to scenario YAML that sets DOCKER_DD_AGENT and host prefix to testdir/host automatically - Groups with containerized scenarios skip t.Parallel (t.Setenv) - Add container_host_prefix_symlink.yaml end-to-end scenario test Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 6 ++++ interp/api.go | 13 +++++++ .../container_host_prefix_symlink.yaml | 19 ++++++++++ tests/scenarios_test.go | 35 +++++++++++++++---- 4 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 2262ca96..68e92710 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -620,6 +620,12 @@ func (s *Sandbox) Readlink(path string, cwd string) (string, error) { return target, nil } +// SetHostPrefix overrides the mount prefix used to translate host-absolute +// symlink targets inside containers. +func (s *Sandbox) SetHostPrefix(prefix string) { + s.hostPrefix = prefix +} + // Close releases all os.Root file descriptors. It is safe to call multiple times. func (s *Sandbox) Close() error { if s == nil { diff --git a/interp/api.go b/interp/api.go index 0f2bc50d..b33acdef 100644 --- a/interp/api.go +++ b/interp/api.go @@ -569,6 +569,19 @@ func AllowedPaths(paths []string) RunnerOption { } } +// HostPrefix sets the mount prefix used to translate host-absolute symlink +// targets inside containers. Defaults to "/host". Only takes effect when +// running in a containerized environment (DOCKER_DD_AGENT set). +// Must be applied after AllowedPaths. +func HostPrefix(prefix string) RunnerOption { + return func(r *Runner) error { + if r.sandbox != nil { + r.sandbox.SetHostPrefix(prefix) + } + return nil + } +} + // AllowedCommands restricts command execution to the specified command names. // Names must use the "rshell:" namespace prefix (e.g. "rshell:cat", // "rshell:find"). Names without a colon separator or with an unknown namespace diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml new file mode 100644 index 00000000..86fefbc5 --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml @@ -0,0 +1,19 @@ +description: Container symlink with host-absolute target is readable after /host prefix +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/var/log/pods/app.log + content: "container log\n" + - path: host/var/log/containers/app.log + symlink: /var/log/pods/app.log +input: + allowed_paths: ["host/var/log/pods", "host/var/log/containers"] + script: |+ + cat host/var/log/containers/app.log +expect: + stdout: |+ + container log + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios_test.go b/tests/scenarios_test.go index a3d6ccb8..51ac50d1 100644 --- a/tests/scenarios_test.go +++ b/tests/scenarios_test.go @@ -34,11 +34,14 @@ const dockerBashImage = "debian:bookworm-slim" // scenario represents a single test scenario. type scenario struct { - Description string `yaml:"description"` - SkipAssertAgainstBash bool `yaml:"skip_assert_against_bash"` // true = skip bash comparison - Setup setup `yaml:"setup"` - Input input `yaml:"input"` - Expect expected `yaml:"expect"` + Description string `yaml:"description"` + SkipAssertAgainstBash bool `yaml:"skip_assert_against_bash"` // true = skip bash comparison + // Containerized simulates a containerized environment by setting + // DOCKER_DD_AGENT before sandbox construction. Disables t.Parallel. + Containerized bool `yaml:"containerized"` + Setup setup `yaml:"setup"` + Input input `yaml:"input"` + Expect expected `yaml:"expect"` } // setup holds optional pre-test configuration such as files to create. @@ -216,6 +219,10 @@ func runScenario(t *testing.T, sc scenario) { // When allow_all_commands is explicitly false and allowed_commands is // empty, no AllowedCommands/AllowAllCommands option is added, so the // interpreter defaults to blocking all commands. + if sc.Containerized { + t.Setenv("DOCKER_DD_AGENT", "true") + opts = append(opts, interp.HostPrefix(filepath.Join(dir, "host"))) + } runner, err := interp.New(opts...) require.NoError(t, err, "failed to create runner") defer runner.Close() @@ -449,12 +456,26 @@ func TestShellScenarios(t *testing.T) { for group, paths := range groups { t.Run(group, func(t *testing.T) { - t.Parallel() + // Check if any scenario in this group needs containerized mode. + // If so, the group cannot be parallel (t.Setenv requirement). + hasContainerized := false + for _, path := range paths { + sc := loadScenario(t, path) + if sc.Containerized { + hasContainerized = true + break + } + } + if !hasContainerized { + t.Parallel() + } for _, path := range paths { sc := loadScenario(t, path) name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) t.Run(name, func(t *testing.T) { - t.Parallel() + if !sc.Containerized { + t.Parallel() + } runScenario(t, sc) }) } From abd098f3519ffac615b9070000a52f119c621c51 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 2 Apr 2026 17:00:18 -0400 Subject: [PATCH 04/14] test(allowedpaths): add container host prefix scenario tests 8 scenarios covering: - cat through container symlink (host/var/log as single allowed root) - ls directory listing through container symlink - grep through container symlink - multiple sequential reads - cat/head/tail/wc/grep all through same container symlink - blocked: symlink to /etc/secret outside allowed root - blocked: symlink to /opt/data outside allowed root - blocked: symlink using parent traversal to escape Co-Authored-By: Claude Opus 4.6 (1M context) --- .../container_host_prefix_file_commands.yaml | 48 +++++++++++++++++++ .../container_host_prefix_grep.yaml | 22 +++++++++ .../container_host_prefix_ls.yaml | 19 ++++++++ .../container_host_prefix_multiple_reads.yaml | 25 ++++++++++ ...container_host_prefix_no_allowed_root.yaml | 19 ++++++++ ...container_host_prefix_outside_blocked.yaml | 19 ++++++++ ...ontainer_host_prefix_parent_traversal.yaml | 19 ++++++++ .../container_host_prefix_symlink.yaml | 2 +- 8 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_file_commands.yaml create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_grep.yaml create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_ls.yaml create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_multiple_reads.yaml create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_no_allowed_root.yaml create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_outside_blocked.yaml create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_parent_traversal.yaml diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_file_commands.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_file_commands.yaml new file mode 100644 index 00000000..b8575286 --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_file_commands.yaml @@ -0,0 +1,48 @@ +description: File-reading commands work through container symlinks with host-absolute targets +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/var/log/pods/app/0.log + content: | + line1 + line2 + line3 + line4 + line5 + - path: host/var/log/containers/app.log + symlink: /var/log/pods/app/0.log +input: + allowed_paths: ["host/var/log"] + script: |+ + echo "=== cat ===" + cat host/var/log/containers/app.log + echo "=== head ===" + head -n 2 host/var/log/containers/app.log + echo "=== tail ===" + tail -n 2 host/var/log/containers/app.log + echo "=== wc ===" + wc -l host/var/log/containers/app.log + echo "=== grep ===" + grep line3 host/var/log/containers/app.log +expect: + stdout: |+ + === cat === + line1 + line2 + line3 + line4 + line5 + === head === + line1 + line2 + === tail === + line4 + line5 + === wc === + 5 host/var/log/containers/app.log + === grep === + line3 + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_grep.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_grep.yaml new file mode 100644 index 00000000..68963a82 --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_grep.yaml @@ -0,0 +1,22 @@ +description: grep works through container symlinks with host-absolute targets +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/var/log/pods/app/0.log + content: | + INFO starting up + ERROR something broke + INFO recovered + - path: host/var/log/containers/app.log + symlink: /var/log/pods/app/0.log +input: + allowed_paths: ["host/var/log"] + script: |+ + grep ERROR host/var/log/containers/app.log +expect: + stdout: |+ + ERROR something broke + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_ls.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_ls.yaml new file mode 100644 index 00000000..d521746f --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_ls.yaml @@ -0,0 +1,19 @@ +description: ls can list files through container symlinks with host-absolute targets +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/var/log/pods/pod-abc/0.log + content: "pod log\n" + - path: host/var/log/containers/app.log + symlink: /var/log/pods/pod-abc/0.log +input: + allowed_paths: ["host/var/log"] + script: |+ + ls host/var/log/containers/ +expect: + stdout: |+ + app.log + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_multiple_reads.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_multiple_reads.yaml new file mode 100644 index 00000000..bf351c8d --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_multiple_reads.yaml @@ -0,0 +1,25 @@ +description: Multiple container symlinks can be read in sequence +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/var/log/pods/pod-a/0.log + content: "log a\n" + - path: host/var/log/pods/pod-b/0.log + content: "log b\n" + - path: host/var/log/containers/a.log + symlink: /var/log/pods/pod-a/0.log + - path: host/var/log/containers/b.log + symlink: /var/log/pods/pod-b/0.log +input: + allowed_paths: ["host/var/log"] + script: |+ + cat host/var/log/containers/a.log + cat host/var/log/containers/b.log +expect: + stdout: |+ + log a + log b + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_no_allowed_root.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_no_allowed_root.yaml new file mode 100644 index 00000000..1ba507ac --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_no_allowed_root.yaml @@ -0,0 +1,19 @@ +description: Container symlink to a host path not under any allowed root is blocked +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/opt/data/secret.txt + content: "secret\n" + - path: host/var/log/containers/sneaky.log + symlink: /opt/data/secret.txt +input: + allowed_paths: ["host/var/log"] + script: |+ + cat host/var/log/containers/sneaky.log +expect: + stdout: |+ + stderr_contains: + - "sneaky.log" + exit_code: 1 diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_outside_blocked.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_outside_blocked.yaml new file mode 100644 index 00000000..1600ab9c --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_outside_blocked.yaml @@ -0,0 +1,19 @@ +description: Container symlink to path outside allowed roots is blocked even with host prefix +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/etc/secret/passwd + content: "secret\n" + - path: host/var/log/containers/escape.log + symlink: /etc/secret/passwd +input: + allowed_paths: ["host/var/log"] + script: |+ + cat host/var/log/containers/escape.log +expect: + stdout: |+ + stderr_contains: + - "escape.log" + exit_code: 1 diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_parent_traversal.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_parent_traversal.yaml new file mode 100644 index 00000000..0e817c98 --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_parent_traversal.yaml @@ -0,0 +1,19 @@ +description: Container symlink using parent traversal to escape allowed root is blocked +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/etc/shadow + content: "root:x\n" + - path: host/var/log/containers/escape.log + symlink: /var/log/../../etc/shadow +input: + allowed_paths: ["host/var/log"] + script: |+ + cat host/var/log/containers/escape.log +expect: + stdout: |+ + stderr_contains: + - "escape.log" + exit_code: 1 diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml index 86fefbc5..07e72dce 100644 --- a/tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_symlink.yaml @@ -9,7 +9,7 @@ setup: - path: host/var/log/containers/app.log symlink: /var/log/pods/app.log input: - allowed_paths: ["host/var/log/pods", "host/var/log/containers"] + allowed_paths: ["host/var/log"] script: |+ cat host/var/log/containers/app.log expect: From 277fd7c55c77e97d376588b85d9788a56105dc73 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 2 Apr 2026 17:07:31 -0400 Subject: [PATCH 05/14] fix(allowedpaths): always prepend host prefix in container mode Remove the conditional check that skipped prepending when the path already started with the host prefix. In containerized environments, the host prefix is always prepended blindly to symlink targets. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 2 +- allowedpaths/sandbox_unix_test.go | 22 ---------------------- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 68e92710..ec2c19a2 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -185,7 +185,7 @@ func (s *Sandbox) resolveRootFollowingSymlinks(absPath string, preserveLast bool // In containers, host symlinks use host-absolute paths // (e.g. /var/log/pods/...) that don't include the /host // mount prefix. Prepend it so the path matches our roots. - if s.containerized && !strings.HasPrefix(absPath, s.hostPrefix+string(filepath.Separator)) { + if s.containerized { absPath = filepath.Join(s.hostPrefix, absPath) } symlinkFound = true diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index 6a3851f1..099de614 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -789,28 +789,6 @@ func TestContainerSymlinkHostPrefixNotAppliedWhenNotContainerized(t *testing.T) assert.Error(t, err, "without container mode, host-absolute symlinks should fail") } -// TestContainerSymlinkHostPrefixAlreadyPresent verifies that the prefix -// is not double-prepended when the symlink target already includes it. -func TestContainerSymlinkHostPrefixAlreadyPresent(t *testing.T) { - hostPrefix, pods, containers := setupContainerDirs(t) - - // Overwrite the symlink to use a target that already starts with the - // host prefix (i.e. a container-aware path, not a host-absolute one). - require.NoError(t, os.Remove(filepath.Join(containers, "app.log"))) - require.NoError(t, os.Symlink(filepath.Join(pods, "app.log"), filepath.Join(containers, "app.log"))) - - sb := newContainerSandbox(t, []string{pods, containers}, hostPrefix) - defer sb.Close() - - f, err := sb.Open("app.log", containers, os.O_RDONLY, 0) - require.NoError(t, err, "symlink with prefix already present should not be double-prefixed") - defer f.Close() - - buf := make([]byte, 64) - n, _ := f.Read(buf) - assert.Equal(t, "log line", string(buf[:n])) -} - // TestContainerSymlinkHostPrefixOutsideAllRoots verifies that a container // symlink pointing outside all allowed roots is still blocked even with // the host prefix applied. From ea5203028d95bdd4a691deeb4045ffcfe8574b9f Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 08:57:12 -0400 Subject: [PATCH 06/14] fix(tests): skip containerized scenario tests on Windows The container host prefix feature is Linux-only (host mounts under /host). Skip containerized scenarios on Windows where symlinks work differently and the /host prefix concept doesn't apply. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/scenarios_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/scenarios_test.go b/tests/scenarios_test.go index 51ac50d1..f9e846a3 100644 --- a/tests/scenarios_test.go +++ b/tests/scenarios_test.go @@ -473,7 +473,11 @@ func TestShellScenarios(t *testing.T) { sc := loadScenario(t, path) name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) t.Run(name, func(t *testing.T) { - if !sc.Containerized { + if sc.Containerized { + if runtime.GOOS == "windows" { + t.Skip("containerized tests are Linux-only") + } + } else { t.Parallel() } runScenario(t, sc) From 885371c97c54685521843a4c45f567add0d12a11 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 09:00:31 -0400 Subject: [PATCH 07/14] fix(allowedpaths): normalize host prefix in SetHostPrefix Clean trailing slashes and other path artifacts via filepath.Clean to prevent malformed prefix comparisons. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index ec2c19a2..aa8253dc 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -623,7 +623,7 @@ func (s *Sandbox) Readlink(path string, cwd string) (string, error) { // SetHostPrefix overrides the mount prefix used to translate host-absolute // symlink targets inside containers. func (s *Sandbox) SetHostPrefix(prefix string) { - s.hostPrefix = prefix + s.hostPrefix = filepath.Clean(prefix) } // Close releases all os.Root file descriptors. It is safe to call multiple times. From 05e18911515e20a66e85950e64a31de64df74f6d Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 09:06:39 -0400 Subject: [PATCH 08/14] docs(allowedpaths): document why host prefix is always prepended Explain that the Private Action Runner in the Datadog agent mounts host volumes exclusively under /host, so the blind prepend is correct for our deployment. Note the HostPrefix option for future changes. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index aa8253dc..fcd399de 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -184,7 +184,10 @@ func (s *Sandbox) resolveRootFollowingSymlinks(absPath string, preserveLast bool absPath = filepath.Clean(target) // In containers, host symlinks use host-absolute paths // (e.g. /var/log/pods/...) that don't include the /host - // mount prefix. Prepend it so the path matches our roots. + // mount prefix. We always prepend the prefix because the + // Private Action Runner in the Datadog agent mounts host + // volumes exclusively under /host. If the mount prefix + // changes, use the HostPrefix RunnerOption to configure it. if s.containerized { absPath = filepath.Join(s.hostPrefix, absPath) } From f4ebd481cea69c89de06cc67b5dc9c360aa88ab9 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 09:16:33 -0400 Subject: [PATCH 09/14] fix(interp): make HostPrefix order-independent Store the host prefix on the runner and apply it to the sandbox in New() after all options are processed. HostPrefix can now be applied before or after AllowedPaths without silently dropping the value. Co-Authored-By: Claude Opus 4.6 (1M context) --- interp/api.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/interp/api.go b/interp/api.go index b33acdef..8aa16286 100644 --- a/interp/api.go +++ b/interp/api.go @@ -56,6 +56,11 @@ type runnerConfig struct { // are set, so the output target is independent of option ordering. sandboxWarnings []byte + // hostPrefix is stored here so HostPrefix can be applied before or + // after AllowedPaths. Applied to the sandbox in New() after all + // options are processed. + hostPrefix string + // allowedCommands is the set of command names (builtins or external) that // the interpreter is permitted to execute. If nil and allowAllCommands is // false, no commands are allowed. @@ -240,6 +245,11 @@ func New(opts ...RunnerOption) (*Runner, error) { if r.stdout == nil || r.stderr == nil { StdIO(r.stdin, r.stdout, r.stderr)(r) } + // Apply host prefix if set, now that both HostPrefix and AllowedPaths + // have been processed regardless of option ordering. + if r.hostPrefix != "" && r.sandbox != nil { + r.sandbox.SetHostPrefix(r.hostPrefix) + } // Flush any sandbox warnings now that stderr is guaranteed to be set. if len(r.sandboxWarnings) > 0 { r.stderr.Write(r.sandboxWarnings) @@ -572,12 +582,10 @@ func AllowedPaths(paths []string) RunnerOption { // HostPrefix sets the mount prefix used to translate host-absolute symlink // targets inside containers. Defaults to "/host". Only takes effect when // running in a containerized environment (DOCKER_DD_AGENT set). -// Must be applied after AllowedPaths. +// Can be applied before or after AllowedPaths. func HostPrefix(prefix string) RunnerOption { return func(r *Runner) error { - if r.sandbox != nil { - r.sandbox.SetHostPrefix(prefix) - } + r.hostPrefix = prefix return nil } } From cf58cb92d9b84bd7abb5121f73c68712aeb1a1a9 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 09:21:06 -0400 Subject: [PATCH 10/14] test(interp): verify HostPrefix works in any option order Add tests for: - HostPrefix after AllowedPaths - HostPrefix before AllowedPaths - HostPrefix without AllowedPaths (silently ignored) - Default prefix when HostPrefix is not called Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 5 +++ interp/allowed_paths_internal_test.go | 53 +++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index fcd399de..ae79278e 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -629,6 +629,11 @@ func (s *Sandbox) SetHostPrefix(prefix string) { s.hostPrefix = filepath.Clean(prefix) } +// GetHostPrefix returns the current host mount prefix. +func (s *Sandbox) GetHostPrefix() string { + return s.hostPrefix +} + // Close releases all os.Root file descriptors. It is safe to call multiple times. func (s *Sandbox) Close() error { if s == nil { diff --git a/interp/allowed_paths_internal_test.go b/interp/allowed_paths_internal_test.go index a4469a0d..f48a0374 100644 --- a/interp/allowed_paths_internal_test.go +++ b/interp/allowed_paths_internal_test.go @@ -200,3 +200,56 @@ func TestAllowedPathsExecDefaultBlocksAll(t *testing.T) { assert.Equal(t, 127, exitCode) assert.Contains(t, stderr, "command not found") } + +// TestHostPrefixAfterAllowedPaths verifies that HostPrefix applied after +// AllowedPaths correctly sets the sandbox's host prefix. +func TestHostPrefixAfterAllowedPaths(t *testing.T) { + dir := t.TempDir() + runner, err := New( + AllowedPaths([]string{dir}), + HostPrefix("/custom"), + ) + require.NoError(t, err) + defer runner.Close() + + assert.Equal(t, "/custom", runner.sandbox.GetHostPrefix()) +} + +// TestHostPrefixBeforeAllowedPaths verifies that HostPrefix applied before +// AllowedPaths still correctly sets the sandbox's host prefix. +func TestHostPrefixBeforeAllowedPaths(t *testing.T) { + dir := t.TempDir() + runner, err := New( + HostPrefix("/custom"), + AllowedPaths([]string{dir}), + ) + require.NoError(t, err) + defer runner.Close() + + assert.Equal(t, "/custom", runner.sandbox.GetHostPrefix()) +} + +// TestHostPrefixWithoutAllowedPaths verifies that HostPrefix is silently +// ignored when no AllowedPaths is configured (sandbox is nil). +func TestHostPrefixWithoutAllowedPaths(t *testing.T) { + runner, err := New( + HostPrefix("/custom"), + ) + require.NoError(t, err) + defer runner.Close() + + assert.Nil(t, runner.sandbox, "sandbox should be nil when AllowedPaths is not set") +} + +// TestHostPrefixDefaultWhenNotSet verifies that the sandbox uses the +// default host prefix when HostPrefix is not called. +func TestHostPrefixDefaultWhenNotSet(t *testing.T) { + dir := t.TempDir() + runner, err := New( + AllowedPaths([]string{dir}), + ) + require.NoError(t, err) + defer runner.Close() + + assert.Equal(t, "/host", runner.sandbox.GetHostPrefix()) +} From 45ac08bca29d54ea72e5a4328214a2354e3b34f1 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 09:35:43 -0400 Subject: [PATCH 11/14] refactor: rename GetHostPrefix to HostPrefix, avoid double-loading scenarios - Rename GetHostPrefix() to HostPrefix() per Effective Go naming (no Get prefix on getters) - Load scenarios once per group and reuse, avoiding double loadScenario calls in the containerized check loop - Fix skip message: containerized tests are unsupported on Windows, not Linux-only Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 4 ++-- interp/allowed_paths_internal_test.go | 6 +++--- tests/scenarios_test.go | 24 ++++++++++++++---------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index ae79278e..7bb1222e 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -629,8 +629,8 @@ func (s *Sandbox) SetHostPrefix(prefix string) { s.hostPrefix = filepath.Clean(prefix) } -// GetHostPrefix returns the current host mount prefix. -func (s *Sandbox) GetHostPrefix() string { +// HostPrefix returns the current host mount prefix. +func (s *Sandbox) HostPrefix() string { return s.hostPrefix } diff --git a/interp/allowed_paths_internal_test.go b/interp/allowed_paths_internal_test.go index f48a0374..957974be 100644 --- a/interp/allowed_paths_internal_test.go +++ b/interp/allowed_paths_internal_test.go @@ -212,7 +212,7 @@ func TestHostPrefixAfterAllowedPaths(t *testing.T) { require.NoError(t, err) defer runner.Close() - assert.Equal(t, "/custom", runner.sandbox.GetHostPrefix()) + assert.Equal(t, "/custom", runner.sandbox.HostPrefix()) } // TestHostPrefixBeforeAllowedPaths verifies that HostPrefix applied before @@ -226,7 +226,7 @@ func TestHostPrefixBeforeAllowedPaths(t *testing.T) { require.NoError(t, err) defer runner.Close() - assert.Equal(t, "/custom", runner.sandbox.GetHostPrefix()) + assert.Equal(t, "/custom", runner.sandbox.HostPrefix()) } // TestHostPrefixWithoutAllowedPaths verifies that HostPrefix is silently @@ -251,5 +251,5 @@ func TestHostPrefixDefaultWhenNotSet(t *testing.T) { require.NoError(t, err) defer runner.Close() - assert.Equal(t, "/host", runner.sandbox.GetHostPrefix()) + assert.Equal(t, "/host", runner.sandbox.HostPrefix()) } diff --git a/tests/scenarios_test.go b/tests/scenarios_test.go index f9e846a3..085fe04a 100644 --- a/tests/scenarios_test.go +++ b/tests/scenarios_test.go @@ -456,31 +456,35 @@ func TestShellScenarios(t *testing.T) { for group, paths := range groups { t.Run(group, func(t *testing.T) { - // Check if any scenario in this group needs containerized mode. - // If so, the group cannot be parallel (t.Setenv requirement). + // Load all scenarios once and check if any needs containerized + // mode. If so, the group cannot be parallel (t.Setenv requirement). + type namedScenario struct { + name string + sc scenario + } + loaded := make([]namedScenario, 0, len(paths)) hasContainerized := false for _, path := range paths { sc := loadScenario(t, path) + name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) + loaded = append(loaded, namedScenario{name: name, sc: sc}) if sc.Containerized { hasContainerized = true - break } } if !hasContainerized { t.Parallel() } - for _, path := range paths { - sc := loadScenario(t, path) - name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) - t.Run(name, func(t *testing.T) { - if sc.Containerized { + for _, ns := range loaded { + t.Run(ns.name, func(t *testing.T) { + if ns.sc.Containerized { if runtime.GOOS == "windows" { - t.Skip("containerized tests are Linux-only") + t.Skip("containerized tests are not supported on Windows") } } else { t.Parallel() } - runScenario(t, sc) + runScenario(t, ns.sc) }) } }) From fd33dcd2b602928549c592ab9763b62e94c58cae Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 09:43:37 -0400 Subject: [PATCH 12/14] fix(tests): skip HostPrefix tests on Windows Container host prefix is not supported on Windows. Skip the three HostPrefix ordering tests that fail due to filepath.Clean converting forward slashes to backslashes. Co-Authored-By: Claude Opus 4.6 (1M context) --- interp/allowed_paths_internal_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/interp/allowed_paths_internal_test.go b/interp/allowed_paths_internal_test.go index 957974be..68d9268f 100644 --- a/interp/allowed_paths_internal_test.go +++ b/interp/allowed_paths_internal_test.go @@ -204,6 +204,9 @@ func TestAllowedPathsExecDefaultBlocksAll(t *testing.T) { // TestHostPrefixAfterAllowedPaths verifies that HostPrefix applied after // AllowedPaths correctly sets the sandbox's host prefix. func TestHostPrefixAfterAllowedPaths(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("container host prefix is not supported on Windows") + } dir := t.TempDir() runner, err := New( AllowedPaths([]string{dir}), @@ -218,6 +221,9 @@ func TestHostPrefixAfterAllowedPaths(t *testing.T) { // TestHostPrefixBeforeAllowedPaths verifies that HostPrefix applied before // AllowedPaths still correctly sets the sandbox's host prefix. func TestHostPrefixBeforeAllowedPaths(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("container host prefix is not supported on Windows") + } dir := t.TempDir() runner, err := New( HostPrefix("/custom"), @@ -244,6 +250,9 @@ func TestHostPrefixWithoutAllowedPaths(t *testing.T) { // TestHostPrefixDefaultWhenNotSet verifies that the sandbox uses the // default host prefix when HostPrefix is not called. func TestHostPrefixDefaultWhenNotSet(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("container host prefix is not supported on Windows") + } dir := t.TempDir() runner, err := New( AllowedPaths([]string{dir}), From 202abe0092c48952b80c34594d4d063299ac992c Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 10:05:22 -0400 Subject: [PATCH 13/14] refactor(allowedpaths): make containerization user-configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove auto-detection via DOCKER_DD_AGENT env var. Container symlink resolution is now enabled by calling HostPrefix — a non-empty prefix activates the feature. This removes: - containerized bool from Sandbox - os.Getenv("DOCKER_DD_AGENT") from New() - defaultHostMountPrefix const - t.Setenv and parallel restrictions from test harness The scenario harness sets HostPrefix directly via the RunnerOption. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 28 +++++++-------------- allowedpaths/sandbox_unix_test.go | 9 +++---- analysis/symbols_allowedpaths.go | 1 - interp/allowed_paths_internal_test.go | 9 +++---- interp/api.go | 6 ++--- tests/scenarios_test.go | 36 ++++++--------------------- 6 files changed, 27 insertions(+), 62 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 7bb1222e..731479ce 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -37,18 +37,12 @@ type root struct { root *os.Root } -// defaultHostMountPrefix is prepended to symlink targets when running -// inside a container. Host filesystems are mounted under this prefix, -// but symlinks created on the host use paths without it. -const defaultHostMountPrefix = "/host" - // Sandbox restricts filesystem access to a set of allowed directories. // The restriction is enforced using os.Root (Go 1.24+), which uses openat // syscalls for atomic path validation — immune to symlink and ".." traversal attacks. type Sandbox struct { - roots []root - containerized bool // true when running inside a container (DOCKER_DD_AGENT set) - hostPrefix string // mount prefix for host paths inside the container + roots []root + hostPrefix string // when non-empty, enables container symlink resolution } // New creates a sandbox from an allowlist of directory paths. Paths that do @@ -76,11 +70,7 @@ func New(paths []string) (sb *Sandbox, warnings []byte, err error) { } roots = append(roots, root{absPath: abs, root: r}) } - return &Sandbox{ - roots: roots, - containerized: os.Getenv("DOCKER_DD_AGENT") != "", - hostPrefix: defaultHostMountPrefix, - }, buf.Bytes(), nil + return &Sandbox{roots: roots}, buf.Bytes(), nil } // isPathEscapeError reports whether err is the unexported "path escapes @@ -183,12 +173,12 @@ func (s *Sandbox) resolveRootFollowingSymlinks(absPath string, preserveLast bool } absPath = filepath.Clean(target) // In containers, host symlinks use host-absolute paths - // (e.g. /var/log/pods/...) that don't include the /host - // mount prefix. We always prepend the prefix because the - // Private Action Runner in the Datadog agent mounts host - // volumes exclusively under /host. If the mount prefix - // changes, use the HostPrefix RunnerOption to configure it. - if s.containerized { + // (e.g. /var/log/pods/...) that don't include the mount + // prefix. We always prepend the prefix because the Private + // Action Runner in the Datadog agent mounts host volumes + // exclusively under /host. If the mount prefix changes, + // use the HostPrefix RunnerOption to configure it. + if s.hostPrefix != "" { absPath = filepath.Join(s.hostPrefix, absPath) } symlinkFound = true diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index 099de614..93d0ed72 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -699,15 +699,14 @@ func TestCrossRootSymlinkSiblingDirs(t *testing.T) { // --- Container /host prefix tests --- -// newContainerSandbox creates a sandbox with containerized=true and a -// custom host prefix, simulating a containerized environment where host -// filesystems are mounted under prefix. +// newContainerSandbox creates a sandbox with a host prefix set, +// simulating a containerized environment where host filesystems +// are mounted under prefix. func newContainerSandbox(t *testing.T, paths []string, hostPrefix string) *Sandbox { t.Helper() sb, _, err := New(paths) require.NoError(t, err) - sb.containerized = true - sb.hostPrefix = hostPrefix + sb.SetHostPrefix(hostPrefix) return sb } diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index ee60baac..73cc7c7b 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -38,7 +38,6 @@ var allowedpathsAllowedSymbols = []string{ "os.ErrPermission", // 🟢 sentinel error for permission denied; pure constant. "os.File", // 🟠 file handle returned by os.Root.Open; needed for cross-root symlink fallback. "os.FileMode", // 🟢 file permission bits type; pure type. - "os.Getenv", // 🟠 reads environment variable; used once at construction to detect containerized environments. "os.Getgid", // 🟠 returns the numeric group id of the caller; read-only syscall. "os.Getgroups", // 🟠 returns supplementary group ids; read-only syscall. "os.Getuid", // 🟠 returns the numeric user id of the caller; read-only syscall. diff --git a/interp/allowed_paths_internal_test.go b/interp/allowed_paths_internal_test.go index 68d9268f..869b79f5 100644 --- a/interp/allowed_paths_internal_test.go +++ b/interp/allowed_paths_internal_test.go @@ -247,12 +247,9 @@ func TestHostPrefixWithoutAllowedPaths(t *testing.T) { assert.Nil(t, runner.sandbox, "sandbox should be nil when AllowedPaths is not set") } -// TestHostPrefixDefaultWhenNotSet verifies that the sandbox uses the -// default host prefix when HostPrefix is not called. +// TestHostPrefixDefaultWhenNotSet verifies that the sandbox has no host +// prefix when HostPrefix is not called (container resolution disabled). func TestHostPrefixDefaultWhenNotSet(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("container host prefix is not supported on Windows") - } dir := t.TempDir() runner, err := New( AllowedPaths([]string{dir}), @@ -260,5 +257,5 @@ func TestHostPrefixDefaultWhenNotSet(t *testing.T) { require.NoError(t, err) defer runner.Close() - assert.Equal(t, "/host", runner.sandbox.HostPrefix()) + assert.Empty(t, runner.sandbox.HostPrefix()) } diff --git a/interp/api.go b/interp/api.go index 8aa16286..9fec73ba 100644 --- a/interp/api.go +++ b/interp/api.go @@ -579,9 +579,9 @@ func AllowedPaths(paths []string) RunnerOption { } } -// HostPrefix sets the mount prefix used to translate host-absolute symlink -// targets inside containers. Defaults to "/host". Only takes effect when -// running in a containerized environment (DOCKER_DD_AGENT set). +// HostPrefix enables container symlink resolution and sets the mount prefix +// used to translate host-absolute symlink targets. When set, symlink targets +// resolved during cross-root fallback are prepended with this prefix. // Can be applied before or after AllowedPaths. func HostPrefix(prefix string) RunnerOption { return func(r *Runner) error { diff --git a/tests/scenarios_test.go b/tests/scenarios_test.go index 085fe04a..d53e2dfa 100644 --- a/tests/scenarios_test.go +++ b/tests/scenarios_test.go @@ -36,8 +36,8 @@ const dockerBashImage = "debian:bookworm-slim" type scenario struct { Description string `yaml:"description"` SkipAssertAgainstBash bool `yaml:"skip_assert_against_bash"` // true = skip bash comparison - // Containerized simulates a containerized environment by setting - // DOCKER_DD_AGENT before sandbox construction. Disables t.Parallel. + // Containerized enables container symlink resolution by setting + // HostPrefix to the test directory's host/ subdirectory. Containerized bool `yaml:"containerized"` Setup setup `yaml:"setup"` Input input `yaml:"input"` @@ -220,7 +220,6 @@ func runScenario(t *testing.T, sc scenario) { // empty, no AllowedCommands/AllowAllCommands option is added, so the // interpreter defaults to blocking all commands. if sc.Containerized { - t.Setenv("DOCKER_DD_AGENT", "true") opts = append(opts, interp.HostPrefix(filepath.Join(dir, "host"))) } runner, err := interp.New(opts...) @@ -456,35 +455,16 @@ func TestShellScenarios(t *testing.T) { for group, paths := range groups { t.Run(group, func(t *testing.T) { - // Load all scenarios once and check if any needs containerized - // mode. If so, the group cannot be parallel (t.Setenv requirement). - type namedScenario struct { - name string - sc scenario - } - loaded := make([]namedScenario, 0, len(paths)) - hasContainerized := false + t.Parallel() for _, path := range paths { sc := loadScenario(t, path) name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) - loaded = append(loaded, namedScenario{name: name, sc: sc}) - if sc.Containerized { - hasContainerized = true - } - } - if !hasContainerized { - t.Parallel() - } - for _, ns := range loaded { - t.Run(ns.name, func(t *testing.T) { - if ns.sc.Containerized { - if runtime.GOOS == "windows" { - t.Skip("containerized tests are not supported on Windows") - } - } else { - t.Parallel() + t.Run(name, func(t *testing.T) { + if sc.Containerized && runtime.GOOS == "windows" { + t.Skip("containerized tests are not supported on Windows") } - runScenario(t, ns.sc) + t.Parallel() + runScenario(t, sc) }) } }) From 614609e7c75509b6a291c280c42abaa959e6104c Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 3 Apr 2026 10:10:46 -0400 Subject: [PATCH 14/14] fix(allowedpaths): skip host prefix for relative symlinks already under prefix Relative symlinks (e.g. ../pods/app.log) resolve to paths that already include the host prefix. Skip prepending to avoid double-prefixing. Added unit test and scenario test for relative symlinks in container mode. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 9 +++---- allowedpaths/sandbox_unix_test.go | 25 +++++++++++++++++++ ...ontainer_host_prefix_relative_symlink.yaml | 19 ++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 tests/scenarios/shell/allowed_paths/container_host_prefix_relative_symlink.yaml diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 731479ce..2d04d9cf 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -174,11 +174,10 @@ func (s *Sandbox) resolveRootFollowingSymlinks(absPath string, preserveLast bool absPath = filepath.Clean(target) // In containers, host symlinks use host-absolute paths // (e.g. /var/log/pods/...) that don't include the mount - // prefix. We always prepend the prefix because the Private - // Action Runner in the Datadog agent mounts host volumes - // exclusively under /host. If the mount prefix changes, - // use the HostPrefix RunnerOption to configure it. - if s.hostPrefix != "" { + // prefix. Prepend it so the path matches our roots. Skip + // if the path already starts with the prefix (e.g. a + // relative symlink that resolved within the same root). + if s.hostPrefix != "" && !strings.HasPrefix(absPath, s.hostPrefix+string(filepath.Separator)) { absPath = filepath.Join(s.hostPrefix, absPath) } symlinkFound = true diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index 93d0ed72..e6a85924 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -809,3 +809,28 @@ func TestContainerSymlinkHostPrefixOutsideAllRoots(t *testing.T) { _, err := sb.Open("escape.log", containers, os.O_RDONLY, 0) assert.Error(t, err, "container symlink to path outside allowed roots should be blocked") } + +// TestContainerSymlinkRelativeTarget verifies that a relative symlink in +// container mode resolves correctly without double-prepending the prefix. +func TestContainerSymlinkRelativeTarget(t *testing.T) { + root := t.TempDir() + hostPrefix := root + pods := filepath.Join(root, "var", "log", "pods") + containers := filepath.Join(root, "var", "log", "containers") + require.NoError(t, os.MkdirAll(pods, 0755)) + require.NoError(t, os.MkdirAll(containers, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(pods, "app.log"), []byte("relative"), 0644)) + // Relative symlink — target already resolves within the host prefix. + require.NoError(t, os.Symlink("../pods/app.log", filepath.Join(containers, "app.log"))) + + sb := newContainerSandbox(t, []string{pods, containers}, hostPrefix) + defer sb.Close() + + f, err := sb.Open("app.log", containers, os.O_RDONLY, 0) + require.NoError(t, err, "relative symlink in container mode should not double-prepend prefix") + defer f.Close() + + buf := make([]byte, 64) + n, _ := f.Read(buf) + assert.Equal(t, "relative", string(buf[:n])) +} diff --git a/tests/scenarios/shell/allowed_paths/container_host_prefix_relative_symlink.yaml b/tests/scenarios/shell/allowed_paths/container_host_prefix_relative_symlink.yaml new file mode 100644 index 00000000..7ae74469 --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/container_host_prefix_relative_symlink.yaml @@ -0,0 +1,19 @@ +description: Relative symlinks in container mode resolve without double-prepending the host prefix +# skip: container host-prefix symlink resolution is an rshell-specific feature +skip_assert_against_bash: true +containerized: true +setup: + files: + - path: host/var/log/pods/app.log + content: "relative log\n" + - path: host/var/log/containers/app.log + symlink: ../pods/app.log +input: + allowed_paths: ["host/var/log"] + script: |+ + cat host/var/log/containers/app.log +expect: + stdout: |+ + relative log + stderr: |+ + exit_code: 0