diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index 86f13675..84b3049d 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -78,8 +78,6 @@ var builtinPerCommandSymbols = map[string][]string{ "io/fs.ModeSocket", // file mode bit constant for sockets; pure constant. "io/fs.ModeSymlink", // file mode bit constant for symlinks; pure constant. "io/fs.ReadDirFile", // read-only directory handle interface; no write capability. - "math.Ceil", // pure arithmetic; no side effects. - "math.Floor", // pure arithmetic; no side effects. "math.MaxInt64", // integer constant; no side effects. "os.IsNotExist", // checks if error is "not exist"; pure function, no I/O. "os.PathError", // error type for path operations; pure type. @@ -89,10 +87,6 @@ var builtinPerCommandSymbols = map[string][]string{ "strconv.ParseInt", // string-to-int conversion; pure function, no I/O. "strings.HasPrefix", // pure function for prefix matching; no I/O. "strings.ToLower", // converts string to lowercase; pure function, no I/O. - "time.Duration", // duration type; pure integer alias, no I/O. - "time.Hour", // constant representing one hour; no side effects. - "time.Minute", // constant representing one minute; no side effects. - "time.Second", // constant representing one second; no side effects. "time.Time", // time value type; pure data, no side effects. "unicode/utf8.DecodeRuneInString", // decodes first UTF-8 rune from a string; pure function, no I/O. }, @@ -328,8 +322,6 @@ var builtinAllowedSymbols = []string{ "io/fs.ModeSticky", // file mode bit constant for sticky bit; pure constant. "io/fs.ModeSymlink", // file mode bit constant for symlinks; pure constant. "io/fs.ReadDirFile", // read-only directory handle interface; no write capability. - "math.Ceil", // pure arithmetic; no side effects. - "math.Floor", // pure arithmetic; no side effects. "math.Inf", // returns positive or negative infinity; pure function, no I/O. "math.MaxInt32", // integer constant; no side effects. "math.MaxInt64", // integer constant; no side effects. @@ -368,10 +360,6 @@ var builtinAllowedSymbols = []string{ "strings.TrimSpace", // removes leading/trailing whitespace; pure function. "syscall.EISDIR", // error number constant for "is a directory"; pure constant, no I/O. "syscall.Errno", // error type for system call error numbers; pure type, no I/O. - "time.Duration", // duration type; pure integer alias, no I/O. - "time.Hour", // constant representing one hour; no side effects. - "time.Minute", // constant representing one minute; no side effects. - "time.Second", // constant representing one second; no side effects. "time.Time", // time value type; pure data, no side effects. "unicode.Cc", // control character category range table; pure data, no I/O. "unicode.Cf", // format character category range table; pure data, no I/O. diff --git a/allowedsymbols/symbols_interp.go b/allowedsymbols/symbols_interp.go index 13513e54..4a9e39cc 100644 --- a/allowedsymbols/symbols_interp.go +++ b/allowedsymbols/symbols_interp.go @@ -57,7 +57,6 @@ var interpAllowedSymbols = []string{ "sync.Mutex", // mutual exclusion lock; concurrency primitive, no I/O. "sync.Once", // ensures a function runs exactly once; concurrency primitive, no I/O. "sync.WaitGroup", // waits for goroutines to finish; concurrency primitive, no I/O. - "time.Now", // returns current time; read-only, no mutation. // --- mvdan.cc/sh/v3/expand --- (shell word expansion library) diff --git a/allowedsymbols/symbols_restrictedtime.go b/allowedsymbols/symbols_restrictedtime.go new file mode 100644 index 00000000..1a347f06 --- /dev/null +++ b/allowedsymbols/symbols_restrictedtime.go @@ -0,0 +1,29 @@ +// 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 allowedsymbols + +// restrictedtimeAllowedSymbols lists every "importpath.Symbol" that may be used by +// non-test Go files in restrictedtime/. Each entry must be in "importpath.Symbol" +// form, where importpath is the full Go import path. +// +// Each symbol must have a comment explaining what it does and why it is safe +// to use inside the time-comparison package. +// +// Internal module imports (github.com/DataDog/rshell/*) are auto-allowed +// and do not appear here. +// +// The permanently banned packages (reflect, unsafe) apply here too. +var restrictedtimeAllowedSymbols = []string{ + "math.Ceil", // pure arithmetic; rounds up fractional minutes for ceiling bucketing. + "math.Floor", // pure arithmetic; rounds down fractional days for floor bucketing. + "math.MaxInt64", // integer constant; used for overflow guards. + "time.Duration", // duration type; pure integer alias, no I/O. + "time.Hour", // constant representing one hour; no side effects. + "time.Minute", // constant representing one minute; no side effects. + "time.Now", // captures invocation timestamp once; read-only, no mutation. + "time.Second", // constant representing one second; no side effects. + "time.Time", // time value type; pure data, no side effects. +} diff --git a/allowedsymbols/symbols_restrictedtime_test.go b/allowedsymbols/symbols_restrictedtime_test.go new file mode 100644 index 00000000..d9f6304f --- /dev/null +++ b/allowedsymbols/symbols_restrictedtime_test.go @@ -0,0 +1,37 @@ +// 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 allowedsymbols + +import ( + "strings" + "testing" +) + +// restrictedtimeCheckConfig returns the allowedSymbolsConfig used to enforce +// symbol-level import restrictions on restrictedtime/. Verification tests reuse +// this function to ensure they test the exact same configuration. +func restrictedtimeCheckConfig() allowedSymbolsConfig { + return allowedSymbolsConfig{ + Symbols: restrictedtimeAllowedSymbols, + TargetDir: "restrictedtime", + CollectFiles: func(dir string) ([]string, error) { + return collectFlatGoFiles(dir) + }, + ExemptImport: func(importPath string) bool { + return strings.HasPrefix(importPath, "github.com/DataDog/rshell/") + }, + ListName: "restrictedtimeAllowedSymbols", + MinFiles: 1, + } +} + +// TestTimecompAllowedSymbols enforces symbol-level import restrictions on +// non-test Go files in restrictedtime/. Every imported symbol must be explicitly +// listed in restrictedtimeAllowedSymbols. Internal module imports +// (github.com/DataDog/rshell/*) are auto-allowed. +func TestTimecompAllowedSymbols(t *testing.T) { + checkAllowedSymbols(t, restrictedtimeCheckConfig()) +} diff --git a/allowedsymbols/symbols_restrictedtime_verification_test.go b/allowedsymbols/symbols_restrictedtime_verification_test.go new file mode 100644 index 00000000..be8c4bfc --- /dev/null +++ b/allowedsymbols/symbols_restrictedtime_verification_test.go @@ -0,0 +1,67 @@ +// 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 allowedsymbols + +import ( + "path/filepath" + "strings" + "testing" +) + +// restrictedtimeVerifyCfg returns a restrictedtimeCheckConfig with RepoRootOverride and +// Errors set for verification testing. +func restrictedtimeVerifyCfg(tempRoot string, errs *[]string) allowedSymbolsConfig { + cfg := restrictedtimeCheckConfig() + cfg.RepoRootOverride = tempRoot + cfg.Errors = errs + return cfg +} + +func TestVerificationTimecompCleanPass(t *testing.T) { + root := repoRoot(t) + tmp := t.TempDir() + copyDir(t, filepath.Join(root, "restrictedtime"), filepath.Join(tmp, "restrictedtime")) + + var errs []string + checkAllowedSymbols(t, restrictedtimeVerifyCfg(tmp, &errs)) + + if len(errs) > 0 { + t.Errorf("expected no errors on clean copy, got:\n%s", strings.Join(errs, "\n")) + } +} + +func TestVerificationTimecompUnlistedSymbol(t *testing.T) { + root := repoRoot(t) + tmp := t.TempDir() + copyDir(t, filepath.Join(root, "restrictedtime"), filepath.Join(tmp, "restrictedtime")) + + target := findFirstFlatGoFile(t, filepath.Join(tmp, "restrictedtime")) + injectUnlistedSymbol(t, target) + + var errs []string + checkAllowedSymbols(t, restrictedtimeVerifyCfg(tmp, &errs)) + + if !errContains(errs, "os") || !errContains(errs, "not in the allowlist") { + t.Errorf("expected 'not in the allowlist' error for os import, got: %v", errs) + } +} + +func TestVerificationTimecompExemptImport(t *testing.T) { + root := repoRoot(t) + tmp := t.TempDir() + copyDir(t, filepath.Join(root, "restrictedtime"), filepath.Join(tmp, "restrictedtime")) + + target := findFirstFlatGoFile(t, filepath.Join(tmp, "restrictedtime")) + // Internal module imports (github.com/DataDog/rshell/*) are exempt. + injectImport(t, target, `fakepkg "github.com/DataDog/rshell/fakepkg"`, "var _ = fakepkg.Foo") + + var errs []string + checkAllowedSymbols(t, restrictedtimeVerifyCfg(tmp, &errs)) + + if errContains(errs, "github.com/DataDog/rshell/fakepkg") { + t.Errorf("exempt import should not be flagged, got: %v", errs) + } +} diff --git a/builtins/builtins.go b/builtins/builtins.go index 95e5e17a..edacb57a 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -121,10 +121,17 @@ type CallContext struct { // PortableErr normalizes an OS error to a POSIX-style message. PortableErr func(err error) string - // Now returns the current time. Builtins should use this instead of - // calling time.Now() directly, so the time source is consistent and - // testable. - Now func() time.Time + // MatchMtime checks whether a file's modification time satisfies a + // -mtime predicate (day-granularity). cmp: -1 = less, 0 = exact, 1 = more. + MatchMtime func(modTime time.Time, n int64, cmp int) bool + + // MatchMmin checks whether a file's modification time satisfies a + // -mmin predicate (minute-granularity). cmp: -1 = less, 0 = exact, 1 = more. + MatchMmin func(modTime time.Time, n int64, cmp int) bool + + // IsRecentEnough reports whether modTime is within the given number of + // months before the invocation time and not in the future. + IsRecentEnough func(modTime time.Time, months int) bool // FileIdentity extracts canonical file identity from FileInfo. // On Unix: dev+inode from Stat_t. On Windows: volume serial + file index diff --git a/builtins/find/eval.go b/builtins/find/eval.go index 66259b83..70a6bac7 100644 --- a/builtins/find/eval.go +++ b/builtins/find/eval.go @@ -8,7 +8,6 @@ package find import ( "context" iofs "io/fs" - "math" "time" "github.com/DataDog/rshell/builtins" @@ -24,7 +23,6 @@ type evalResult struct { type evalContext struct { callCtx *builtins.CallContext ctx context.Context - now time.Time relPath string // path relative to starting point info iofs.FileInfo // file info (lstat or stat depending on -L) depth int // current depth @@ -175,76 +173,14 @@ func evalNewer(ec *evalContext, refPath string) bool { return ec.info.ModTime().After(refTime) } -// evalMtime checks modification time in days. -// GNU find uses different comparison strategies for -mtime: -// - Exact (N): day-bucketed comparison — N*86400 <= delta < (N+1)*86400. -// - +N: raw second comparison — delta > (N+1)*86400. -// - -N: raw second comparison — delta < N*86400. -// -// GNU find captures 'now' via time() (second precision) but gets file mtime -// from stat() (nanosecond precision). This means for very fresh files, -// delta can be slightly negative, causing -mtime -0 to match files created -// within the same second. We replicate this by truncating now to seconds -// for +N/-N comparisons. -// -// maxMtimeN is the largest N for which (N+1)*24*time.Hour does not overflow. -const maxMtimeN = int64(math.MaxInt64/(int64(24*time.Hour))) - 1 - +// evalMtime delegates to CallContext.MatchMtime which encapsulates +// the GNU find day-bucketing logic without exposing wall-clock time. func evalMtime(ec *evalContext, n int64, cmp cmpOp) bool { - modTime := ec.info.ModTime() - switch cmp { - case cmpMore: // +N: strictly older than (N+1) days - if n > maxMtimeN { - return false // threshold beyond representable duration - } - // Truncate now to second precision to match GNU find's time(). - diff := ec.now.Truncate(time.Second).Sub(modTime) - return diff >= time.Duration(n+1)*24*time.Hour - case cmpLess: // -N: strictly newer than N days - if n > maxMtimeN { - return true // threshold beyond representable duration - } - // Truncate now to second precision to match GNU find's time(). - diff := ec.now.Truncate(time.Second).Sub(modTime) - return diff < time.Duration(n)*24*time.Hour - default: // N: day-bucketed exact match - // Do not clamp negative diff — future-dated files must produce - // negative day buckets so they never match non-negative N, - // matching GNU find behavior. - diff := ec.now.Sub(modTime) - days := int64(math.Floor(diff.Hours() / 24)) - return days == n - } + return ec.callCtx.MatchMtime(ec.info.ModTime(), n, int(cmp)) } -// evalMmin checks modification time in minutes. -// GNU find uses different comparison strategies: -// - Exact (N): ceiling-bucketed comparison — a 5s-old file is in bucket 1. -// - +N: raw second comparison — delta_seconds > N*60. -// - -N: raw second comparison — delta_seconds < N*60. -// -// This matches GNU findutils behavior where +N/-N compare against raw -// seconds while exact N uses a window check. -// maxMminN is the largest N for which time.Duration(N)*time.Minute -// does not overflow int64 nanoseconds. -const maxMminN = int64(math.MaxInt64 / int64(time.Minute)) - +// evalMmin delegates to CallContext.MatchMmin which encapsulates +// the GNU find minute-bucketing logic without exposing wall-clock time. func evalMmin(ec *evalContext, n int64, cmp cmpOp) bool { - modTime := ec.info.ModTime() - diff := ec.now.Sub(modTime) - switch cmp { - case cmpMore: // +N: strictly older than N minutes - if n > maxMminN { - return false // threshold is beyond representable duration; nothing qualifies - } - return diff > time.Duration(n)*time.Minute - case cmpLess: // -N: strictly newer than N minutes - if n > maxMminN { - return true // threshold is beyond representable duration; everything qualifies - } - return diff < time.Duration(n)*time.Minute - default: // N: ceiling-bucketed exact match - mins := int64(math.Ceil(diff.Minutes())) - return mins == n - } + return ec.callCtx.MatchMmin(ec.info.ModTime(), n, int(cmp)) } diff --git a/builtins/find/eval_test.go b/builtins/find/eval_test.go index 5978376e..ff5d6b9e 100644 --- a/builtins/find/eval_test.go +++ b/builtins/find/eval_test.go @@ -9,7 +9,6 @@ import ( "context" "io" iofs "io/fs" - "math" "strings" "testing" "time" @@ -18,173 +17,6 @@ import ( "github.com/stretchr/testify/assert" ) -// TestEvalMminCeiling verifies that -mmin uses ceiling rounding. -// GNU find rounds up fractional minutes: a file 5 seconds old is in -// minute bucket 1 (not 0). This prevents regression to math.Floor. -func TestEvalMminCeiling(t *testing.T) { - now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) - - tests := []struct { - name string - age time.Duration // how old the file is - n int64 - cmp cmpOp - matched bool - }{ - // Exact match uses ceiling bucketing: ceil(delta_sec / 60) - // +N/-N use raw second comparison: delta_sec > N*60 / delta_sec < N*60 - - // 0 seconds old → ceil(0) = 0 → bucket 0 - {"0s exact 0", 0, 0, cmpExact, true}, - {"0s gt 0", 0, 0, cmpMore, false}, // 0 > 0 = false - {"0s lt 1", 0, 1, cmpLess, true}, // 0 < 60 = true - - // 1 second old → ceil(1/60) = 1 → bucket 1 - {"1s exact 0", 1 * time.Second, 0, cmpExact, false}, - {"1s exact 1", 1 * time.Second, 1, cmpExact, true}, - {"1s gt 0", 1 * time.Second, 0, cmpMore, true}, // 1 > 0 = true - {"1s lt 1", 1 * time.Second, 1, cmpLess, true}, // 1 < 60 = true (GNU find matches) - - // 5 seconds old → ceil(5/60) = 1 → bucket 1 - {"5s exact 0", 5 * time.Second, 0, cmpExact, false}, - {"5s exact 1", 5 * time.Second, 1, cmpExact, true}, - {"5s gt 0", 5 * time.Second, 0, cmpMore, true}, // 5 > 0 = true - {"5s lt 1", 5 * time.Second, 1, cmpLess, true}, // 5 < 60 = true (key regression test) - - // 30 seconds old — the specific case from codex P1 - {"30s lt 1", 30 * time.Second, 1, cmpLess, true}, // 30 < 60 = true - - // 59 seconds old → ceil(59/60) = 1 → bucket 1 - {"59s exact 1", 59 * time.Second, 1, cmpExact, true}, - {"59s exact 0", 59 * time.Second, 0, cmpExact, false}, - {"59s lt 1", 59 * time.Second, 1, cmpLess, true}, // 59 < 60 = true - - // 60 seconds old → ceil(60/60) = 1 → bucket 1 - {"60s exact 1", 60 * time.Second, 1, cmpExact, true}, - {"60s exact 2", 60 * time.Second, 2, cmpExact, false}, - {"60s gt 1", 60 * time.Second, 1, cmpMore, false}, // 60 > 60 = false - {"60s lt 1", 60 * time.Second, 1, cmpLess, false}, // 60 < 60 = false - - // 61 seconds old → ceil(61/60) = 2 → bucket 2 - {"61s exact 1", 61 * time.Second, 1, cmpExact, false}, - {"61s exact 2", 61 * time.Second, 2, cmpExact, true}, - {"61s gt 1", 61 * time.Second, 1, cmpMore, true}, // 61 > 60 = true - {"61s lt 2", 61 * time.Second, 2, cmpLess, true}, // 61 < 120 = true - - // 5 minutes old → ceil(300/60) = 5 → bucket 5 - {"5m exact 5", 5 * time.Minute, 5, cmpExact, true}, - {"5m gt 4", 5 * time.Minute, 4, cmpMore, true}, // 300 > 240 = true - {"5m lt 6", 5 * time.Minute, 6, cmpLess, true}, // 300 < 360 = true - - // 5 minutes 1 second old → ceil(301/60) = 6 → bucket 6 - {"5m1s exact 6", 5*time.Minute + 1*time.Second, 6, cmpExact, true}, - {"5m1s exact 5", 5*time.Minute + 1*time.Second, 5, cmpExact, false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - modTime := now.Add(-tt.age) - ec := &evalContext{ - now: now, - info: &fakeFileInfo{modTime: modTime}, - } - got := evalMmin(ec, tt.n, tt.cmp) - assert.Equal(t, tt.matched, got, "evalMmin(age=%v, n=%d, cmp=%s)", tt.age, tt.n, tt.cmp) - }) - } -} - -// TestEvalMminOverflow verifies that evalMmin handles values exceeding -// maxMminN without integer overflow. For +N (cmpMore), overflow values -// should return false (nothing qualifies). For -N (cmpLess), overflow -// values should return true (everything qualifies). -func TestEvalMminOverflow(t *testing.T) { - now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) - // File is 1 hour old — a normal age for testing overflow thresholds. - modTime := now.Add(-1 * time.Hour) - ec := &evalContext{ - now: now, - info: &fakeFileInfo{modTime: modTime}, - } - - tests := []struct { - name string - n int64 - cmp cmpOp - matched bool - }{ - // At the overflow boundary: maxMminN is the largest safe value. - {"maxMminN +N", maxMminN, cmpMore, false}, // threshold is ~292K years; 1h file is newer - {"maxMminN -N", maxMminN, cmpLess, true}, // 1h < ~292K years - {"maxMminN exact", maxMminN, cmpExact, false}, // exact match impossible - - // Just past the boundary: these would overflow without the guard. - {"maxMminN+1 +N", maxMminN + 1, cmpMore, false}, // overflow guard → false - {"maxMminN+1 -N", maxMminN + 1, cmpLess, true}, // overflow guard → true - - // Very large values that would definitely overflow. - {"huge +N", math.MaxInt64 / 2, cmpMore, false}, - {"huge -N", math.MaxInt64 / 2, cmpLess, true}, - {"maxint64 +N", math.MaxInt64, cmpMore, false}, - {"maxint64 -N", math.MaxInt64, cmpLess, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := evalMmin(ec, tt.n, tt.cmp) - assert.Equal(t, tt.matched, got, "evalMmin(n=%d, cmp=%s)", tt.n, tt.cmp) - }) - } -} - -// TestEvalMtimeFloor verifies that -mtime uses floor rounding (NOT ceiling). -// A file 5 hours old should be in day bucket 0 (not 1). -func TestEvalMtimeFloor(t *testing.T) { - now := time.Date(2026, 1, 10, 12, 0, 0, 0, time.UTC) - - tests := []struct { - name string - age time.Duration - n int64 - cmp cmpOp - matched bool - }{ - // 0 hours → floor(0/24) = 0 - {"0h exact 0", 0, 0, cmpExact, true}, - {"0h gt 0", 0, 0, cmpMore, false}, - - // 5 hours → floor(5/24) = 0 - {"5h exact 0", 5 * time.Hour, 0, cmpExact, true}, - {"5h exact 1", 5 * time.Hour, 1, cmpExact, false}, - - // 23 hours → floor(23/24) = 0 - {"23h exact 0", 23 * time.Hour, 0, cmpExact, true}, - - // 24 hours → floor(24/24) = 1 - {"24h exact 1", 24 * time.Hour, 1, cmpExact, true}, - {"24h exact 0", 24 * time.Hour, 0, cmpExact, false}, - - // 25 hours → floor(25/24) = 1 - {"25h exact 1", 25 * time.Hour, 1, cmpExact, true}, - - // 48 hours → floor(48/24) = 2 - {"48h exact 2", 48 * time.Hour, 2, cmpExact, true}, - {"48h gt 1", 48 * time.Hour, 1, cmpMore, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - modTime := now.Add(-tt.age) - ec := &evalContext{ - now: now, - info: &fakeFileInfo{modTime: modTime}, - } - got := evalMtime(ec, tt.n, tt.cmp) - assert.Equal(t, tt.matched, got, "evalMtime(age=%v, n=%d, cmp=%s)", tt.age, tt.n, tt.cmp) - }) - } -} - // TestCompareSizeOverflow verifies overflow-safe ceiling division. func TestCompareSizeOverflow(t *testing.T) { tests := []struct { diff --git a/builtins/find/find.go b/builtins/find/find.go index e909a8a4..0b5c8b84 100644 --- a/builtins/find/find.go +++ b/builtins/find/find.go @@ -207,10 +207,6 @@ optLoop: } } - // Capture invocation time once so -mtime/-mmin predicates use a - // consistent reference across all root paths (matches GNU find). - now := callCtx.Now() - // GNU find treats a missing -newer reference as a fatal argument error // and produces no result set, so skip the walk entirely. if !failed { @@ -232,7 +228,6 @@ optLoop: followLinks: followLinks, maxDepth: maxDepth, minDepth: minDepth, - now: now, eagerNewerErrors: eagerNewerErrors, }) { failed = true @@ -263,7 +258,6 @@ type walkOptions struct { followLinks bool maxDepth int minDepth int - now time.Time eagerNewerErrors map[string]bool } @@ -275,7 +269,6 @@ func walkPath( startPath string, opts walkOptions, ) bool { - now := opts.now failed := false newerCache := map[string]time.Time{} newerErrors := map[string]bool{} @@ -362,7 +355,6 @@ func walkPath( ec := &evalContext{ callCtx: callCtx, ctx: ctx, - now: now, relPath: path, info: info, depth: depth, diff --git a/builtins/find/now_test.go b/builtins/find/now_test.go index a1bdedf0..42ea7440 100644 --- a/builtins/find/now_test.go +++ b/builtins/find/now_test.go @@ -21,11 +21,11 @@ import ( "github.com/DataDog/rshell/builtins" ) -// TestNowCalledOnce verifies that find captures the invocation timestamp -// once in run(), not per root path. GNU find evaluates -mtime/-mmin -// relative to a single invocation time, so multi-path invocations must -// use a consistent reference. -func TestNowCalledOnce(t *testing.T) { +// TestMatchMminCalledConsistently verifies that find delegates -mmin +// evaluation through CallContext.MatchMmin and that the function is +// called for each matching file (consistent invocation-scoped time +// reference is guaranteed by the runner's closure). +func TestMatchMminCalledConsistently(t *testing.T) { // Create two directories with one file each. tmp := t.TempDir() dir1 := filepath.Join(tmp, "a") @@ -35,16 +35,15 @@ func TestNowCalledOnce(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir1, "f1.txt"), []byte("x"), 0644)) require.NoError(t, os.WriteFile(filepath.Join(dir2, "f2.txt"), []byte("y"), 0644)) - var nowCalls atomic.Int32 - fixedNow := time.Now() + var matchMminCalls atomic.Int32 var stdout, stderr bytes.Buffer callCtx := &builtins.CallContext{ Stdout: &stdout, Stderr: &stderr, - Now: func() time.Time { - nowCalls.Add(1) - return fixedNow + MatchMmin: func(_ time.Time, _ int64, _ int) bool { + matchMminCalls.Add(1) + return true // all files match }, LstatFile: func(_ context.Context, path string) (fs.FileInfo, error) { return os.Lstat(filepath.Join(tmp, path)) @@ -71,8 +70,12 @@ func TestNowCalledOnce(t *testing.T) { result := run(context.Background(), callCtx, []string{"a", "b", "-mmin", "-60"}) assert.Equal(t, uint8(0), result.Code, "find should succeed") - assert.Equal(t, int32(1), nowCalls.Load(), - "Now() should be called exactly once per find invocation, not per root path") + // MatchMmin must be called for each file evaluated across both root paths. + // Timestamp consistency across roots is guaranteed by construction: the + // runner captures time.Now() once in NewCallbacks() and passes the same + // closures for the entire command invocation. + assert.GreaterOrEqual(t, matchMminCalls.Load(), int32(2), + "MatchMmin should be called at least once per file (one in each root)") assert.Contains(t, stdout.String(), "f1.txt") assert.Contains(t, stdout.String(), "f2.txt") } diff --git a/builtins/ls/ls.go b/builtins/ls/ls.go index 5163e485..ccf0e34d 100644 --- a/builtins/ls/ls.go +++ b/builtins/ls/ls.go @@ -140,8 +140,6 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{} } - now := callCtx.Now() - // Determine the effective sort mode. When both -S and -t are given, // the last one specified wins, matching GNU ls behaviour. // @@ -242,7 +240,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { if len(files) > 0 { sortEntries(files, opts, func(a pathArg) iofs.FileInfo { return a.info }, func(a pathArg) string { return a.name }) for _, f := range files { - printEntry(callCtx, f.name, f.info, opts, now) + printEntry(callCtx, f.name, f.info, opts) } } @@ -259,7 +257,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { } callCtx.Outf("%s:\n", d.name) } - if err := listDir(ctx, callCtx, d.name, opts, 0, now); err != nil { + if err := listDir(ctx, callCtx, d.name, opts, 0); err != nil { failed = true } } @@ -292,7 +290,7 @@ type pathArg struct { info iofs.FileInfo } -func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opts *options, depth int, now time.Time) error { +func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opts *options, depth int) error { if depth > maxRecursionDepth { callCtx.Errf("ls: recursion depth limit exceeded at '%s'\n", dir) return errFailed @@ -389,7 +387,7 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt if ctx.Err() != nil { break } - printEntry(callCtx, ei.name, ei.info, opts, now) + printEntry(callCtx, ei.name, ei.info, opts) } // Only warn on implicit truncation (no explicit --offset/--limit). @@ -421,7 +419,7 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt } subdir := joinPath(dir, ei.name) callCtx.Outf("\n%s:\n", subdir) - if err := listDir(ctx, callCtx, subdir, opts, depth+1, now); err != nil { + if err := listDir(ctx, callCtx, subdir, opts, depth+1); err != nil { failed = true } } @@ -443,7 +441,7 @@ func readDir(ctx context.Context, callCtx *builtins.CallContext, dir string, off return entries, false, err } -func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, opts *options, now time.Time) { +func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, opts *options) { if opts.longFmt { mode := formatMode(info) size := info.Size() @@ -456,7 +454,7 @@ func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, sizeStr = fmt.Sprintf("%d", size) } - timeStr := formatTime(modTime, now) + timeStr := formatTime(callCtx, modTime) callCtx.Outf("%s %s %s %s%s\n", mode, sizeStr, timeStr, name, indicator(info, opts)) } else { callCtx.Outf("%s%s\n", name, indicator(info, opts)) @@ -646,11 +644,10 @@ func humanSize(size int64) string { return fmt.Sprintf("%.0fP", val) } -func formatTime(t time.Time, now time.Time) string { - sixMonthsAgo := now.AddDate(0, -6, 0) - if t.Before(sixMonthsAgo) || t.After(now) { - // Old or future: show year instead of time. - return t.Format("Jan _2 2006") +func formatTime(callCtx *builtins.CallContext, t time.Time) string { + if callCtx.IsRecentEnough(t, 6) { + return t.Format("Jan _2 15:04") } - return t.Format("Jan _2 15:04") + // Old or future: show year instead of time. + return t.Format("Jan _2 2006") } diff --git a/interp/runner_exec.go b/interp/runner_exec.go index 7f220cee..c189e84e 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -13,13 +13,13 @@ import ( "os" "path/filepath" "sync" - "time" "mvdan.cc/sh/v3/expand" "mvdan.cc/sh/v3/syntax" "github.com/DataDog/rshell/allowedpaths" "github.com/DataDog/rshell/builtins" + "github.com/DataDog/rshell/restrictedtime" ) func (r *Runner) stmt(ctx context.Context, st *syntax.Stmt) { @@ -266,6 +266,7 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { } if fn, ok := builtins.Lookup(name); ok { + matchMtime, matchMmin, isRecentEnough := restrictedtime.NewCallbacks() call := &builtins.CallContext{ Stdout: r.stdout, Stderr: r.stderr, @@ -295,8 +296,10 @@ 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) }, - PortableErr: allowedpaths.PortableErrMsg, - Now: time.Now, + PortableErr: allowedpaths.PortableErrMsg, + MatchMtime: matchMtime, + MatchMmin: matchMmin, + IsRecentEnough: isRecentEnough, FileIdentity: func(path string, info fs.FileInfo) (builtins.FileID, bool) { absPath := path if !filepath.IsAbs(absPath) { diff --git a/restrictedtime/timecomp.go b/restrictedtime/timecomp.go new file mode 100644 index 00000000..27d0d2e7 --- /dev/null +++ b/restrictedtime/timecomp.go @@ -0,0 +1,111 @@ +// 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 restrictedtime provides time-comparison helpers for the shell interpreter. +// Builtins never import this package directly — they receive boolean-returning +// closures via CallContext, so they cannot obtain raw time values. +package restrictedtime + +import ( + "math" + "time" +) + +// Comparison operators matching find's cmpOp values. +const ( + CmpLess = -1 + CmpExact = 0 + CmpMore = 1 +) + +// MaxMtimeN is the largest N for which (N+1)*24*time.Hour does not overflow. +const MaxMtimeN = int64(math.MaxInt64/(int64(24*time.Hour))) - 1 + +// MaxMminN is the largest N for which time.Duration(N)*time.Minute +// does not overflow int64 nanoseconds. +const MaxMminN = int64(math.MaxInt64 / int64(time.Minute)) + +// MatchMtime checks modification time in days using GNU find semantics. +// +// Comparison modes: +// - CmpExact (N): day-bucketed — floor(delta_hours/24) == N. +// - CmpMore (+N): raw seconds — delta >= (N+1)*86400. +// - CmpLess (-N): raw seconds — delta < N*86400. +// +// now is truncated to second precision for +N/-N to match GNU find's time(). +func MatchMtime(now, modTime time.Time, n int64, cmp int) bool { + switch cmp { + case CmpMore: // +N: strictly older than (N+1) days + if n > MaxMtimeN { + return false // threshold beyond representable duration + } + diff := now.Truncate(time.Second).Sub(modTime) + return diff >= time.Duration(n+1)*24*time.Hour + case CmpLess: // -N: strictly newer than N days + if n > MaxMtimeN { + return true // threshold beyond representable duration + } + diff := now.Truncate(time.Second).Sub(modTime) + return diff < time.Duration(n)*24*time.Hour + default: // N: day-bucketed exact match + diff := now.Sub(modTime) + days := int64(math.Floor(diff.Hours() / 24)) + return days == n + } +} + +// MatchMmin checks modification time in minutes using GNU find semantics. +// +// Comparison modes: +// - CmpExact (N): ceiling-bucketed — ceil(delta_seconds/60) == N. +// - CmpMore (+N): raw seconds — delta > N*60. +// - CmpLess (-N): raw seconds — delta < N*60. +func MatchMmin(now, modTime time.Time, n int64, cmp int) bool { + diff := now.Sub(modTime) + switch cmp { + case CmpMore: // +N: strictly older than N minutes + if n > MaxMminN { + return false // threshold beyond representable duration; nothing qualifies + } + return diff > time.Duration(n)*time.Minute + case CmpLess: // -N: strictly newer than N minutes + if n > MaxMminN { + return true // threshold beyond representable duration; everything qualifies + } + return diff < time.Duration(n)*time.Minute + default: // N: ceiling-bucketed exact match + mins := int64(math.Ceil(diff.Minutes())) + return mins == n + } +} + +// IsRecentEnough reports whether modTime is within the given number of months +// before now and not in the future. Used by ls to decide between showing +// HH:MM (recent) or year (old/future) in long listing format. +func IsRecentEnough(now, modTime time.Time, months int) bool { + cutoff := now.AddDate(0, -months, 0) + return !modTime.Before(cutoff) && !modTime.After(now) +} + +// NewCallbacks returns MatchMtime, MatchMmin, and IsRecentEnough closures +// that capture a single invocation timestamp internally via time.Now(). +// This allows the caller to wire time-comparison callbacks without importing +// the time package directly. +func NewCallbacks() ( + matchMtime func(time.Time, int64, int) bool, + matchMmin func(time.Time, int64, int) bool, + isRecentEnough func(time.Time, int) bool, +) { + now := time.Now() + return func(modTime time.Time, n int64, cmp int) bool { + return MatchMtime(now, modTime, n, cmp) + }, + func(modTime time.Time, n int64, cmp int) bool { + return MatchMmin(now, modTime, n, cmp) + }, + func(modTime time.Time, months int) bool { + return IsRecentEnough(now, modTime, months) + } +} diff --git a/restrictedtime/timecomp_test.go b/restrictedtime/timecomp_test.go new file mode 100644 index 00000000..c126638b --- /dev/null +++ b/restrictedtime/timecomp_test.go @@ -0,0 +1,202 @@ +// 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 restrictedtime + +import ( + "math" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +// TestMatchMminCeiling verifies that MatchMmin uses ceiling rounding. +// GNU find rounds up fractional minutes: a file 5 seconds old is in +// minute bucket 1 (not 0). This prevents regression to math.Floor. +func TestMatchMminCeiling(t *testing.T) { + now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) + + tests := []struct { + name string + age time.Duration // how old the file is + n int64 + cmp int + matched bool + }{ + // Exact match uses ceiling bucketing: ceil(delta_sec / 60) + // +N/-N use raw second comparison: delta_sec > N*60 / delta_sec < N*60 + + // 0 seconds old → ceil(0) = 0 → bucket 0 + {"0s exact 0", 0, 0, CmpExact, true}, + {"0s gt 0", 0, 0, CmpMore, false}, // 0 > 0 = false + {"0s lt 1", 0, 1, CmpLess, true}, // 0 < 60 = true + + // 1 second old → ceil(1/60) = 1 → bucket 1 + {"1s exact 0", 1 * time.Second, 0, CmpExact, false}, + {"1s exact 1", 1 * time.Second, 1, CmpExact, true}, + {"1s gt 0", 1 * time.Second, 0, CmpMore, true}, // 1 > 0 = true + {"1s lt 1", 1 * time.Second, 1, CmpLess, true}, // 1 < 60 = true (GNU find matches) + + // 5 seconds old → ceil(5/60) = 1 → bucket 1 + {"5s exact 0", 5 * time.Second, 0, CmpExact, false}, + {"5s exact 1", 5 * time.Second, 1, CmpExact, true}, + {"5s gt 0", 5 * time.Second, 0, CmpMore, true}, // 5 > 0 = true + {"5s lt 1", 5 * time.Second, 1, CmpLess, true}, // 5 < 60 = true (key regression test) + + // 30 seconds old — the specific case from codex P1 + {"30s lt 1", 30 * time.Second, 1, CmpLess, true}, // 30 < 60 = true + + // 59 seconds old → ceil(59/60) = 1 → bucket 1 + {"59s exact 1", 59 * time.Second, 1, CmpExact, true}, + {"59s exact 0", 59 * time.Second, 0, CmpExact, false}, + {"59s lt 1", 59 * time.Second, 1, CmpLess, true}, // 59 < 60 = true + + // 60 seconds old → ceil(60/60) = 1 → bucket 1 + {"60s exact 1", 60 * time.Second, 1, CmpExact, true}, + {"60s exact 2", 60 * time.Second, 2, CmpExact, false}, + {"60s gt 1", 60 * time.Second, 1, CmpMore, false}, // 60 > 60 = false + {"60s lt 1", 60 * time.Second, 1, CmpLess, false}, // 60 < 60 = false + + // 61 seconds old → ceil(61/60) = 2 → bucket 2 + {"61s exact 1", 61 * time.Second, 1, CmpExact, false}, + {"61s exact 2", 61 * time.Second, 2, CmpExact, true}, + {"61s gt 1", 61 * time.Second, 1, CmpMore, true}, // 61 > 60 = true + {"61s lt 2", 61 * time.Second, 2, CmpLess, true}, // 61 < 120 = true + + // 5 minutes old → ceil(300/60) = 5 → bucket 5 + {"5m exact 5", 5 * time.Minute, 5, CmpExact, true}, + {"5m gt 4", 5 * time.Minute, 4, CmpMore, true}, // 300 > 240 = true + {"5m lt 6", 5 * time.Minute, 6, CmpLess, true}, // 300 < 360 = true + + // 5 minutes 1 second old → ceil(301/60) = 6 → bucket 6 + {"5m1s exact 6", 5*time.Minute + 1*time.Second, 6, CmpExact, true}, + {"5m1s exact 5", 5*time.Minute + 1*time.Second, 5, CmpExact, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modTime := now.Add(-tt.age) + got := MatchMmin(now, modTime, tt.n, tt.cmp) + assert.Equal(t, tt.matched, got, "MatchMmin(age=%v, n=%d, cmp=%d)", tt.age, tt.n, tt.cmp) + }) + } +} + +// TestMatchMminOverflow verifies that MatchMmin handles values exceeding +// MaxMminN without integer overflow. +func TestMatchMminOverflow(t *testing.T) { + now := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) + modTime := now.Add(-1 * time.Hour) + + tests := []struct { + name string + n int64 + cmp int + matched bool + }{ + {"MaxMminN +N", MaxMminN, CmpMore, false}, + {"MaxMminN -N", MaxMminN, CmpLess, true}, + {"MaxMminN exact", MaxMminN, CmpExact, false}, + {"MaxMminN+1 +N", MaxMminN + 1, CmpMore, false}, + {"MaxMminN+1 -N", MaxMminN + 1, CmpLess, true}, + {"huge +N", math.MaxInt64 / 2, CmpMore, false}, + {"huge -N", math.MaxInt64 / 2, CmpLess, true}, + {"maxint64 +N", math.MaxInt64, CmpMore, false}, + {"maxint64 -N", math.MaxInt64, CmpLess, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := MatchMmin(now, modTime, tt.n, tt.cmp) + assert.Equal(t, tt.matched, got, "MatchMmin(n=%d, cmp=%d)", tt.n, tt.cmp) + }) + } +} + +// TestMatchMtimeFloor verifies that MatchMtime uses floor rounding (NOT ceiling). +// A file 5 hours old should be in day bucket 0 (not 1). +func TestMatchMtimeFloor(t *testing.T) { + now := time.Date(2026, 1, 10, 12, 0, 0, 0, time.UTC) + + tests := []struct { + name string + age time.Duration + n int64 + cmp int + matched bool + }{ + // 0 hours → floor(0/24) = 0 + {"0h exact 0", 0, 0, CmpExact, true}, + {"0h gt 0", 0, 0, CmpMore, false}, + + // 5 hours → floor(5/24) = 0 + {"5h exact 0", 5 * time.Hour, 0, CmpExact, true}, + {"5h exact 1", 5 * time.Hour, 1, CmpExact, false}, + + // 23 hours → floor(23/24) = 0 + {"23h exact 0", 23 * time.Hour, 0, CmpExact, true}, + + // 24 hours → floor(24/24) = 1 + {"24h exact 1", 24 * time.Hour, 1, CmpExact, true}, + {"24h exact 0", 24 * time.Hour, 0, CmpExact, false}, + + // 25 hours → floor(25/24) = 1 + {"25h exact 1", 25 * time.Hour, 1, CmpExact, true}, + + // 48 hours → floor(48/24) = 2 + {"48h exact 2", 48 * time.Hour, 2, CmpExact, true}, + {"48h gt 1", 48 * time.Hour, 1, CmpMore, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + modTime := now.Add(-tt.age) + got := MatchMtime(now, modTime, tt.n, tt.cmp) + assert.Equal(t, tt.matched, got, "MatchMtime(age=%v, n=%d, cmp=%d)", tt.age, tt.n, tt.cmp) + }) + } +} + +// TestNewCallbacks verifies that NewCallbacks returns working closures +// that delegate to MatchMtime, MatchMmin, and IsRecentEnough with a +// captured invocation timestamp. +func TestNewCallbacks(t *testing.T) { + matchMtime, matchMmin, isRecentEnough := NewCallbacks() + // Use a time safely in the past (NewCallbacks captures time.Now() + // internally, so modTime must not be after the captured instant). + recent := time.Now().Add(-30 * time.Second) + // 30s ago should be in day bucket 0 + assert.True(t, matchMtime(recent, 0, CmpExact)) + // 30s ago → ceil(30/60) = 1 → minute bucket 1 + assert.True(t, matchMmin(recent, 1, CmpExact)) + // 30s ago is well within 6 months + assert.True(t, isRecentEnough(recent, 6)) +} + +// TestIsRecentEnough verifies the ls time-format decision logic. +func TestIsRecentEnough(t *testing.T) { + now := time.Date(2026, 7, 15, 12, 0, 0, 0, time.UTC) + + tests := []struct { + name string + mod time.Time + months int + want bool + }{ + {"recent file", time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC), 6, true}, + {"exactly 6 months ago", now.AddDate(0, -6, 0), 6, true}, // boundary: not before cutoff + {"old file", time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), 6, false}, + {"future file", time.Date(2027, 1, 1, 0, 0, 0, 0, time.UTC), 6, false}, + {"just now", now, 6, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IsRecentEnough(now, tt.mod, tt.months) + assert.Equal(t, tt.want, got) + }) + } +}