From ee95f8bc943a7be6e5a4f9bd6be34b8404813307 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Wed, 13 May 2026 14:23:25 +0200 Subject: [PATCH 1/2] feat(builtins): add logrotate demo builtin for host-remediation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps the existing AllowedPaths Truncate capability behind a logrotate- shaped command intended for agent-driven disk remediation. Supports -h, -s SIZE (threshold), -k N (informational only — sandbox has no rename capability), and -v. Not a substitute for logrotate(8): no config, no rename-based rotation, no compression, no post-rotate scripts. Co-Authored-By: Claude Opus 4.7 (1M context) --- SHELL_FEATURES.md | 1 + analysis/symbols_builtins.go | 6 + builtins/logrotate/logrotate.go | 210 ++++++++++++++++++ interp/register_builtins.go | 2 + .../scenarios/cmd/logrotate/basic/multi.yaml | 25 +++ .../cmd/logrotate/basic/truncate.yaml | 17 ++ .../cmd/logrotate/errors/invalid_size.yaml | 17 ++ .../cmd/logrotate/errors/missing_file.yaml | 13 ++ .../cmd/logrotate/errors/missing_target.yaml | 13 ++ tests/scenarios/cmd/logrotate/help/help.yaml | 14 ++ .../cmd/logrotate/threshold/above.yaml | 18 ++ .../cmd/logrotate/threshold/below.yaml | 17 ++ .../cmd/logrotate/verbose/skipped.yaml | 16 ++ .../cmd/logrotate/verbose/truncated.yaml | 16 ++ 14 files changed, 385 insertions(+) create mode 100644 builtins/logrotate/logrotate.go create mode 100644 tests/scenarios/cmd/logrotate/basic/multi.yaml create mode 100644 tests/scenarios/cmd/logrotate/basic/truncate.yaml create mode 100644 tests/scenarios/cmd/logrotate/errors/invalid_size.yaml create mode 100644 tests/scenarios/cmd/logrotate/errors/missing_file.yaml create mode 100644 tests/scenarios/cmd/logrotate/errors/missing_target.yaml create mode 100644 tests/scenarios/cmd/logrotate/help/help.yaml create mode 100644 tests/scenarios/cmd/logrotate/threshold/above.yaml create mode 100644 tests/scenarios/cmd/logrotate/threshold/below.yaml create mode 100644 tests/scenarios/cmd/logrotate/verbose/skipped.yaml create mode 100644 tests/scenarios/cmd/logrotate/verbose/truncated.yaml diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index 466e6c0b..8ea0bace 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -25,6 +25,7 @@ The in-shell `help` command mirrors these feature categories: run `help` for a c - ✅ `ip route get ADDRESS` — show the route selected by longest-prefix-match for ADDRESS (Linux only); write ops (`add`, `del`, `flush`, `replace`, `change`, `save`, `restore`) are blocked; `-6` (IPv6 routing) is not supported - ✅ `sort [-rnhubfds] [-k KEYDEF] [-t SEP] [-c|-C] [FILE]...` — sort lines of text files; `-h`/`--human-numeric-sort` orders by SI suffix (none < K/k < M < G < T < P < E < Z < Y < R < Q) then by numeric value (single-letter suffixes only — `Ki`, `Mi`, etc. are not recognised); `-o`, `--compress-program`, and `-T` are rejected (filesystem write / exec) - ✅ `ss [-tuaxlans4689Hoehs] [OPTION]...` — display network socket statistics; reads kernel socket state directly via `os.Open` (bypassing `AllowedPaths`) from: Linux: `/proc/net/`; macOS: sysctl; Windows: iphlpapi.dll; `-F`/`--filter` (GTFOBins file-read), `-p`/`--processes` (PID disclosure), `-K`/`--kill`, `-E`/`--events`, and `-N`/`--net` are rejected +- ✅ `logrotate [-s SIZE] [-k N] [-v] FILE...` — demo log-rotation helper that truncates each FILE to zero through `AllowedPaths`; `-s SIZE` skips files smaller than SIZE (binary `K/M/G/T` or decimal `KB/MB/GB/TB` suffixes); `-k N` is recorded for `-v` reporting only — the sandbox has no rename capability so prior copies are not retained; `-v` prints a per-file `truncated` or `skipping` line. Not a substitute for real `logrotate(8)`: no config file, compression, rename-based rotation, or post-rotate scripts. - ✅ `ls [-1aAdFhlpRrSt] [--offset N] [--limit N] [FILE]...` — list directory contents; `--offset`/`--limit` are non-standard pagination flags (single-directory only, silently ignored with `-R` or multiple arguments, capped at 1,000 entries per call); offset operates on filesystem order (not sorted order) for O(n) memory - ✅ `ping [-c N] [-W DURATION] [-i DURATION] [-q] [-4|-6] [-h] HOST` — send ICMP echo requests to a network host and report round-trip statistics; `-f` (flood), `-b` (broadcast), `-s` (packet size), `-I` (interface), `-p` (pattern), and `-R` (record route) are blocked; count/wait/interval are clamped to safe ranges with a warning; multicast, unspecified (`0.0.0.0`/`::`), and broadcast addresses (IPv4 last-octet `.255`) are rejected — note: directed broadcasts on non-standard subnets (e.g. `.127` on a `/25`) are not blocked without subnet-mask knowledge - ✅ `ps [-e|-A] [-f] [-p PIDLIST]` — report process status; default shows current-session processes; `-e`/`-A` shows all; `-f` adds UID/PPID/STIME columns; `-p` selects by PID list diff --git a/analysis/symbols_builtins.go b/analysis/symbols_builtins.go index cff47252..237d94dc 100644 --- a/analysis/symbols_builtins.go +++ b/analysis/symbols_builtins.go @@ -173,6 +173,12 @@ var builtinPerCommandSymbols = map[string][]string{ "os.O_RDONLY", // 🟢 read-only file flag constant; cannot open files by itself. "strconv.ParseInt", // 🟢 string-to-int conversion with base/bit-size; pure function, no I/O. }, + "logrotate": { + "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "errors.New", // 🟢 creates a sentinel error value; pure function, no I/O. + "math.MaxInt64", // 🟢 integer constant; no side effects. + "strconv.ParseInt", // 🟢 string-to-int conversion with base/bit-size; pure function, no I/O. + }, "ls": { "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. "errors.New", // 🟢 creates a simple error value; pure function, no I/O. diff --git a/builtins/logrotate/logrotate.go b/builtins/logrotate/logrotate.go new file mode 100644 index 00000000..aecd2b89 --- /dev/null +++ b/builtins/logrotate/logrotate.go @@ -0,0 +1,210 @@ +// 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 logrotate implements a demo-grade logrotate builtin. +// +// logrotate — rotate log files by truncating them in place +// +// Usage: logrotate [OPTION]... FILE... +// +// This is a deliberately minimal subset of the real GNU/util-linux logrotate +// tool, intended for agent-driven host remediation (e.g. "free up disk by +// emptying a runaway log file"). It only truncates each FILE through the +// AllowedPaths sandbox; no rename-based rotation, compression, or config +// parsing is performed. +// +// Accepted flags: +// +// -s SIZE, --size=SIZE +// Only rotate files whose current size is at least SIZE bytes. SIZE +// is a non-negative integer with an optional K/M/G/T binary suffix +// (1024-based; leading letter case-insensitive) or KB/MB/GB/TB +// decimal suffix (1000-based). Files smaller than SIZE are skipped. +// +// -k N, --keep=N +// Informational. Real logrotate keeps N rotated copies; this builtin +// has no sandbox capability to rename files, so the flag is recorded +// and printed by -v but does not actually retain prior copies. N +// must be a non-negative integer. +// +// -v, --verbose +// Print a line per file describing what happened (truncated / +// skipped) along with the pre-rotation size and keep count. +// +// -h, --help +// Print usage to stdout and exit 0. +// +// Exit codes: +// +// 0 All files processed successfully. +// 1 Bad flag value, missing operand, or at least one per-file failure. +// Processing continues across operands so a single failure does not +// abort the run. +package logrotate + +import ( + "context" + "errors" + "math" + "strconv" + + "github.com/DataDog/rshell/builtins" +) + +// Cmd is the logrotate builtin command descriptor. +var Cmd = builtins.Command{ + Name: "logrotate", + Description: "rotate log files by truncating them (demo)", + MakeFlags: registerFlags, +} + +var errInvalidSize = errors.New("invalid size") + +func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { + help := fs.BoolP("help", "h", false, "print usage and exit") + sizeStr := fs.StringP("size", "s", "", "only rotate files larger than SIZE bytes") + keep := fs.IntP("keep", "k", 0, "informational: number of rotated copies to keep") + verbose := fs.BoolP("verbose", "v", false, "print each rotated or skipped file") + + return func(ctx context.Context, callCtx *builtins.CallContext, files []string) builtins.Result { + if *help { + callCtx.Out("Usage: logrotate [OPTION]... FILE...\n") + callCtx.Out("Rotate each FILE by truncating it to zero bytes through the\n") + callCtx.Out("AllowedPaths sandbox. -k is recorded but not enforced because the\n") + callCtx.Out("sandbox does not currently expose a rename capability.\n\n") + fs.SetOutput(callCtx.Stdout) + fs.PrintDefaults() + return builtins.Result{} + } + + if callCtx.Truncate == nil { + callCtx.Errf("logrotate: filesystem capability not available\n") + return builtins.Result{Code: 1} + } + + var threshold int64 + if fs.Changed("size") { + n, err := parseSize(*sizeStr) + if err != nil { + callCtx.Errf("logrotate: invalid size %q\n", *sizeStr) + return builtins.Result{Code: 1} + } + threshold = n + } + + if *keep < 0 { + callCtx.Errf("logrotate: --keep must be >= 0\n") + return builtins.Result{Code: 1} + } + + if len(files) == 0 { + callCtx.Errf("logrotate: 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} + } + + // Stat first when we need the size (either for the threshold + // check or to report it under -v). When neither is in play we + // skip the stat so a missing file still rotates correctly via + // create=false (which surfaces ErrNotExist as a per-file error). + var sizeBefore int64 + needStat := threshold > 0 || *verbose + if needStat { + if callCtx.StatFile == nil { + callCtx.Errf("logrotate: stat capability not available\n") + return builtins.Result{Code: 1} + } + info, err := callCtx.StatFile(ctx, file) + if err != nil { + callCtx.Errf("logrotate: %q: %s\n", file, callCtx.PortableErr(err)) + failed = true + continue + } + sizeBefore = info.Size() + if threshold > 0 && sizeBefore < threshold { + if *verbose { + callCtx.Outf("logrotate: %s: %d bytes below threshold %d, skipping\n", file, sizeBefore, threshold) + } + continue + } + } + + if err := callCtx.Truncate(ctx, file, 0, false); err != nil { + callCtx.Errf("logrotate: %q: %s\n", file, callCtx.PortableErr(err)) + failed = true + continue + } + + if *verbose { + callCtx.Outf("logrotate: %s: truncated %d bytes (keep=%d)\n", file, sizeBefore, *keep) + } + } + + if failed { + return builtins.Result{Code: 1} + } + return builtins.Result{} + } +} + +// parseSize parses a non-negative byte count with an optional K/M/G/T binary +// suffix (1024-based; leading letter case-insensitive) or KB/MB/GB/TB decimal +// suffix (1000-based). Demo-grade: a strict subset of truncate's grammar that +// is enough for the size-threshold use case. Relative-size modifiers and the +// P/E suffixes are intentionally not supported. +func parseSize(s string) (int64, error) { + if s == "" { + return 0, errInvalidSize + } + 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:] + + var mult int64 + switch suffix { + case "": + mult = 1 + case "K", "k", "KiB", "kiB": + mult = 1 << 10 + case "M", "m", "MiB", "miB": + mult = 1 << 20 + case "G", "g", "GiB", "giB": + mult = 1 << 30 + case "T", "t", "TiB", "tiB": + mult = 1 << 40 + case "KB", "kB": + mult = 1000 + case "MB", "mB": + mult = 1000 * 1000 + case "GB", "gB": + mult = 1000 * 1000 * 1000 + case "TB", "tB": + mult = 1000 * 1000 * 1000 * 1000 + default: + return 0, errInvalidSize + } + + 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/interp/register_builtins.go b/interp/register_builtins.go index 25dddbb5..e3187223 100644 --- a/interp/register_builtins.go +++ b/interp/register_builtins.go @@ -23,6 +23,7 @@ import ( "github.com/DataDog/rshell/builtins/head" "github.com/DataDog/rshell/builtins/help" "github.com/DataDog/rshell/builtins/ip" + "github.com/DataDog/rshell/builtins/logrotate" "github.com/DataDog/rshell/builtins/ls" "github.com/DataDog/rshell/builtins/ping" printfcmd "github.com/DataDog/rshell/builtins/printf" @@ -63,6 +64,7 @@ func registerBuiltins() { head.Cmd, help.Cmd, ip.Cmd, + logrotate.Cmd, ls.Cmd, ping.Cmd, sortcmd.Cmd, diff --git a/tests/scenarios/cmd/logrotate/basic/multi.yaml b/tests/scenarios/cmd/logrotate/basic/multi.yaml new file mode 100644 index 00000000..45cdc94d --- /dev/null +++ b/tests/scenarios/cmd/logrotate/basic/multi.yaml @@ -0,0 +1,25 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: logrotate processes every operand and reports exit 0 when all succeed. +setup: + files: + - path: a.log + content: "alpha\n" + - path: b.log + content: "beta\n" + - path: c.log + content: "gamma\n" +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate a.log b.log c.log + wc -c < a.log + wc -c < b.log + wc -c < c.log +expect: + stdout: |+ + 0 + 0 + 0 + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/cmd/logrotate/basic/truncate.yaml b/tests/scenarios/cmd/logrotate/basic/truncate.yaml new file mode 100644 index 00000000..f4ed13cd --- /dev/null +++ b/tests/scenarios/cmd/logrotate/basic/truncate.yaml @@ -0,0 +1,17 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: logrotate empties a single log file via the sandbox. +setup: + files: + - path: app.log + content: "request 1\nrequest 2\nrequest 3\n" +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate app.log + wc -c < app.log +expect: + stdout: |+ + 0 + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/cmd/logrotate/errors/invalid_size.yaml b/tests/scenarios/cmd/logrotate/errors/invalid_size.yaml new file mode 100644 index 00000000..3acc370f --- /dev/null +++ b/tests/scenarios/cmd/logrotate/errors/invalid_size.yaml @@ -0,0 +1,17 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: A non-numeric -s value is rejected with exit 1. +setup: + files: + - path: app.log + content: "" +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate -s xyz app.log +expect: + stdout: |+ + stderr_contains: + - "logrotate:" + - "invalid size" + exit_code: 1 diff --git a/tests/scenarios/cmd/logrotate/errors/missing_file.yaml b/tests/scenarios/cmd/logrotate/errors/missing_file.yaml new file mode 100644 index 00000000..9b4f7dfc --- /dev/null +++ b/tests/scenarios/cmd/logrotate/errors/missing_file.yaml @@ -0,0 +1,13 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: logrotate without operands rejects with exit 1. +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate +expect: + stdout: |+ + stderr_contains: + - "logrotate:" + - "missing file operand" + exit_code: 1 diff --git a/tests/scenarios/cmd/logrotate/errors/missing_target.yaml b/tests/scenarios/cmd/logrotate/errors/missing_target.yaml new file mode 100644 index 00000000..67420085 --- /dev/null +++ b/tests/scenarios/cmd/logrotate/errors/missing_target.yaml @@ -0,0 +1,13 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: logrotate on a non-existent file fails with exit 1. +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate nope.log +expect: + stdout: |+ + stderr_contains: + - "logrotate:" + - "nope.log" + exit_code: 1 diff --git a/tests/scenarios/cmd/logrotate/help/help.yaml b/tests/scenarios/cmd/logrotate/help/help.yaml new file mode 100644 index 00000000..b4f7dda4 --- /dev/null +++ b/tests/scenarios/cmd/logrotate/help/help.yaml @@ -0,0 +1,14 @@ +# skip: --help output format is implementation-specific. +skip_assert_against_bash: true +description: logrotate --help prints usage to stdout and exits 0. +input: + script: |+ + logrotate --help +expect: + stdout_contains: + - "Usage: logrotate" + - "--size" + - "--keep" + - "--verbose" + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/cmd/logrotate/threshold/above.yaml b/tests/scenarios/cmd/logrotate/threshold/above.yaml new file mode 100644 index 00000000..dea2dca1 --- /dev/null +++ b/tests/scenarios/cmd/logrotate/threshold/above.yaml @@ -0,0 +1,18 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: -s SIZE truncates when the file is at or above the threshold. +setup: + files: + - path: big.log + # 1100 bytes, well above the 1K threshold. + content: "0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghi" +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate -s 1K big.log + wc -c < big.log +expect: + stdout: |+ + 0 + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/cmd/logrotate/threshold/below.yaml b/tests/scenarios/cmd/logrotate/threshold/below.yaml new file mode 100644 index 00000000..642f7c2d --- /dev/null +++ b/tests/scenarios/cmd/logrotate/threshold/below.yaml @@ -0,0 +1,17 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: -s SIZE leaves a file untouched when it is below the threshold. +setup: + files: + - path: small.log + content: "tiny\n" +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate -s 1K small.log + wc -c < small.log +expect: + stdout: |+ + 5 + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/cmd/logrotate/verbose/skipped.yaml b/tests/scenarios/cmd/logrotate/verbose/skipped.yaml new file mode 100644 index 00000000..a183116d --- /dev/null +++ b/tests/scenarios/cmd/logrotate/verbose/skipped.yaml @@ -0,0 +1,16 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: -v reports skipped files when -s threshold filters them out. +setup: + files: + - path: small.log + content: "tiny\n" +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate -v -s 1K small.log +expect: + stdout: |+ + logrotate: small.log: 5 bytes below threshold 1024, skipping + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/cmd/logrotate/verbose/truncated.yaml b/tests/scenarios/cmd/logrotate/verbose/truncated.yaml new file mode 100644 index 00000000..9c2bf157 --- /dev/null +++ b/tests/scenarios/cmd/logrotate/verbose/truncated.yaml @@ -0,0 +1,16 @@ +# skip: logrotate is a demo builtin with no bash equivalent. +skip_assert_against_bash: true +description: -v reports the pre-rotation size and keep count. +setup: + files: + - path: app.log + content: "hello\n" +input: + allowed_paths: ["$DIR"] + script: |+ + logrotate -v -k 3 app.log +expect: + stdout: |+ + logrotate: app.log: truncated 6 bytes (keep=3) + stderr: |+ + exit_code: 0 From 8b1b308344107fca85ee74e9d6fdcc78e0cc2757 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Wed, 13 May 2026 14:44:44 +0200 Subject: [PATCH 2/2] fix(logrotate): close threshold-check TOCTOU via TruncateIfLarger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Logrotate previously called callCtx.StatFile to gate -s and then callCtx.Truncate, which reopens the path separately — a concurrent swap of a smaller file under the same path between the two calls could fool the threshold check. Add Sandbox.TruncateIfLarger, which opens the path with O_WRONLY, fstats the resulting fd, and ftruncates only when the fd's size is at least minSize. Stat and truncate share a single fd, so the size used for the threshold decision is the size of the inode that is actually truncated. Existing safety properties of Sandbox.Truncate (IsRegular check, write-symlink rejection, EINVAL on negative size) are preserved. Expose the primitive on CallContext, wire it up in interp's two CallContext construction sites, and refactor logrotate to drop the separate StatFile call. Addresses Codex review on PR #247. Co-Authored-By: Claude Opus 4.7 (1M context) --- allowedpaths/sandbox.go | 60 +++++++++++++++++++++ allowedpaths/sandbox_test.go | 96 +++++++++++++++++++++++++++++++++ builtins/builtins.go | 9 ++++ builtins/logrotate/logrotate.go | 47 +++++++--------- interp/runner_exec.go | 6 +++ 5 files changed, 190 insertions(+), 28 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 1d1c0bde..fc6b1135 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -454,6 +454,66 @@ func (s *Sandbox) Truncate(path string, cwd string, size int64, create bool) err return closeErr } +// TruncateIfLarger opens path with O_WRONLY (and optionally O_CREATE), +// fstats the resulting fd, and ftruncates to newSize only when the fd's +// pre-truncation size is at least minSize. The fstat and ftruncate share +// the same fd so the size check cannot race with a path swap: a file +// substituted under the same path between resolve and open is caught by +// the IsRegular check inherited from Truncate, and any size-based decision +// is made against the inode that will actually be truncated. +// +// minSize == 0 is equivalent to Truncate(path, cwd, newSize, create) but +// still returns the pre-truncation size so callers can report it. When +// minSize > 0 and the file is smaller, the function returns +// (sizeBefore, false, nil) without altering the file. +// +// All other safety properties — sandbox path resolution, non-regular +// file rejection, write-symlink TOCTOU rejection, EINVAL for negative +// sizes, deferred-close error semantics — match Truncate exactly. +func (s *Sandbox) TruncateIfLarger(path, cwd string, minSize, newSize int64, create bool) (int64, bool, error) { + if newSize < 0 { + return 0, false, &os.PathError{Op: "truncate", Path: path, Err: syscall.EINVAL} + } + + absPath := toAbs(path, cwd) + + ar, relPath, ok := s.resolve(absPath) + if !ok { + return 0, false, &os.PathError{Op: "truncate", Path: path, Err: os.ErrPermission} + } + + flag := os.O_WRONLY | syscall.O_NONBLOCK + if create { + flag |= os.O_CREATE + } + f, err := ar.root.OpenFile(relPath, flag, 0666) + if err != nil { + return 0, false, err + } + info, err := f.Stat() + if err != nil { + f.Close() + return 0, false, err + } + if !info.Mode().IsRegular() { + f.Close() + return 0, false, &os.PathError{Op: "truncate", Path: path, Err: errors.New("not a regular file")} + } + sizeBefore := info.Size() + if sizeBefore < minSize { + // Close-only path: nothing was written, so a Close error here + // cannot mask user-visible data loss. Drop it. + f.Close() + return sizeBefore, false, nil + } + truncErr := f.Truncate(newSize) + closeErr := f.Close() + if truncErr != nil { + return sizeBefore, false, truncErr + } + return sizeBefore, true, 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 a16e37b6..1b7947c5 100644 --- a/allowedpaths/sandbox_test.go +++ b/allowedpaths/sandbox_test.go @@ -291,6 +291,102 @@ func TestSandboxTruncateMethodSymlinkEscapeRejected(t *testing.T) { assert.Equal(t, "untouched", string(got), "symlink target must not be reachable for writes") } +// TestSandboxTruncateIfLargerAboveThreshold verifies that a file at or +// above minSize is truncated to newSize and the pre-size is reported. +func TestSandboxTruncateIfLargerAboveThreshold(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() + + sizeBefore, truncated, err := sb.TruncateIfLarger("log.txt", dir, 5, 0, false) + require.NoError(t, err) + assert.Equal(t, int64(10), sizeBefore) + assert.True(t, truncated) + + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, int64(0), info.Size()) +} + +// TestSandboxTruncateIfLargerBelowThreshold verifies that a file smaller +// than minSize is left untouched and (size, false, nil) is returned. +func TestSandboxTruncateIfLargerBelowThreshold(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() + + sizeBefore, truncated, err := sb.TruncateIfLarger("log.txt", dir, 1024, 0, false) + require.NoError(t, err) + assert.Equal(t, int64(3), sizeBefore) + assert.False(t, truncated) + + got, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, "abc", string(got), "below-threshold file must not be modified") +} + +// TestSandboxTruncateIfLargerZeroMinSize verifies that minSize == 0 +// is equivalent to Truncate: the file is always truncated. +func TestSandboxTruncateIfLargerZeroMinSize(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "log.txt") + require.NoError(t, os.WriteFile(path, []byte("xyz"), 0644)) + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + sizeBefore, truncated, err := sb.TruncateIfLarger("log.txt", dir, 0, 0, false) + require.NoError(t, err) + assert.Equal(t, int64(3), sizeBefore) + assert.True(t, truncated) + + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, int64(0), info.Size()) +} + +// TestSandboxTruncateIfLargerOutsideAllowedPath verifies that paths +// outside the sandbox are rejected with permission denied before any I/O. +func TestSandboxTruncateIfLargerOutsideAllowedPath(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.TruncateIfLarger(target, allowed, 0, 0, false) + assert.ErrorIs(t, err, os.ErrPermission) + + got, ferr := os.ReadFile(target) + require.NoError(t, ferr) + assert.Equal(t, "untouched", string(got)) +} + +// TestSandboxTruncateIfLargerNoCreate verifies that missing files surface +// os.ErrNotExist when create=false. +func TestSandboxTruncateIfLargerNoCreate(t *testing.T) { + dir := t.TempDir() + + sb, _, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + _, _, err = sb.TruncateIfLarger("missing.txt", dir, 0, 0, false) + assert.ErrorIs(t, err, fs.ErrNotExist) +} + func TestSandboxOpenReadStillWorks(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "test.txt"), []byte("data"), 0644)) diff --git a/builtins/builtins.go b/builtins/builtins.go index e4681567..04895736 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -159,6 +159,15 @@ type CallContext struct { // Negative sizes are rejected. Truncate func(ctx context.Context, path string, size int64, create bool) error + // TruncateIfLarger opens path through the sandbox, fstats the open + // fd, and ftruncates to newSize only when the pre-truncation size is + // at least minSize. The size check and truncate share a single fd so + // the threshold cannot race a path swap. Returns the pre-truncation + // size (always populated when the open succeeds) and a flag indicating + // whether ftruncate ran. When minSize == 0 the check is skipped and + // the file is always truncated. + TruncateIfLarger func(ctx context.Context, path string, minSize, newSize int64, create bool) (sizeBefore int64, truncated bool, err error) + // PortableErr normalizes an OS error to a POSIX-style message. PortableErr func(err error) string diff --git a/builtins/logrotate/logrotate.go b/builtins/logrotate/logrotate.go index aecd2b89..ad5668c1 100644 --- a/builtins/logrotate/logrotate.go +++ b/builtins/logrotate/logrotate.go @@ -15,6 +15,11 @@ // AllowedPaths sandbox; no rename-based rotation, compression, or config // parsing is performed. // +// Threshold safety: the size check used by -s and the ftruncate share a +// single open fd via callCtx.TruncateIfLarger, so an attacker with write +// access to the directory cannot swap a small file under the same path +// between a separate stat and truncate to fool the threshold gate. +// // Accepted flags: // // -s SIZE, --size=SIZE @@ -79,7 +84,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{} } - if callCtx.Truncate == nil { + if callCtx.TruncateIfLarger == nil { callCtx.Errf("logrotate: filesystem capability not available\n") return builtins.Result{Code: 1} } @@ -110,38 +115,24 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{Code: 1} } - // Stat first when we need the size (either for the threshold - // check or to report it under -v). When neither is in play we - // skip the stat so a missing file still rotates correctly via - // create=false (which surfaces ErrNotExist as a per-file error). - var sizeBefore int64 - needStat := threshold > 0 || *verbose - if needStat { - if callCtx.StatFile == nil { - callCtx.Errf("logrotate: stat capability not available\n") - return builtins.Result{Code: 1} - } - info, err := callCtx.StatFile(ctx, file) - if err != nil { - callCtx.Errf("logrotate: %q: %s\n", file, callCtx.PortableErr(err)) - failed = true - continue - } - sizeBefore = info.Size() - if threshold > 0 && sizeBefore < threshold { - if *verbose { - callCtx.Outf("logrotate: %s: %d bytes below threshold %d, skipping\n", file, sizeBefore, threshold) - } - continue - } - } - - if err := callCtx.Truncate(ctx, file, 0, false); err != nil { + // One sandbox call: open, fstat, conditionally ftruncate, all + // on the same fd. This closes the path-stat to path-truncate + // TOCTOU window — the size used for the threshold decision is + // the size of the inode that will actually be truncated. + sizeBefore, truncated, err := callCtx.TruncateIfLarger(ctx, file, threshold, 0, false) + if err != nil { callCtx.Errf("logrotate: %q: %s\n", file, callCtx.PortableErr(err)) failed = true continue } + if !truncated { + if *verbose { + callCtx.Outf("logrotate: %s: %d bytes below threshold %d, skipping\n", file, sizeBefore, threshold) + } + continue + } + if *verbose { callCtx.Outf("logrotate: %s: truncated %d bytes (keep=%d)\n", file, sizeBefore, *keep) } diff --git a/interp/runner_exec.go b/interp/runner_exec.go index 97a21e11..37796138 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -611,6 +611,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { Truncate: func(ctx context.Context, path string, size int64, create bool) error { return r.sandbox.Truncate(path, dir, size, create) }, + TruncateIfLarger: func(ctx context.Context, path string, minSize, newSize int64, create bool) (int64, bool, error) { + return r.sandbox.TruncateIfLarger(path, dir, minSize, newSize, create) + }, PortableErr: allowedpaths.PortableErrMsg, Now: r.startTime, FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) { @@ -720,6 +723,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { 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) }, + TruncateIfLarger: func(ctx context.Context, path string, minSize, newSize int64, create bool) (int64, bool, error) { + return r.sandbox.TruncateIfLarger(path, HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir, minSize, newSize, create) + }, PortableErr: allowedpaths.PortableErrMsg, Now: r.startTime, FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) {