Skip to content

feat(uname): implement uname builtin (Linux only, proc-based)#146

Merged
matt-dz merged 20 commits intomainfrom
matt-dz/implement-uname
Mar 26, 2026
Merged

feat(uname): implement uname builtin (Linux only, proc-based)#146
matt-dz merged 20 commits intomainfrom
matt-dz/implement-uname

Conversation

@matt-dz
Copy link
Copy Markdown
Collaborator

@matt-dz matt-dz commented Mar 25, 2026

Summary

  • New uname builtin that reads system information from /proc/sys/kernel/ pseudo-files
  • Linux only — returns exit 1 with "not supported" message on other platforms
  • Proc path configurable via --proc-path / interp.ProcPath() (supports /host/proc for containers)
  • Adds ProcPath() accessor to ProcProvider

Flags (POSIX)

Flag Source Description
-s sys/kernel/ostype Kernel name (default)
-n sys/kernel/hostname Network node hostname
-r sys/kernel/osrelease Kernel release
-v sys/kernel/version Kernel version
-m sys/kernel/arch Machine hardware name
-a all of the above Print all info
-h Help

Test plan

  • 14 Go tests with fake proc tree covering every flag, combined flags, errors, custom proc path
  • go test ./builtins/tests/uname/... passes
  • go test ./allowedsymbols/... passes
  • go test ./... full suite passes
  • CI passes

🤖 Generated with Claude Code

matt-dz and others added 4 commits March 25, 2026 15:44
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 25, 2026

@codex conduct a comprehensive security and code review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6244031185

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/uname/uname.go
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 25, 2026

@codex conduct a comprehensive security and code review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69d2faa978

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/proc_provider.go Outdated
Comment thread builtins/tests/uname/uname_test.go
matt-dz and others added 2 commits March 25, 2026 16:30
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 25, 2026

@codex conduct a comprehensive security and code review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a90396b25c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/proc_provider.go Outdated
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 25, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a90396b25c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/proc_provider.go Outdated
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 25, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a6940b38c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/proc_provider.go Outdated
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 25, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

@matt-dz matt-dz closed this Mar 25, 2026
@matt-dz matt-dz reopened this Mar 25, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7bf18fac5d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/proc_provider.go Outdated
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 25, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70d0acd793

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/proc_provider.go Outdated
Comment thread builtins/proc_provider.go Outdated
Comment thread builtins/proc_provider.go
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 26, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98f63209d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/internal/procsyskernel/procsyskernel.go Outdated
Comment thread builtins/uname/uname.go Outdated
matt-dz and others added 2 commits March 26, 2026 10:06
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 26, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1a0208d06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/internal/procsyskernel/procsyskernel.go
Comment thread tests/scenarios/cmd/uname/errors/extra_operand.yaml Outdated
matt-dz and others added 2 commits March 26, 2026 10:24
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 26, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b2917e025

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/uname/uname.go Outdated
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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 26, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

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) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c02f36e7a1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/scenarios/cmd/uname/basic/help.yaml
Comment thread tests/scenarios/cmd/uname/errors/unknown_flag.yaml
Comment thread builtins/uname/uname.go Outdated
matt-dz and others added 3 commits March 26, 2026 11:24
Last 2.6 LTS (2.6.32) reached EOL February 2016.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
… 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) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Mar 26, 2026

@codex conduct a comprehensive security and code review. ignore the symlink kernel file reads in uname reads.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@matt-dz matt-dz marked this pull request as ready for review March 26, 2026 15:48
@@ -0,0 +1,11 @@
# skip: GNU uname --help includes flags we don't support (-p, -i, -o, --version)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have scenario tests for each of the flags?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Opus 4.6] Cannot add scenario tests for flag output — the scenario test framework has no platform-skip mechanism, and uname reads from /proc (Linux-only). Scenarios fail on macOS and Windows CI runners. This is the same limitation documented in AGENTS.md for ip route:

"ip route show/ip route get happy-path scenario tests cannot be added. The scenario test framework has no platform-skip mechanism."

Flag coverage is in the Go tests (builtins/tests/uname/uname_test.go) which use requireLinux(t) to skip gracefully on non-Linux. Every flag (-s, -n, -r, -v, -m, -a) plus combined flags, flag ordering, and edge cases are covered there with a fake proc tree.

@matt-dz matt-dz added this pull request to the merge queue Mar 26, 2026
Merged via the queue into main with commit c944642 Mar 26, 2026
34 of 35 checks passed
@matt-dz matt-dz deleted the matt-dz/implement-uname branch March 26, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants