From 1f2f188ae0d4d7268fa54c49f8e598b39989becf Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 15:40:32 -0400 Subject: [PATCH 01/20] feat(uname): implement uname builtin (Linux only, proc-based) Add uname command that reads system information from /proc/sys/kernel/ pseudo-files. Supports POSIX flags: -s (kernel name, default), -n (hostname), -r (release), -v (version), -m (machine), -a (all). The proc path is configurable via --proc-path / interp.ProcPath(), supporting containerized environments where /proc is mounted at /host/proc or similar. Adds ProcPath() accessor to ProcProvider so builtins can construct proc file paths from the configured base path. Co-Authored-By: Claude Opus 4.6 (1M context) --- SHELL_FEATURES.md | 1 + allowedsymbols/symbols_builtins.go | 11 ++ builtins/proc_provider.go | 5 + builtins/tests/uname/uname_test.go | 207 +++++++++++++++++++++++++++++ builtins/uname/uname.go | 163 +++++++++++++++++++++++ interp/register_builtins.go | 2 + 6 files changed, 389 insertions(+) create mode 100644 builtins/tests/uname/uname_test.go create mode 100644 builtins/uname/uname.go diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index 0f6663f3..05417fb4 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -31,6 +31,7 @@ Blocked features are rejected before execution with exit code 2. - ✅ `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 +- ✅ `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 - ❌ All other commands — return exit code 127 with `: not found` unless an ExecHandler is configured diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index dc806d86..9ca265fb 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -278,6 +278,14 @@ var builtinPerCommandSymbols = map[string][]string{ "true": { "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. }, + "uname": { + "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "io.EOF", // 🟢 sentinel error value; pure constant. + "os.Open", // 🟠 opens proc pseudo-files for reading kernel info; reads only /proc/sys/kernel/*. + "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. + "strings.Join", // 🟢 joins string slices; pure function, no I/O. + "strings.TrimRight", // 🟢 trims trailing characters; pure function, no I/O. + }, "uniq": { "bufio.NewScanner", // 🟢 line-by-line input reading (e.g. head, cat); no write or exec capability. "bufio.SplitFunc", // 🟢 type for custom scanner split functions; pure type, no I/O. @@ -463,9 +471,11 @@ var builtinAllowedSymbols = []string{ "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. "os.O_RDONLY", // 🟢 read-only file flag constant; cannot open files by itself. + "os.Open", // 🟠 opens a file for reading; used by uname to read /proc/sys/kernel/ pseudo-files. "os.PathError", // 🟢 error type for filesystem path errors; pure type, no I/O. "path/filepath.Dir", // 🟢 returns the directory component of a path; pure function, no I/O. "path/filepath.IsAbs", // 🟢 reports whether a path is absolute; pure function, no I/O. + "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. "path/filepath.ToSlash", // 🟢 converts OS path separators to forward slashes; pure function, no I/O. "regexp.Compile", // 🟢 compiles a regular expression; pure function, no I/O. Uses RE2 engine (linear-time, no backtracking). "regexp.QuoteMeta", // 🟢 escapes all special regex characters in a string; pure function, no I/O. @@ -495,6 +505,7 @@ var builtinAllowedSymbols = []string{ "strings.ReplaceAll", // 🟢 replaces all occurrences of a substring; pure function, no I/O. "strings.Split", // 🟢 splits a string by separator into a slice; pure function, no I/O. "strings.ToLower", // 🟢 converts string to lowercase; pure function, no I/O. + "strings.TrimRight", // 🟢 trims trailing characters from a string; pure function, no I/O. "strings.TrimSpace", // 🟢 removes leading/trailing whitespace; pure function. "syscall.ByHandleFileInformation", // 🟢 Windows file info struct for extracting nlink; read-only type, no I/O. "syscall.EACCES", // 🟢 POSIX errno constant for permission denied; pure constant, no I/O. diff --git a/builtins/proc_provider.go b/builtins/proc_provider.go index 9b832797..171cc5fd 100644 --- a/builtins/proc_provider.go +++ b/builtins/proc_provider.go @@ -26,6 +26,11 @@ func NewProcProvider(path string) *ProcProvider { return &ProcProvider{path: path} } +// ProcPath returns the configured proc filesystem path (e.g. "/proc" or "/host/proc"). +func (p *ProcProvider) ProcPath() string { + return p.path +} + // ListAll returns all running processes. func (p *ProcProvider) ListAll(ctx context.Context) ([]procinfo.ProcInfo, error) { return procinfo.ListAll(ctx, p.path) diff --git a/builtins/tests/uname/uname_test.go b/builtins/tests/uname/uname_test.go new file mode 100644 index 00000000..57d1925f --- /dev/null +++ b/builtins/tests/uname/uname_test.go @@ -0,0 +1,207 @@ +// 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 uname_test + +import ( + "bytes" + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "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" +) + +// writeFakeProc creates a fake /proc/sys/kernel/ tree in dir. +func writeFakeProc(t *testing.T, dir string, vals map[string]string) { + t.Helper() + kernelDir := filepath.Join(dir, "sys", "kernel") + require.NoError(t, os.MkdirAll(kernelDir, 0755)) + for name, val := range vals { + require.NoError(t, os.WriteFile(filepath.Join(kernelDir, name), []byte(val+"\n"), 0644)) + } +} + +// defaultFakeProc returns a standard set of fake proc values. +func defaultFakeProc() map[string]string { + return map[string]string{ + "ostype": "Linux", + "hostname": "testhost", + "osrelease": "5.15.0-test", + "version": "#1 SMP Test", + "arch": "x86_64", + } +} + +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), "") + require.NoError(t, 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...) + require.NoError(t, err) + defer runner.Close() + if dir != "" { + runner.Dir = dir + } + err = runner.Run(ctx, prog) + exitCode := 0 + if err != nil { + var es interp.ExitStatus + if errors.As(err, &es) { + exitCode = int(es) + } else if ctx.Err() == nil { + t.Fatalf("unexpected error: %v", err) + } + } + return outBuf.String(), errBuf.String(), exitCode +} + +func cmdRun(t *testing.T, script, procDir string) (stdout, stderr string, exitCode int) { + t.Helper() + return runScript(t, script, procDir, interp.ProcPath(procDir)) +} + +// --- Tests --- + +func TestUnameDefault(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux\n", stdout) +} + +func TestUnameS(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -s", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux\n", stdout) +} + +func TestUnameN(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -n", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "testhost\n", stdout) +} + +func TestUnameR(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -r", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "5.15.0-test\n", stdout) +} + +func TestUnameV(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -v", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "#1 SMP Test\n", stdout) +} + +func TestUnameM(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -m", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "x86_64\n", stdout) +} + +func TestUnameA(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -a", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux testhost 5.15.0-test #1 SMP Test x86_64\n", stdout) +} + +func TestUnameCombinedFlags(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -sn", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux testhost\n", stdout) +} + +func TestUnameCombinedFlagsMR(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -mr", dir) + assert.Equal(t, 0, code) + // Output order follows POSIX: s, n, r, v, m — so -mr gives "release machine" + assert.Equal(t, "5.15.0-test x86_64\n", stdout) +} + +func TestUnameUnknownFlag(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + _, stderr, code := cmdRun(t, "uname -z", dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "uname:") +} + +func TestUnameMissingProcFile(t *testing.T) { + dir := t.TempDir() + // Don't create any proc files — all reads should fail. + _, stderr, code := cmdRun(t, "uname", dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "uname: cannot read") +} + +func TestUnameCustomProcPath(t *testing.T) { + dir := t.TempDir() + customProc := filepath.Join(dir, "host", "proc") + writeFakeProc(t, customProc, map[string]string{ + "ostype": "Linux", + "hostname": "container-host", + "osrelease": "6.1.0-custom", + "version": "#42 SMP Custom", + "arch": "aarch64", + }) + stdout, _, code := runScript(t, "uname -a", dir, interp.ProcPath(customProc)) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux container-host 6.1.0-custom #42 SMP Custom aarch64\n", stdout) +} + +func TestUnameHelp(t *testing.T) { + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname --help", dir) + assert.Equal(t, 0, code) + assert.Contains(t, stdout, "uname") +} + +func TestUnameNoProcFiles(t *testing.T) { + // Point proc path at an empty directory — no kernel files exist. + dir := t.TempDir() + emptyProc := filepath.Join(dir, "empty_proc") + require.NoError(t, os.MkdirAll(emptyProc, 0755)) + _, stderr, code := runScript(t, "uname", dir, interp.ProcPath(emptyProc)) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "uname: cannot read") +} diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go new file mode 100644 index 00000000..a739edfb --- /dev/null +++ b/builtins/uname/uname.go @@ -0,0 +1,163 @@ +// 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 uname implements the uname builtin command. +// +// uname — print system information +// +// Usage: uname [-asnrvm] [--help] +// +// Print certain system information. With no flags, same as -s. +// +// Reads system information from /proc/sys/kernel/ pseudo-files. The proc +// path is configurable via the --proc-path CLI flag or interp.ProcPath() +// API option (e.g., /host/proc for containerized environments). +// +// Linux only — returns exit code 1 with "not supported" on other platforms. +// +// Flags: +// +// -s Print the kernel name (default when no flags given) +// -n Print the network node hostname +// -r Print the kernel release +// -v Print the kernel version +// -m Print the machine hardware name +// -a Print all of the above, in the order shown +// -h, --help Display help and exit +// +// Data sources (relative to proc path): +// +// -s sys/kernel/ostype +// -n sys/kernel/hostname +// -r sys/kernel/osrelease +// -v sys/kernel/version +// -m sys/kernel/arch +// +// Exit codes: +// +// 0 Success — requested information was written. +// 1 Error — unsupported platform, missing proc file, or invalid flag. +package uname + +import ( + "context" + "io" + "os" + "path/filepath" + "strings" + + "github.com/DataDog/rshell/builtins" +) + +// Cmd is the uname builtin command descriptor. +var Cmd = builtins.Command{ + Name: "uname", + Description: "print system information", + Help: `uname: uname [-asnrvm] + Print system information. + + With no flags, print the kernel name (same as -s). + Reads from /proc/sys/kernel/ (configurable via --proc-path). + Linux only.`, + MakeFlags: makeFlags, +} + +// procKernelFiles maps each flag letter to the proc pseudo-file that +// provides the corresponding value. Order matches POSIX -a output. +var procKernelFiles = [...]struct { + flag string + file string +}{ + {"s", "sys/kernel/ostype"}, + {"n", "sys/kernel/hostname"}, + {"r", "sys/kernel/osrelease"}, + {"v", "sys/kernel/version"}, + {"m", "sys/kernel/arch"}, +} + +func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { + help := fs.BoolP("help", "h", false, "print usage and exit") + var flags [len(procKernelFiles)]*bool + for i, entry := range procKernelFiles { + flags[i] = fs.BoolP(entry.flag, entry.flag, false, "") + } + allFlag := fs.BoolP("a", "a", false, "print all information") + + return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { + if *help { + callCtx.Outf("Usage: uname [-asnrvm]\n") + callCtx.Outf("Print system information. With no flags, same as -s.\n\n") + callCtx.Outf(" -s kernel name\n") + callCtx.Outf(" -n network node hostname\n") + callCtx.Outf(" -r kernel release\n") + callCtx.Outf(" -v kernel version\n") + callCtx.Outf(" -m machine hardware name\n") + callCtx.Outf(" -a print all information\n") + callCtx.Outf(" -h, --help display this help and exit\n") + return builtins.Result{} + } + return run(ctx, callCtx, flags, allFlag) + } +} + +func run(ctx context.Context, callCtx *builtins.CallContext, flags [len(procKernelFiles)]*bool, allFlag *bool) builtins.Result { + if callCtx.Proc == nil { + callCtx.Errf("uname: not supported (no proc filesystem configured)\n") + return builtins.Result{Code: 1} + } + + procPath := callCtx.Proc.ProcPath() + + // Default: -s when no flags given. + anySet := *allFlag + if !anySet { + for _, f := range flags { + if *f { + anySet = true + break + } + } + } + if !anySet { + *flags[0] = true // -s + } + + var parts []string + for i, entry := range procKernelFiles { + if !*allFlag && !*flags[i] { + continue + } + if ctx.Err() != nil { + return builtins.Result{Code: 1} + } + val, err := readProcFile(filepath.Join(procPath, entry.file)) + if err != nil { + callCtx.Errf("uname: cannot read %s: %s\n", entry.file, err) + return builtins.Result{Code: 1} + } + parts = append(parts, val) + } + + callCtx.Outf("%s\n", strings.Join(parts, " ")) + return builtins.Result{} +} + +// readProcFile reads a single-line value from a proc pseudo-file. +// The value is trimmed of trailing whitespace/newlines. +func readProcFile(path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() + + // Proc files are tiny — cap read at 4 KiB for safety. + buf := make([]byte, 4096) + n, err := f.Read(buf) + if err != nil && err != io.EOF { + return "", err + } + return strings.TrimRight(string(buf[:n]), " \t\r\n"), nil +} diff --git a/interp/register_builtins.go b/interp/register_builtins.go index bcb766d3..d16f1b69 100644 --- a/interp/register_builtins.go +++ b/interp/register_builtins.go @@ -33,6 +33,7 @@ import ( "github.com/DataDog/rshell/builtins/testcmd" "github.com/DataDog/rshell/builtins/tr" truecmd "github.com/DataDog/rshell/builtins/true" + "github.com/DataDog/rshell/builtins/uname" "github.com/DataDog/rshell/builtins/uniq" "github.com/DataDog/rshell/builtins/wc" ) @@ -67,6 +68,7 @@ func registerBuiltins() { testcmd.BracketCmd, tr.Cmd, truecmd.Cmd, + uname.Cmd, uniq.Cmd, wc.Cmd, } { From 2c6a0c3c964c7b00a8b3fb642a2c867edf85fe13 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 15:52:52 -0400 Subject: [PATCH 02/20] refactor(uname): use ProcProvider.ReadKernelFile instead of raw os calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move proc file reading into ProcProvider.ReadKernelFile(), following the same pattern as ps which delegates to procinfo. The uname builtin now has zero direct os package usage — all proc I/O goes through the ProcProvider abstraction. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_builtins.go | 11 +-- builtins/proc_provider.go | 14 ++++ builtins/uname/uname.go | 117 +++++++++++------------------ 3 files changed, 60 insertions(+), 82 deletions(-) diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index 9ca265fb..432733c5 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -279,12 +279,8 @@ var builtinPerCommandSymbols = map[string][]string{ "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. }, "uname": { - "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. - "io.EOF", // 🟢 sentinel error value; pure constant. - "os.Open", // 🟠 opens proc pseudo-files for reading kernel info; reads only /proc/sys/kernel/*. - "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. - "strings.Join", // 🟢 joins string slices; pure function, no I/O. - "strings.TrimRight", // 🟢 trims trailing characters; pure function, no I/O. + "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "strings.Join", // 🟢 joins string slices; pure function, no I/O. }, "uniq": { "bufio.NewScanner", // 🟢 line-by-line input reading (e.g. head, cat); no write or exec capability. @@ -471,11 +467,9 @@ var builtinAllowedSymbols = []string{ "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. "os.O_RDONLY", // 🟢 read-only file flag constant; cannot open files by itself. - "os.Open", // 🟠 opens a file for reading; used by uname to read /proc/sys/kernel/ pseudo-files. "os.PathError", // 🟢 error type for filesystem path errors; pure type, no I/O. "path/filepath.Dir", // 🟢 returns the directory component of a path; pure function, no I/O. "path/filepath.IsAbs", // 🟢 reports whether a path is absolute; pure function, no I/O. - "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. "path/filepath.ToSlash", // 🟢 converts OS path separators to forward slashes; pure function, no I/O. "regexp.Compile", // 🟢 compiles a regular expression; pure function, no I/O. Uses RE2 engine (linear-time, no backtracking). "regexp.QuoteMeta", // 🟢 escapes all special regex characters in a string; pure function, no I/O. @@ -505,7 +499,6 @@ var builtinAllowedSymbols = []string{ "strings.ReplaceAll", // 🟢 replaces all occurrences of a substring; pure function, no I/O. "strings.Split", // 🟢 splits a string by separator into a slice; pure function, no I/O. "strings.ToLower", // 🟢 converts string to lowercase; pure function, no I/O. - "strings.TrimRight", // 🟢 trims trailing characters from a string; pure function, no I/O. "strings.TrimSpace", // 🟢 removes leading/trailing whitespace; pure function. "syscall.ByHandleFileInformation", // 🟢 Windows file info struct for extracting nlink; read-only type, no I/O. "syscall.EACCES", // 🟢 POSIX errno constant for permission denied; pure constant, no I/O. diff --git a/builtins/proc_provider.go b/builtins/proc_provider.go index 171cc5fd..cc3452f7 100644 --- a/builtins/proc_provider.go +++ b/builtins/proc_provider.go @@ -7,6 +7,9 @@ package builtins import ( "context" + "os" + "path/filepath" + "strings" "github.com/DataDog/rshell/builtins/internal/procinfo" ) @@ -45,3 +48,14 @@ func (p *ProcProvider) GetSession(ctx context.Context) ([]procinfo.ProcInfo, err func (p *ProcProvider) GetByPIDs(ctx context.Context, pids []int) ([]procinfo.ProcInfo, error) { return procinfo.GetByPIDs(ctx, p.path, pids) } + +// ReadKernelFile reads a single-line value from a /proc/sys/kernel/ pseudo-file. +// name is the filename relative to sys/kernel/ (e.g. "ostype", "hostname"). +// The returned value is trimmed of trailing whitespace. +func (p *ProcProvider) ReadKernelFile(name string) (string, error) { + data, err := os.ReadFile(filepath.Join(p.path, "sys", "kernel", name)) + if err != nil { + return "", err + } + return strings.TrimRight(string(data), " \t\r\n"), nil +} diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index a739edfb..9a18e716 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -11,11 +11,9 @@ // // Print certain system information. With no flags, same as -s. // -// Reads system information from /proc/sys/kernel/ pseudo-files. The proc -// path is configurable via the --proc-path CLI flag or interp.ProcPath() -// API option (e.g., /host/proc for containerized environments). -// -// Linux only — returns exit code 1 with "not supported" on other platforms. +// Reads system information from /proc/sys/kernel/ pseudo-files via the +// ProcProvider. The proc path is configurable via the --proc-path CLI +// flag or interp.ProcPath() API option (e.g., /host/proc for containers). // // Flags: // @@ -43,9 +41,6 @@ package uname import ( "context" - "io" - "os" - "path/filepath" "strings" "github.com/DataDog/rshell/builtins" @@ -59,28 +54,27 @@ var Cmd = builtins.Command{ Print system information. With no flags, print the kernel name (same as -s). - Reads from /proc/sys/kernel/ (configurable via --proc-path). - Linux only.`, + Reads from /proc/sys/kernel/ (configurable via --proc-path).`, MakeFlags: makeFlags, } -// procKernelFiles maps each flag letter to the proc pseudo-file that +// kernelFiles maps each flag letter to the proc pseudo-file that // provides the corresponding value. Order matches POSIX -a output. -var procKernelFiles = [...]struct { +var kernelFiles = [...]struct { flag string file string }{ - {"s", "sys/kernel/ostype"}, - {"n", "sys/kernel/hostname"}, - {"r", "sys/kernel/osrelease"}, - {"v", "sys/kernel/version"}, - {"m", "sys/kernel/arch"}, + {"s", "ostype"}, + {"n", "hostname"}, + {"r", "osrelease"}, + {"v", "version"}, + {"m", "arch"}, } func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { help := fs.BoolP("help", "h", false, "print usage and exit") - var flags [len(procKernelFiles)]*bool - for i, entry := range procKernelFiles { + var flags [len(kernelFiles)]*bool + for i, entry := range kernelFiles { flags[i] = fs.BoolP(entry.flag, entry.flag, false, "") } allFlag := fs.BoolP("a", "a", false, "print all information") @@ -98,66 +92,43 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { callCtx.Outf(" -h, --help display this help and exit\n") return builtins.Result{} } - return run(ctx, callCtx, flags, allFlag) - } -} - -func run(ctx context.Context, callCtx *builtins.CallContext, flags [len(procKernelFiles)]*bool, allFlag *bool) builtins.Result { - if callCtx.Proc == nil { - callCtx.Errf("uname: not supported (no proc filesystem configured)\n") - return builtins.Result{Code: 1} - } - - procPath := callCtx.Proc.ProcPath() - // Default: -s when no flags given. - anySet := *allFlag - if !anySet { - for _, f := range flags { - if *f { - anySet = true - break - } + if callCtx.Proc == nil { + callCtx.Errf("uname: not supported (no proc filesystem configured)\n") + return builtins.Result{Code: 1} } - } - if !anySet { - *flags[0] = true // -s - } - var parts []string - for i, entry := range procKernelFiles { - if !*allFlag && !*flags[i] { - continue - } - if ctx.Err() != nil { - return builtins.Result{Code: 1} + // Default: -s when no flags given. + anySet := *allFlag + if !anySet { + for _, f := range flags { + if *f { + anySet = true + break + } + } } - val, err := readProcFile(filepath.Join(procPath, entry.file)) - if err != nil { - callCtx.Errf("uname: cannot read %s: %s\n", entry.file, err) - return builtins.Result{Code: 1} + if !anySet { + *flags[0] = true // -s } - parts = append(parts, val) - } - - callCtx.Outf("%s\n", strings.Join(parts, " ")) - return builtins.Result{} -} -// readProcFile reads a single-line value from a proc pseudo-file. -// The value is trimmed of trailing whitespace/newlines. -func readProcFile(path string) (string, error) { - f, err := os.Open(path) - if err != nil { - return "", err - } - defer f.Close() + var parts []string + for i, entry := range kernelFiles { + if !*allFlag && !*flags[i] { + continue + } + if ctx.Err() != nil { + return builtins.Result{Code: 1} + } + val, err := callCtx.Proc.ReadKernelFile(entry.file) + if err != nil { + callCtx.Errf("uname: cannot read %s: %s\n", entry.file, err) + return builtins.Result{Code: 1} + } + parts = append(parts, val) + } - // Proc files are tiny — cap read at 4 KiB for safety. - buf := make([]byte, 4096) - n, err := f.Read(buf) - if err != nil && err != io.EOF { - return "", err + callCtx.Outf("%s\n", strings.Join(parts, " ")) + return builtins.Result{} } - return strings.TrimRight(string(buf[:n]), " \t\r\n"), nil } From 16d10264d685df31c273bca9e1dd17a3842863cd Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 16:00:06 -0400 Subject: [PATCH 03/20] fix(uname): show clear platform error on non-Linux systems Previously showed a confusing "cannot read ostype: no such file" error on macOS/Windows. Now checks runtime.GOOS first and prints "uname: not supported on darwin (Linux only)". Tests skip proc-dependent cases on non-Linux, with a dedicated TestUnameNonLinuxPlatform test verifying the error message. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_builtins.go | 1 + builtins/tests/uname/uname_test.go | 31 ++++++++++++++++++++++++++++++ builtins/uname/uname.go | 6 ++++++ 3 files changed, 38 insertions(+) diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index 432733c5..707125db 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -280,6 +280,7 @@ var builtinPerCommandSymbols = map[string][]string{ }, "uname": { "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. "strings.Join", // 🟢 joins string slices; pure function, no I/O. }, "uniq": { diff --git a/builtins/tests/uname/uname_test.go b/builtins/tests/uname/uname_test.go index 57d1925f..f77dadcc 100644 --- a/builtins/tests/uname/uname_test.go +++ b/builtins/tests/uname/uname_test.go @@ -11,6 +11,7 @@ import ( "errors" "os" "path/filepath" + "runtime" "strings" "testing" @@ -82,9 +83,17 @@ func cmdRun(t *testing.T, script, procDir string) (stdout, stderr string, exitCo return runScript(t, script, procDir, interp.ProcPath(procDir)) } +func requireLinux(t *testing.T) { + t.Helper() + if runtime.GOOS != "linux" { + t.Skip("uname reads from /proc; skipping on " + runtime.GOOS) + } +} + // --- Tests --- func TestUnameDefault(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname", dir) @@ -93,6 +102,7 @@ func TestUnameDefault(t *testing.T) { } func TestUnameS(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -s", dir) @@ -101,6 +111,7 @@ func TestUnameS(t *testing.T) { } func TestUnameN(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -n", dir) @@ -109,6 +120,7 @@ func TestUnameN(t *testing.T) { } func TestUnameR(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -r", dir) @@ -117,6 +129,7 @@ func TestUnameR(t *testing.T) { } func TestUnameV(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -v", dir) @@ -125,6 +138,7 @@ func TestUnameV(t *testing.T) { } func TestUnameM(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -m", dir) @@ -133,6 +147,7 @@ func TestUnameM(t *testing.T) { } func TestUnameA(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -a", dir) @@ -141,6 +156,7 @@ func TestUnameA(t *testing.T) { } func TestUnameCombinedFlags(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -sn", dir) @@ -149,6 +165,7 @@ func TestUnameCombinedFlags(t *testing.T) { } func TestUnameCombinedFlagsMR(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) stdout, _, code := cmdRun(t, "uname -mr", dir) @@ -158,6 +175,7 @@ func TestUnameCombinedFlagsMR(t *testing.T) { } func TestUnameUnknownFlag(t *testing.T) { + requireLinux(t) dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) _, stderr, code := cmdRun(t, "uname -z", dir) @@ -166,6 +184,7 @@ func TestUnameUnknownFlag(t *testing.T) { } func TestUnameMissingProcFile(t *testing.T) { + requireLinux(t) dir := t.TempDir() // Don't create any proc files — all reads should fail. _, stderr, code := cmdRun(t, "uname", dir) @@ -174,6 +193,7 @@ func TestUnameMissingProcFile(t *testing.T) { } func TestUnameCustomProcPath(t *testing.T) { + requireLinux(t) dir := t.TempDir() customProc := filepath.Join(dir, "host", "proc") writeFakeProc(t, customProc, map[string]string{ @@ -197,6 +217,7 @@ func TestUnameHelp(t *testing.T) { } func TestUnameNoProcFiles(t *testing.T) { + requireLinux(t) // Point proc path at an empty directory — no kernel files exist. dir := t.TempDir() emptyProc := filepath.Join(dir, "empty_proc") @@ -205,3 +226,13 @@ func TestUnameNoProcFiles(t *testing.T) { assert.Equal(t, 1, code) assert.Contains(t, stderr, "uname: cannot read") } + +func TestUnameNonLinuxPlatform(t *testing.T) { + if runtime.GOOS == "linux" { + t.Skip("this test verifies non-Linux behavior") + } + dir := t.TempDir() + _, stderr, code := cmdRun(t, "uname", dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "not supported") +} diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index 9a18e716..3a6f346c 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -41,6 +41,7 @@ package uname import ( "context" + "runtime" "strings" "github.com/DataDog/rshell/builtins" @@ -93,6 +94,11 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{} } + if runtime.GOOS != "linux" { + callCtx.Errf("uname: not supported on %s (Linux only)\n", runtime.GOOS) + return builtins.Result{Code: 1} + } + if callCtx.Proc == nil { callCtx.Errf("uname: not supported (no proc filesystem configured)\n") return builtins.Result{Code: 1} From 6244031185139f8bcea399aab9596d00b71d56a7 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 16:07:35 -0400 Subject: [PATCH 04/20] test(uname): add comprehensive unit and scenario tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go tests (Linux-only via requireLinux): - Duplicate flags (-ss), all flags explicit (-snrvm), flag order independence (-mrvns), -a overrides individual flags - Partial proc tree (success and failure paths) - Whitespace trimming in proc values, empty proc file - Pipe integration, variable capture via command substitution - Context cancellation All-platform tests: - Help via -h and --help, stderr empty on help - Extra positional arguments ignored (POSIX behavior) YAML scenarios: - errors/unknown_flag — rejects unknown flags - basic/help — --help prints usage to stdout Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/tests/uname/uname_test.go | 140 ++++++++++++++++++ tests/scenarios/cmd/uname/basic/help.yaml | 9 ++ .../cmd/uname/errors/unknown_flag.yaml | 9 ++ 3 files changed, 158 insertions(+) create mode 100644 tests/scenarios/cmd/uname/basic/help.yaml create mode 100644 tests/scenarios/cmd/uname/errors/unknown_flag.yaml diff --git a/builtins/tests/uname/uname_test.go b/builtins/tests/uname/uname_test.go index f77dadcc..da01c2a5 100644 --- a/builtins/tests/uname/uname_test.go +++ b/builtins/tests/uname/uname_test.go @@ -236,3 +236,143 @@ func TestUnameNonLinuxPlatform(t *testing.T) { assert.Equal(t, 1, code) assert.Contains(t, stderr, "not supported") } + +func TestUnameDuplicateFlags(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + // -ss should print kernel name once, not twice. + stdout, _, code := cmdRun(t, "uname -ss", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux\n", stdout) +} + +func TestUnameAllFlagsExplicit(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + // -snrvm should produce the same output as -a. + stdout, _, code := cmdRun(t, "uname -snrvm", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux testhost 5.15.0-test #1 SMP Test x86_64\n", stdout) +} + +func TestUnameFlagOrderDoesntMatter(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + // -mrvns (reverse order) should still output in POSIX order: s,n,r,v,m. + stdout, _, code := cmdRun(t, "uname -mrvns", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux testhost 5.15.0-test #1 SMP Test x86_64\n", stdout) +} + +func TestUnameAllOverridesIndividual(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + // -as should produce the same output as -a. + stdout, _, code := cmdRun(t, "uname -as", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux testhost 5.15.0-test #1 SMP Test x86_64\n", stdout) +} + +func TestUnamePartialProcTreeSuccess(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + // Only create ostype — requesting -s should succeed. + writeFakeProc(t, dir, map[string]string{"ostype": "Linux"}) + stdout, _, code := cmdRun(t, "uname -s", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux\n", stdout) +} + +func TestUnamePartialProcTreeFailure(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + // Only create ostype — requesting -n should fail (hostname missing). + writeFakeProc(t, dir, map[string]string{"ostype": "Linux"}) + _, stderr, code := cmdRun(t, "uname -n", dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "uname: cannot read hostname") +} + +func TestUnameWhitespaceInProcValues(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, map[string]string{ + "ostype": "Linux", + "hostname": "myhost \t", + "osrelease": "5.15.0", + "version": "#1 SMP", + "arch": "x86_64", + }) + stdout, _, code := cmdRun(t, "uname -n", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "myhost\n", stdout, "trailing whitespace should be trimmed") +} + +func TestUnameEmptyProcFile(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + // ostype exists but writeFakeProc adds "\n" — write truly empty file. + kernelDir := filepath.Join(dir, "sys", "kernel") + require.NoError(t, os.MkdirAll(kernelDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(kernelDir, "ostype"), []byte(""), 0644)) + stdout, _, code := cmdRun(t, "uname -s", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "\n", stdout, "empty proc file should produce empty field") +} + +func TestUnamePipeIntegration(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, "uname -s | cat", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux\n", stdout) +} + +func TestUnameVariableCapture(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + stdout, _, code := cmdRun(t, `x=$(uname -s); echo "$x"`, dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux\n", stdout) +} + +func TestUnameContextCancellation(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately. + _, _, code := runScriptCtx(ctx, t, "uname -a", dir, interp.ProcPath(dir)) + assert.NotEqual(t, 0, code, "cancelled context should result in non-zero exit") +} + +func TestUnameHelpShortFlag(t *testing.T) { + dir := t.TempDir() + stdout, stderr, code := cmdRun(t, "uname -h", dir) + assert.Equal(t, 0, code) + assert.Contains(t, stdout, "uname") + assert.Empty(t, stderr, "help should not write to stderr") +} + +func TestUnameHelpStderrEmpty(t *testing.T) { + dir := t.TempDir() + _, stderr, code := cmdRun(t, "uname --help", dir) + assert.Equal(t, 0, code) + assert.Empty(t, stderr) +} + +func TestUnameExtraArgsIgnored(t *testing.T) { + requireLinux(t) + dir := t.TempDir() + writeFakeProc(t, dir, defaultFakeProc()) + // POSIX uname ignores extra arguments. + stdout, _, code := cmdRun(t, "uname foo bar", dir) + assert.Equal(t, 0, code) + assert.Equal(t, "Linux\n", stdout) +} diff --git a/tests/scenarios/cmd/uname/basic/help.yaml b/tests/scenarios/cmd/uname/basic/help.yaml new file mode 100644 index 00000000..a83d5b2f --- /dev/null +++ b/tests/scenarios/cmd/uname/basic/help.yaml @@ -0,0 +1,9 @@ +description: uname --help prints usage to stdout +skip_assert_against_bash: true +input: + script: |+ + uname --help +expect: + stdout_contains: ["uname", "kernel"] + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/uname/errors/unknown_flag.yaml b/tests/scenarios/cmd/uname/errors/unknown_flag.yaml new file mode 100644 index 00000000..d6235b3f --- /dev/null +++ b/tests/scenarios/cmd/uname/errors/unknown_flag.yaml @@ -0,0 +1,9 @@ +description: uname rejects unknown flags +skip_assert_against_bash: true +input: + script: |+ + uname -z +expect: + stdout: "" + stderr_contains: ["uname:"] + exit_code: 1 From 69d2faa978ea71c2d69db6c07faa89f1dd7035e8 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 16:20:01 -0400 Subject: [PATCH 05/20] fix(uname): reject extra positional operands GNU uname rejects extra operands with exit 1: uname: extra operand 'foo' Our implementation was silently ignoring them. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/tests/uname/uname_test.go | 11 +++++------ builtins/uname/uname.go | 5 +++++ tests/scenarios/cmd/uname/errors/extra_operand.yaml | 9 +++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 tests/scenarios/cmd/uname/errors/extra_operand.yaml diff --git a/builtins/tests/uname/uname_test.go b/builtins/tests/uname/uname_test.go index da01c2a5..5a84f98d 100644 --- a/builtins/tests/uname/uname_test.go +++ b/builtins/tests/uname/uname_test.go @@ -367,12 +367,11 @@ func TestUnameHelpStderrEmpty(t *testing.T) { assert.Empty(t, stderr) } -func TestUnameExtraArgsIgnored(t *testing.T) { - requireLinux(t) +func TestUnameExtraOperandRejected(t *testing.T) { dir := t.TempDir() writeFakeProc(t, dir, defaultFakeProc()) - // POSIX uname ignores extra arguments. - stdout, _, code := cmdRun(t, "uname foo bar", dir) - assert.Equal(t, 0, code) - assert.Equal(t, "Linux\n", stdout) + // GNU uname rejects extra operands. + _, stderr, code := cmdRun(t, "uname foo", dir) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "uname: extra operand") } diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index 3a6f346c..8a0457f0 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -94,6 +94,11 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{} } + if len(args) > 0 { + callCtx.Errf("uname: extra operand '%s'\n", args[0]) + return builtins.Result{Code: 1} + } + if runtime.GOOS != "linux" { callCtx.Errf("uname: not supported on %s (Linux only)\n", runtime.GOOS) return builtins.Result{Code: 1} diff --git a/tests/scenarios/cmd/uname/errors/extra_operand.yaml b/tests/scenarios/cmd/uname/errors/extra_operand.yaml new file mode 100644 index 00000000..3230ac83 --- /dev/null +++ b/tests/scenarios/cmd/uname/errors/extra_operand.yaml @@ -0,0 +1,9 @@ +description: uname rejects extra positional operands +skip_assert_against_bash: true +input: + script: |+ + uname foo +expect: + stdout: "" + stderr: "uname: extra operand 'foo'\n" + exit_code: 1 From 1bd4097499416d25fe44e9725b91223a0f430be5 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 16:30:28 -0400 Subject: [PATCH 06/20] fix(proc): bound ReadKernelFile reads to 4 KiB Replace unbounded os.ReadFile with os.Open + io.LimitReader(4096). Prevents DoS if --proc-path points at a large file or FIFO instead of actual proc pseudo-files. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/proc_provider.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/builtins/proc_provider.go b/builtins/proc_provider.go index cc3452f7..e9404796 100644 --- a/builtins/proc_provider.go +++ b/builtins/proc_provider.go @@ -7,6 +7,7 @@ package builtins import ( "context" + "io" "os" "path/filepath" "strings" @@ -53,7 +54,14 @@ func (p *ProcProvider) GetByPIDs(ctx context.Context, pids []int) ([]procinfo.Pr // name is the filename relative to sys/kernel/ (e.g. "ostype", "hostname"). // The returned value is trimmed of trailing whitespace. func (p *ProcProvider) ReadKernelFile(name string) (string, error) { - data, err := os.ReadFile(filepath.Join(p.path, "sys", "kernel", name)) + f, err := os.Open(filepath.Join(p.path, "sys", "kernel", name)) + if err != nil { + return "", err + } + defer f.Close() + // Proc kernel files are tiny single-line values. Cap at 4 KiB to + // prevent unbounded reads if --proc-path points at a non-proc tree. + data, err := io.ReadAll(io.LimitReader(f, 4096)) if err != nil { return "", err } From a90396b25c48f74260a648a2f07c620dba2f231a Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 16:33:50 -0400 Subject: [PATCH 07/20] fix(test): map context cancellation to non-zero exit in runScriptCtx The test helper returned exit code 0 when runner.Run failed with a context cancellation error (no ExitStatus unwrapped). Now returns 1 for cancelled contexts, fixing TestUnameContextCancellation. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/tests/uname/uname_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtins/tests/uname/uname_test.go b/builtins/tests/uname/uname_test.go index 5a84f98d..0567dfc9 100644 --- a/builtins/tests/uname/uname_test.go +++ b/builtins/tests/uname/uname_test.go @@ -71,7 +71,9 @@ func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts .. var es interp.ExitStatus if errors.As(err, &es) { exitCode = int(es) - } else if ctx.Err() == nil { + } else if ctx.Err() != nil { + exitCode = 1 // Context cancelled/timed out. + } else { t.Fatalf("unexpected error: %v", err) } } From 4a6940b38c711f809249e4bb84c2b4f857bbc83f Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 17:06:40 -0400 Subject: [PATCH 08/20] fix(proc): reject non-regular files in ReadKernelFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check file type via fstat after open — reject FIFOs, sockets, etc. that could block indefinitely. Allow regular files and char devices (proc pseudo-files may appear as char devices on some configs). Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/proc_provider.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/builtins/proc_provider.go b/builtins/proc_provider.go index e9404796..eba25a0c 100644 --- a/builtins/proc_provider.go +++ b/builtins/proc_provider.go @@ -7,6 +7,7 @@ package builtins import ( "context" + "fmt" "io" "os" "path/filepath" @@ -54,11 +55,23 @@ func (p *ProcProvider) GetByPIDs(ctx context.Context, pids []int) ([]procinfo.Pr // name is the filename relative to sys/kernel/ (e.g. "ostype", "hostname"). // The returned value is trimmed of trailing whitespace. func (p *ProcProvider) ReadKernelFile(name string) (string, error) { - f, err := os.Open(filepath.Join(p.path, "sys", "kernel", name)) + path := filepath.Join(p.path, "sys", "kernel", name) + f, err := os.Open(path) if err != nil { return "", err } defer f.Close() + // Reject non-regular files (FIFOs, devices, etc.) to prevent blocking + // reads when --proc-path points at a non-proc tree. + info, err := f.Stat() + if err != nil { + return "", err + } + if !info.Mode().IsRegular() && info.Mode().Type()&os.ModeCharDevice == 0 { + // Allow regular files and char devices (proc pseudo-files appear as + // char devices on some configurations). Reject FIFOs, sockets, etc. + return "", fmt.Errorf("not a regular file: %s", path) + } // Proc kernel files are tiny single-line values. Cap at 4 KiB to // prevent unbounded reads if --proc-path points at a non-proc tree. data, err := io.ReadAll(io.LimitReader(f, 4096)) From 7bf18fac5d2e29265e011b5d14e3880c374ac46b Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 17:18:02 -0400 Subject: [PATCH 09/20] fix(proc): stat before open in ReadKernelFile to prevent FIFO hang os.Open on a FIFO blocks in open(2) before reaching the fstat type check, causing uname to hang when --proc-path points at a tree with mkfifo'd entries. Move the type check to os.Stat before os.Open so FIFOs are rejected without blocking. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/proc_provider.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/builtins/proc_provider.go b/builtins/proc_provider.go index eba25a0c..a02a6cd6 100644 --- a/builtins/proc_provider.go +++ b/builtins/proc_provider.go @@ -56,14 +56,10 @@ func (p *ProcProvider) GetByPIDs(ctx context.Context, pids []int) ([]procinfo.Pr // The returned value is trimmed of trailing whitespace. func (p *ProcProvider) ReadKernelFile(name string) (string, error) { path := filepath.Join(p.path, "sys", "kernel", name) - f, err := os.Open(path) - if err != nil { - return "", err - } - defer f.Close() - // Reject non-regular files (FIFOs, devices, etc.) to prevent blocking - // reads when --proc-path points at a non-proc tree. - info, err := f.Stat() + // Stat before opening to reject FIFOs and other blocking file types + // without hanging in open(2). This prevents DoS when --proc-path + // points at a non-proc tree with mkfifo'd entries. + info, err := os.Stat(path) if err != nil { return "", err } @@ -72,6 +68,11 @@ func (p *ProcProvider) ReadKernelFile(name string) (string, error) { // char devices on some configurations). Reject FIFOs, sockets, etc. return "", fmt.Errorf("not a regular file: %s", path) } + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() // Proc kernel files are tiny single-line values. Cap at 4 KiB to // prevent unbounded reads if --proc-path points at a non-proc tree. data, err := io.ReadAll(io.LimitReader(f, 4096)) From 70d0acd7933abe737933e5625d61f58152523387 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 25 Mar 2026 17:33:35 -0400 Subject: [PATCH 10/20] fix(proc): atomic open+fstat in ReadKernelFile to close TOCTOU race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous stat-then-open approach had a TOCTOU gap where an attacker could swap a regular file for a FIFO between the stat and open calls. Now opens with O_NONBLOCK (prevents blocking on FIFOs), then validates file type via fstat on the opened fd — atomic check. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/proc_provider.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/builtins/proc_provider.go b/builtins/proc_provider.go index a02a6cd6..45b28e80 100644 --- a/builtins/proc_provider.go +++ b/builtins/proc_provider.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "github.com/DataDog/rshell/builtins/internal/procinfo" ) @@ -56,10 +57,15 @@ func (p *ProcProvider) GetByPIDs(ctx context.Context, pids []int) ([]procinfo.Pr // The returned value is trimmed of trailing whitespace. func (p *ProcProvider) ReadKernelFile(name string) (string, error) { path := filepath.Join(p.path, "sys", "kernel", name) - // Stat before opening to reject FIFOs and other blocking file types - // without hanging in open(2). This prevents DoS when --proc-path - // points at a non-proc tree with mkfifo'd entries. - info, err := os.Stat(path) + // Open with O_NONBLOCK to prevent blocking on FIFOs, then validate + // the file type via fstat on the opened fd. This is atomic — no + // TOCTOU gap between type check and open. + f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) + if err != nil { + return "", err + } + defer f.Close() + info, err := f.Stat() if err != nil { return "", err } @@ -68,11 +74,6 @@ func (p *ProcProvider) ReadKernelFile(name string) (string, error) { // char devices on some configurations). Reject FIFOs, sockets, etc. return "", fmt.Errorf("not a regular file: %s", path) } - f, err := os.Open(path) - if err != nil { - return "", err - } - defer f.Close() // Proc kernel files are tiny single-line values. Cap at 4 KiB to // prevent unbounded reads if --proc-path points at a non-proc tree. data, err := io.ReadAll(io.LimitReader(f, 4096)) From 98f63209d8b0248b3b2b63a872f3d68a8d95efd3 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 09:11:57 -0400 Subject: [PATCH 11/20] refactor(uname): move kernel reads to builtins/internal/procsyskernel Per AlexandreYang's review: move ReadKernelFile logic into a new builtins/internal/procsyskernel package for consistency with the existing procinfo, procnetsocket, and procnetroute internal packages. ProcProvider.ReadKernelFile now delegates to procsyskernel.ReadFile. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_internal.go | 17 +++++ .../internal/procsyskernel/procsyskernel.go | 62 +++++++++++++++++++ builtins/proc_provider.go | 33 +--------- 3 files changed, 81 insertions(+), 31 deletions(-) create mode 100644 builtins/internal/procsyskernel/procsyskernel.go diff --git a/allowedsymbols/symbols_internal.go b/allowedsymbols/symbols_internal.go index ccb6fde0..6a9a8e0a 100644 --- a/allowedsymbols/symbols_internal.go +++ b/allowedsymbols/symbols_internal.go @@ -56,6 +56,17 @@ var internalPerPackageSymbols = map[string][]string{ "procpath": { // No stdlib symbols needed — this package only defines a string constant. }, + "procsyskernel": { + "fmt.Errorf", // 🟢 error formatting; pure function, no I/O. + "io.ReadAll", // 🟠 reads all data from a reader; bounded by io.LimitReader below. + "io.LimitReader", // 🟢 wraps a reader with a byte cap; pure wrapper, no I/O by itself. + "os.ModeCharDevice", // 🟢 file mode constant; pure constant. + "os.O_RDONLY", // 🟢 read-only file flag; pure constant. + "os.OpenFile", // 🟠 opens kernel pseudo-files for reading; bypasses AllowedPaths by design. + "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. + "strings.TrimRight", // 🟢 trims trailing characters; pure function, no I/O. + "syscall.O_NONBLOCK", // 🟢 non-blocking open flag; prevents FIFO hang. Pure constant. + }, "procnetroute": { "bufio.NewScanner", // 🟢 line-by-line reading of /proc/net/route; no write capability. "github.com/DataDog/rshell/builtins/internal/procpath.Default", // 🟢 canonical /proc filesystem root path constant; pure constant, no I/O. @@ -128,8 +139,13 @@ var internalAllowedSymbols = []string{ "fmt.Errorf", // 🟢 error formatting; pure function, no I/O. "os.ErrNotExist", // 🟢 procinfo: sentinel error value indicating a file or directory does not exist; read-only constant, no I/O. "fmt.Sprintf", // 🟢 string formatting; pure function, no I/O. + "io.LimitReader", // 🟢 procsyskernel: wraps a reader with a byte cap; pure wrapper, no I/O by itself. + "io.ReadAll", // 🟠 procsyskernel: reads all data from a bounded reader; used with LimitReader for 4KiB cap. "os.Getpid", // 🟠 procinfo: returns the current process ID; read-only, no side effects. + "os.ModeCharDevice", // 🟢 procsyskernel: file mode constant for char device detection; pure constant. + "os.O_RDONLY", // 🟢 procsyskernel: read-only open flag; pure constant. "os.Open", // 🟠 procinfo: opens a file read-only; needed to stream /proc/stat line-by-line. + "os.OpenFile", // 🟠 procsyskernel: opens kernel pseudo-files with O_NONBLOCK; bypasses AllowedPaths by design. "os.ReadDir", // 🟠 procinfo: reads a directory listing; needed to enumerate /proc entries. "os.ReadFile", // 🟠 procinfo: reads a whole file; needed to read /proc/[pid]/{stat,cmdline,status}. "os.Stat", // 🟠 procinfo: validates that the proc path exists before enumeration; read-only metadata, no write capability. @@ -153,6 +169,7 @@ var internalAllowedSymbols = []string{ "strings.TrimSpace", // 🟢 procinfo: removes leading/trailing whitespace; pure function, no I/O. "syscall.Errno", // 🟢 winnet: wraps DLL return code as an error type; pure type, no I/O. "syscall.Getsid", // 🟠 procinfo: returns the session ID of a process; read-only syscall, no write/exec. + "syscall.O_NONBLOCK", // 🟢 procsyskernel: non-blocking open flag to prevent FIFO hang; pure constant. "syscall.MustLoadDLL", // 🔴 winnet: loads iphlpapi.dll once at program init; read-only OS loader call. "syscall.Proc", // 🟢 winnet: DLL procedure handle type used in function signature; pure type, no I/O. "time.Now", // 🟠 procinfo: returns the current wall-clock time; read-only, no side effects. diff --git a/builtins/internal/procsyskernel/procsyskernel.go b/builtins/internal/procsyskernel/procsyskernel.go new file mode 100644 index 00000000..8c6c407b --- /dev/null +++ b/builtins/internal/procsyskernel/procsyskernel.go @@ -0,0 +1,62 @@ +// 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 procsyskernel reads Linux kernel information from /proc/sys/kernel/. +// +// This package is in builtins/internal/ and is therefore exempt from the +// builtinAllowedSymbols allowlist check. It may use OS-specific APIs freely. +// +// # Sandbox bypass +// +// ReadFile intentionally bypasses the AllowedPaths sandbox (callCtx.OpenFile) +// and calls os.OpenFile directly. This is safe because procPath is always a +// kernel-managed pseudo-filesystem root (/proc by default) that is hardcoded +// by the caller — it is never derived from user-supplied input and cannot be +// redirected by a shell script. The caller is responsible for ensuring that +// procPath remains a safe, non-user-controlled path. +package procsyskernel + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" + "syscall" +) + +// ReadFile reads a single-line value from a /proc/sys/kernel/ pseudo-file. +// name is the filename (e.g. "ostype", "hostname"). procPath is the base +// proc path (e.g. "/proc" or "/host/proc"). +// +// The file is opened with O_NONBLOCK to prevent blocking on FIFOs, then +// validated via fstat to reject non-regular files. Reads are bounded to +// 4 KiB. The returned value is trimmed of trailing whitespace. +func ReadFile(procPath, name string) (string, error) { + path := filepath.Join(procPath, "sys", "kernel", name) + // Open with O_NONBLOCK to prevent blocking on FIFOs, then validate + // the file type via fstat on the opened fd. This is atomic — no + // TOCTOU gap between type check and open. + f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) + if err != nil { + return "", err + } + defer f.Close() + info, err := f.Stat() + if err != nil { + return "", err + } + if !info.Mode().IsRegular() && info.Mode().Type()&os.ModeCharDevice == 0 { + // Allow regular files and char devices (proc pseudo-files appear as + // char devices on some configurations). Reject FIFOs, sockets, etc. + return "", fmt.Errorf("not a regular file: %s", path) + } + // Proc kernel files are tiny single-line values. Cap at 4 KiB. + data, err := io.ReadAll(io.LimitReader(f, 4096)) + if err != nil { + return "", err + } + return strings.TrimRight(string(data), " \t\r\n"), nil +} diff --git a/builtins/proc_provider.go b/builtins/proc_provider.go index 45b28e80..e80da0c3 100644 --- a/builtins/proc_provider.go +++ b/builtins/proc_provider.go @@ -7,14 +7,9 @@ package builtins import ( "context" - "fmt" - "io" - "os" - "path/filepath" - "strings" - "syscall" "github.com/DataDog/rshell/builtins/internal/procinfo" + "github.com/DataDog/rshell/builtins/internal/procsyskernel" ) // ProcProvider gives builtins controlled access to the proc filesystem. @@ -56,29 +51,5 @@ func (p *ProcProvider) GetByPIDs(ctx context.Context, pids []int) ([]procinfo.Pr // name is the filename relative to sys/kernel/ (e.g. "ostype", "hostname"). // The returned value is trimmed of trailing whitespace. func (p *ProcProvider) ReadKernelFile(name string) (string, error) { - path := filepath.Join(p.path, "sys", "kernel", name) - // Open with O_NONBLOCK to prevent blocking on FIFOs, then validate - // the file type via fstat on the opened fd. This is atomic — no - // TOCTOU gap between type check and open. - f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0) - if err != nil { - return "", err - } - defer f.Close() - info, err := f.Stat() - if err != nil { - return "", err - } - if !info.Mode().IsRegular() && info.Mode().Type()&os.ModeCharDevice == 0 { - // Allow regular files and char devices (proc pseudo-files appear as - // char devices on some configurations). Reject FIFOs, sockets, etc. - return "", fmt.Errorf("not a regular file: %s", path) - } - // Proc kernel files are tiny single-line values. Cap at 4 KiB to - // prevent unbounded reads if --proc-path points at a non-proc tree. - data, err := io.ReadAll(io.LimitReader(f, 4096)) - if err != nil { - return "", err - } - return strings.TrimRight(string(data), " \t\r\n"), nil + return procsyskernel.ReadFile(p.path, name) } From 8f3aad17b6b6a1a5b3881607361fd18b5747305e Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 10:06:45 -0400 Subject: [PATCH 12/20] fix(procsyskernel): reject ".." in procPath for defence-in-depth Add the same ".." traversal guard used by procnetroute and procnetsocket. Rejects paths like "/proc/../etc/passwd" before filepath.Clean can normalize them away. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_internal.go | 20 ++++++++++--------- .../internal/procsyskernel/procsyskernel.go | 8 +++++++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/allowedsymbols/symbols_internal.go b/allowedsymbols/symbols_internal.go index 6a9a8e0a..30470236 100644 --- a/allowedsymbols/symbols_internal.go +++ b/allowedsymbols/symbols_internal.go @@ -57,15 +57,17 @@ var internalPerPackageSymbols = map[string][]string{ // No stdlib symbols needed — this package only defines a string constant. }, "procsyskernel": { - "fmt.Errorf", // 🟢 error formatting; pure function, no I/O. - "io.ReadAll", // 🟠 reads all data from a reader; bounded by io.LimitReader below. - "io.LimitReader", // 🟢 wraps a reader with a byte cap; pure wrapper, no I/O by itself. - "os.ModeCharDevice", // 🟢 file mode constant; pure constant. - "os.O_RDONLY", // 🟢 read-only file flag; pure constant. - "os.OpenFile", // 🟠 opens kernel pseudo-files for reading; bypasses AllowedPaths by design. - "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. - "strings.TrimRight", // 🟢 trims trailing characters; pure function, no I/O. - "syscall.O_NONBLOCK", // 🟢 non-blocking open flag; prevents FIFO hang. Pure constant. + "fmt.Errorf", // 🟢 error formatting; pure function, no I/O. + "io.LimitReader", // 🟢 wraps a reader with a byte cap; pure wrapper, no I/O by itself. + "io.ReadAll", // 🟠 reads all data from a reader; bounded by io.LimitReader. + "os.ModeCharDevice", // 🟢 file mode constant; pure constant. + "os.O_RDONLY", // 🟢 read-only file flag; pure constant. + "os.OpenFile", // 🟠 opens kernel pseudo-files for reading; bypasses AllowedPaths by design. + "path/filepath.Clean", // 🟢 normalises path before use; pure function, no I/O. + "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. + "strings.Contains", // 🟢 checks for ".." traversal in procPath; pure function, no I/O. + "strings.TrimRight", // 🟢 trims trailing characters; pure function, no I/O. + "syscall.O_NONBLOCK", // 🟢 non-blocking open flag; prevents FIFO hang. Pure constant. }, "procnetroute": { "bufio.NewScanner", // 🟢 line-by-line reading of /proc/net/route; no write capability. diff --git a/builtins/internal/procsyskernel/procsyskernel.go b/builtins/internal/procsyskernel/procsyskernel.go index 8c6c407b..404d7c35 100644 --- a/builtins/internal/procsyskernel/procsyskernel.go +++ b/builtins/internal/procsyskernel/procsyskernel.go @@ -35,7 +35,13 @@ import ( // validated via fstat to reject non-regular files. Reads are bounded to // 4 KiB. The returned value is trimmed of trailing whitespace. func ReadFile(procPath, name string) (string, error) { - path := filepath.Join(procPath, "sys", "kernel", name) + // Defence-in-depth: reject ".." in the original path before Clean + // so traversal like "/proc/../etc/passwd" is caught. Matches the + // equivalent guard in procnetroute and procnetsocket. + if strings.Contains(procPath, "..") { + return "", fmt.Errorf("procsyskernel: unsafe procPath %q (must not contain \"..\" components)", procPath) + } + path := filepath.Join(filepath.Clean(procPath), "sys", "kernel", name) // Open with O_NONBLOCK to prevent blocking on FIFOs, then validate // the file type via fstat on the opened fd. This is atomic — no // TOCTOU gap between type check and open. From f1a0208d0628035a8ef1241f815c357dfe02f205 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 10:10:35 -0400 Subject: [PATCH 13/20] fix(uname): use GNU-compatible long flag names instead of one-letter BoolP("s","s",...) registered --s as a long form. GNU uname --v prints the coreutils version (not kernel version), making --v incompatible. Use descriptive long names matching GNU coreutils: --kernel-name, --nodename, --kernel-release, --kernel-version, --machine, --all Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/uname/uname.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index 8a0457f0..c0e81266 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -62,23 +62,24 @@ var Cmd = builtins.Command{ // kernelFiles maps each flag letter to the proc pseudo-file that // provides the corresponding value. Order matches POSIX -a output. var kernelFiles = [...]struct { - flag string - file string + short string + long string + file string }{ - {"s", "ostype"}, - {"n", "hostname"}, - {"r", "osrelease"}, - {"v", "version"}, - {"m", "arch"}, + {"s", "kernel-name", "ostype"}, + {"n", "nodename", "hostname"}, + {"r", "kernel-release", "osrelease"}, + {"v", "kernel-version", "version"}, + {"m", "machine", "arch"}, } func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { help := fs.BoolP("help", "h", false, "print usage and exit") var flags [len(kernelFiles)]*bool for i, entry := range kernelFiles { - flags[i] = fs.BoolP(entry.flag, entry.flag, false, "") + flags[i] = fs.BoolP(entry.long, entry.short, false, "") } - allFlag := fs.BoolP("a", "a", false, "print all information") + allFlag := fs.BoolP("all", "a", false, "print all information") return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { From 251be0ff7b4b533225768c549346c809234007a0 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 10:24:43 -0400 Subject: [PATCH 14/20] fix(procsyskernel): validate name is a plain basename ReadFile's name parameter is currently hardcoded from uname.go, but the API is exported. Reject absolute paths, "..", and path separators in name to prevent path escape via the exported ReadKernelFile API. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_internal.go | 2 ++ builtins/internal/procsyskernel/procsyskernel.go | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/allowedsymbols/symbols_internal.go b/allowedsymbols/symbols_internal.go index 30470236..9237cc24 100644 --- a/allowedsymbols/symbols_internal.go +++ b/allowedsymbols/symbols_internal.go @@ -63,6 +63,7 @@ var internalPerPackageSymbols = map[string][]string{ "os.ModeCharDevice", // 🟢 file mode constant; pure constant. "os.O_RDONLY", // 🟢 read-only file flag; pure constant. "os.OpenFile", // 🟠 opens kernel pseudo-files for reading; bypasses AllowedPaths by design. + "path/filepath.Base", // 🟢 returns the last element of a path; validates name is a plain basename. "path/filepath.Clean", // 🟢 normalises path before use; pure function, no I/O. "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. "strings.Contains", // 🟢 checks for ".." traversal in procPath; pure function, no I/O. @@ -151,6 +152,7 @@ var internalAllowedSymbols = []string{ "os.ReadDir", // 🟠 procinfo: reads a directory listing; needed to enumerate /proc entries. "os.ReadFile", // 🟠 procinfo: reads a whole file; needed to read /proc/[pid]/{stat,cmdline,status}. "os.Stat", // 🟠 procinfo: validates that the proc path exists before enumeration; read-only metadata, no write capability. + "path/filepath.Base", // 🟢 procsyskernel: returns the last element of a path; validates name is a plain basename. "path/filepath.Clean", // 🟢 procnetroute/procnetsocket: normalises procPath before ".." safety check; pure function, no I/O. "path/filepath.Join", // 🟢 procinfo: joins path elements to construct /proc//stat paths; pure function, no I/O. "strconv.Atoi", // 🟢 string-to-int conversion; pure function, no I/O. diff --git a/builtins/internal/procsyskernel/procsyskernel.go b/builtins/internal/procsyskernel/procsyskernel.go index 404d7c35..7481af52 100644 --- a/builtins/internal/procsyskernel/procsyskernel.go +++ b/builtins/internal/procsyskernel/procsyskernel.go @@ -41,6 +41,10 @@ func ReadFile(procPath, name string) (string, error) { if strings.Contains(procPath, "..") { return "", fmt.Errorf("procsyskernel: unsafe procPath %q (must not contain \"..\" components)", procPath) } + // Reject path components in name — must be a plain basename (e.g. "ostype"). + if name != filepath.Base(name) || strings.Contains(name, "..") { + return "", fmt.Errorf("procsyskernel: unsafe name %q (must be a plain filename)", name) + } path := filepath.Join(filepath.Clean(procPath), "sys", "kernel", name) // Open with O_NONBLOCK to prevent blocking on FIFOs, then validate // the file type via fstat on the opened fd. This is atomic — no From 0b2917e0254dcc56b3e264a5239b6548b564c56a Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 10:28:23 -0400 Subject: [PATCH 15/20] fix(uname): match GNU error format and enable bash comparison Add "Try 'uname --help' for more information." line to match GNU coreutils error format. Remove skip_assert_against_bash since uname is available on Ubuntu CI runners. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/uname/uname.go | 1 + tests/scenarios/cmd/uname/errors/extra_operand.yaml | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index c0e81266..a4c8d24d 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -97,6 +97,7 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { if len(args) > 0 { callCtx.Errf("uname: extra operand '%s'\n", args[0]) + callCtx.Errf("Try 'uname --help' for more information.\n") return builtins.Result{Code: 1} } diff --git a/tests/scenarios/cmd/uname/errors/extra_operand.yaml b/tests/scenarios/cmd/uname/errors/extra_operand.yaml index 3230ac83..2e9cb400 100644 --- a/tests/scenarios/cmd/uname/errors/extra_operand.yaml +++ b/tests/scenarios/cmd/uname/errors/extra_operand.yaml @@ -1,9 +1,8 @@ description: uname rejects extra positional operands -skip_assert_against_bash: true input: script: |+ uname foo expect: stdout: "" - stderr: "uname: extra operand 'foo'\n" + stderr: "uname: extra operand 'foo'\nTry 'uname --help' for more information.\n" exit_code: 1 From c02f36e7a1e67701c9b341635a51f5eaae73af9e Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 11:10:33 -0400 Subject: [PATCH 16/20] fix(uname): fall back to runtime.GOARCH for -m when proc file missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When /proc/sys/kernel/arch is unavailable (older kernels, restricted environments), fall back to runtime.GOARCH mapped to Linux kernel machine names (amd64→x86_64, arm64→aarch64, 386→i686, etc.). Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_builtins.go | 2 ++ builtins/uname/uname.go | 33 ++++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index 707125db..ed8d299d 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -280,6 +280,7 @@ var builtinPerCommandSymbols = map[string][]string{ }, "uname": { "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "runtime.GOARCH", // 🟢 current architecture constant; fallback for -m when /proc/sys/kernel/arch is unavailable. "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. "strings.Join", // 🟢 joins string slices; pure function, no I/O. }, @@ -475,6 +476,7 @@ var builtinAllowedSymbols = []string{ "regexp.Compile", // 🟢 compiles a regular expression; pure function, no I/O. Uses RE2 engine (linear-time, no backtracking). "regexp.QuoteMeta", // 🟢 escapes all special regex characters in a string; pure function, no I/O. "regexp.Regexp", // 🟢 compiled regular expression type; no I/O side effects. All matching methods are linear-time (RE2). + "runtime.GOARCH", // 🟢 current architecture constant; pure constant, no I/O. "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. "slices.Reverse", // 🟢 reverses a slice in-place; pure function, no I/O. "slices.SortFunc", // 🟢 sorts a slice with a comparison function; pure function, no I/O. diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index a4c8d24d..afdce45d 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -135,8 +135,14 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { } val, err := callCtx.Proc.ReadKernelFile(entry.file) if err != nil { - callCtx.Errf("uname: cannot read %s: %s\n", entry.file, err) - return builtins.Result{Code: 1} + // Fallback for -m (arch): use runtime.GOARCH mapped to + // kernel names when /proc/sys/kernel/arch is unavailable. + if entry.file == "arch" { + val = goarchToKernel(runtime.GOARCH) + } else { + callCtx.Errf("uname: cannot read %s: %s\n", entry.file, err) + return builtins.Result{Code: 1} + } } parts = append(parts, val) } @@ -145,3 +151,26 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{} } } + +// goarchToKernel maps Go's runtime.GOARCH to Linux kernel machine names. +// Used as a fallback when /proc/sys/kernel/arch is unavailable. +func goarchToKernel(goarch string) string { + switch goarch { + case "amd64": + return "x86_64" + case "arm64": + return "aarch64" + case "386": + return "i686" + case "arm": + return "armv7l" + case "ppc64le": + return "ppc64le" + case "s390x": + return "s390x" + case "riscv64": + return "riscv64" + default: + return goarch + } +} From bb3c3b89621e612026ba943a4832365b911f43a9 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 11:20:03 -0400 Subject: [PATCH 17/20] revert(uname): remove runtime.GOARCH fallback for -m runtime.GOARCH returns the compile-time target architecture, not the host system's architecture. With --proc-path pointing at a remote host's proc tree, the fallback would silently return wrong data. The proc file is the only correct source; if missing, fail honestly. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedsymbols/symbols_builtins.go | 2 -- builtins/uname/uname.go | 33 ++---------------------------- 2 files changed, 2 insertions(+), 33 deletions(-) diff --git a/allowedsymbols/symbols_builtins.go b/allowedsymbols/symbols_builtins.go index ed8d299d..707125db 100644 --- a/allowedsymbols/symbols_builtins.go +++ b/allowedsymbols/symbols_builtins.go @@ -280,7 +280,6 @@ var builtinPerCommandSymbols = map[string][]string{ }, "uname": { "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. - "runtime.GOARCH", // 🟢 current architecture constant; fallback for -m when /proc/sys/kernel/arch is unavailable. "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. "strings.Join", // 🟢 joins string slices; pure function, no I/O. }, @@ -476,7 +475,6 @@ var builtinAllowedSymbols = []string{ "regexp.Compile", // 🟢 compiles a regular expression; pure function, no I/O. Uses RE2 engine (linear-time, no backtracking). "regexp.QuoteMeta", // 🟢 escapes all special regex characters in a string; pure function, no I/O. "regexp.Regexp", // 🟢 compiled regular expression type; no I/O side effects. All matching methods are linear-time (RE2). - "runtime.GOARCH", // 🟢 current architecture constant; pure constant, no I/O. "runtime.GOOS", // 🟢 current OS name constant; pure constant, no I/O. "slices.Reverse", // 🟢 reverses a slice in-place; pure function, no I/O. "slices.SortFunc", // 🟢 sorts a slice with a comparison function; pure function, no I/O. diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index afdce45d..a4c8d24d 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -135,14 +135,8 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { } val, err := callCtx.Proc.ReadKernelFile(entry.file) if err != nil { - // Fallback for -m (arch): use runtime.GOARCH mapped to - // kernel names when /proc/sys/kernel/arch is unavailable. - if entry.file == "arch" { - val = goarchToKernel(runtime.GOARCH) - } else { - callCtx.Errf("uname: cannot read %s: %s\n", entry.file, err) - return builtins.Result{Code: 1} - } + callCtx.Errf("uname: cannot read %s: %s\n", entry.file, err) + return builtins.Result{Code: 1} } parts = append(parts, val) } @@ -151,26 +145,3 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return builtins.Result{} } } - -// goarchToKernel maps Go's runtime.GOARCH to Linux kernel machine names. -// Used as a fallback when /proc/sys/kernel/arch is unavailable. -func goarchToKernel(goarch string) string { - switch goarch { - case "amd64": - return "x86_64" - case "arm64": - return "aarch64" - case "386": - return "i686" - case "arm": - return "armv7l" - case "ppc64le": - return "ppc64le" - case "s390x": - return "s390x" - case "riscv64": - return "riscv64" - default: - return goarch - } -} From 3a96e3394d2c140fc1a6e7a3af2e37ff74726e0a Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 11:24:38 -0400 Subject: [PATCH 18/20] docs(uname): note /proc/sys/kernel/arch availability since Linux 2.6 Last 2.6 LTS (2.6.32) reached EOL February 2016. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/uname/uname.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index a4c8d24d..a50b3154 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -70,7 +70,7 @@ var kernelFiles = [...]struct { {"n", "nodename", "hostname"}, {"r", "kernel-release", "osrelease"}, {"v", "kernel-version", "version"}, - {"m", "machine", "arch"}, + {"m", "machine", "arch"}, // Available since Linux 2.6 (2003); last 2.6 LTS (2.6.32) EOL Feb 2016. } func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { From c3a1e11840dfd25b876a55a2d8e75006dd55b498 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 11:27:39 -0400 Subject: [PATCH 19/20] fix(uname): include long option names in --help output Help now shows both short and long forms (e.g. -s, --kernel-name) matching the GNU coreutils style. Updated help scenario assertions to verify long option names are present. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/uname/uname.go | 18 +++++++++--------- tests/scenarios/cmd/uname/basic/help.yaml | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/builtins/uname/uname.go b/builtins/uname/uname.go index a50b3154..cc06dc01 100644 --- a/builtins/uname/uname.go +++ b/builtins/uname/uname.go @@ -83,15 +83,15 @@ func makeFlags(fs *builtins.FlagSet) builtins.HandlerFunc { return func(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if *help { - callCtx.Outf("Usage: uname [-asnrvm]\n") - callCtx.Outf("Print system information. With no flags, same as -s.\n\n") - callCtx.Outf(" -s kernel name\n") - callCtx.Outf(" -n network node hostname\n") - callCtx.Outf(" -r kernel release\n") - callCtx.Outf(" -v kernel version\n") - callCtx.Outf(" -m machine hardware name\n") - callCtx.Outf(" -a print all information\n") - callCtx.Outf(" -h, --help display this help and exit\n") + callCtx.Outf("Usage: uname [OPTION]...\n") + callCtx.Outf("Print system information. With no OPTION, same as -s.\n\n") + callCtx.Outf(" -s, --kernel-name print the kernel name\n") + callCtx.Outf(" -n, --nodename print the network node hostname\n") + callCtx.Outf(" -r, --kernel-release print the kernel release\n") + callCtx.Outf(" -v, --kernel-version print the kernel version\n") + callCtx.Outf(" -m, --machine print the machine hardware name\n") + callCtx.Outf(" -a, --all print all information\n") + callCtx.Outf(" --help display this help and exit\n") return builtins.Result{} } diff --git a/tests/scenarios/cmd/uname/basic/help.yaml b/tests/scenarios/cmd/uname/basic/help.yaml index a83d5b2f..3b69158c 100644 --- a/tests/scenarios/cmd/uname/basic/help.yaml +++ b/tests/scenarios/cmd/uname/basic/help.yaml @@ -4,6 +4,6 @@ input: script: |+ uname --help expect: - stdout_contains: ["uname", "kernel"] + stdout_contains: ["uname", "kernel-name", "nodename", "kernel-release", "kernel-version", "machine"] stderr: "" exit_code: 0 From 2407658fa5b3b038f6b8654c8703a7823f9f15c8 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 26 Mar 2026 11:32:47 -0400 Subject: [PATCH 20/20] docs(test): add comments explaining skip_assert_against_bash in uname scenarios - unknown_flag: pflag error format differs from GNU coreutils - help: GNU includes flags we don't support (-p, -i, -o, --version) Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/scenarios/cmd/uname/basic/help.yaml | 2 ++ tests/scenarios/cmd/uname/errors/unknown_flag.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/scenarios/cmd/uname/basic/help.yaml b/tests/scenarios/cmd/uname/basic/help.yaml index 3b69158c..10a2cf48 100644 --- a/tests/scenarios/cmd/uname/basic/help.yaml +++ b/tests/scenarios/cmd/uname/basic/help.yaml @@ -1,3 +1,5 @@ +# skip: GNU uname --help includes flags we don't support (-p, -i, -o, --version) +# and GNU-specific footer text. Help format intentionally differs. description: uname --help prints usage to stdout skip_assert_against_bash: true input: diff --git a/tests/scenarios/cmd/uname/errors/unknown_flag.yaml b/tests/scenarios/cmd/uname/errors/unknown_flag.yaml index d6235b3f..a8436255 100644 --- a/tests/scenarios/cmd/uname/errors/unknown_flag.yaml +++ b/tests/scenarios/cmd/uname/errors/unknown_flag.yaml @@ -1,3 +1,5 @@ +# skip: pflag formats unknown-flag errors differently from GNU coreutils +# ("unknown shorthand flag" vs "invalid option"). Exit code matches (1). description: uname rejects unknown flags skip_assert_against_bash: true input: