Implement POSIX test and [ builtin commands by AlexandreYang · Pull Request #23 · DataDog/rshell

AlexandreYang · 2026-03-10T10:44:46Z

What does this PR do?

Implements the POSIX test and [ builtin commands as a new testcmd package. These commands evaluate conditional expressions and return exit code 0 (true), 1 (false), or 2 (syntax error).

Also adds StatFile and LstatFile capabilities to CallContext backed by sandboxed os.Root.Stat/os.Root.Lstat operations, enabling file metadata queries without bypassing the path sandbox.

Motivation

The test/[ commands are fundamental POSIX shell builtins needed for conditional logic (e.g., test -f file && echo exists, [ "$x" = "y" ] && ...). They are essential for AI agent scripts that need to check file existence, compare strings/integers, and make decisions based on conditions.

Testing

68+ Go unit tests covering all operators, edge cases, and error paths (testcmd_test.go, testcmd_gnu_compat_test.go)
Platform-specific tests: Unix (symlinks, pipes, executable bit) and Windows (reserved names) with build tags
17 YAML scenario tests across strings, integers, files, logical, bracket, errors, and help categories
19 exploratory pentest cases covering integer overflow/underflow, special files (/dev/null), path traversal, empty filenames, context timeouts, nested parentheses, and large argument counts
All tests pass with go test ./interp/... ./tests/...
go vet clean, gofmt applied

Checklist

Tests added/updated
Documentation updated (if applicable)

PR by Bits
View session in Datadog

Comment @DataDog to request changes

Co-authored-by: AlexandreYang <49917914+AlexandreYang@users.noreply.github.com>

datadog-official · 2026-03-10T10:44:48Z

View session in Datadog

Bits Dev status: ✅ Done

Comment @DataDog to request changes

AlexandreYang · 2026-03-10T11:24:16Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e590d16e3

ℹ️ 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".

…d copyright headers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…rands Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…l.Access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

AlexandreYang · 2026-03-10T12:01:02Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bfdd72b494

ℹ️ 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 · 2026-03-10T13:51:01Z

[Security Audit] LOW: TOCTOU Between test and Subsequent Use (CWE-367)

This is an inherent limitation of the test command in all POSIX shells. The result of test -e FILE can become stale between the check and the subsequent action (e.g., test -f file && cat file).

Mitigating factors: Both the test and the subsequent operation are independently validated through os.Root, so the sandbox boundary cannot be crossed via TOCTOU. The worst case is a misleading true/false result followed by a normal error on the subsequent operation.

Remediation: No action required. Defense-in-depth from os.Root neutralizes the security risk. This is noted for completeness.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Fix -w/-x to check effective UID/GID permission class (owner/group/other) instead of any-class mode bits. Uses syscall.Stat_t on Unix; falls back to any-class bits on Windows. - Add pentest tests for 1000+ deep nesting, null bytes in file paths, 100KB string arguments, and 100KB file paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

datadog-prod-us1-6 · 2026-03-10T16:31:59Z

✅ Code Quality ✅ Code Vulnerabilities ✅ Library Vulnerabilities ✅ Secrets

🎉 All green!

🛠️ No new code quality issues
🛡️ No new code vulnerabilities
📚 No new vulnerable libraries detected
🔑 No new secrets detected

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 619d206 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

matt-dz

Security Audit — POSIX `test` and `[` Builtins

Overall Risk: LOW — No sandbox escape vectors identified.

#	Severity	Finding	File
1	Medium	Unbounded recursion via `!` operator (no depth limit)	`testcmd.go:254`
2	Medium	TOCTOU window between `Open()` and `Stat()` in `access()`	`allowed_paths.go:100-113`
3	Low	`effectiveHasPerm` only checks primary GID, not supplementary groups	`portable_unix.go:45`
4	Low	Raw OS errors returned from `stat`/`lstat`/`access` API (mitigated in `test` by silent error swallowing)	`allowed_paths.go`

Positive Observations

os.Root sandbox (Go 1.24 openat syscalls) is immune to symlink traversal and ../ escape — excellent design
Import allowlist enforced at AST level prevents builtins from importing os, exec, net, etc.
Empty path handling correctly returns false before any filesystem access
Paren depth limiting properly capped at 128 with pentest coverage
Integer overflow correctly handled — clamped to MinInt64/MaxInt64, base-10 only
Error messages silently suppressed in file tests, preventing information leakage
Comprehensive pentest test suite covering path traversal, null bytes, deep nesting, integer edge cases

Nice work overall — the defense-in-depth layering is solid. See inline comments for details on each finding.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- S1: Add depth limit to parseNot() recursion via existing depth counter - S2: Fix TOCTOU in access() by using f.Stat() on open fd instead of separate ar.root.Stat() call; wrap errors with portablePathError() - S3: Check supplementary groups in effectiveHasPerm via os.Getgroups() - S4: Wrap raw OS errors with portablePathError() in access() (done as part of S2 fix) - Simplify boolean expression in portable_windows.go (Datadog code quality) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The builtins.Command struct uses MakeFlags (not Run). Updated test and [ command registrations to use builtins.NoFlags(handler) matching the pattern used by all other builtins. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- access(): fall through to Stat path when Open() fails on a directory (isErrIsDirectory), fixing test -r <dir> on Windows - evalFileTest(): add nil guards for StatFile/LstatFile callbacks, matching the existing AccessFile nil check pattern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…cket-builtins # Conflicts: # interp/register_builtins.go # tests/import_allowlist_test.go

AlexandreYang · 2026-03-10T18:37:35Z

+	"github.com/spf13/pflag.ContinueOnError",
+	// pflag.NewFlagSet — CLI flag parsing; operates only on string slices, no I/O.
+	"github.com/spf13/pflag.NewFlagSet",


those should be removed

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

thieman

Changes to the fs sandbox and import allowlist look good

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

AlexandreYang · 2026-03-10T18:51:28Z

/merge

gh-worker-devflow-routing-ef8351 · 2026-03-10T18:51:33Z

View all feedbacks in Devflow UI.

2026-03-10 18:51:33 UTC ℹ️ Start processing command /merge

2026-03-10 18:51:38 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 45s (p90).

2026-03-10 18:52:16 UTC ℹ️ MergeQueue: This merge request was merged

Implement POSIX test and [ builtin commands

334d0c8

Co-authored-by: AlexandreYang <49917914+AlexandreYang@users.noreply.github.com>

datadog-datadog-prod-us1 Bot added the Bits AI label Mar 10, 2026

datadog-prod-us1-3 Bot and others added 2 commits March 10, 2026 11:06

Merge branch 'main' into dd/implement-test-bracket-builtins

5d43d5e

Merge branch 'main' into dd/implement-test-bracket-builtins

8e590d1

chatgpt-codex-connector Bot reviewed Mar 10, 2026

View reviewed changes

Comment thread interp/builtins/testcmd/testcmd.go Outdated

Comment thread interp/builtins/testcmd/testcmd.go

Comment thread interp/builtins/testcmd/testcmd.go

AlexandreYang and others added 4 commits March 10, 2026 12:33

Fix test builtin: match bash behavior for -o unary, error formats, an…

271c7d0

…d copyright headers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix test builtin: handle lone '(' as string and reject empty file ope…

f2565a0

…rands Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Use real access checks for test -r/-w/-x via sandbox AccessFile callback

93878bd

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix access checks to go through os.Root sandbox instead of raw syscal…

bfdd72b

…l.Access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

AlexandreYang marked this pull request as ready for review March 10, 2026 12:00

AlexandreYang requested review from matt-dz and thieman as code owners March 10, 2026 12:00

chatgpt-codex-connector Bot reviewed Mar 10, 2026

View reviewed changes

Comment thread interp/builtins/testcmd/testcmd.go Outdated

Comment thread interp/builtins/testcmd/testcmd.go Outdated

Comment thread interp/allowed_paths.go Outdated

matt-dz reviewed Mar 10, 2026

View reviewed changes

Comment thread interp/builtins/testcmd/testcmd.go

Comment thread interp/builtins/testcmd/testcmd.go

Comment thread interp/builtins/testcmd/testcmd.go

Comment thread interp/builtins/testcmd/testcmd_pentest_test.go

AlexandreYang and others added 5 commits March 10, 2026 14:57

Merge branch 'main' into dd/implement-test-bracket-builtins

2c880c8

Merge branch 'main' into dd/implement-test-bracket-builtins

215a289

Fix !/( literal parsing in binary expressions and add parser depth limit

4fe2162

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Merge branch 'main' into dd/implement-test-bracket-builtins

89e3be1

datadog-official Bot reviewed Mar 10, 2026

View reviewed changes

Comment thread interp/portable_windows.go Outdated

Merge branch 'main' into dd/implement-test-bracket-builtins

54f7d49

matt-dz reviewed Mar 10, 2026

View reviewed changes

Comment thread interp/builtins/testcmd/testcmd.go Outdated

Comment thread interp/allowed_paths.go Outdated

Comment thread interp/portable_unix.go

Comment thread interp/allowed_paths.go

AlexandreYang and others added 3 commits March 10, 2026 17:50

Merge branch 'main' into dd/implement-test-bracket-builtins

be99dc1

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Merge branch 'main' into dd/implement-test-bracket-builtins

222428e

Merge branch 'main' into dd/implement-test-bracket-builtins

045676a

AlexandreYang and others added 3 commits March 10, 2026 18:39

Merge branch 'main' into dd/implement-test-bracket-builtins

d8503ca

AlexandreYang commented Mar 10, 2026

View reviewed changes

Comment thread interp/allowed_paths.go Outdated

AlexandreYang commented Mar 10, 2026

View reviewed changes

Comment thread interp/builtins/testcmd/testcmd.go

AlexandreYang and others added 2 commits March 10, 2026 19:17

Merge remote-tracking branch 'origin/main' into dd/implement-test-bra…

4caa800

…cket-builtins # Conflicts: # interp/register_builtins.go # tests/import_allowlist_test.go

AlexandreYang commented Mar 10, 2026

View reviewed changes

thieman reviewed Mar 10, 2026

View reviewed changes

Comment thread tests/import_allowlist_test.go Outdated

Address code review: nil guard, explicit Close, fix YAML bash skips

d1587a0

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

thieman approved these changes Mar 10, 2026

View reviewed changes

Remove pflag symbols from allowlist per PR #27 MakeFlags refactor

619d206

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gh-worker-dd-devflow-36fce6 Bot added mergequeue-status: queued mergequeue-status: in_progress and removed mergequeue-status: queued labels Mar 10, 2026

gh-worker-dd-mergequeue-cf854d Bot merged commit 1833336 into main Mar 10, 2026
9 checks passed

gh-worker-dd-mergequeue-cf854d Bot deleted the dd/implement-test-bracket-builtins branch March 10, 2026 18:52

gh-worker-dd-devflow-36fce6 Bot added mergequeue-status: done and removed mergequeue-status: in_progress labels Mar 10, 2026

AlexandreYang mentioned this pull request Mar 12, 2026

Implement printf builtin command #57

Merged

4 tasks

Conversation

AlexandreYang commented Mar 10, 2026

What does this PR do?

Motivation

Testing

Checklist

Uh oh!

datadog-official Bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexandreYang commented Mar 10, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexandreYang commented Mar 10, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matt-dz commented Mar 10, 2026

Uh oh!

datadog-prod-us1-6 Bot commented Mar 10, 2026 • edited by datadog-prod-us1-5 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

matt-dz left a comment

Choose a reason for hiding this comment

Security Audit — POSIX test and [ Builtins

Positive Observations

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexandreYang Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thieman left a comment

Choose a reason for hiding this comment

Uh oh!

AlexandreYang commented Mar 10, 2026

Uh oh!

gh-worker-devflow-routing-ef8351 Bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

datadog-official Bot commented Mar 10, 2026 •

edited

Loading

datadog-prod-us1-6 Bot commented Mar 10, 2026 •

edited by datadog-prod-us1-5 Bot

Loading

Security Audit — POSIX `test` and `[` Builtins

gh-worker-devflow-routing-ef8351 Bot commented Mar 10, 2026 •

edited

Loading