From 3300592e066873da7de4bfcb9f2a1caf76b08cb7 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Mon, 4 May 2026 12:23:03 +0200 Subject: [PATCH 1/5] feat(builtins): add truncate builtin for sandbox-mediated file truncation Implements `truncate -s SIZE [-c] FILE...` as the first writing builtin. Targets the host disk-space remediation flow where `truncate -s 0 ` is the demo's "nuclear option" for reclaiming space when logrotate cannot run. All file mutations route through a new `Sandbox.Truncate` method that mirrors the existing `Sandbox.Open` write policy: paths are resolved within a single `os.Root`, the cross-root symlink fallback is disabled (TOCTOU-unsafe for writes), and non-regular targets (FIFO/socket/device) are rejected before the open syscall to avoid blocking on FIFO opens. Supports `-s` with the full GNU suffix grammar (binary K/k/KiB, decimal KB, and analogous M/G/T forms) plus `-c`/`--no-create` and `--help`. Deferred: `-r`/`--reference`, `-o`/`--io-blocks`, and the GNU relative-size operators (`+N`/`-N`/`N`/`/N`/`%N`), all rejected with clear errors. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/fuzz.yml | 3 + SHELL_FEATURES.md | 1 + allowedpaths/sandbox.go | 74 ++++ allowedpaths/sandbox_test.go | 131 ++++++ allowedpaths/sandbox_unix_test.go | 27 ++ analysis/symbols_allowedpaths.go | 1 + analysis/symbols_builtins.go | 9 + builtins/builtins.go | 6 + builtins/tests/truncate/hardening_test.go | 188 +++++++++ builtins/tests/truncate/helpers_test.go | 72 ++++ builtins/tests/truncate/truncate_fuzz_test.go | 261 ++++++++++++ builtins/tests/truncate/truncate_test.go | 396 ++++++++++++++++++ builtins/truncate/truncate.go | 254 +++++++++++ builtins/truncate/truncate_test.go | 152 +++++++ interp/builtin_truncate_pentest_test.go | 305 ++++++++++++++ interp/register_builtins.go | 2 + interp/runner_exec.go | 6 + .../scenarios/cmd/truncate/basic/extend.yaml | 15 + .../cmd/truncate/basic/long_form.yaml | 15 + .../scenarios/cmd/truncate/basic/shrink.yaml | 16 + .../cmd/truncate/basic/suffix_k.yaml | 15 + .../cmd/truncate/basic/suffix_kb.yaml | 15 + .../cmd/truncate/basic/zero_size.yaml | 15 + .../cmd/truncate/errors/directory_target.yaml | 16 + .../cmd/truncate/errors/invalid_size.yaml | 17 + .../cmd/truncate/errors/missing_file.yaml | 13 + .../cmd/truncate/errors/missing_size.yaml | 17 + .../cmd/truncate/errors/negative_size.yaml | 17 + .../errors/outside_allowed_paths.yaml | 19 + .../cmd/truncate/errors/relative_size.yaml | 17 + .../cmd/truncate/hardening/double_dash.yaml | 15 + .../cmd/truncate/hardening/unknown_flag.yaml | 16 + tests/scenarios/cmd/truncate/help/help.yaml | 13 + .../cmd/truncate/multi_file/all_zero.yaml | 23 + .../cmd/truncate/multi_file/some_missing.yaml | 19 + .../truncate/no_create/missing_silent.yaml | 15 + .../truncate/no_create/no_flag_creates.yaml | 15 + .../no_create/no_flag_creates_with_size.yaml | 15 + 38 files changed, 2226 insertions(+) create mode 100644 builtins/tests/truncate/hardening_test.go create mode 100644 builtins/tests/truncate/helpers_test.go create mode 100644 builtins/tests/truncate/truncate_fuzz_test.go create mode 100644 builtins/tests/truncate/truncate_test.go create mode 100644 builtins/truncate/truncate.go create mode 100644 builtins/truncate/truncate_test.go create mode 100644 interp/builtin_truncate_pentest_test.go create mode 100644 tests/scenarios/cmd/truncate/basic/extend.yaml create mode 100644 tests/scenarios/cmd/truncate/basic/long_form.yaml create mode 100644 tests/scenarios/cmd/truncate/basic/shrink.yaml create mode 100644 tests/scenarios/cmd/truncate/basic/suffix_k.yaml create mode 100644 tests/scenarios/cmd/truncate/basic/suffix_kb.yaml create mode 100644 tests/scenarios/cmd/truncate/basic/zero_size.yaml create mode 100644 tests/scenarios/cmd/truncate/errors/directory_target.yaml create mode 100644 tests/scenarios/cmd/truncate/errors/invalid_size.yaml create mode 100644 tests/scenarios/cmd/truncate/errors/missing_file.yaml create mode 100644 tests/scenarios/cmd/truncate/errors/missing_size.yaml create mode 100644 tests/scenarios/cmd/truncate/errors/negative_size.yaml create mode 100644 tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml create mode 100644 tests/scenarios/cmd/truncate/errors/relative_size.yaml create mode 100644 tests/scenarios/cmd/truncate/hardening/double_dash.yaml create mode 100644 tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml create mode 100644 tests/scenarios/cmd/truncate/help/help.yaml create mode 100644 tests/scenarios/cmd/truncate/multi_file/all_zero.yaml create mode 100644 tests/scenarios/cmd/truncate/multi_file/some_missing.yaml create mode 100644 tests/scenarios/cmd/truncate/no_create/missing_silent.yaml create mode 100644 tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml create mode 100644 tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index a68cd536d..ced620a5a 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -94,6 +94,9 @@ jobs: - pkg: ./builtins/tests/xargs/ name: xargs corpus_path: builtins/tests/xargs + - pkg: ./builtins/tests/truncate/ + name: truncate + corpus_path: builtins/tests/truncate steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0 diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index d81e60b35..d626f4d16 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -37,6 +37,7 @@ The in-shell `help` command mirrors these feature categories: run `help` for a c - ✅ `test EXPRESSION` / `[ EXPRESSION ]` — evaluate conditional expression (file tests, string/integer comparison, logical operators) - ✅ `tr [-cdsCt] SET1 [SET2]` — translate, squeeze, and/or delete characters from stdin - ✅ `true` — return exit code 0 +- ✅ `truncate [-c] -s SIZE FILE...` — shrink or extend each FILE to SIZE bytes inside `AllowedPaths` (creates files unless `-c`); `-s` accepts `K`/`k`/`KiB`/`KB`/`M`/`m`/`MiB`/`MB`/`G`/`g`/`GiB`/`GB`/`T`/`t`/`TiB`/`TB` suffixes (binary or decimal); `-r`/`--reference`, `-o`/`--io-blocks`, and the GNU relative-size modifiers (`+N`/`-N`/`N`/`/N`/`%N`) are rejected - ✅ `uname [-asnrvm]` — print system information (Linux only; reads from `/proc/sys/kernel/`, respects `--proc-path`) - ✅ `uniq [OPTION]... [INPUT]` — report or omit repeated lines - ✅ `wc [-l] [-w] [-c] [-m] [-L] [FILE]...` — count lines, words, bytes, characters, or max line length diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 1679b5131..03685d8f4 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -17,6 +17,7 @@ import ( "path/filepath" "slices" "strings" + "syscall" ) // Access mode bits for permission checks. @@ -367,6 +368,79 @@ func (s *Sandbox) Open(path string, cwd string, flag int, perm os.FileMode) (io. return f, nil } +// Truncate sets the size of the file at path to size bytes. When create is +// true, a missing file is created with mode 0644; when create is false, a +// missing file returns os.ErrNotExist (the caller, e.g. truncate -c, decides +// whether to treat that as an error or a silent skip). +// +// Like Open, the operation goes through os.Root for atomic openat-based path +// validation. The cross-root symlink fallback is intentionally NOT used: +// resolving a symlink that escapes one root and then writing through the +// resolved path is the classic TOCTOU footgun. Writes must stay within a +// single allowed root. +// +// Non-regular targets (FIFO, socket, char/block device) are rejected before +// the open syscall. Mirrors the redirect-side guard in +// interp.Runner.rejectNonRegularRedirectTarget — opening a FIFO with +// O_WRONLY blocks until a reader connects, which would hang the shell. +// /dev/null and similar are handled by sandbox path resolution and never +// reach this method through normal AllowedPaths. +// +// Negative sizes are rejected with EINVAL. Sizes within int64 range are +// passed through to the kernel; the kernel/filesystem rejects values it +// cannot represent (e.g. exceeding the filesystem's maximum file size). +// +// The flag passed to OpenFile is constructed locally from a fixed set of +// constants, so the open-flag allowlist enforced in Open is not relevant +// here — there is no caller-controlled flag bit that could leak through. +func (s *Sandbox) Truncate(path string, cwd string, size int64, create bool) error { + if size < 0 { + return &os.PathError{Op: "truncate", Path: path, Err: syscall.EINVAL} + } + + absPath := toAbs(path, cwd) + + ar, relPath, ok := s.resolve(absPath) + if !ok { + return &os.PathError{Op: "truncate", Path: path, Err: os.ErrPermission} + } + + // Stat (follow symlinks within the root) so a symlink whose final + // target is a FIFO is caught here before OpenFile blocks on the + // open. Mirrors the policy in interp.Runner.rejectNonRegularRedirectTarget. + // ENOENT and other Stat errors are intentionally ignored: O_CREATE + // will produce a regular file, and any genuine permission error will + // surface from the subsequent OpenFile call. + if info, err := ar.root.Stat(relPath); err == nil && !info.Mode().IsRegular() { + return &os.PathError{Op: "truncate", Path: path, Err: errors.New("not a regular file")} + } + + flag := os.O_WRONLY + if create { + flag |= os.O_CREATE + } + f, err := ar.root.OpenFile(relPath, flag, 0644) + if err != nil { + // Return the raw error so callers can use errors.Is against + // fs.ErrNotExist / fs.ErrPermission. The handler renders user- + // facing messages via PortableErrMsg, so the wrapping that + // PortablePathError performs is not needed here. Wrapping would + // hide os.ErrNotExist behind a fresh errors.New value, which + // would silently break the truncate -c silent-skip path. + return err + } + truncErr := f.Truncate(size) + // Surface a deferred Close error only when Truncate itself succeeded; + // the open path is read-only-on-the-fd at this point so a failed Close + // after a successful ftruncate is the only case where a Close error + // reflects user-visible data loss (flush failure on write-back). + closeErr := f.Close() + if truncErr != nil { + return truncErr + } + return closeErr +} + // ReadDir implements the restricted directory-read policy. func (s *Sandbox) ReadDir(path string, cwd string) ([]fs.DirEntry, error) { return s.readDirN(path, cwd, -1) diff --git a/allowedpaths/sandbox_test.go b/allowedpaths/sandbox_test.go index 6ded67994..a16e37b6b 100644 --- a/allowedpaths/sandbox_test.go +++ b/allowedpaths/sandbox_test.go @@ -160,6 +160,137 @@ func TestSandboxWriteRejectsUnknownFlag(t *testing.T) { assert.ErrorIs(t, err, os.ErrPermission) } +// TestSandboxTruncateMethodShrink covers the happy path of the new +// Sandbox.Truncate API: an existing file in an allowed root is shrunk to +// the requested size, leaving the leading bytes intact. +func TestSandboxTruncateMethodShrink(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "log.txt") + require.NoError(t, os.WriteFile(path, []byte("0123456789"), 0644)) + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + require.NoError(t, sb.Truncate("log.txt", dir, 4, false)) + + got, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, "0123", string(got)) +} + +// TestSandboxTruncateMethodExtend covers the case where SIZE is larger +// than the current file: the file is zero-extended. +func TestSandboxTruncateMethodExtend(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "log.txt") + require.NoError(t, os.WriteFile(path, []byte("abc"), 0644)) + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + require.NoError(t, sb.Truncate("log.txt", dir, 1024, false)) + + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, int64(1024), info.Size()) +} + +// TestSandboxTruncateMethodCreates covers the create-by-default behaviour +// used when truncate is invoked without -c. +func TestSandboxTruncateMethodCreates(t *testing.T) { + dir := t.TempDir() + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + require.NoError(t, sb.Truncate("fresh.txt", dir, 100, true)) + + info, err := os.Stat(filepath.Join(dir, "fresh.txt")) + require.NoError(t, err) + assert.Equal(t, int64(100), info.Size()) +} + +// TestSandboxTruncateMethodNoCreate covers create=false: the call returns +// os.ErrNotExist for missing files (the truncate -c silent-skip path +// depends on errors.Is matching). +func TestSandboxTruncateMethodNoCreate(t *testing.T) { + dir := t.TempDir() + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + err = sb.Truncate("missing.txt", dir, 0, false) + assert.ErrorIs(t, err, fs.ErrNotExist) + _, statErr := os.Stat(filepath.Join(dir, "missing.txt")) + assert.True(t, os.IsNotExist(statErr), "no-create must not create missing.txt") +} + +// TestSandboxTruncateMethodOutsideAllowedPath verifies that paths outside +// the sandbox are rejected with a permission error before any I/O. +func TestSandboxTruncateMethodOutsideAllowedPath(t *testing.T) { + allowed := t.TempDir() + outside := t.TempDir() + target := filepath.Join(outside, "log.txt") + require.NoError(t, os.WriteFile(target, []byte("untouched"), 0644)) + + sb, _, err := New([]string{allowed}) + require.NoError(t, err) + defer sb.Close() + + err = sb.Truncate(target, allowed, 0, true) + assert.ErrorIs(t, err, os.ErrPermission) + + got, ferr := os.ReadFile(target) + require.NoError(t, ferr) + assert.Equal(t, "untouched", string(got), "outside file must not be touched") +} + +// TestSandboxTruncateMethodNegativeSize verifies that negative sizes are +// rejected with EINVAL. +func TestSandboxTruncateMethodNegativeSize(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "log.txt") + require.NoError(t, os.WriteFile(path, []byte("data"), 0644)) + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + err = sb.Truncate("log.txt", dir, -1, false) + assert.Error(t, err) + got, ferr := os.ReadFile(path) + require.NoError(t, ferr) + assert.Equal(t, "data", string(got), "negative size must not modify the file") +} + +// TestSandboxTruncateMethodSymlinkEscapeRejected mirrors +// TestSandboxWriteThroughSymlinkEscapeRejected for the new API: writes +// must not follow a symlink that escapes the os.Root, even via the +// Truncate code path. +func TestSandboxTruncateMethodSymlinkEscapeRejected(t *testing.T) { + allowed := t.TempDir() + outside := t.TempDir() + linkPath := filepath.Join(allowed, "escape") + target := filepath.Join(outside, "target.txt") + require.NoError(t, os.WriteFile(target, []byte("untouched"), 0644)) + require.NoError(t, os.Symlink(target, linkPath)) + + sb, _, err := New([]string{allowed}) + require.NoError(t, err) + defer sb.Close() + + err = sb.Truncate("escape", allowed, 0, true) + assert.Error(t, err) + + got, ferr := os.ReadFile(target) + require.NoError(t, ferr) + assert.Equal(t, "untouched", string(got), "symlink target must not be reachable for writes") +} + func TestSandboxOpenReadStillWorks(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644)) diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index e6a85924b..2f1738a15 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -834,3 +834,30 @@ func TestContainerSymlinkRelativeTarget(t *testing.T) { n, _ := f.Read(buf) assert.Equal(t, "relative", string(buf[:n])) } + +// TestSandboxTruncateMethodFIFODoesNotBlock verifies that Sandbox.Truncate +// rejects a FIFO target before the open syscall can block. Without the +// non-regular-file pre-check, opening a FIFO with O_WRONLY would wait for +// a reader and hang the shell until the executor timeout fires. +func TestSandboxTruncateMethodFIFODoesNotBlock(t *testing.T) { + dir := t.TempDir() + fifoPath := filepath.Join(dir, "pipe") + require.NoError(t, syscall.Mkfifo(fifoPath, 0644)) + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + done := make(chan error, 1) + go func() { + done <- sb.Truncate("pipe", dir, 0, false) + }() + + select { + case err := <-done: + assert.Error(t, err, "FIFO target must be rejected, not silently truncated") + assert.Contains(t, err.Error(), "not a regular file") + case <-time.After(2 * time.Second): + t.Fatal("Truncate blocked on FIFO — expected fast rejection via non-regular-file guard") + } +} diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index c591b467e..b89929aaa 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -68,6 +68,7 @@ var allowedpathsAllowedSymbols = []string{ "strings.Join", // 🟢 joins string slices; pure function, no I/O. "strings.Split", // 🟢 splits a string by separator; pure function, no I/O. "syscall.ByHandleFileInformation", // 🟢 Windows file identity structure; pure type for file metadata. + "syscall.EINVAL", // 🟢 "invalid argument" errno constant; pure constant. Used by Sandbox.Truncate to reject negative sizes. "syscall.EISDIR", // 🟢 "is a directory" errno constant; pure constant. "syscall.Errno", // 🟢 system call error number type; pure type. "syscall.GetFileInformationByHandle", // 🟠 Windows API for file identity (vol serial + file index); read-only syscall. diff --git a/analysis/symbols_builtins.go b/analysis/symbols_builtins.go index 6da0effbf..cff472520 100644 --- a/analysis/symbols_builtins.go +++ b/analysis/symbols_builtins.go @@ -357,6 +357,14 @@ var builtinPerCommandSymbols = map[string][]string{ "true": { "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. }, + "truncate": { + "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "errors.Is", // 🟢 error comparison; pure function, no I/O. + "errors.New", // 🟢 creates a sentinel error value; pure function, no I/O. + "math.MaxInt64", // 🟢 integer constant; no side effects. + "os.ErrNotExist", // 🟢 sentinel error value; pure constant. + "strconv.ParseInt", // 🟢 string-to-int conversion with base/bit-size; pure function, no I/O. + }, "uname": { "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. @@ -577,6 +585,7 @@ var builtinAllowedSymbols = []string{ "net.Interface", // 🟢 OS network interface descriptor; read-only struct, no network connections. "net.Interfaces", // 🟠 read-only OS interface enumeration function; no network connections or writes. "os.ErrDeadlineExceeded", // 🟢 sentinel error value for *os.File read/write deadline expiry; pure constant. + "os.ErrNotExist", // 🟢 sentinel error value for "does not exist"; pure constant. "os.File", // 🟠 *os.File type, used for type-asserting callCtx.Stdin to access SetReadDeadline/Stat (e.g. read -t timeout, TTY detection); no constructors invoked. "os.FileInfo", // 🟢 file metadata interface returned by Stat; no I/O side effects. "os.IsNotExist", // 🟢 checks if error is "not exist"; pure function, no I/O. diff --git a/builtins/builtins.go b/builtins/builtins.go index 8eaeefe76..e46815672 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -153,6 +153,12 @@ type CallContext struct { // within the shell's path restrictions. Mode: 0x04=read, 0x02=write, 0x01=execute. AccessFile func(ctx context.Context, path string, mode uint32) error + // Truncate sets the size of the file at path within the shell's path + // restrictions. When create is true, a missing file is created (mode + // 0644); when create is false, a missing file returns os.ErrNotExist. + // Negative sizes are rejected. + Truncate func(ctx context.Context, path string, size int64, create bool) error + // PortableErr normalizes an OS error to a POSIX-style message. PortableErr func(err error) string diff --git a/builtins/tests/truncate/hardening_test.go b/builtins/tests/truncate/hardening_test.go new file mode 100644 index 000000000..a1e22d608 --- /dev/null +++ b/builtins/tests/truncate/hardening_test.go @@ -0,0 +1,188 @@ +// 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 truncate_test + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestHardenCompactShortFlag verifies that pflag's combined short-flag form +// (-cs0) is recognised: -c is boolean, -s takes a value, the value can be +// glued. POSIX-style short-flag chaining is part of pflag's contract; we +// just confirm we have not accidentally disabled it. +func TestHardenCompactShortFlag(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "a.txt", "content") + _, stderr, code := truncateRun(t, "truncate -cs0 a.txt", dir) + if code != 0 { + t.Fatalf("exit %d, stderr=%q", code, stderr) + } + if got := fileSize(t, a); got != 0 { + t.Errorf("size = %d, want 0", got) + } +} + +// TestHardenLastSizeWins verifies pflag's default last-value-wins behaviour +// for repeated flags. We document the behaviour in case a future change +// switches to first-value-wins or rejects duplicates. +func TestHardenLastSizeWins(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "a.txt", "abcdef") + _, _, code := truncateRun(t, "truncate -s 100 -s 0 a.txt", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + if got := fileSize(t, a); got != 0 { + t.Errorf("size = %d, want 0 (last -s wins)", got) + } +} + +// TestHardenDoubleDashTreatsLaterFlagsAsFiles verifies that "--" forces +// every following token to be treated as a positional argument, even +// tokens that look like flags. This is critical for safety: a user- +// supplied filename that happens to start with -- must not silently mutate +// behaviour. +func TestHardenDoubleDashTreatsLaterFlagsAsFiles(t *testing.T) { + dir := t.TempDir() + // Filename literally named "--size=99". Without -- it would be + // parsed as the --size flag value. + weird := writeFile(t, dir, "--size=99", "content") + _, _, code := truncateRun(t, "truncate -s 0 -- '--size=99'", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + if got := fileSize(t, weird); got != 0 { + t.Errorf("size of '--size=99' = %d, want 0", got) + } +} + +// TestHardenMissingSizeWithNoCreate verifies that -c alone (no -s) is +// rejected just like the no-flag case. -c is a modifier of behaviour, +// not a substitute for --size. +func TestHardenMissingSizeWithNoCreate(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "a.txt", "abc") + _, stderr, code := truncateRun(t, "truncate -c a.txt", dir) + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } + if !strings.Contains(stderr, "--size") { + t.Errorf("stderr should hint at --size: %q", stderr) + } +} + +// TestHardenLargeSparseSize covers the case where the user requests an +// extension to a very large but kernel-acceptable size (1 GiB sparse). +// The file system records the size in metadata only; no allocation +// happens. We use 1 << 30 to exercise the suffix path and confirm we +// don't crash for sizes larger than typical files. +func TestHardenLargeSparseSize(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "f.bin", "") + _, stderr, code := truncateRun(t, "truncate -s 1G f.bin", dir) + if code != 0 { + t.Fatalf("exit %d, stderr=%q", code, stderr) + } + if got := fileSize(t, a); got != 1<<30 { + t.Errorf("size = %d, want %d", got, 1<<30) + } + // Sanity check: this should be a sparse file, but we don't assert on + // disk usage because that depends on the filesystem (APFS, ext4, NTFS + // all handle sparse differently). The stat'd size is what matters. +} + +// TestHardenDirectoryTarget verifies that calling truncate on a directory +// returns exit 1 with a clear error and does not panic. +func TestHardenDirectoryTarget(t *testing.T) { + dir := t.TempDir() + subdir := filepath.Join(dir, "subdir") + if err := os.MkdirAll(subdir, 0755); err != nil { + t.Fatal(err) + } + _, stderr, code := truncateRun(t, "truncate -s 0 subdir", dir) + if code != 1 { + t.Fatalf("exit %d, want 1; stderr=%q", code, stderr) + } + if !strings.Contains(stderr, "subdir") { + t.Errorf("stderr should mention subdir: %q", stderr) + } +} + +// TestHardenCreatePreservesMode verifies that newly-created files use a +// reasonable default mode (0644 minus umask). This guards against an +// accidental switch to 0666 or 0777. +func TestHardenCreatePreservesMode(t *testing.T) { + dir := t.TempDir() + _, _, code := truncateRun(t, "truncate -s 0 fresh.bin", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + info, err := os.Stat(filepath.Join(dir, "fresh.bin")) + if err != nil { + t.Fatal(err) + } + mode := info.Mode().Perm() + // On Unix the umask reduces the requested 0644 to whatever the test + // process's umask permits. We require: no setuid/setgid/sticky bits, + // no group-/world-writable bits, no execute bits. + if mode&0o111 != 0 { + t.Errorf("created file should not be executable: mode=%o", mode) + } + if mode&0o022 != 0 { + t.Errorf("created file should not be group/world writable: mode=%o", mode) + } + if info.Mode()&(os.ModeSetuid|os.ModeSetgid|os.ModeSticky) != 0 { + t.Errorf("created file should have no special bits: mode=%o", info.Mode()) + } +} + +// TestHardenLargeSizeRejected verifies that truncate refuses to multiply +// past int64 — a request that overflows the multiplier ceiling fails +// before reaching the kernel. +func TestHardenLargeSizeRejected(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "a.txt", "abc") + // 8388608T = 8 EiB which is one above MaxInt64; parseSize should + // reject before the kernel sees it. + _, stderr, code := truncateRun(t, "truncate -s 8388608T a.txt", dir) + if code != 1 { + t.Fatalf("exit %d, want 1; stderr=%q", code, stderr) + } + if !strings.Contains(stderr, "invalid size") { + t.Errorf("stderr missing 'invalid size': %q", stderr) + } +} + +// TestHardenZeroSizeOnAlreadyEmpty verifies that truncating an already- +// empty file is a no-op, not a failure. +func TestHardenZeroSizeOnAlreadyEmpty(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "empty.txt", "") + _, stderr, code := truncateRun(t, "truncate -s 0 empty.txt", dir) + if code != 0 { + t.Fatalf("exit %d, stderr=%q", code, stderr) + } + if got := fileSize(t, a); got != 0 { + t.Errorf("size = %d, want 0", got) + } +} + +// TestHardenSpecialCharsInFilename verifies that filenames with embedded +// spaces and unicode are handled correctly when shell-quoted. +func TestHardenSpecialCharsInFilename(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "weird name with spaces and é.txt", "abcdef") + _, _, code := truncateRun(t, `truncate -s 0 'weird name with spaces and é.txt'`, dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + if got := fileSize(t, a); got != 0 { + t.Errorf("size = %d, want 0", got) + } +} diff --git a/builtins/tests/truncate/helpers_test.go b/builtins/tests/truncate/helpers_test.go new file mode 100644 index 000000000..158e43b92 --- /dev/null +++ b/builtins/tests/truncate/helpers_test.go @@ -0,0 +1,72 @@ +// 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 truncate_test + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + + "mvdan.cc/sh/v3/syntax" + + "github.com/DataDog/rshell/internal/interpoption" + "github.com/DataDog/rshell/interp" +) + +// runScript parses and runs the given shell script through interp.Runner with +// AllowAllCommands set, plus any additional options. It returns stdout, +// stderr, and the resolved exit code. +func runScript(t *testing.T, script, dir string, opts ...interp.RunnerOption) (string, string, int) { + t.Helper() + return runScriptCtx(context.Background(), t, script, dir, opts...) +} + +func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts ...interp.RunnerOption) (string, string, int) { + t.Helper() + parser := syntax.NewParser() + prog, err := parser.Parse(strings.NewReader(script), "") + if err != nil { + t.Fatal(err) + } + var outBuf, errBuf bytes.Buffer + allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf), interpoption.AllowAllCommands().(interp.RunnerOption)}, opts...) + runner, err := interp.New(allOpts...) + if err != nil { + t.Fatal(err) + } + defer runner.Close() + if dir != "" { + runner.Dir = dir + } + runErr := runner.Run(ctx, prog) + exitCode := 0 + if runErr != nil { + var es interp.ExitStatus + if errors.As(runErr, &es) { + exitCode = int(es) + } else if ctx.Err() == nil { + t.Fatalf("unexpected error: %v", runErr) + } + } + return outBuf.String(), errBuf.String(), exitCode +} + +// truncateRun runs script with AllowedPaths restricted to dir. Use this +// wrapper for tests where the sandbox boundary is the temp dir itself. +func truncateRun(t *testing.T, script, dir string) (string, string, int) { + t.Helper() + return runScript(t, script, dir, interp.AllowedPaths([]string{dir})) +} + +// newCancelledContext returns a context that is already cancelled. Used by +// tests that exercise the handler's per-iteration ctx.Err() check. +func newCancelledContext() (context.Context, context.CancelFunc) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + return ctx, cancel +} diff --git a/builtins/tests/truncate/truncate_fuzz_test.go b/builtins/tests/truncate/truncate_fuzz_test.go new file mode 100644 index 000000000..3df84ee1d --- /dev/null +++ b/builtins/tests/truncate/truncate_fuzz_test.go @@ -0,0 +1,261 @@ +// 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 truncate_test + +import ( + "context" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/DataDog/rshell/builtins/testutil" + "github.com/DataDog/rshell/interp" +) + +// FuzzTruncateSize fuzzes the -s SIZE parser by routing arbitrary +// byte-strings through the runner. The corpus seeds every distinct +// numeric / suffix / modifier shape parseSize is expected to handle, and +// the fuzz body asserts only that exit code is 0 or 1 (no panic, no +// hang, no other status). +func FuzzTruncateSize(f *testing.F) { + // Bare digits — boundaries. + f.Add("0") + f.Add("1") + f.Add("9223372036854775807") // exact MaxInt64. + f.Add("9223372036854775808") // one above. + f.Add("99999999999999999999") // far past int64. + f.Add("00000000000000000000") + + // Every accepted suffix. + for _, s := range []string{ + "K", "k", "KB", "KiB", + "M", "m", "MB", "MiB", + "G", "g", "GB", "GiB", + "T", "t", "TB", "TiB", + } { + f.Add("0" + s) + f.Add("1" + s) + f.Add("123" + s) + } + + // Suffix-overflow neighbours. + f.Add("8388607T") // largest accepted T-suffix. + f.Add("8388608T") // overflow. + f.Add("9007199254740992K") + + // GNU relative-size modifiers — must always reject with errRelativeSize. + for _, p := range []string{"+", "-", "<", ">", "/", "%"} { + f.Add(p) + f.Add(p + "0") + f.Add(p + "10") + f.Add(p + "10K") + } + + // Malformed inputs. + f.Add("") + f.Add(" ") + f.Add(" ") + f.Add("abc") + f.Add("1.5") + f.Add("0x10") + f.Add("0b10") + f.Add("1KIB") + f.Add("1kB") + f.Add("1MiB1") + f.Add("K") + f.Add("KB") + f.Add("--size=0") + + // CVE-class adversarial inputs: huge integer string, embedded null + // bytes, control characters. The shell parser may strip some of + // these before they reach truncate; we rely on the runner to model + // realistic shell behaviour. + f.Add(strings.Repeat("9", 100)) + f.Add("0\x00") + f.Add("\x00") + f.Add("\x1b[0m") + + baseDir := f.TempDir() + var counter atomic.Int64 + + f.Fuzz(func(t *testing.T, sizeArg string) { + if t.Context().Err() != nil { + return + } + // Cap size string length to keep iterations cheap; an attacker- + // controlled size larger than 1 KiB is not a realistic scenario. + if len(sizeArg) > 1024 { + return + } + // Reject size strings containing shell metacharacters that would + // alter parsing — we want to exercise parseSize, not the shell. + if strings.ContainsAny(sizeArg, "'\"\\$`\n\r") { + return + } + + dir, cleanup := testutil.FuzzIterDir(t, baseDir, &counter) + defer cleanup() + + if err := os.WriteFile(filepath.Join(dir, "f.txt"), []byte("data"), 0644); err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + script := "truncate -s '" + sizeArg + "' f.txt" + _, _, code := runScriptCtxFuzz(ctx, t, script, dir) + if t.Context().Err() != nil { + return + } + if code != 0 && code != 1 { + t.Errorf("unexpected exit code %d for size %q", code, sizeArg) + } + }) +} + +// FuzzTruncateFlags fuzzes the flag parser by submitting arbitrary +// argument strings before a fixed --size 0 and a single file operand. +// Confirms that no flag combination crashes the runner — only exit codes +// 0 and 1 are valid outcomes. +func FuzzTruncateFlags(f *testing.F) { + // Known flags and combinations. + f.Add("") + f.Add("-c") + f.Add("--no-create") + f.Add("--help") + f.Add("-h") + f.Add("-cs") // missing value for -s; expected reject. + f.Add("-c -s") // separated, missing value. + f.Add("--size=0 -c") + f.Add("-c -c") // duplicate boolean. + f.Add("--size=0 --size=1") // duplicate value flag. + // Unknown / deferred flags — must be rejected. + f.Add("-r ref.txt") + f.Add("--reference=ref.txt") + f.Add("-o") + f.Add("--io-blocks") + f.Add("--unknown") + f.Add("-X") + // Double-dash separator games. + f.Add("--") + f.Add("-- -file") + f.Add("-- --size=0") + // Numeric-looking flags (which would matter if NormalizeArgs were ever + // added later — currently truncate has no normaliser, so -5 is just an + // unknown short flag). + f.Add("-5") + f.Add("--size -5") + + baseDir := f.TempDir() + var counter atomic.Int64 + + f.Fuzz(func(t *testing.T, flagArgs string) { + if t.Context().Err() != nil { + return + } + if len(flagArgs) > 256 { + return + } + if strings.ContainsAny(flagArgs, "'\"\\$`\n\r;|&><()") { + return + } + + dir, cleanup := testutil.FuzzIterDir(t, baseDir, &counter) + defer cleanup() + if err := os.WriteFile(filepath.Join(dir, "f.txt"), []byte("data"), 0644); err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + script := "truncate " + flagArgs + " f.txt" + _, _, code := runScriptCtxFuzz(ctx, t, script, dir) + if t.Context().Err() != nil { + return + } + if code != 0 && code != 1 { + t.Errorf("unexpected exit code %d for flags %q", code, flagArgs) + } + }) +} + +// FuzzTruncateFilenames fuzzes the operand path. Exit codes other than 0 +// or 1 indicate a panic or unhandled error path. AllowedPaths is set to +// dir, so paths outside dir must be rejected by the sandbox cleanly. +func FuzzTruncateFilenames(f *testing.F) { + // Plain names. + f.Add("file.txt") + f.Add("a") + f.Add("dir/file") + // Path-traversal payloads. + f.Add("../escape") + f.Add("../../etc/hosts") + f.Add("./././file") + f.Add("//double//slash//file") + // Absolute paths outside the sandbox. + f.Add("/etc/hosts") + f.Add("/dev/null") + f.Add("/tmp/escape") + // Special characters and unicode. + f.Add("file with spaces.txt") + f.Add("file\twith\ttabs.txt") + f.Add("é-unicode.txt") + f.Add("漢字.txt") + // Long names. + f.Add(strings.Repeat("a", 200) + ".txt") + // Names that look like flags (will be passed positional, not as flag). + f.Add("-dashfile") + f.Add("--dashed-file") + f.Add("-c") + + baseDir := f.TempDir() + var counter atomic.Int64 + + f.Fuzz(func(t *testing.T, filename string) { + if t.Context().Err() != nil { + return + } + if len(filename) == 0 || len(filename) > 512 { + return + } + // Filter shell-special chars; we want to exercise the sandbox / + // open path, not shell parsing. + if strings.ContainsAny(filename, "'\"\\$`\n\r;|&><()") { + return + } + // Reject strings with embedded NULs which the OS rejects on most + // filesystems before reaching truncate's logic. + if strings.ContainsRune(filename, 0) { + return + } + + dir, cleanup := testutil.FuzzIterDir(t, baseDir, &counter) + defer cleanup() + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + script := "truncate -s 0 -- '" + filename + "'" + _, _, code := runScriptCtxFuzz(ctx, t, script, dir) + if t.Context().Err() != nil { + return + } + if code != 0 && code != 1 { + t.Errorf("unexpected exit code %d for filename %q", code, filename) + } + }) +} + +// runScriptCtxFuzz wraps runScriptCtx with AllowedPaths set to dir so the +// sandbox boundary matches the temp directory each fuzz iteration uses. +// The name is distinct from runScriptCtx to avoid future collisions if +// the integration-test helper grows additional positional parameters. +func runScriptCtxFuzz(ctx context.Context, t *testing.T, script, dir string) (string, string, int) { + t.Helper() + return runScriptCtx(ctx, t, script, dir, interp.AllowedPaths([]string{dir})) +} diff --git a/builtins/tests/truncate/truncate_test.go b/builtins/tests/truncate/truncate_test.go new file mode 100644 index 000000000..9ef700ed3 --- /dev/null +++ b/builtins/tests/truncate/truncate_test.go @@ -0,0 +1,396 @@ +// 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 truncate_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/DataDog/rshell/interp" +) + +// fileSize returns the size of path, or fails the test if it cannot be stat'd. +func fileSize(t *testing.T, path string) int64 { + t.Helper() + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat %s: %v", path, err) + } + return info.Size() +} + +// writeFile writes content to dir/name. Used to seed test fixtures. +func writeFile(t *testing.T, dir, name, content string) string { + t.Helper() + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("write %s: %v", path, err) + } + return path +} + +// TestTruncateZeroSize covers the demo's "nuclear option" — truncate -s 0 on +// a populated file zeros it out without removing the inode. +func TestTruncateZeroSize(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "log.txt", "abcdefghij") + stdout, stderr, code := truncateRun(t, "truncate -s 0 log.txt", dir) + if code != 0 { + t.Fatalf("exit %d, stderr=%q", code, stderr) + } + if stdout != "" { + t.Errorf("unexpected stdout: %q", stdout) + } + if stderr != "" { + t.Errorf("unexpected stderr: %q", stderr) + } + if got := fileSize(t, path); got != 0 { + t.Errorf("post-truncate size = %d, want 0", got) + } +} + +// TestTruncateExtend covers the case where SIZE is larger than the current +// file: bytes are zero-extended (sparse on most filesystems). +func TestTruncateExtend(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "log.txt", "abc") + _, stderr, code := truncateRun(t, "truncate -s 1024 log.txt", dir) + if code != 0 { + t.Fatalf("exit %d, stderr=%q", code, stderr) + } + if got := fileSize(t, path); got != 1024 { + t.Errorf("post-extend size = %d, want 1024", got) + } +} + +// TestTruncateShrink covers the case where SIZE is smaller than the current +// file: trailing bytes are dropped, leading bytes are preserved verbatim. +func TestTruncateShrink(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "log.txt", "0123456789") + _, _, code := truncateRun(t, "truncate -s 5 log.txt", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + if got := fileSize(t, path); got != 5 { + t.Errorf("size = %d, want 5", got) + } + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(body) != "01234" { + t.Errorf("content = %q, want %q", body, "01234") + } +} + +// TestTruncateSizeSuffixes covers each accepted suffix end-to-end via the +// shell, in addition to the unit-level coverage in parseSize tests. +func TestTruncateSizeSuffixes(t *testing.T) { + cases := []struct { + size string + want int64 + }{ + {"512", 512}, + {"1K", 1024}, + {"1KiB", 1024}, + {"1KB", 1000}, + {"1M", 1 << 20}, + {"2MiB", 2 << 20}, + {"1MB", 1000 * 1000}, + } + for _, tc := range cases { + t.Run(tc.size, func(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "f.bin", "") + script := "truncate -s " + tc.size + " f.bin" + _, stderr, code := truncateRun(t, script, dir) + if code != 0 { + t.Fatalf("exit %d, stderr=%q", code, stderr) + } + if got := fileSize(t, path); got != tc.want { + t.Errorf("size = %d, want %d", got, tc.want) + } + }) + } +} + +// TestTruncateLongFlag confirms --size= is accepted and equivalent to -s. +func TestTruncateLongFlag(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "f.bin", "abcde") + _, _, code := truncateRun(t, "truncate --size=0 f.bin", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + if got := fileSize(t, path); got != 0 { + t.Errorf("size = %d, want 0", got) + } +} + +// TestTruncateMultipleFiles covers two related properties: +// - Without -c, every operand is processed in order — including missing +// files, which are created. Exit code is 0. +// - With -c, missing files become permission-denied-like errors only +// when the missing file is outside an allowed root; an ordinarily +// missing file is silently skipped (exit 0). To exercise the failure +// path under -c, we drop a sub-operand outside AllowedPaths. +func TestTruncateMultipleFiles(t *testing.T) { + dir := t.TempDir() + a := writeFile(t, dir, "a.txt", "hello a") + b := writeFile(t, dir, "b.txt", "hello b") + stdout, stderr, code := truncateRun(t, "truncate -s 0 a.txt fresh.txt b.txt", dir) + if code != 0 { + t.Fatalf("exit %d, want 0; stdout=%q stderr=%q", code, stdout, stderr) + } + if fileSize(t, a) != 0 { + t.Error("a.txt was not truncated") + } + if fileSize(t, b) != 0 { + t.Error("b.txt was not truncated") + } + if got := fileSize(t, filepath.Join(dir, "fresh.txt")); got != 0 { + t.Errorf("fresh.txt was not created at size 0: %d", got) + } +} + +// TestTruncateMultipleFilesPartialFailure verifies that one failing +// operand does not abort the loop: the surviving files are still +// truncated and the final exit code is 1. +func TestTruncateMultipleFilesPartialFailure(t *testing.T) { + dir := t.TempDir() + insideDir := filepath.Join(dir, "inside") + if err := os.MkdirAll(insideDir, 0755); err != nil { + t.Fatal(err) + } + a := writeFile(t, insideDir, "a.txt", "alpha") + b := writeFile(t, insideDir, "b.txt", "beta") + // outside.txt is reachable on disk but blocked by AllowedPaths. + outsidePath := writeFile(t, dir, "outside.txt", "untouched") + + _, stderr, code := runScript(t, "truncate -s 0 a.txt ../outside.txt b.txt", insideDir, + interp.AllowedPaths([]string{insideDir})) + if code != 1 { + t.Fatalf("exit %d, want 1; stderr=%q", code, stderr) + } + if !strings.Contains(stderr, "outside.txt") { + t.Errorf("stderr should mention outside.txt: %q", stderr) + } + if fileSize(t, a) != 0 { + t.Error("a.txt was not truncated despite later failure") + } + if fileSize(t, b) != 0 { + t.Error("b.txt was not truncated despite earlier failure") + } + if got := fileSize(t, outsidePath); got != int64(len("untouched")) { + t.Errorf("outside.txt was modified despite sandbox: size %d", got) + } +} + +// TestTruncateNoCreateMissingFile confirms that -c silently skips a missing +// file, returning exit 0 and not creating the file. +func TestTruncateNoCreateMissingFile(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := truncateRun(t, "truncate -c -s 0 missing.txt", dir) + if code != 0 { + t.Fatalf("exit %d, stderr=%q", code, stderr) + } + if stdout != "" || stderr != "" { + t.Errorf("unexpected output: stdout=%q stderr=%q", stdout, stderr) + } + if _, err := os.Stat(filepath.Join(dir, "missing.txt")); !os.IsNotExist(err) { + t.Error("missing.txt was created when -c was passed") + } +} + +// TestTruncateCreatesByDefault confirms that without -c, missing files are +// created. +func TestTruncateCreatesByDefault(t *testing.T) { + dir := t.TempDir() + _, _, code := truncateRun(t, "truncate -s 100 fresh.bin", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + path := filepath.Join(dir, "fresh.bin") + if got := fileSize(t, path); got != 100 { + t.Errorf("size = %d, want 100", got) + } +} + +// TestTruncateMissingSize verifies that running truncate without -s/--size +// is rejected with exit 1 and a clear error message. +func TestTruncateMissingSize(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "a.txt", "") + _, stderr, code := truncateRun(t, "truncate a.txt", dir) + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } + if !strings.Contains(stderr, "--size") { + t.Errorf("stderr should hint at --size: %q", stderr) + } +} + +// TestTruncateMissingFile verifies that -s SIZE without any file operand is +// rejected with exit 1. +func TestTruncateMissingFile(t *testing.T) { + dir := t.TempDir() + _, stderr, code := truncateRun(t, "truncate -s 0", dir) + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } + if !strings.Contains(stderr, "missing file operand") { + t.Errorf("stderr should say missing file operand: %q", stderr) + } +} + +// TestTruncateRejectsRelativeSize verifies every GNU relative-size modifier +// (+, -, <, >, /, %) is rejected with errRelativeSize wording. +// +// The size argument is single-quoted in the shell command so that the +// parser does not interpret '<' or '>' as redirection operators. +func TestTruncateRejectsRelativeSize(t *testing.T) { + prefixes := []string{"+", "-", "<", ">", "/", "%"} + for _, p := range prefixes { + t.Run(p, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "a.txt", "abc") + script := "truncate -s '" + p + "10' a.txt" + _, stderr, code := truncateRun(t, script, dir) + if code != 1 { + t.Fatalf("exit %d, want 1; stderr=%q", code, stderr) + } + if !strings.Contains(stderr, "relative size operators not supported") { + t.Errorf("stderr should explain unsupported relative size: %q", stderr) + } + }) + } +} + +// TestTruncateRejectsInvalidSize verifies non-numeric and overflow inputs +// produce exit 1 with the generic "invalid size" message. +func TestTruncateRejectsInvalidSize(t *testing.T) { + cases := []string{"abc", "1.5", "9999999999999999999999", "1KIB", "1kB"} + for _, c := range cases { + t.Run(c, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "a.txt", "abc") + script := "truncate -s " + c + " a.txt" + _, stderr, code := truncateRun(t, script, dir) + if code != 1 { + t.Fatalf("exit %d, want 1; stderr=%q", code, stderr) + } + if !strings.Contains(stderr, "invalid size") { + t.Errorf("stderr missing 'invalid size': %q", stderr) + } + }) + } +} + +// TestTruncateUnknownFlag verifies that flags we deliberately did not +// implement (--reference, -o) are rejected as unknown, never silently +// accepted. +func TestTruncateUnknownFlag(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "a.txt", "abc") + writeFile(t, dir, "ref.txt", "1234567890") + _, stderr, code := truncateRun(t, "truncate --reference=ref.txt a.txt", dir) + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } + if !strings.Contains(stderr, "truncate") { + t.Errorf("stderr should mention truncate: %q", stderr) + } +} + +// TestTruncateOutsideAllowedPath verifies that the sandbox rejects targets +// outside AllowedPaths before any open syscall is issued. +func TestTruncateOutsideAllowedPath(t *testing.T) { + dir := t.TempDir() + insideDir := filepath.Join(dir, "inside") + if err := os.MkdirAll(insideDir, 0755); err != nil { + t.Fatal(err) + } + outsidePath := filepath.Join(dir, "outside.txt") + writeFile(t, dir, "outside.txt", "leave me alone") + + _, stderr, code := runScript(t, "truncate -s 0 ../outside.txt", insideDir, + interp.AllowedPaths([]string{insideDir})) + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } + if !strings.Contains(stderr, "permission denied") { + t.Errorf("stderr should report permission denied: %q", stderr) + } + if got := fileSize(t, outsidePath); got != int64(len("leave me alone")) { + t.Errorf("outside.txt was modified: size %d", got) + } +} + +// TestTruncateDoubleDash verifies that "--" lets users target a filename +// that begins with "-". +func TestTruncateDoubleDash(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "-dashfile", "abcdef") + _, _, code := truncateRun(t, "truncate -s 0 -- -dashfile", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + if got := fileSize(t, filepath.Join(dir, "-dashfile")); got != 0 { + t.Errorf("size = %d, want 0", got) + } +} + +// TestTruncateHelp covers --help: usage on stdout, exit 0, no stderr. +func TestTruncateHelp(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := truncateRun(t, "truncate --help", dir) + if code != 0 { + t.Fatalf("exit %d", code) + } + if stderr != "" { + t.Errorf("--help should write nothing to stderr: %q", stderr) + } + for _, want := range []string{"Usage: truncate", "--size", "--no-create"} { + if !strings.Contains(stdout, want) { + t.Errorf("--help stdout missing %q: %q", want, stdout) + } + } +} + +// TestTruncateContextCancellation verifies that a cancelled context aborts +// the iteration without panicking. We assert no panic and that the per- +// iteration ctx.Err() check leaves at least one operand unmodified. +func TestTruncateContextCancellation(t *testing.T) { + dir := t.TempDir() + paths := make([]string, 5) + for i := range paths { + paths[i] = writeFile(t, dir, "f"+string(rune('0'+i))+".txt", "abc") + } + ctx, cancel := newCancelledContext() + defer cancel() + + // Returning at all (no panic) is the primary pass condition. + _, _, _ = runScriptCtx(ctx, t, "truncate -s 0 f0.txt f1.txt f2.txt f3.txt f4.txt", dir, + interp.AllowedPaths([]string{dir})) + + // The handler checks ctx.Err() before each operand, so at least the + // final operand should be left at its original 3 bytes when ctx was + // cancelled before any work began. If every file was zeroed, the + // cancellation check is silently broken. + allZeroed := true + for _, p := range paths { + if fileSize(t, p) != 0 { + allZeroed = false + break + } + } + if allZeroed { + t.Errorf("expected ctx.Err() check to abort iteration; all files were truncated") + } +} diff --git a/builtins/truncate/truncate.go b/builtins/truncate/truncate.go new file mode 100644 index 000000000..c2c7dada8 --- /dev/null +++ b/builtins/truncate/truncate.go @@ -0,0 +1,254 @@ +// 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 truncate implements the truncate builtin command. +// +// truncate — shrink or extend the size of a file to a specified size +// +// Usage: truncate [OPTION]... FILE... +// +// Shrink or extend the size of each FILE to the specified size. A file that +// is larger than the specified size is truncated; a file that is smaller is +// extended (the extension reads as zero bytes). When the file does not yet +// exist it is created (mode 0644) unless --no-create is given. +// +// All file operations go through the AllowedPaths sandbox. Targets outside +// the sandbox are rejected with a permission error before any open syscall +// is issued. +// +// Accepted flags: +// +// -s SIZE, --size=SIZE +// Set or adjust the file size to SIZE bytes. SIZE is a non-negative +// integer with an optional suffix: +// +// K = k = KiB = 1024 KB = 1000 +// M = m = MiB = 1024^2 MB = 1000^2 +// G = g = GiB = 1024^3 GB = 1000^3 +// T = t = TiB = 1024^4 TB = 1000^4 +// +// The bare single-letter forms accept either case. The "iB" and "B" +// multi-letter forms are case-sensitive on the leading letter, as +// GNU truncate does. +// +// -c, --no-create +// Do not create files that do not already exist. With -c, missing +// files are silently skipped (matching GNU truncate). +// +// -h, --help +// Print this usage message to stdout and exit 0. +// +// Out of scope (not implemented; rejected as unknown flags): +// +// -r REF, --reference=FILE set size from a reference file +// -o, --io-blocks treat SIZE as a block count +// relative size modifiers in -s (+, -, <, >, /, %) +// +// Exit codes: +// +// 0 All files processed successfully. +// 1 At least one file failed (invalid size, permission denied, missing +// file without -c, etc.). Processing continues across all operands so +// that a single failure does not abort the run; exit 1 is returned at +// the end if any operand failed. +// +// Memory safety: +// +// truncate performs no I/O on file contents — only metadata. The sandbox +// opens the file with O_WRONLY (+ O_CREATE when allowed) and calls +// ftruncate(2) on the resulting fd. No buffers are allocated proportional +// to user input; the only user-controlled numeric is the size argument, +// which is validated for overflow before reaching the kernel. +package truncate + +import ( + "context" + "errors" + "math" + "os" + "strconv" + + "github.com/DataDog/rshell/builtins" +) + +// Cmd is the truncate builtin command descriptor. +var Cmd = builtins.Command{ + Name: "truncate", + Description: "shrink or extend file size", + MakeFlags: registerFlags, +} + +// errInvalidSize is returned by parseSize for any non-numeric, malformed, +// or out-of-range input. The handler wraps it into a POSIX-style error +// message that includes the offending size string. +var errInvalidSize = errors.New("invalid size") + +// errRelativeSize is returned by parseSize when the size value carries a +// leading +/-/<>//% modifier, which GNU truncate uses for relative size +// adjustments. We surface a dedicated error so the handler can hint that +// these forms are intentionally not supported. +var errRelativeSize = errors.New("relative size operators not supported") + +func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { + help := fs.BoolP("help", "h", false, "print usage and exit") + sizeStr := fs.StringP("size", "s", "", "set or adjust file size to SIZE bytes") + noCreate := fs.BoolP("no-create", "c", false, "do not create files that do not exist") + + return func(ctx context.Context, callCtx *builtins.CallContext, files []string) builtins.Result { + if *help { + callCtx.Out("Usage: truncate [OPTION]... FILE...\n") + callCtx.Out("Shrink or extend the size of each FILE to the specified size.\n") + callCtx.Out("A FILE smaller than SIZE is extended with zero bytes; a FILE\n") + callCtx.Out("larger than SIZE is truncated. Missing files are created unless\n") + callCtx.Out("--no-create is given.\n\n") + fs.SetOutput(callCtx.Stdout) + fs.PrintDefaults() + return builtins.Result{} + } + + // Capability check first — it is a setup invariant rather than a + // per-input validation. Failing fast here keeps the error message + // stable even when the user supplies a malformed --size. + if callCtx.Truncate == nil { + callCtx.Errf("truncate: filesystem capability not available\n") + return builtins.Result{Code: 1} + } + + if !fs.Changed("size") { + callCtx.Errf("truncate: you must specify --size\n") + return builtins.Result{Code: 1} + } + + size, err := parseSize(*sizeStr) + if err != nil { + if errors.Is(err, errRelativeSize) { + callCtx.Errf("truncate: invalid size %q: %s\n", *sizeStr, err) + } else { + callCtx.Errf("truncate: invalid size %q\n", *sizeStr) + } + return builtins.Result{Code: 1} + } + + if len(files) == 0 { + callCtx.Errf("truncate: missing file operand\n") + return builtins.Result{Code: 1} + } + + var failed bool + for _, file := range files { + if ctx.Err() != nil { + return builtins.Result{Code: 1} + } + err := callCtx.Truncate(ctx, file, size, !*noCreate) + if err == nil { + continue + } + if *noCreate && errors.Is(err, os.ErrNotExist) { + continue + } + // The underlying failure can come from open (permission + // denied, not-a-regular-file, ENOENT without -c) or from + // ftruncate (ENOSPC, EFBIG, EINVAL); use a phase-neutral + // message so the operator is not misled when an open + // succeeded but the size change failed. + callCtx.Errf("truncate: %q: %s\n", file, callCtx.PortableErr(err)) + failed = true + } + + if failed { + return builtins.Result{Code: 1} + } + return builtins.Result{} + } +} + +// sizeMultipliers maps suffix tokens accepted by -s to their byte +// multipliers, matching GNU coreutils: +// +// - bare single-letter binary forms accept either case (K=k=KiB=1024) +// - the explicit "B" decimal forms (KB, MB, GB, TB) and the "iB" forms +// (KiB, MiB, GiB, TiB) are case-sensitive on the leading letter, as +// GNU truncate does +// +// "" maps to 1 so a bare digit string falls through with no multiplication. +var sizeMultipliers = map[string]int64{ + "": 1, + "K": 1 << 10, + "k": 1 << 10, + "KiB": 1 << 10, + "KB": 1000, + "M": 1 << 20, + "m": 1 << 20, + "MiB": 1 << 20, + "MB": 1000 * 1000, + "G": 1 << 30, + "g": 1 << 30, + "GiB": 1 << 30, + "GB": 1000 * 1000 * 1000, + "T": 1 << 40, + "t": 1 << 40, + "TiB": 1 << 40, + "TB": 1000 * 1000 * 1000 * 1000, +} + +// parseSize parses the value of -s/--size into a non-negative byte count. +// +// The grammar matches GNU truncate: +// +// size := digit+ suffix? +// suffix := "K" | "k" | "KB" | "KiB" | +// "M" | "m" | "MB" | "MiB" | +// "G" | "g" | "GB" | "GiB" | +// "T" | "t" | "TB" | "TiB" +// +// The bare single-letter binary forms accept either case; the multi-letter +// "B" decimal and "iB" binary forms are case-sensitive on the leading +// letter (e.g. "KB" is accepted, "kB" is not — GNU rejects it too). +// +// Leading +/-/<>//% modifiers (the GNU relative-size syntax) are rejected +// with errRelativeSize so the caller can surface a hint that these forms +// are intentionally unsupported. Any other malformed input, or any value +// whose product overflows int64, returns errInvalidSize. +func parseSize(s string) (int64, error) { + if s == "" { + return 0, errInvalidSize + } + switch s[0] { + case '+', '-', '<', '>', '/', '%': + return 0, errRelativeSize + } + + // Locate the digit/suffix boundary by scanning for the first non-digit + // byte. ParseInt would otherwise consume the suffix as part of the + // number for hexadecimal-looking inputs. + i := 0 + for i < len(s) && s[i] >= '0' && s[i] <= '9' { + i++ + } + if i == 0 { + return 0, errInvalidSize + } + digits, suffix := s[:i], s[i:] + + mult, ok := sizeMultipliers[suffix] + if !ok { + return 0, errInvalidSize + } + + // digits is guaranteed to be a non-empty all-digit string by the loop + // above, so ParseInt cannot fail on parse — only on overflow (ErrRange). + n, err := strconv.ParseInt(digits, 10, 64) + if err != nil { + return 0, errInvalidSize + } + + if mult == 1 { + return n, nil + } + if n > math.MaxInt64/mult { + return 0, errInvalidSize + } + return n * mult, nil +} diff --git a/builtins/truncate/truncate_test.go b/builtins/truncate/truncate_test.go new file mode 100644 index 000000000..93c0410e1 --- /dev/null +++ b/builtins/truncate/truncate_test.go @@ -0,0 +1,152 @@ +// 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 truncate + +import ( + "errors" + "math" + "testing" +) + +// TestParseSizeAccepts covers every accepted suffix and key boundary value. +// Any change to the suffix table or numeric handling should keep these +// outputs byte-identical to GNU truncate's behaviour for the same inputs. +func TestParseSizeAccepts(t *testing.T) { + cases := []struct { + input string + want int64 + }{ + // Bare digits. + {"0", 0}, + {"1", 1}, + {"123456", 123456}, + {"9223372036854775807", math.MaxInt64}, // exact int64 ceiling. + + // Binary suffixes (1024-based) — both cases. + {"1K", 1 << 10}, + {"1k", 1 << 10}, + {"1KiB", 1 << 10}, + {"2K", 2 << 10}, + {"1M", 1 << 20}, + {"1m", 1 << 20}, + {"1MiB", 1 << 20}, + {"1G", 1 << 30}, + {"1g", 1 << 30}, + {"1GiB", 1 << 30}, + {"1T", 1 << 40}, + {"1t", 1 << 40}, + {"1TiB", 1 << 40}, + + // Decimal suffixes (1000-based) — case-sensitive on the leading letter. + {"1KB", 1000}, + {"1MB", 1000 * 1000}, + {"1GB", 1000 * 1000 * 1000}, + {"1TB", 1000 * 1000 * 1000 * 1000}, + + // Zero with suffix is still zero. + {"0K", 0}, + {"0MB", 0}, + } + for _, tc := range cases { + got, err := parseSize(tc.input) + if err != nil { + t.Errorf("parseSize(%q) returned error %v, want %d", tc.input, err, tc.want) + continue + } + if got != tc.want { + t.Errorf("parseSize(%q) = %d, want %d", tc.input, got, tc.want) + } + } +} + +// TestParseSizeRejects covers every malformed-input class. The expected +// error sentinel matters because the handler distinguishes errRelativeSize +// from errInvalidSize when formatting user-facing messages. +func TestParseSizeRejects(t *testing.T) { + cases := []struct { + input string + want error + }{ + // Empty, whitespace, garbage. + {"", errInvalidSize}, + {" ", errInvalidSize}, + {" 10", errInvalidSize}, // leading whitespace. + {"abc", errInvalidSize}, + {"1.5", errInvalidSize}, // floats not supported. + {"1KIB", errInvalidSize}, // multi-letter suffix is case-sensitive. + {"1kB", errInvalidSize}, // GNU rejects mixed-case "kB" too. + {"1XB", errInvalidSize}, + {"1KB1", errInvalidSize}, // trailing junk. + {"1Ki", errInvalidSize}, // partial "iB" suffix. + {"K", errInvalidSize}, // suffix without digits. + {"+", errRelativeSize}, // bare modifier. + {"-", errRelativeSize}, // bare modifier. + {"%5", errRelativeSize}, // GNU shell-percent rounding mode. + {"/2", errRelativeSize}, // GNU divide-by-N mode. + {"<10", errRelativeSize}, // GNU shrink-to-at-most mode. + {">10", errRelativeSize}, // GNU expand-to-at-least mode. + {"+10", errRelativeSize}, // GNU relative add. + {"-10", errRelativeSize}, // GNU relative subtract. + {"+10K", errRelativeSize}, // modifier with suffix. + + // Overflow boundaries. + {"9223372036854775808", errInvalidSize}, // int64 max + 1. + {"99999999999999999999", errInvalidSize}, // far past int64. + {"8388608T", errInvalidSize}, // 2^23 TiB overflows int64 multiplier. + } + for _, tc := range cases { + _, err := parseSize(tc.input) + if err == nil { + t.Errorf("parseSize(%q) returned no error, want %v", tc.input, tc.want) + continue + } + if !errors.Is(err, tc.want) { + t.Errorf("parseSize(%q) returned %v, want %v", tc.input, err, tc.want) + } + } +} + +// TestParseSizeMaxIntBoundary pins the exact T-suffix ceiling to guard +// against off-by-one regressions in the overflow check. +// +// MaxInt64 / TiB == 8388607, so: +// - 8388607T must succeed (multiplies to 8388607 * 2^40, just under MaxInt64) +// - 8388608T must fail (would multiply to MaxInt64+1) +func TestParseSizeMaxIntBoundary(t *testing.T) { + maxT := int64(math.MaxInt64) / (1 << 40) + if maxT != 8388607 { + t.Fatalf("test assumes maxT == 8388607, got %d", maxT) + } + largest := strconvFormat(maxT) + "T" + got, err := parseSize(largest) + if err != nil { + t.Errorf("parseSize(%q) should succeed at the ceiling, got %v", largest, err) + } + if got != maxT*(1<<40) { + t.Errorf("parseSize(%q) = %d, want %d", largest, got, maxT*(1<<40)) + } + overflow := strconvFormat(maxT+1) + "T" + if _, err := parseSize(overflow); err == nil { + t.Errorf("parseSize(%q) must reject one above the ceiling", overflow) + } +} + +// strconvFormat is a tiny helper that avoids pulling strconv import +// duplication into the test file's import set. +func strconvFormat(v int64) string { + if v == 0 { + return "0" + } + var buf [20]byte + pos := len(buf) + n := v + for n > 0 { + pos-- + buf[pos] = byte('0' + n%10) + n /= 10 + } + return string(buf[pos:]) +} diff --git a/interp/builtin_truncate_pentest_test.go b/interp/builtin_truncate_pentest_test.go new file mode 100644 index 000000000..7dec7bf41 --- /dev/null +++ b/interp/builtin_truncate_pentest_test.go @@ -0,0 +1,305 @@ +// 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 interp_test + +import ( + "bytes" + "context" + "errors" + "os" + "path/filepath" + "runtime" + "strconv" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "mvdan.cc/sh/v3/syntax" + + "github.com/DataDog/rshell/internal/interpoption" + "github.com/DataDog/rshell/interp" +) + +// runTruncatePentest is a self-contained runner specialised for these +// adversarial scenarios. It accepts a per-test context so cancellation and +// hang detection are explicit; AllowedPaths is set to the supplied dir so +// every pentest exercises the sandbox boundary. +func runTruncatePentest(ctx context.Context, t *testing.T, script, dir string) (string, string, int) { + t.Helper() + parser := syntax.NewParser() + prog, err := parser.Parse(strings.NewReader(script), "") + require.NoError(t, err) + + var stdout, stderr bytes.Buffer + opts := []interp.RunnerOption{ + interp.StdIO(nil, &stdout, &stderr), + interpoption.AllowAllCommands().(interp.RunnerOption), + interp.AllowedPaths([]string{dir}), + } + runner, err := interp.New(opts...) + require.NoError(t, err) + defer runner.Close() + runner.Dir = dir + + runErr := runner.Run(ctx, prog) + exitCode := 0 + if runErr != nil { + var es interp.ExitStatus + if errors.As(runErr, &es) { + exitCode = int(es) + } else if ctx.Err() == nil { + t.Fatalf("unexpected error: %v", runErr) + } + } + return stdout.String(), stderr.String(), exitCode +} + +// TestTruncatePentestNegativeSize rejects every form of negative size at +// either the parseSize layer (most cases) or, for completeness, the +// sandbox EINVAL guard. +func TestTruncatePentestNegativeSize(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "f.txt"), []byte("data"), 0644)) + + cases := []string{"-1", "-9999999999", "-9223372036854775808"} + for _, c := range cases { + t.Run(c, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _, stderr, code := runTruncatePentest(ctx, t, "truncate -s '"+c+"' f.txt", dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "truncate") + }) + } +} + +// TestTruncatePentestIntegerEdges exercises every notable int64 boundary, +// confirming none of them hang, panic, or crash the runner. +// +// "want" semantics: +// - 0 = parseSize accepts AND the kernel accepts on a typical FS (small +// sizes that any FS can represent). +// - 1 = parseSize must reject (overflow / malformed / empty). +// - any = parseSize accepts, but the kernel may or may not depending on +// the FS's maximum file size — we tolerate both 0 and 1, just no +// timeout or panic. +func TestTruncatePentestIntegerEdges(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "f.txt"), []byte("data"), 0644)) + + const ( + mustSucceed = 0 + mustFail = 1 + anyCode = -1 + ) + cases := []struct { + size string + want int + }{ + {"0", mustSucceed}, + {"1", mustSucceed}, + {"9223372036854775807", anyCode}, // exact MaxInt64; kernel may reject. + {"9223372036854775808", mustFail}, + {"99999999999999999999", mustFail}, + {"8388608T", mustFail}, // 8 EiB overflows int64. + {"8388607T", anyCode}, // parseSize accepts; kernel may reject. + {"", mustFail}, + {" ", mustFail}, + } + for _, tc := range cases { + t.Run(tc.size, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + script := "truncate -s '" + tc.size + "' f.txt" + _, _, code := runTruncatePentest(ctx, t, script, dir) + switch tc.want { + case mustSucceed: + if code != 0 { + t.Errorf("size %q: exit %d, want success", tc.size, code) + } + case mustFail: + if code != 1 { + t.Errorf("size %q: exit %d, want failure", tc.size, code) + } + } + // anyCode: any clean exit (0 or 1) is acceptable; the test + // passes as long as we did not hang or panic. + }) + } + // Restore the file before any subsequent test depends on its content. + require.NoError(t, os.WriteFile(filepath.Join(dir, "f.txt"), []byte("data"), 0644)) +} + +// TestTruncatePentestPathTraversal verifies that classic path-traversal +// payloads are blocked by the sandbox, never silently followed to an +// outside file. +func TestTruncatePentestPathTraversal(t *testing.T) { + dir := t.TempDir() + insideDir := filepath.Join(dir, "inside") + require.NoError(t, os.MkdirAll(insideDir, 0755)) + outsidePath := filepath.Join(dir, "outside.txt") + require.NoError(t, os.WriteFile(outsidePath, []byte("untouched"), 0644)) + + payloads := []string{ + "../outside.txt", + "../../../outside.txt", + "./../outside.txt", + "../inside/../outside.txt", + "//../outside.txt", + "/etc/hosts", // absolute path that does not match AllowedPaths. + } + for _, p := range payloads { + t.Run(p, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + script := "truncate -s 0 '" + p + "'" + _, _, code := runTruncatePentest(ctx, t, script, insideDir) + assert.Equal(t, 1, code, "traversal %q should be blocked", p) + body, err := os.ReadFile(outsidePath) + require.NoError(t, err) + assert.Equal(t, "untouched", string(body), + "traversal %q reached outside.txt", p) + }) + } +} + +// TestTruncatePentestSymlinkEscape confirms that a symlink inside the +// sandbox pointing OUT cannot be used to truncate the external target. +// Same property the os.Root/openat path enforces; we re-check at the +// builtin layer. +func TestTruncatePentestSymlinkEscape(t *testing.T) { + if _, err := os.Stat("/dev/null"); err != nil { + t.Skip("symlink test requires Unix-like FS semantics") + } + dir := t.TempDir() + allowed := filepath.Join(dir, "allowed") + require.NoError(t, os.MkdirAll(allowed, 0755)) + outside := filepath.Join(dir, "secret.txt") + require.NoError(t, os.WriteFile(outside, []byte("secret"), 0644)) + + // Create a symlink inside the sandbox that points to outside. + link := filepath.Join(allowed, "link") + require.NoError(t, os.Symlink(outside, link)) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _, _, code := runTruncatePentest(ctx, t, "truncate -s 0 link", allowed) + assert.Equal(t, 1, code, "symlink escape should be rejected") + body, err := os.ReadFile(outside) + require.NoError(t, err) + assert.Equal(t, "secret", string(body), "symlink target was modified") +} + +// (FIFO hang regression coverage lives in +// allowedpaths/sandbox_unix_test.go::TestSandboxTruncateMethodFIFODoesNotBlock, +// which uses the Unix-only syscall.Mkfifo. Keeping the syscall dependency +// in a build-tagged file avoids leaking it into this cross-platform suite.) + +// TestTruncatePentestFlagInjection verifies that user-controlled values +// are not interpreted as flags: a filename starting with "--" or "-" is +// only treated as a flag in the absence of the "--" separator. +func TestTruncatePentestFlagInjection(t *testing.T) { + dir := t.TempDir() + tricky := filepath.Join(dir, "--no-create=evil") + require.NoError(t, os.WriteFile(tricky, []byte("data"), 0644)) + + // Without "--", pflag would interpret "--no-create=evil" as a flag + // assignment with value "evil" — pflag should reject this because the + // no-create flag is boolean. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _, stderr, code := runTruncatePentest(ctx, t, "truncate -s 0 '--no-create=evil'", dir) + assert.Equal(t, 1, code, "flag-shaped filename without -- should not be accepted as a positional") + assert.NotEmpty(t, stderr) + body, _ := os.ReadFile(tricky) + assert.Equal(t, "data", string(body), "tricky file was unexpectedly modified") + + // With "--", the same token is treated as a positional and the file is + // truncated. + _, _, code = runTruncatePentest(ctx, t, "truncate -s 0 -- '--no-create=evil'", dir) + assert.Equal(t, 0, code) + info, err := os.Stat(tricky) + require.NoError(t, err) + assert.Equal(t, int64(0), info.Size()) +} + +// TestTruncatePentestManyFiles verifies that processing many operands does +// not leak file descriptors. We use 200 files, well under per-process +// limits but enough to surface a leak via repeated open/close churn. +func TestTruncatePentestManyFiles(t *testing.T) { + dir := t.TempDir() + const n = 200 + names := make([]string, n) + var sb strings.Builder + sb.WriteString("truncate -s 0") + for i := 0; i < n; i++ { + name := "f" + strconv.Itoa(i) + ".txt" + names[i] = name + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte("data"), 0644)) + sb.WriteString(" '") + sb.WriteString(name) + sb.WriteString("'") + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _, stderr, code := runTruncatePentest(ctx, t, sb.String(), dir) + assert.Equal(t, 0, code, "many-file run should succeed cleanly; stderr=%q", stderr) + for _, name := range names { + info, err := os.Stat(filepath.Join(dir, name)) + require.NoError(t, err) + assert.Equal(t, int64(0), info.Size(), "%s was not truncated", name) + } +} + +// TestTruncatePentestMassiveSparseSize confirms that a very large but +// kernel-acceptable size succeeds without OOM. On modern filesystems this +// is a metadata-only operation. Skipped on Windows where the test temp +// directory typically lives on NTFS but the FS-level rejection wording +// differs in ways not worth specialising for. +func TestTruncatePentestMassiveSparseSize(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("massive-sparse-size FS error wording is platform-specific") + } + dir := t.TempDir() + path := filepath.Join(dir, "huge.bin") + require.NoError(t, os.WriteFile(path, []byte(""), 0644)) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _, stderr, code := runTruncatePentest(ctx, t, "truncate -s 100G huge.bin", dir) + if code != 0 { + // Some filesystems (tmpfs, FAT) cannot represent a 100 GiB file; + // the kernel returns EFBIG/ENOSPC. Either is acceptable; what + // matters is that we did not crash. + t.Logf("kernel rejected 100G extension on this FS: %s", stderr) + return + } + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, int64(100)*(1<<30), info.Size()) +} + +// TestTruncatePentestEmptyFlagValues exercises the empty/whitespace-string +// paths that often slip past flag validators. parseSize should reject +// these, not pass them to ParseInt. +func TestTruncatePentestEmptyFlagValues(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "f.txt"), []byte("data"), 0644)) + + cases := []string{"''", `""`, "' '", `" "`} + for _, c := range cases { + t.Run(c, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + script := "truncate -s " + c + " f.txt" + _, stderr, code := runTruncatePentest(ctx, t, script, dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "invalid size") + }) + } +} diff --git a/interp/register_builtins.go b/interp/register_builtins.go index 0f8458878..25dddbb56 100644 --- a/interp/register_builtins.go +++ b/interp/register_builtins.go @@ -37,6 +37,7 @@ import ( "github.com/DataDog/rshell/builtins/testcmd" "github.com/DataDog/rshell/builtins/tr" truecmd "github.com/DataDog/rshell/builtins/true" + truncatecmd "github.com/DataDog/rshell/builtins/truncate" "github.com/DataDog/rshell/builtins/uname" "github.com/DataDog/rshell/builtins/uniq" "github.com/DataDog/rshell/builtins/wc" @@ -77,6 +78,7 @@ func registerBuiltins() { testcmd.BracketCmd, tr.Cmd, truecmd.Cmd, + truncatecmd.Cmd, uname.Cmd, uniq.Cmd, wc.Cmd, diff --git a/interp/runner_exec.go b/interp/runner_exec.go index a9c5af047..97a21e11a 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -608,6 +608,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { AccessFile: func(ctx context.Context, path string, mode uint32) error { return r.sandbox.Access(path, dir, mode) }, + Truncate: func(ctx context.Context, path string, size int64, create bool) error { + return r.sandbox.Truncate(path, dir, size, create) + }, PortableErr: allowedpaths.PortableErrMsg, Now: r.startTime, FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) { @@ -714,6 +717,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { AccessFile: func(ctx context.Context, path string, mode uint32) error { return r.sandbox.Access(path, HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir, mode) }, + Truncate: func(ctx context.Context, path string, size int64, create bool) error { + return r.sandbox.Truncate(path, HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir, size, create) + }, PortableErr: allowedpaths.PortableErrMsg, Now: r.startTime, FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) { diff --git a/tests/scenarios/cmd/truncate/basic/extend.yaml b/tests/scenarios/cmd/truncate/basic/extend.yaml new file mode 100644 index 000000000..5fa46a1db --- /dev/null +++ b/tests/scenarios/cmd/truncate/basic/extend.yaml @@ -0,0 +1,15 @@ +description: truncate -s N extends a smaller file with zero bytes. +setup: + files: + - path: small.bin + content: "abc" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 1024 small.bin + wc -c < small.bin +expect: + stdout: |+ + 1024 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/long_form.yaml b/tests/scenarios/cmd/truncate/basic/long_form.yaml new file mode 100644 index 000000000..072bbf068 --- /dev/null +++ b/tests/scenarios/cmd/truncate/basic/long_form.yaml @@ -0,0 +1,15 @@ +description: --size= long form is accepted and equivalent to -s. +setup: + files: + - path: log.txt + content: "hello\n" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate --size=0 log.txt + wc -c < log.txt +expect: + stdout: |+ + 0 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/shrink.yaml b/tests/scenarios/cmd/truncate/basic/shrink.yaml new file mode 100644 index 000000000..8e59617f1 --- /dev/null +++ b/tests/scenarios/cmd/truncate/basic/shrink.yaml @@ -0,0 +1,16 @@ +description: truncate -s N shrinks a larger file to N bytes. +setup: + files: + - path: big.txt + content: "0123456789abcdefghij" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 5 big.txt + cat big.txt + wc -c < big.txt +expect: + stdout: |+ + 012345 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/suffix_k.yaml b/tests/scenarios/cmd/truncate/basic/suffix_k.yaml new file mode 100644 index 000000000..56b4d0afa --- /dev/null +++ b/tests/scenarios/cmd/truncate/basic/suffix_k.yaml @@ -0,0 +1,15 @@ +description: K suffix is interpreted as 1024 bytes (binary). +setup: + files: + - path: f.bin + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 1K f.bin + wc -c < f.bin +expect: + stdout: |+ + 1024 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/suffix_kb.yaml b/tests/scenarios/cmd/truncate/basic/suffix_kb.yaml new file mode 100644 index 000000000..5106fd6c3 --- /dev/null +++ b/tests/scenarios/cmd/truncate/basic/suffix_kb.yaml @@ -0,0 +1,15 @@ +description: KB suffix is interpreted as 1000 bytes (decimal). +setup: + files: + - path: f.bin + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 1KB f.bin + wc -c < f.bin +expect: + stdout: |+ + 1000 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/zero_size.yaml b/tests/scenarios/cmd/truncate/basic/zero_size.yaml new file mode 100644 index 000000000..5920b02ae --- /dev/null +++ b/tests/scenarios/cmd/truncate/basic/zero_size.yaml @@ -0,0 +1,15 @@ +description: truncate -s 0 empties a file while preserving the inode. +setup: + files: + - path: log.txt + content: "hello world\n" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 0 log.txt + wc -c < log.txt +expect: + stdout: |+ + 0 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/errors/directory_target.yaml b/tests/scenarios/cmd/truncate/errors/directory_target.yaml new file mode 100644 index 000000000..c28572e0e --- /dev/null +++ b/tests/scenarios/cmd/truncate/errors/directory_target.yaml @@ -0,0 +1,16 @@ +# skip: error wording for "is a directory" varies between GNU and our PortableErr. +skip_assert_against_bash: true +description: truncate refuses to act on a directory and exits 1. +setup: + files: + - path: subdir/.keep + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 0 subdir +expect: + stdout: "" + stderr_contains: + - "truncate:" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/errors/invalid_size.yaml b/tests/scenarios/cmd/truncate/errors/invalid_size.yaml new file mode 100644 index 000000000..9a0511b85 --- /dev/null +++ b/tests/scenarios/cmd/truncate/errors/invalid_size.yaml @@ -0,0 +1,17 @@ +# skip: GNU truncate's exact error wording differs; we test our own message. +skip_assert_against_bash: true +description: A non-numeric -s value is rejected with exit 1. +setup: + files: + - path: a.txt + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s xyz a.txt +expect: + stdout: "" + stderr_contains: + - "truncate:" + - "invalid size" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/errors/missing_file.yaml b/tests/scenarios/cmd/truncate/errors/missing_file.yaml new file mode 100644 index 000000000..a12a7b0f4 --- /dev/null +++ b/tests/scenarios/cmd/truncate/errors/missing_file.yaml @@ -0,0 +1,13 @@ +# skip: GNU truncate's exact error wording differs; we test our own message. +skip_assert_against_bash: true +description: truncate -s SIZE without an operand rejects with exit 1. +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 0 +expect: + stdout: "" + stderr_contains: + - "truncate:" + - "missing file operand" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/errors/missing_size.yaml b/tests/scenarios/cmd/truncate/errors/missing_size.yaml new file mode 100644 index 000000000..d8f2ee296 --- /dev/null +++ b/tests/scenarios/cmd/truncate/errors/missing_size.yaml @@ -0,0 +1,17 @@ +# skip: GNU truncate's exact error wording differs; we test our own message. +skip_assert_against_bash: true +description: truncate without -s rejects with exit 1. +setup: + files: + - path: a.txt + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate a.txt +expect: + stdout: "" + stderr_contains: + - "truncate:" + - "--size" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/errors/negative_size.yaml b/tests/scenarios/cmd/truncate/errors/negative_size.yaml new file mode 100644 index 000000000..4567d2dee --- /dev/null +++ b/tests/scenarios/cmd/truncate/errors/negative_size.yaml @@ -0,0 +1,17 @@ +# skip: GNU truncate accepts -10 as a relative size modifier; we reject it as unsupported. +skip_assert_against_bash: true +description: A leading - is rejected (relative size modifiers are unsupported). +setup: + files: + - path: a.txt + content: "abc" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s=-10 a.txt +expect: + stdout: "" + stderr_contains: + - "truncate:" + - "relative size operators not supported" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml b/tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml new file mode 100644 index 000000000..40fae6c62 --- /dev/null +++ b/tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml @@ -0,0 +1,19 @@ +# skip: AllowedPaths sandbox restriction is rshell-specific; bash has no such concept. +skip_assert_against_bash: true +description: truncate refuses to act on files outside AllowedPaths. +setup: + files: + - path: inside/.keep + content: "" + - path: outside.txt + content: "untouched\n" +input: + allowed_paths: ["inside"] + script: |+ + truncate -s 0 ../outside.txt +expect: + stdout: "" + stderr_contains: + - "truncate:" + - "permission denied" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/errors/relative_size.yaml b/tests/scenarios/cmd/truncate/errors/relative_size.yaml new file mode 100644 index 000000000..c4cfc2466 --- /dev/null +++ b/tests/scenarios/cmd/truncate/errors/relative_size.yaml @@ -0,0 +1,17 @@ +# skip: GNU truncate accepts +10/<10/>10 as relative modifiers; we reject them as unsupported. +skip_assert_against_bash: true +description: A leading +/ in -s is rejected (relative size modifiers unsupported). +setup: + files: + - path: a.txt + content: "abc" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s=+10 a.txt +expect: + stdout: "" + stderr_contains: + - "truncate:" + - "relative size operators not supported" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/hardening/double_dash.yaml b/tests/scenarios/cmd/truncate/hardening/double_dash.yaml new file mode 100644 index 000000000..48fbbe42e --- /dev/null +++ b/tests/scenarios/cmd/truncate/hardening/double_dash.yaml @@ -0,0 +1,15 @@ +description: -- separates flags from a filename literally starting with a dash. +setup: + files: + - path: -dashfile + content: "abcdef" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 0 -- -dashfile + wc -c < -dashfile +expect: + stdout: |+ + 0 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml b/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml new file mode 100644 index 000000000..e19fa0184 --- /dev/null +++ b/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml @@ -0,0 +1,16 @@ +description: truncate rejects deferred flags (--reference, -o) as unknown flags. +setup: + files: + - path: a.txt + content: "" + - path: ref.txt + content: "abc" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate --reference=ref.txt a.txt +expect: + stdout: "" + stderr_contains: + - "truncate:" + exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/help/help.yaml b/tests/scenarios/cmd/truncate/help/help.yaml new file mode 100644 index 000000000..59b004da1 --- /dev/null +++ b/tests/scenarios/cmd/truncate/help/help.yaml @@ -0,0 +1,13 @@ +# skip: --help output format is implementation-specific. +skip_assert_against_bash: true +description: truncate --help prints usage to stdout and exits 0. +input: + script: |+ + truncate --help +expect: + stdout_contains: + - "Usage: truncate" + - "--size" + - "--no-create" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/multi_file/all_zero.yaml b/tests/scenarios/cmd/truncate/multi_file/all_zero.yaml new file mode 100644 index 000000000..5b3fb7240 --- /dev/null +++ b/tests/scenarios/cmd/truncate/multi_file/all_zero.yaml @@ -0,0 +1,23 @@ +description: truncate -s 0 zeros every operand. +setup: + files: + - path: a.txt + content: "alpha\n" + - path: b.txt + content: "beta\n" + - path: c.txt + content: "gamma\n" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 0 a.txt b.txt c.txt + wc -c < a.txt + wc -c < b.txt + wc -c < c.txt +expect: + stdout: |+ + 0 + 0 + 0 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/multi_file/some_missing.yaml b/tests/scenarios/cmd/truncate/multi_file/some_missing.yaml new file mode 100644 index 000000000..b48013a62 --- /dev/null +++ b/tests/scenarios/cmd/truncate/multi_file/some_missing.yaml @@ -0,0 +1,19 @@ +description: truncate continues after a missing operand and exits 1 at the end. +setup: + files: + - path: a.txt + content: "alpha\n" + - path: c.txt + content: "gamma\n" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -c -s 0 a.txt b.txt c.txt + wc -c < a.txt + wc -c < c.txt +expect: + stdout: |+ + 0 + 0 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/no_create/missing_silent.yaml b/tests/scenarios/cmd/truncate/no_create/missing_silent.yaml new file mode 100644 index 000000000..f3e010781 --- /dev/null +++ b/tests/scenarios/cmd/truncate/no_create/missing_silent.yaml @@ -0,0 +1,15 @@ +description: truncate -c silently skips missing files (exit 0, file not created). +setup: + files: + - path: .keep + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -c -s 0 missing.txt + test ! -e missing.txt && echo "not created" +expect: + stdout: |+ + not created + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml b/tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml new file mode 100644 index 000000000..f3bfc4fcd --- /dev/null +++ b/tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml @@ -0,0 +1,15 @@ +description: Without -c, missing files are created at the requested size. +setup: + files: + - path: .keep + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 0 fresh.txt + wc -c < fresh.txt +expect: + stdout: |+ + 0 + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml b/tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml new file mode 100644 index 000000000..3232260b3 --- /dev/null +++ b/tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml @@ -0,0 +1,15 @@ +description: Without -c, missing files are created and extended to SIZE. +setup: + files: + - path: .keep + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + truncate -s 100 fresh.bin + wc -c < fresh.bin +expect: + stdout: |+ + 100 + stderr: "" + exit_code: 0 From befd5b440d5a8d39a7ea5c4ced0d8e6208e3b001 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Mon, 4 May 2026 16:20:11 +0200 Subject: [PATCH 2/5] =?UTF-8?q?fix(allowedpaths):=20close=20Stat=E2=86=92O?= =?UTF-8?q?pen=20TOCTOU=20in=20Sandbox.Truncate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review on PR #219 flagged a real race: between the pre-open Stat guard and the OpenFile call, a regular file can be swapped for a FIFO, and the subsequent O_WRONLY open will block waiting for a reader. The in-builtin ctx.Err() loop cannot interrupt that wait. Replace the racy pre-Stat with an atomic open-and-fstat sequence: - O_NONBLOCK on the open makes O_WRONLY on a reader-less FIFO return ENXIO immediately. It is benign on regular files and a no-op on Windows where syscall.O_NONBLOCK == 0. - fstat on the resulting fd verifies the file is regular before any ftruncate runs. Even a regular file swapped for a FIFO with a reader is rejected at the type check; the size change never reaches the kernel. Add a regression test that opens the read end of a FIFO before calling Truncate, exercising the post-fd fstat path that the no-reader test cannot reach. Co-Authored-By: Claude Opus 4.7 (1M context) --- allowedpaths/sandbox.go | 48 +++++++++++++++------------ allowedpaths/sandbox_unix_test.go | 54 +++++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 27 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 03685d8f4..3e59ea299 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -379,12 +379,20 @@ func (s *Sandbox) Open(path string, cwd string, flag int, perm os.FileMode) (io. // resolved path is the classic TOCTOU footgun. Writes must stay within a // single allowed root. // -// Non-regular targets (FIFO, socket, char/block device) are rejected before -// the open syscall. Mirrors the redirect-side guard in -// interp.Runner.rejectNonRegularRedirectTarget — opening a FIFO with -// O_WRONLY blocks until a reader connects, which would hang the shell. -// /dev/null and similar are handled by sandbox path resolution and never -// reach this method through normal AllowedPaths. +// Non-regular targets (FIFO, socket, char/block device) are rejected by an +// atomic open-and-fstat sequence: +// +// 1. The open includes O_NONBLOCK so that an O_WRONLY open of a FIFO with +// no reader returns ENXIO immediately instead of blocking the shell +// waiting for a connection. (O_NONBLOCK is benign on regular files — +// it sets the fd's status flag but does not change open semantics — +// and is a no-op on platforms where the constant is zero, e.g. Windows.) +// 2. After a successful open, fstat on the returned fd verifies the file +// is regular before any ftruncate runs. This closes the TOCTOU window +// that a pre-open Stat would have left open: even if a regular file +// is swapped for a FIFO between path resolution and the open syscall, +// the resulting fd is rejected before the size change reaches the +// kernel. // // Negative sizes are rejected with EINVAL. Sizes within int64 range are // passed through to the kernel; the kernel/filesystem rejects values it @@ -405,17 +413,7 @@ func (s *Sandbox) Truncate(path string, cwd string, size int64, create bool) err return &os.PathError{Op: "truncate", Path: path, Err: os.ErrPermission} } - // Stat (follow symlinks within the root) so a symlink whose final - // target is a FIFO is caught here before OpenFile blocks on the - // open. Mirrors the policy in interp.Runner.rejectNonRegularRedirectTarget. - // ENOENT and other Stat errors are intentionally ignored: O_CREATE - // will produce a regular file, and any genuine permission error will - // surface from the subsequent OpenFile call. - if info, err := ar.root.Stat(relPath); err == nil && !info.Mode().IsRegular() { - return &os.PathError{Op: "truncate", Path: path, Err: errors.New("not a regular file")} - } - - flag := os.O_WRONLY + flag := os.O_WRONLY | syscall.O_NONBLOCK if create { flag |= os.O_CREATE } @@ -429,11 +427,21 @@ func (s *Sandbox) Truncate(path string, cwd string, size int64, create bool) err // would silently break the truncate -c silent-skip path. return err } + // fstat the fd we actually opened (not the path) so a swap between + // path resolution and open is caught before ftruncate runs. + info, err := f.Stat() + if err != nil { + f.Close() + return err + } + if !info.Mode().IsRegular() { + f.Close() + return &os.PathError{Op: "truncate", Path: path, Err: errors.New("not a regular file")} + } truncErr := f.Truncate(size) // Surface a deferred Close error only when Truncate itself succeeded; - // the open path is read-only-on-the-fd at this point so a failed Close - // after a successful ftruncate is the only case where a Close error - // reflects user-visible data loss (flush failure on write-back). + // a failed Close after a successful ftruncate is the only case where a + // Close error reflects user-visible data loss (flush failure on write-back). closeErr := f.Close() if truncErr != nil { return truncErr diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index 2f1738a15..b40583d62 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -835,11 +835,13 @@ func TestContainerSymlinkRelativeTarget(t *testing.T) { assert.Equal(t, "relative", string(buf[:n])) } -// TestSandboxTruncateMethodFIFODoesNotBlock verifies that Sandbox.Truncate -// rejects a FIFO target before the open syscall can block. Without the -// non-regular-file pre-check, opening a FIFO with O_WRONLY would wait for -// a reader and hang the shell until the executor timeout fires. -func TestSandboxTruncateMethodFIFODoesNotBlock(t *testing.T) { +// TestSandboxTruncateMethodFIFONoReaderDoesNotBlock verifies that +// Sandbox.Truncate rejects a FIFO with no reader without blocking. The +// O_NONBLOCK flag on the open call makes the kernel return ENXIO +// immediately instead of waiting for a connection, which a plain +// O_WRONLY open would do and which the in-builtin ctx.Err() loop cannot +// interrupt. +func TestSandboxTruncateMethodFIFONoReaderDoesNotBlock(t *testing.T) { dir := t.TempDir() fifoPath := filepath.Join(dir, "pipe") require.NoError(t, syscall.Mkfifo(fifoPath, 0644)) @@ -856,8 +858,46 @@ func TestSandboxTruncateMethodFIFODoesNotBlock(t *testing.T) { select { case err := <-done: assert.Error(t, err, "FIFO target must be rejected, not silently truncated") - assert.Contains(t, err.Error(), "not a regular file") case <-time.After(2 * time.Second): - t.Fatal("Truncate blocked on FIFO — expected fast rejection via non-regular-file guard") + t.Fatal("Truncate blocked on FIFO without reader — O_NONBLOCK regressed") + } +} + +// TestSandboxTruncateMethodFIFOWithReaderRejected verifies the post-fd +// fstat guard: when a reader is connected, O_NONBLOCK no longer returns +// ENXIO and the open succeeds, so the in-fd type check is the safety net +// that rejects the FIFO before any ftruncate runs. +// +// This is the regression test for the Stat→Open TOCTOU window: a real +// attacker would swap a regular file for a FIFO between resolution and +// open, but a connected-reader FIFO at open time exercises the same +// branch without needing a race. +func TestSandboxTruncateMethodFIFOWithReaderRejected(t *testing.T) { + dir := t.TempDir() + fifoPath := filepath.Join(dir, "pipe") + require.NoError(t, syscall.Mkfifo(fifoPath, 0644)) + + // Open the read end so the kernel allows O_WRONLY|O_NONBLOCK opens + // to succeed instead of returning ENXIO. + reader, err := os.OpenFile(fifoPath, os.O_RDONLY|syscall.O_NONBLOCK, 0) + require.NoError(t, err) + defer reader.Close() + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + done := make(chan error, 1) + go func() { + done <- sb.Truncate("pipe", dir, 0, false) + }() + + select { + case err := <-done: + require.Error(t, err, "FIFO with reader must be rejected post-open, not silently truncated") + assert.Contains(t, err.Error(), "not a regular file", + "post-fd fstat guard should be the rejection path here") + case <-time.After(2 * time.Second): + t.Fatal("Truncate blocked on FIFO with reader — post-fd fstat guard regressed") } } From 83d1a00d7d23aa59edbeb733a7b9e66ed87e0e7a Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Mon, 4 May 2026 18:28:38 +0200 Subject: [PATCH 3/5] fix(truncate): match GNU suffix grammar and skip bash compare for unknown_flag Codex review on PR #219 found two issues: 1. tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml lacked skip_assert_against_bash. GNU truncate accepts --reference; rshell intentionally rejects it as a deferred flag, so the bash-comparison suite would diverge. 2. parseSize rejected the lowercase-leading multi-letter suffixes (1kB, 1kiB, 1mB, 1miB, ...) that GNU truncate accepts. The actual GNU rule is "leading letter case-insensitive, trailing B / iB case-sensitive", not "bare single-letter case-insensitive, multi-letter case-sensitive on the leading letter". Updated the suffix map, doc comments, unit tests, integration tests, fuzz seeds, and SHELL_FEATURES.md. Verified byte-for-byte agreement with `gtruncate` across all 27 leading-letter / trailing-form combinations (six accepted plus the rejected variants 1KIB, 1Kib, 1kb). Co-Authored-By: Claude Opus 4.7 (1M context) --- SHELL_FEATURES.md | 2 +- builtins/tests/truncate/truncate_fuzz_test.go | 14 +++--- builtins/tests/truncate/truncate_test.go | 2 +- builtins/truncate/truncate.go | 48 +++++++++++-------- builtins/truncate/truncate_test.go | 22 +++++++-- .../cmd/truncate/hardening/unknown_flag.yaml | 4 ++ 6 files changed, 59 insertions(+), 33 deletions(-) diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index d626f4d16..7dbec74bf 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -37,7 +37,7 @@ The in-shell `help` command mirrors these feature categories: run `help` for a c - ✅ `test EXPRESSION` / `[ EXPRESSION ]` — evaluate conditional expression (file tests, string/integer comparison, logical operators) - ✅ `tr [-cdsCt] SET1 [SET2]` — translate, squeeze, and/or delete characters from stdin - ✅ `true` — return exit code 0 -- ✅ `truncate [-c] -s SIZE FILE...` — shrink or extend each FILE to SIZE bytes inside `AllowedPaths` (creates files unless `-c`); `-s` accepts `K`/`k`/`KiB`/`KB`/`M`/`m`/`MiB`/`MB`/`G`/`g`/`GiB`/`GB`/`T`/`t`/`TiB`/`TB` suffixes (binary or decimal); `-r`/`--reference`, `-o`/`--io-blocks`, and the GNU relative-size modifiers (`+N`/`-N`/`N`/`/N`/`%N`) are rejected +- ✅ `truncate [-c] -s SIZE FILE...` — shrink or extend each FILE to SIZE bytes inside `AllowedPaths` (creates files unless `-c`); `-s` accepts the GNU suffix grammar where the leading letter is case-insensitive and the trailing `B`/`iB` is case-sensitive (e.g. `1k`/`1K`/`1kiB`/`1KiB` = 1024, `1kB`/`1KB` = 1000, same for `M`/`G`/`T`); `-r`/`--reference`, `-o`/`--io-blocks`, and the GNU relative-size modifiers (`+N`/`-N`/`N`/`/N`/`%N`) are rejected - ✅ `uname [-asnrvm]` — print system information (Linux only; reads from `/proc/sys/kernel/`, respects `--proc-path`) - ✅ `uniq [OPTION]... [INPUT]` — report or omit repeated lines - ✅ `wc [-l] [-w] [-c] [-m] [-L] [FILE]...` — count lines, words, bytes, characters, or max line length diff --git a/builtins/tests/truncate/truncate_fuzz_test.go b/builtins/tests/truncate/truncate_fuzz_test.go index 3df84ee1d..0cb5e877f 100644 --- a/builtins/tests/truncate/truncate_fuzz_test.go +++ b/builtins/tests/truncate/truncate_fuzz_test.go @@ -32,12 +32,13 @@ func FuzzTruncateSize(f *testing.F) { f.Add("99999999999999999999") // far past int64. f.Add("00000000000000000000") - // Every accepted suffix. + // Every accepted suffix (the leading letter is case-insensitive in + // every form GNU truncate accepts). for _, s := range []string{ - "K", "k", "KB", "KiB", - "M", "m", "MB", "MiB", - "G", "g", "GB", "GiB", - "T", "t", "TB", "TiB", + "K", "k", "KB", "kB", "KiB", "kiB", + "M", "m", "MB", "mB", "MiB", "miB", + "G", "g", "GB", "gB", "GiB", "giB", + "T", "t", "TB", "tB", "TiB", "tiB", } { f.Add("0" + s) f.Add("1" + s) @@ -66,7 +67,8 @@ func FuzzTruncateSize(f *testing.F) { f.Add("0x10") f.Add("0b10") f.Add("1KIB") - f.Add("1kB") + f.Add("1Kib") + f.Add("1kb") f.Add("1MiB1") f.Add("K") f.Add("KB") diff --git a/builtins/tests/truncate/truncate_test.go b/builtins/tests/truncate/truncate_test.go index 9ef700ed3..aa08bbe58 100644 --- a/builtins/tests/truncate/truncate_test.go +++ b/builtins/tests/truncate/truncate_test.go @@ -275,7 +275,7 @@ func TestTruncateRejectsRelativeSize(t *testing.T) { // TestTruncateRejectsInvalidSize verifies non-numeric and overflow inputs // produce exit 1 with the generic "invalid size" message. func TestTruncateRejectsInvalidSize(t *testing.T) { - cases := []string{"abc", "1.5", "9999999999999999999999", "1KIB", "1kB"} + cases := []string{"abc", "1.5", "9999999999999999999999", "1KIB", "1kb", "1Kib"} for _, c := range cases { t.Run(c, func(t *testing.T) { dir := t.TempDir() diff --git a/builtins/truncate/truncate.go b/builtins/truncate/truncate.go index c2c7dada8..ff30476d2 100644 --- a/builtins/truncate/truncate.go +++ b/builtins/truncate/truncate.go @@ -22,16 +22,14 @@ // // -s SIZE, --size=SIZE // Set or adjust the file size to SIZE bytes. SIZE is a non-negative -// integer with an optional suffix: +// integer with an optional suffix. The leading letter is case- +// insensitive (K = k, M = m, G = g, T = t); the trailing characters +// are case-sensitive ("B" and "iB" must use exact casing): // -// K = k = KiB = 1024 KB = 1000 -// M = m = MiB = 1024^2 MB = 1000^2 -// G = g = GiB = 1024^3 GB = 1000^3 -// T = t = TiB = 1024^4 TB = 1000^4 -// -// The bare single-letter forms accept either case. The "iB" and "B" -// multi-letter forms are case-sensitive on the leading letter, as -// GNU truncate does. +// K = k = KiB = kiB = 1024 KB = kB = 1000 +// M = m = MiB = miB = 1024^2 MB = mB = 1000^2 +// G = g = GiB = giB = 1024^3 GB = gB = 1000^3 +// T = t = TiB = tiB = 1024^4 TB = tB = 1000^4 // // -c, --no-create // Do not create files that do not already exist. With -c, missing @@ -167,10 +165,10 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // sizeMultipliers maps suffix tokens accepted by -s to their byte // multipliers, matching GNU coreutils: // -// - bare single-letter binary forms accept either case (K=k=KiB=1024) -// - the explicit "B" decimal forms (KB, MB, GB, TB) and the "iB" forms -// (KiB, MiB, GiB, TiB) are case-sensitive on the leading letter, as -// GNU truncate does +// - the leading letter is case-insensitive (K = k, M = m, G = g, T = t) +// - the trailing characters are case-sensitive: "B" (decimal, 1000-based) +// and "iB" (binary, 1024-based) must use the exact casing shown — GNU +// accepts "kB" and "kiB" but rejects "KIB", "KIb", "kIB", etc. // // "" maps to 1 so a bare digit string falls through with no multiplication. var sizeMultipliers = map[string]int64{ @@ -178,19 +176,27 @@ var sizeMultipliers = map[string]int64{ "K": 1 << 10, "k": 1 << 10, "KiB": 1 << 10, + "kiB": 1 << 10, "KB": 1000, + "kB": 1000, "M": 1 << 20, "m": 1 << 20, "MiB": 1 << 20, + "miB": 1 << 20, "MB": 1000 * 1000, + "mB": 1000 * 1000, "G": 1 << 30, "g": 1 << 30, "GiB": 1 << 30, + "giB": 1 << 30, "GB": 1000 * 1000 * 1000, + "gB": 1000 * 1000 * 1000, "T": 1 << 40, "t": 1 << 40, "TiB": 1 << 40, + "tiB": 1 << 40, "TB": 1000 * 1000 * 1000 * 1000, + "tB": 1000 * 1000 * 1000 * 1000, } // parseSize parses the value of -s/--size into a non-negative byte count. @@ -198,14 +204,14 @@ var sizeMultipliers = map[string]int64{ // The grammar matches GNU truncate: // // size := digit+ suffix? -// suffix := "K" | "k" | "KB" | "KiB" | -// "M" | "m" | "MB" | "MiB" | -// "G" | "g" | "GB" | "GiB" | -// "T" | "t" | "TB" | "TiB" -// -// The bare single-letter binary forms accept either case; the multi-letter -// "B" decimal and "iB" binary forms are case-sensitive on the leading -// letter (e.g. "KB" is accepted, "kB" is not — GNU rejects it too). +// suffix := [Kk] | [Kk]B | [Kk]iB | +// [Mm] | [Mm]B | [Mm]iB | +// [Gg] | [Gg]B | [Gg]iB | +// [Tt] | [Tt]B | [Tt]iB +// +// The leading letter is case-insensitive; the trailing "B" and "iB" +// characters must use exact casing (GNU accepts "1kB" and "1kiB" but +// rejects "1KIB", "1KIb", "1kIB", etc.). // // Leading +/-/<>//% modifiers (the GNU relative-size syntax) are rejected // with errRelativeSize so the caller can surface a hint that these forms diff --git a/builtins/truncate/truncate_test.go b/builtins/truncate/truncate_test.go index 93c0410e1..0e7cddcca 100644 --- a/builtins/truncate/truncate_test.go +++ b/builtins/truncate/truncate_test.go @@ -25,26 +25,36 @@ func TestParseSizeAccepts(t *testing.T) { {"123456", 123456}, {"9223372036854775807", math.MaxInt64}, // exact int64 ceiling. - // Binary suffixes (1024-based) — both cases. + // Binary suffixes (1024-based). Leading letter is case-insensitive; + // the trailing "iB" must keep exact casing. {"1K", 1 << 10}, {"1k", 1 << 10}, {"1KiB", 1 << 10}, + {"1kiB", 1 << 10}, {"2K", 2 << 10}, {"1M", 1 << 20}, {"1m", 1 << 20}, {"1MiB", 1 << 20}, + {"1miB", 1 << 20}, {"1G", 1 << 30}, {"1g", 1 << 30}, {"1GiB", 1 << 30}, + {"1giB", 1 << 30}, {"1T", 1 << 40}, {"1t", 1 << 40}, {"1TiB", 1 << 40}, + {"1tiB", 1 << 40}, - // Decimal suffixes (1000-based) — case-sensitive on the leading letter. + // Decimal suffixes (1000-based). Same rule: leading letter + // case-insensitive, trailing "B" must be uppercase. {"1KB", 1000}, + {"1kB", 1000}, {"1MB", 1000 * 1000}, + {"1mB", 1000 * 1000}, {"1GB", 1000 * 1000 * 1000}, + {"1gB", 1000 * 1000 * 1000}, {"1TB", 1000 * 1000 * 1000 * 1000}, + {"1tB", 1000 * 1000 * 1000 * 1000}, // Zero with suffix is still zero. {"0K", 0}, @@ -76,8 +86,12 @@ func TestParseSizeRejects(t *testing.T) { {" 10", errInvalidSize}, // leading whitespace. {"abc", errInvalidSize}, {"1.5", errInvalidSize}, // floats not supported. - {"1KIB", errInvalidSize}, // multi-letter suffix is case-sensitive. - {"1kB", errInvalidSize}, // GNU rejects mixed-case "kB" too. + {"1KIB", errInvalidSize}, // GNU rejects all-caps "iB". + {"1Kib", errInvalidSize}, // and lowercase "ib". + {"1KIb", errInvalidSize}, // and "Ib". + {"1kIB", errInvalidSize}, // any non-"iB" trailing form is invalid. + {"1kb", errInvalidSize}, // trailing "b" must be uppercase. + {"1Kb", errInvalidSize}, {"1XB", errInvalidSize}, {"1KB1", errInvalidSize}, // trailing junk. {"1Ki", errInvalidSize}, // partial "iB" suffix. diff --git a/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml b/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml index e19fa0184..dd76f6873 100644 --- a/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml +++ b/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml @@ -1,3 +1,7 @@ +# skip: GNU truncate accepts --reference; rshell intentionally rejects it as +# out of scope for this PR (deferred flag), so the bash-comparison suite would +# diverge. +skip_assert_against_bash: true description: truncate rejects deferred flags (--reference, -o) as unknown flags. setup: files: From b91c1bd7a391921ae095fe2a51accae22f77a2a2 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Tue, 5 May 2026 14:57:32 +0200 Subject: [PATCH 4/5] fix(truncate): add P/E suffixes; convert empty scenario expectations to |+ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review on PR #219 found two issues: 1. Suffix table stopped at T/t. GNU truncate accepts P (PiB) and E (EiB) plus their PB/EB and PiB/EiB forms. GNU also lists Z/Y/R/Q in --help, but on 64-bit-uintmax_t systems (the standard platform) GNU rejects them with "Value too large to be stored in data type" — which matches our int64 ceiling, so rejecting them is not a real divergence. Empirically (verified locally with gtruncate from coreutils): - K/M/G/T: leading letter case-insensitive (1k = 1K = 1024). - P/E: leading letter UPPERCASE-ONLY (1p, 1pB, 1piB rejected). - The trailing "B" / "iB" is always case-sensitive in every form. - Z/Y/R/Q: rejected. Encode the asymmetry exactly. All 18 spot-checked combinations (P/PB/PiB/p/pB/piB/E/EB/EiB/e/eB/eiB/Z/Y/R/Q/7E/8E) now agree with gtruncate byte-for-byte. 2. New scenario YAMLs used quoted-empty scalars (`stderr: ""`) instead of the |+ block scalars AGENTS.md mandates. Convert all of them. Co-Authored-By: Claude Opus 4.7 (1M context) --- SHELL_FEATURES.md | 2 +- builtins/tests/truncate/truncate_fuzz_test.go | 17 +++++- builtins/truncate/truncate.go | 52 +++++++++++++++---- builtins/truncate/truncate_test.go | 23 ++++++++ .../scenarios/cmd/truncate/basic/extend.yaml | 2 +- .../cmd/truncate/basic/long_form.yaml | 2 +- .../scenarios/cmd/truncate/basic/shrink.yaml | 2 +- .../cmd/truncate/basic/suffix_k.yaml | 2 +- .../cmd/truncate/basic/suffix_kb.yaml | 2 +- .../cmd/truncate/basic/zero_size.yaml | 2 +- .../cmd/truncate/errors/directory_target.yaml | 2 +- .../cmd/truncate/errors/invalid_size.yaml | 2 +- .../cmd/truncate/errors/missing_file.yaml | 2 +- .../cmd/truncate/errors/missing_size.yaml | 2 +- .../cmd/truncate/errors/negative_size.yaml | 2 +- .../errors/outside_allowed_paths.yaml | 2 +- .../cmd/truncate/errors/relative_size.yaml | 2 +- .../cmd/truncate/hardening/double_dash.yaml | 2 +- .../cmd/truncate/hardening/unknown_flag.yaml | 2 +- tests/scenarios/cmd/truncate/help/help.yaml | 2 +- .../cmd/truncate/multi_file/all_zero.yaml | 2 +- .../cmd/truncate/multi_file/some_missing.yaml | 2 +- .../truncate/no_create/missing_silent.yaml | 2 +- .../truncate/no_create/no_flag_creates.yaml | 2 +- .../no_create/no_flag_creates_with_size.yaml | 2 +- 25 files changed, 101 insertions(+), 35 deletions(-) diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index 7dbec74bf..466e6c0bb 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -37,7 +37,7 @@ The in-shell `help` command mirrors these feature categories: run `help` for a c - ✅ `test EXPRESSION` / `[ EXPRESSION ]` — evaluate conditional expression (file tests, string/integer comparison, logical operators) - ✅ `tr [-cdsCt] SET1 [SET2]` — translate, squeeze, and/or delete characters from stdin - ✅ `true` — return exit code 0 -- ✅ `truncate [-c] -s SIZE FILE...` — shrink or extend each FILE to SIZE bytes inside `AllowedPaths` (creates files unless `-c`); `-s` accepts the GNU suffix grammar where the leading letter is case-insensitive and the trailing `B`/`iB` is case-sensitive (e.g. `1k`/`1K`/`1kiB`/`1KiB` = 1024, `1kB`/`1KB` = 1000, same for `M`/`G`/`T`); `-r`/`--reference`, `-o`/`--io-blocks`, and the GNU relative-size modifiers (`+N`/`-N`/`N`/`/N`/`%N`) are rejected +- ✅ `truncate [-c] -s SIZE FILE...` — shrink or extend each FILE to SIZE bytes inside `AllowedPaths` (creates files unless `-c`); `-s` accepts the GNU suffix grammar — for K/M/G/T the leading letter is case-insensitive and the trailing `B`/`iB` is case-sensitive (e.g. `1k`/`1K`/`1kiB`/`1KiB` = 1024, `1kB`/`1KB` = 1000), for P/E the leading letter is uppercase-only (matching GNU); Z/Y/R/Q are rejected because their multipliers exceed int64 (GNU rejects these too on 64-bit-uintmax_t systems); `-r`/`--reference`, `-o`/`--io-blocks`, and the GNU relative-size modifiers (`+N`/`-N`/`N`/`/N`/`%N`) are rejected - ✅ `uname [-asnrvm]` — print system information (Linux only; reads from `/proc/sys/kernel/`, respects `--proc-path`) - ✅ `uniq [OPTION]... [INPUT]` — report or omit repeated lines - ✅ `wc [-l] [-w] [-c] [-m] [-L] [FILE]...` — count lines, words, bytes, characters, or max line length diff --git a/builtins/tests/truncate/truncate_fuzz_test.go b/builtins/tests/truncate/truncate_fuzz_test.go index 0cb5e877f..a1772557f 100644 --- a/builtins/tests/truncate/truncate_fuzz_test.go +++ b/builtins/tests/truncate/truncate_fuzz_test.go @@ -32,18 +32,31 @@ func FuzzTruncateSize(f *testing.F) { f.Add("99999999999999999999") // far past int64. f.Add("00000000000000000000") - // Every accepted suffix (the leading letter is case-insensitive in - // every form GNU truncate accepts). + // Every accepted suffix. K/M/G/T accept either case on the leading + // letter; P/E are uppercase-only (matching GNU truncate). for _, s := range []string{ "K", "k", "KB", "kB", "KiB", "kiB", "M", "m", "MB", "mB", "MiB", "miB", "G", "g", "GB", "gB", "GiB", "giB", "T", "t", "TB", "tB", "TiB", "tiB", + "P", "PB", "PiB", + "E", "EB", "EiB", } { f.Add("0" + s) f.Add("1" + s) f.Add("123" + s) } + // Lowercase P/E and the Z/Y/R/Q suffixes — must always reject. + for _, s := range []string{ + "p", "pB", "piB", + "e", "eB", "eiB", + "Z", "ZB", "ZiB", "z", "zB", + "Y", "YB", "YiB", "y", + "R", "RB", "RiB", + "Q", "QB", "QiB", + } { + f.Add("1" + s) + } // Suffix-overflow neighbours. f.Add("8388607T") // largest accepted T-suffix. diff --git a/builtins/truncate/truncate.go b/builtins/truncate/truncate.go index ff30476d2..630520b54 100644 --- a/builtins/truncate/truncate.go +++ b/builtins/truncate/truncate.go @@ -22,14 +22,23 @@ // // -s SIZE, --size=SIZE // Set or adjust the file size to SIZE bytes. SIZE is a non-negative -// integer with an optional suffix. The leading letter is case- -// insensitive (K = k, M = m, G = g, T = t); the trailing characters -// are case-sensitive ("B" and "iB" must use exact casing): +// integer with an optional suffix. +// +// For K/M/G/T the leading letter is case-insensitive; for P/E the +// leading letter is uppercase-only (matching GNU truncate exactly). +// The trailing "B" and "iB" characters are always case-sensitive: // // K = k = KiB = kiB = 1024 KB = kB = 1000 // M = m = MiB = miB = 1024^2 MB = mB = 1000^2 // G = g = GiB = giB = 1024^3 GB = gB = 1000^3 // T = t = TiB = tiB = 1024^4 TB = tB = 1000^4 +// P = PiB = 1024^5 PB = 1000^5 +// E = EiB = 1024^6 EB = 1000^6 +// +// Z/Y/R/Q (zetta/yotta/ronna/quetta) are rejected because their +// multipliers exceed int64. GNU coreutils with the standard 64-bit +// uintmax_t rejects these as well, so this is not a divergence in +// practice. // // -c, --no-create // Do not create files that do not already exist. With -c, missing @@ -165,10 +174,20 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { // sizeMultipliers maps suffix tokens accepted by -s to their byte // multipliers, matching GNU coreutils: // -// - the leading letter is case-insensitive (K = k, M = m, G = g, T = t) -// - the trailing characters are case-sensitive: "B" (decimal, 1000-based) -// and "iB" (binary, 1024-based) must use the exact casing shown — GNU -// accepts "kB" and "kiB" but rejects "KIB", "KIb", "kIB", etc. +// - For K/M/G/T the leading letter is case-insensitive +// (K = k, M = m, G = g, T = t) and lowercase-leading multi-letter +// forms ("kB", "kiB", ...) are accepted. +// - For P/E the leading letter is uppercase-only (GNU rejects "1p", +// "1pB", "1piB", "1e", ...; we match that exactly). The "B"/"iB" +// trailing forms keep the same case rules. +// - In every form, the trailing characters are case-sensitive: "B" must +// be uppercase and "iB" must be exactly "iB" — GNU rejects "1KIB", +// "1kb", "1Kib", etc. +// +// Z/Y/R/Q are intentionally NOT supported: their multipliers (1024^7+ +// and 1000^7+) exceed int64. GNU coreutils with the standard 64-bit +// uintmax_t rejects these too ("Value too large to be stored in data +// type"), so this is not a divergence in practice. // // "" maps to 1 so a bare digit string falls through with no multiplication. var sizeMultipliers = map[string]int64{ @@ -197,6 +216,13 @@ var sizeMultipliers = map[string]int64{ "tiB": 1 << 40, "TB": 1000 * 1000 * 1000 * 1000, "tB": 1000 * 1000 * 1000 * 1000, + // P and E: uppercase-only leading letter, matching GNU. + "P": 1 << 50, + "PiB": 1 << 50, + "PB": 1000 * 1000 * 1000 * 1000 * 1000, + "E": 1 << 60, + "EiB": 1 << 60, + "EB": 1000 * 1000 * 1000 * 1000 * 1000 * 1000, } // parseSize parses the value of -s/--size into a non-negative byte count. @@ -207,11 +233,15 @@ var sizeMultipliers = map[string]int64{ // suffix := [Kk] | [Kk]B | [Kk]iB | // [Mm] | [Mm]B | [Mm]iB | // [Gg] | [Gg]B | [Gg]iB | -// [Tt] | [Tt]B | [Tt]iB +// [Tt] | [Tt]B | [Tt]iB | +// P | PB | PiB | +// E | EB | EiB // -// The leading letter is case-insensitive; the trailing "B" and "iB" -// characters must use exact casing (GNU accepts "1kB" and "1kiB" but -// rejects "1KIB", "1KIb", "1kIB", etc.). +// For K/M/G/T the leading letter is case-insensitive; for P/E the leading +// letter is uppercase-only (GNU rejects "1p"/"1pB"/"1e"/"1eB" etc.). The +// trailing "B" / "iB" characters are always case-sensitive (GNU rejects +// "1KIB", "1KIb", "1kIB", "1Kib", etc.). Z/Y/R/Q are intentionally +// rejected — see the sizeMultipliers comment. // // Leading +/-/<>//% modifiers (the GNU relative-size syntax) are rejected // with errRelativeSize so the caller can surface a hint that these forms diff --git a/builtins/truncate/truncate_test.go b/builtins/truncate/truncate_test.go index 0e7cddcca..7b11f0412 100644 --- a/builtins/truncate/truncate_test.go +++ b/builtins/truncate/truncate_test.go @@ -56,9 +56,18 @@ func TestParseSizeAccepts(t *testing.T) { {"1TB", 1000 * 1000 * 1000 * 1000}, {"1tB", 1000 * 1000 * 1000 * 1000}, + // P/E: uppercase-only leading letter, matching GNU. + {"1P", 1 << 50}, + {"1PiB", 1 << 50}, + {"1PB", 1000 * 1000 * 1000 * 1000 * 1000}, + {"1E", 1 << 60}, + {"1EiB", 1 << 60}, + {"1EB", 1000 * 1000 * 1000 * 1000 * 1000 * 1000}, + // Zero with suffix is still zero. {"0K", 0}, {"0MB", 0}, + {"0P", 0}, } for _, tc := range cases { got, err := parseSize(tc.input) @@ -92,6 +101,20 @@ func TestParseSizeRejects(t *testing.T) { {"1kIB", errInvalidSize}, // any non-"iB" trailing form is invalid. {"1kb", errInvalidSize}, // trailing "b" must be uppercase. {"1Kb", errInvalidSize}, + // P and E are uppercase-only on the leading letter (matches GNU). + {"1p", errInvalidSize}, + {"1pB", errInvalidSize}, + {"1piB", errInvalidSize}, + {"1e", errInvalidSize}, + {"1eB", errInvalidSize}, + {"1eiB", errInvalidSize}, + // Z/Y/R/Q multipliers exceed int64; rejected as unknown suffix + // (GNU rejects them too on 64-bit-uintmax_t systems). + {"1Z", errInvalidSize}, + {"1ZB", errInvalidSize}, + {"1Y", errInvalidSize}, + {"1R", errInvalidSize}, + {"1Q", errInvalidSize}, {"1XB", errInvalidSize}, {"1KB1", errInvalidSize}, // trailing junk. {"1Ki", errInvalidSize}, // partial "iB" suffix. diff --git a/tests/scenarios/cmd/truncate/basic/extend.yaml b/tests/scenarios/cmd/truncate/basic/extend.yaml index 5fa46a1db..1946f17e3 100644 --- a/tests/scenarios/cmd/truncate/basic/extend.yaml +++ b/tests/scenarios/cmd/truncate/basic/extend.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 1024 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/long_form.yaml b/tests/scenarios/cmd/truncate/basic/long_form.yaml index 072bbf068..cc88c1067 100644 --- a/tests/scenarios/cmd/truncate/basic/long_form.yaml +++ b/tests/scenarios/cmd/truncate/basic/long_form.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 0 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/shrink.yaml b/tests/scenarios/cmd/truncate/basic/shrink.yaml index 8e59617f1..f52c2812e 100644 --- a/tests/scenarios/cmd/truncate/basic/shrink.yaml +++ b/tests/scenarios/cmd/truncate/basic/shrink.yaml @@ -12,5 +12,5 @@ input: expect: stdout: |+ 012345 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/suffix_k.yaml b/tests/scenarios/cmd/truncate/basic/suffix_k.yaml index 56b4d0afa..ea15c2e8c 100644 --- a/tests/scenarios/cmd/truncate/basic/suffix_k.yaml +++ b/tests/scenarios/cmd/truncate/basic/suffix_k.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 1024 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/suffix_kb.yaml b/tests/scenarios/cmd/truncate/basic/suffix_kb.yaml index 5106fd6c3..cee83bd54 100644 --- a/tests/scenarios/cmd/truncate/basic/suffix_kb.yaml +++ b/tests/scenarios/cmd/truncate/basic/suffix_kb.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 1000 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/basic/zero_size.yaml b/tests/scenarios/cmd/truncate/basic/zero_size.yaml index 5920b02ae..e5ef20d9a 100644 --- a/tests/scenarios/cmd/truncate/basic/zero_size.yaml +++ b/tests/scenarios/cmd/truncate/basic/zero_size.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 0 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/errors/directory_target.yaml b/tests/scenarios/cmd/truncate/errors/directory_target.yaml index c28572e0e..fb5f2453d 100644 --- a/tests/scenarios/cmd/truncate/errors/directory_target.yaml +++ b/tests/scenarios/cmd/truncate/errors/directory_target.yaml @@ -10,7 +10,7 @@ input: script: |+ truncate -s 0 subdir expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/errors/invalid_size.yaml b/tests/scenarios/cmd/truncate/errors/invalid_size.yaml index 9a0511b85..3c99b4dba 100644 --- a/tests/scenarios/cmd/truncate/errors/invalid_size.yaml +++ b/tests/scenarios/cmd/truncate/errors/invalid_size.yaml @@ -10,7 +10,7 @@ input: script: |+ truncate -s xyz a.txt expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" - "invalid size" diff --git a/tests/scenarios/cmd/truncate/errors/missing_file.yaml b/tests/scenarios/cmd/truncate/errors/missing_file.yaml index a12a7b0f4..1cb292dab 100644 --- a/tests/scenarios/cmd/truncate/errors/missing_file.yaml +++ b/tests/scenarios/cmd/truncate/errors/missing_file.yaml @@ -6,7 +6,7 @@ input: script: |+ truncate -s 0 expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" - "missing file operand" diff --git a/tests/scenarios/cmd/truncate/errors/missing_size.yaml b/tests/scenarios/cmd/truncate/errors/missing_size.yaml index d8f2ee296..651c23e55 100644 --- a/tests/scenarios/cmd/truncate/errors/missing_size.yaml +++ b/tests/scenarios/cmd/truncate/errors/missing_size.yaml @@ -10,7 +10,7 @@ input: script: |+ truncate a.txt expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" - "--size" diff --git a/tests/scenarios/cmd/truncate/errors/negative_size.yaml b/tests/scenarios/cmd/truncate/errors/negative_size.yaml index 4567d2dee..f3f13880f 100644 --- a/tests/scenarios/cmd/truncate/errors/negative_size.yaml +++ b/tests/scenarios/cmd/truncate/errors/negative_size.yaml @@ -10,7 +10,7 @@ input: script: |+ truncate -s=-10 a.txt expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" - "relative size operators not supported" diff --git a/tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml b/tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml index 40fae6c62..4d326e1a2 100644 --- a/tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml +++ b/tests/scenarios/cmd/truncate/errors/outside_allowed_paths.yaml @@ -12,7 +12,7 @@ input: script: |+ truncate -s 0 ../outside.txt expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" - "permission denied" diff --git a/tests/scenarios/cmd/truncate/errors/relative_size.yaml b/tests/scenarios/cmd/truncate/errors/relative_size.yaml index c4cfc2466..6d1ca5dd4 100644 --- a/tests/scenarios/cmd/truncate/errors/relative_size.yaml +++ b/tests/scenarios/cmd/truncate/errors/relative_size.yaml @@ -10,7 +10,7 @@ input: script: |+ truncate -s=+10 a.txt expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" - "relative size operators not supported" diff --git a/tests/scenarios/cmd/truncate/hardening/double_dash.yaml b/tests/scenarios/cmd/truncate/hardening/double_dash.yaml index 48fbbe42e..6263deff6 100644 --- a/tests/scenarios/cmd/truncate/hardening/double_dash.yaml +++ b/tests/scenarios/cmd/truncate/hardening/double_dash.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 0 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml b/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml index dd76f6873..e479fe5c0 100644 --- a/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml +++ b/tests/scenarios/cmd/truncate/hardening/unknown_flag.yaml @@ -14,7 +14,7 @@ input: script: |+ truncate --reference=ref.txt a.txt expect: - stdout: "" + stdout: |+ stderr_contains: - "truncate:" exit_code: 1 diff --git a/tests/scenarios/cmd/truncate/help/help.yaml b/tests/scenarios/cmd/truncate/help/help.yaml index 59b004da1..fba6add36 100644 --- a/tests/scenarios/cmd/truncate/help/help.yaml +++ b/tests/scenarios/cmd/truncate/help/help.yaml @@ -9,5 +9,5 @@ expect: - "Usage: truncate" - "--size" - "--no-create" - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/multi_file/all_zero.yaml b/tests/scenarios/cmd/truncate/multi_file/all_zero.yaml index 5b3fb7240..2950e4b13 100644 --- a/tests/scenarios/cmd/truncate/multi_file/all_zero.yaml +++ b/tests/scenarios/cmd/truncate/multi_file/all_zero.yaml @@ -19,5 +19,5 @@ expect: 0 0 0 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/multi_file/some_missing.yaml b/tests/scenarios/cmd/truncate/multi_file/some_missing.yaml index b48013a62..b0f869d54 100644 --- a/tests/scenarios/cmd/truncate/multi_file/some_missing.yaml +++ b/tests/scenarios/cmd/truncate/multi_file/some_missing.yaml @@ -15,5 +15,5 @@ expect: stdout: |+ 0 0 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/no_create/missing_silent.yaml b/tests/scenarios/cmd/truncate/no_create/missing_silent.yaml index f3e010781..787faeea3 100644 --- a/tests/scenarios/cmd/truncate/no_create/missing_silent.yaml +++ b/tests/scenarios/cmd/truncate/no_create/missing_silent.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ not created - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml b/tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml index f3bfc4fcd..070bf3a40 100644 --- a/tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml +++ b/tests/scenarios/cmd/truncate/no_create/no_flag_creates.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 0 - stderr: "" + stderr: |+ exit_code: 0 diff --git a/tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml b/tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml index 3232260b3..3457101ee 100644 --- a/tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml +++ b/tests/scenarios/cmd/truncate/no_create/no_flag_creates_with_size.yaml @@ -11,5 +11,5 @@ input: expect: stdout: |+ 100 - stderr: "" + stderr: |+ exit_code: 0 From 8cbe726a0947551c0d48602eb493533b8e53fa7b Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Tue, 5 May 2026 16:34:35 +0200 Subject: [PATCH 5/5] fix(allowedpaths): pass 0666 to OpenFile so umask determines create mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review on PR #219 noted that hard-coding 0644 makes Sandbox.Truncate more restrictive than GNU truncate / bash redirects under permissive umasks: with `umask 000`, GNU `truncate -s 0 f` produces 0666, but rshell produced 0644. POSIX `open(2)` applies `mode & ~umask`, so passing 0666 lets the process umask alone decide the final mode. Verified locally that this now matches `gtruncate` byte-for-byte across umask 022/000/077/027: umask 022 -> 0644 (was 0644 — unchanged) umask 000 -> 0666 (was 0644) umask 077 -> 0600 (was 0600 — unchanged) umask 027 -> 0640 (was 0640 — unchanged) Add TestSandboxTruncateMethodCreatesHonourUmask to lock this behaviour in. Update the existing TestHardenCreatePreservesMode to lock umask to 022 (the typical operator environment) so its mode assertion stays deterministic, and split the umask helpers into Unix/Windows test files to keep syscall.Umask off the Windows build. Co-Authored-By: Claude Opus 4.7 (1M context) --- allowedpaths/sandbox.go | 13 +++++-- allowedpaths/sandbox_unix_test.go | 39 +++++++++++++++++++ builtins/tests/truncate/hardening_test.go | 26 +++++++------ builtins/tests/truncate/umask_unix_test.go | 26 +++++++++++++ builtins/tests/truncate/umask_windows_test.go | 20 ++++++++++ 5 files changed, 109 insertions(+), 15 deletions(-) create mode 100644 builtins/tests/truncate/umask_unix_test.go create mode 100644 builtins/tests/truncate/umask_windows_test.go diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 3e59ea299..1d1c0bdea 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -369,9 +369,11 @@ func (s *Sandbox) Open(path string, cwd string, flag int, perm os.FileMode) (io. } // Truncate sets the size of the file at path to size bytes. When create is -// true, a missing file is created with mode 0644; when create is false, a -// missing file returns os.ErrNotExist (the caller, e.g. truncate -c, decides -// whether to treat that as an error or a silent skip). +// true, a missing file is created with the open(2) permissive default +// (0666 & ~umask), matching GNU truncate and bash redirect semantics; the +// process umask is what actually decides the mode. When create is false, +// a missing file returns os.ErrNotExist (the caller, e.g. truncate -c, +// decides whether to treat that as an error or a silent skip). // // Like Open, the operation goes through os.Root for atomic openat-based path // validation. The cross-root symlink fallback is intentionally NOT used: @@ -417,7 +419,10 @@ func (s *Sandbox) Truncate(path string, cwd string, size int64, create bool) err if create { flag |= os.O_CREATE } - f, err := ar.root.OpenFile(relPath, flag, 0644) + // 0666 lets the process umask determine the final mode (open(2) applies + // mode & ~umask). This matches GNU truncate and bash >FILE behaviour: + // `umask 000; truncate -s 0 f` produces 0666; `umask 022` yields 0644. + f, err := ar.root.OpenFile(relPath, flag, 0666) if err != nil { // Return the raw error so callers can use errors.Is against // fs.ErrNotExist / fs.ErrPermission. The handler renders user- diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index b40583d62..2a78f93c6 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -8,6 +8,7 @@ package allowedpaths import ( + "fmt" "os" "path/filepath" "syscall" @@ -863,6 +864,44 @@ func TestSandboxTruncateMethodFIFONoReaderDoesNotBlock(t *testing.T) { } } +// TestSandboxTruncateMethodCreatesHonourUmask verifies that newly-created +// files use 0666 & ~umask, matching GNU truncate and bash redirect +// semantics. Hard-coding 0644 on the OpenFile call would make the result +// more restrictive than coreutils when umask is permissive (umask 000 +// should produce 0666, not 0644). +// +// syscall.Umask is process-global so this test cannot run in parallel +// with other umask-sensitive tests. The defer restores the saved value. +func TestSandboxTruncateMethodCreatesHonourUmask(t *testing.T) { + cases := []struct { + umaskBits int + wantMode os.FileMode + }{ + {0o022, 0o644}, + {0o000, 0o666}, + {0o077, 0o600}, + } + for _, tc := range cases { + name := fmt.Sprintf("umask_%03o", tc.umaskBits) + t.Run(name, func(t *testing.T) { + old := syscall.Umask(tc.umaskBits) + defer syscall.Umask(old) + + dir := t.TempDir() + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + require.NoError(t, sb.Truncate("fresh.txt", dir, 0, true)) + + info, err := os.Stat(filepath.Join(dir, "fresh.txt")) + require.NoError(t, err) + assert.Equal(t, tc.wantMode, info.Mode().Perm(), + "umask %03o should yield mode %03o", tc.umaskBits, tc.wantMode) + }) + } +} + // TestSandboxTruncateMethodFIFOWithReaderRejected verifies the post-fd // fstat guard: when a reader is connected, O_NONBLOCK no longer returns // ENXIO and the open succeeds, so the in-fd type check is the safety net diff --git a/builtins/tests/truncate/hardening_test.go b/builtins/tests/truncate/hardening_test.go index a1e22d608..7fda6d56a 100644 --- a/builtins/tests/truncate/hardening_test.go +++ b/builtins/tests/truncate/hardening_test.go @@ -114,10 +114,18 @@ func TestHardenDirectoryTarget(t *testing.T) { } } -// TestHardenCreatePreservesMode verifies that newly-created files use a -// reasonable default mode (0644 minus umask). This guards against an -// accidental switch to 0666 or 0777. +// TestHardenCreatePreservesMode verifies that newly-created files use the +// open(2) default of 0666 & ~umask, matching GNU truncate and bash. We +// also assert that no execute or special bits are set under any umask. +// +// Umask is locked to 022 in the test (the typical operator environment) +// so the mode comparison is deterministic. The umask-honouring property +// itself is verified directly against the sandbox API in +// allowedpaths.TestSandboxTruncateMethodCreatesHonourUmask. func TestHardenCreatePreservesMode(t *testing.T) { + old := umaskOrSkip(t, 0o022) + defer restoreUmask(old) + dir := t.TempDir() _, _, code := truncateRun(t, "truncate -s 0 fresh.bin", dir) if code != 0 { @@ -128,14 +136,10 @@ func TestHardenCreatePreservesMode(t *testing.T) { t.Fatal(err) } mode := info.Mode().Perm() - // On Unix the umask reduces the requested 0644 to whatever the test - // process's umask permits. We require: no setuid/setgid/sticky bits, - // no group-/world-writable bits, no execute bits. - if mode&0o111 != 0 { - t.Errorf("created file should not be executable: mode=%o", mode) - } - if mode&0o022 != 0 { - t.Errorf("created file should not be group/world writable: mode=%o", mode) + // 0666 & ~022 == 0644. Execute/setuid/setgid/sticky bits should not + // appear under any umask. + if mode != 0o644 { + t.Errorf("mode = %#o, want 0644 under umask 022", mode) } if info.Mode()&(os.ModeSetuid|os.ModeSetgid|os.ModeSticky) != 0 { t.Errorf("created file should have no special bits: mode=%o", info.Mode()) diff --git a/builtins/tests/truncate/umask_unix_test.go b/builtins/tests/truncate/umask_unix_test.go new file mode 100644 index 000000000..892bdccdc --- /dev/null +++ b/builtins/tests/truncate/umask_unix_test.go @@ -0,0 +1,26 @@ +// 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 truncate_test + +import ( + "syscall" + "testing" +) + +// umaskOrSkip sets the process umask to the requested value and returns +// the previous value. Callers MUST defer restoreUmask(old) to put it back. +// syscall.Umask is process-global and not safe to run concurrently with +// other umask-sensitive tests. +func umaskOrSkip(_ *testing.T, mask int) int { + return syscall.Umask(mask) +} + +// restoreUmask puts the umask back to the value returned by umaskOrSkip. +func restoreUmask(prev int) { + syscall.Umask(prev) +} diff --git a/builtins/tests/truncate/umask_windows_test.go b/builtins/tests/truncate/umask_windows_test.go new file mode 100644 index 000000000..39dab4241 --- /dev/null +++ b/builtins/tests/truncate/umask_windows_test.go @@ -0,0 +1,20 @@ +// 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 windows + +package truncate_test + +import "testing" + +// umaskOrSkip skips the calling test on Windows: the umask concept does +// not exist there, and Windows ACLs make permission-bit assertions +// irrelevant. +func umaskOrSkip(t *testing.T, _ int) int { + t.Skip("umask is not a Windows concept; permission bits are governed by ACLs") + return 0 +} + +func restoreUmask(_ int) {}