Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
deef1c7
Add AllowedCommands option to restrict executable commands
AlexandreYang Mar 13, 2026
6dc189c
Make nil allowedCommands block all commands; add AllowAllCommands opt…
AlexandreYang Mar 13, 2026
75db43f
Use AllowAllCommands() in CLI instead of builtins.Names()
AlexandreYang Mar 13, 2026
2122329
Rename CLI flags to --allowed-paths/--allowed-commands and use lowerc…
AlexandreYang Mar 13, 2026
64e0229
Remove allowAllCommands bool; fill map with builtin names instead
AlexandreYang Mar 13, 2026
4748471
Use allowed_commands in scenarios instead of skip_assert_against_bash
AlexandreYang Mar 13, 2026
d833bb2
Change error format to 'CMD: command not allowed' and add unknown cmd…
AlexandreYang Mar 14, 2026
37a3ea0
Rename AllowAllCommands to AllowAllBuiltinsCommands
AlexandreYang Mar 14, 2026
49ca2e3
Restore 'command not found' assertions by adding unknown cmds to allo…
AlexandreYang Mar 14, 2026
06ebddf
Apply suggestion from @AlexandreYang
AlexandreYang Mar 14, 2026
549eac5
Remove unnecessary skip_assert_against_bash; use stderr_contains for …
AlexandreYang Mar 14, 2026
96118af
Fix CLI flag shorthands: -s for --script, -c for --allowed-commands, …
AlexandreYang Mar 14, 2026
95dd71c
Use -c for --script, long flags for --allowed-commands/--allowed-path…
AlexandreYang Mar 14, 2026
a1b1823
Revert "Use -c for --script, long flags for --allowed-commands/--allo…
AlexandreYang Mar 14, 2026
23e413a
Fix CLI tests to use -s for --script and long flags for --allowed-com…
AlexandreYang Mar 14, 2026
c17f32e
Use exact stderr match instead of stderr_contains in skip_assert_agai…
AlexandreYang Mar 14, 2026
fe975c0
[iter 1] Address PR review findings: fix dead test, add missing scena…
AlexandreYang Mar 14, 2026
54e28da
Update review-fix-loop: require 3 consecutive Step 3 successes before…
AlexandreYang Mar 14, 2026
c6a790b
[iter 2] Address PR review: fix CLI compat, AllowAllCommands, pipe test
AlexandreYang Mar 14, 2026
a2f1fe4
[iter 3] Fix doc/code mismatch: AllowedCommands default is allow-all,…
AlexandreYang Mar 14, 2026
26c0c92
[iter 4] Revert --allowed-paths back to --allowed-path to avoid break…
AlexandreYang Mar 14, 2026
89ea191
[iter 5] Add pipeline scenario and fix fragile test helper
AlexandreYang Mar 14, 2026
3083682
[iter 5] Treat empty --allowed-commands as explicit deny-all
AlexandreYang Mar 14, 2026
615dfcb
[iter 6] Add CLI tests for --allowed-commands restriction and use Str…
AlexandreYang Mar 14, 2026
96d56bc
[iter 7] Add right-side pipeline scenario, case-insensitive "all" key…
AlexandreYang Mar 14, 2026
f019c0c
[iter 8] Add command substitution and subshell scenarios for AllowedC…
AlexandreYang Mar 14, 2026
b6cc8c8
[iter 9] Trim whitespace from --allowed-commands and --allowed-path e…
AlexandreYang Mar 14, 2026
fe241af
Increase review-fix-loop max iterations from 10 to 20
AlexandreYang Mar 14, 2026
001fb91
Increase review-fix-loop max iterations from 20 to 30
AlexandreYang Mar 14, 2026
38bb825
[iter 1] Fix SKILL.md iteration limit inconsistency (30 vs 20)
AlexandreYang Mar 14, 2026
088739f
[iter 1] Address PR review findings: document exit codes, fix splitAn…
AlexandreYang Mar 14, 2026
052bd07
Update review-fix-loop to show iteration number in Step 2 task subject
AlexandreYang Mar 14, 2026
0f2d9f7
[iter 1] Filter empty entries in splitAndTrim, restore skip_assert_ag…
AlexandreYang Mar 14, 2026
b32161f
[iter 2] Fix separator-only allowlist, stale comment, and CLI help text
AlexandreYang Mar 14, 2026
b488c50
Increase review-fix-loop Step 3 consecutive success requirement to 5
AlexandreYang Mar 14, 2026
04ef3ae
[iter 1] Address PR review comments: honor 'all' in mixed lists, add …
AlexandreYang Mar 14, 2026
00ebaaf
[iter 2] Add missing test scenarios and clarify CLI help text
AlexandreYang Mar 14, 2026
7aa845f
[iter 3] Sort builtin Names(), document 'all' as reserved keyword in …
AlexandreYang Mar 14, 2026
07371a1
[iter 4] Improve AllowAllBuiltinCommands doc to clarify ExecHandler i…
AlexandreYang Mar 14, 2026
b5501c7
[iter 5] Use AllowAllCommands() in test helpers, add ExecHandler inte…
AlexandreYang Mar 14, 2026
77b8af9
Add specs verification step to code-review and update codex review pr…
AlexandreYang Mar 14, 2026
f3d1d51
[iter 6] Clarify AllowedCommands doc: empty slice = deny-all, nil = a…
AlexandreYang Mar 14, 2026
47de068
Update review-fix-loop and code-review skills: increase limits, add i…
AlexandreYang Mar 14, 2026
c2ed053
[iter 7] Add stderr_contains comments, case-insensitive all test, sim…
AlexandreYang Mar 14, 2026
37758ed
Improve address-pr-comments skill: prioritize PR specs over assumptions
AlexandreYang Mar 14, 2026
d4c7257
[iter 1] Address review comments: enforce spec requirements
AlexandreYang Mar 14, 2026
c3cae87
[iter 1] Fix CI test failures: PortableErrMsg unwrap, ls -l total lin…
AlexandreYang Mar 14, 2026
6f2c191
[iter 1] Fix Windows test failures: update stderr_windows and permiss…
AlexandreYang Mar 14, 2026
f77e998
[iter 2] Fix splitAndTrim slice reuse
AlexandreYang Mar 14, 2026
72e02fe
[iter 3] Fix misleading comment and remove dead Names() function
AlexandreYang Mar 14, 2026
7953da1
[iter 4] Make AllowAllCommands authoritative regardless of option ord…
AlexandreYang Mar 14, 2026
b2c3cfc
[iter 6] Rename --allowed-commands to --allowed-command per spec
AlexandreYang Mar 14, 2026
ac9c059
[iter 7] Add exit code comment distinguishing "restricted" from "not …
AlexandreYang Mar 14, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 41 additions & 15 deletions .claude/skills/address-pr-comments/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,21 @@ gh api graphql -f query='

Only process **unresolved** threads with actionable comments.

### 2b. Read the PR specs

Before evaluating any comment, read the PR description to check for a **SPECS** section:

```bash
gh pr view $ARGUMENTS --json body --jq '.body'
```

If a SPECS section is present, **it defines the authoritative requirements for this PR**. Specs override:
- Your assumptions about backward compatibility or design intent
- Inline code comments
- Conventions from other parts of the codebase

Store the specs for use in step 4 (validity evaluation). If a reviewer comment aligns with a spec, the comment is **valid by definition** — even if you think the current implementation is reasonable.

### 3. Understand each comment

For each unresolved review comment:
Expand All @@ -99,36 +114,45 @@ For each unresolved review comment:
| **Nitpick** | Minor optional suggestion | Evaluate — fix if trivial, otherwise reply explaining the tradeoff |
| **Invalid/outdated** | Comment doesn't apply or is based on a misunderstanding | Reply politely explaining why |

### 4. Evaluate validity — bash behavior is the source of truth
### 4. Evaluate validity — specs and bash behavior are the sources of truth

There are two sources of truth, checked in this order:

**The shell must match bash behavior unless it intentionally diverges** (e.g., sandbox restrictions, blocked commands, readonly enforcement). This principle overrides reviewer suggestions.
1. **PR specs** (from step 2b) — if present, specs are the highest authority for what this PR should do
2. **Bash behavior** — the shell must match bash unless it intentionally diverges (sandbox restrictions, blocked commands, readonly enforcement)

**CRITICAL: Never invent justifications for dismissing a comment.** If a reviewer says "the spec requires X" and the spec does require X, the comment is valid — even if you think the current implementation is a reasonable alternative. Do not fabricate reasons like "backward compatibility" or "design intent" unless those reasons are explicitly stated in the specs or CLAUDE.md.

For each comment, determine if it is **valid and actionable**:

1. **Verify against bash** — always check what bash actually does:
1. **Check against PR specs first** — if a SPECS section exists and the comment aligns with a spec, the comment is **valid by definition**. Do not dismiss it.
2. **Verify against bash** — for comments about shell behavior, check what bash actually does:
```bash
docker run --rm debian:bookworm-slim bash -c '<relevant script>'
```
2. **Read the relevant code** in full — not just the diff, but the surrounding implementation
3. **Check project conventions** in `CLAUDE.md` and `AGENTS.md`
4. **Consider side effects** — will the change break other tests or behaviors?
5. **Check for duplicates** — is the same issue raised in multiple comments? Group them
3. **Read the relevant code** in full — not just the diff, but the surrounding implementation
4. **Check project conventions** in `CLAUDE.md` and `AGENTS.md`
5. **Consider side effects** — will the change break other tests or behaviors?
6. **Check for duplicates** — is the same issue raised in multiple comments? Group them

Decision matrix:

| Reviewer says | Bash does | Shell intentionally diverges? | Action |
|--------------|-----------|-------------------------------|--------|
| "This is wrong" | Reviewer is right | No | **Fix the implementation** to match bash |
| "This is wrong" | Current code matches bash | No | **Reply** explaining it matches bash, with proof |
| "This is wrong" | N/A | Yes (sandbox/security) | **Reply** explaining the intentional divergence |
| "Do it differently" | Suggestion matches bash better | No | **Fix the implementation** to match bash |
| "Do it differently" | Current code already matches bash | No | **Reply** — bash compatibility takes priority |
| Reviewer says | Spec says | Bash does | Action |
|--------------|-----------|-----------|--------|
| "Spec requires X" | Spec does require X | N/A | **Fix the implementation** to match the spec |
| "Spec requires X" | No such spec exists | N/A | **Reply** noting the spec doesn't mention this |
| "This is wrong" | No spec relevant | Reviewer is right | **Fix the implementation** to match bash |
| "This is wrong" | No spec relevant | Current code matches bash | **Reply** explaining it matches bash, with proof |
| "This is wrong" | No spec relevant | N/A (sandbox/security) | **Reply** explaining the intentional divergence |
| "Do it differently" | No spec relevant | Suggestion matches bash better | **Fix the implementation** to match bash |
| "Do it differently" | No spec relevant | Current code already matches bash | **Reply** — bash compatibility takes priority |

If a comment is **not valid**:
- Prepare a polite reply with proof (e.g., "This matches bash behavior — verified with `docker run --rm debian:bookworm-slim bash -c '...'`")
- If the divergence is intentional, explain why (sandbox restriction, security, etc.)
- **Never claim "backward compatibility" or "design intent" unless you can point to a specific line in the specs or CLAUDE.md that says so**

If a comment is **valid** (i.e., fixing it brings the shell closer to bash, or addresses a real bug):
If a comment is **valid** (i.e., it aligns with a spec, brings the shell closer to bash, or addresses a real bug):
- Proceed to step 5

### 5. Implement fixes
Expand Down Expand Up @@ -223,6 +247,8 @@ For each comment that was addressed:

For comments that were **not valid** or were **questions**, reply (prefixed with `[<MODELL NAME - VERSION>]`) with an explanation but do NOT resolve — let the reviewer decide.

**IMPORTANT: Never resolve a thread where the reviewer's comment aligns with a PR spec but the implementation doesn't match.** These are valid spec violations — fix the code instead. If you cannot fix it, leave the thread unresolved and explain the blocker.

### 8. Summary

Provide a final summary:
Expand Down
31 changes: 30 additions & 1 deletion .claude/skills/code-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,36 @@ git diff main...HEAD

If no changes are found, inform the user and stop.

### 2. Read and understand all changed code
### 2. Verify specs implementation

Read the PR description and look for a **SPECS** section:

```bash
gh pr view $ARGUMENTS --json body --jq '.body'
```

If a SPECS section is present, it defines the requirements that this PR MUST implement. **Every single spec must be verified against the diff.**
The specs override other instructions (code, inline comments in code, etc). ALL specs MUST be implemented.

For each spec:
1. **Find the code** that implements the spec
2. **Verify correctness** — does the implementation fully satisfy the spec?
3. **Check for missing specs** — is any spec not implemented at all?

Flag any unimplemented or partially implemented spec as a **P1 finding** (missing functionality that was explicitly required).

Include a spec coverage table in the review output:

```markdown
| Spec | Implemented | Location | Notes |
|------|:-----------:|----------|-------|
| Must support `--flag` option | Yes | `interp/api.go:42` | Fully implemented |
| Must return exit code 2 on error | **No** | — | Not found in diff |
```

If no SPECS section is found in the PR description, skip this step.

### 3. Read and understand all changed code

For each changed file:

Expand Down
24 changes: 17 additions & 7 deletions .claude/skills/review-fix-loop/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ You MUST follow this execution protocol. Skipping steps or running them out of o
Your very first action — before reading ANY files, before running ANY commands — is to call TaskCreate exactly 11 times, once for each step/sub-step below. Use these exact subjects:

1. "Step 1: Identify the PR"
2. "Step 2: Run the review-fix loop"
2. "Step 2: Run the review-fix loop" ← **Update subject with iteration number each loop** (e.g. "Step 2: Run the review-fix loop (iteration 1)")
3. "Step 2A1: Self-review (code-review)" ← **parallel with 2A2**
4. "Step 2A2: Request external reviews (@codex)" ← **parallel with 2A1**
5. "Step 2B: Address PR comments (address-pr-comments)"
Expand Down Expand Up @@ -91,7 +91,9 @@ Store the owner and repo name.

**GATE CHECK**: Call TaskList. Step 1 must be `completed`. Set Step 2 to `in_progress`.

Set `iteration = 1`. Maximum iterations: **10**. Repeat sub-steps A through E while `iteration <= 10`:
Set `iteration = 1`. Maximum iterations: **30**. Repeat sub-steps A through E while `iteration <= 30`.

**At the start of each iteration**, update the Step 2 task subject to include the current iteration number using TaskUpdate, e.g. `"Step 2: Run the review-fix loop (iteration 3)"`.

---

Expand All @@ -107,7 +109,11 @@ This analyzes the full diff against main, posts findings as a GitHub PR review w

Post a comment to trigger @codex reviews:
```bash
gh pr comment <pr-number> --body "@codex review"
gh pr comment <pr-number> --body "@codex review this PR

Important: Read the SPECS section of the PR description. If SPECS are present: **make sure the implementation matches ALL the specs**.
The specs override other instructions (code, inline comments in code, etc). ALL specs MUST be implemented.
"
```
The external reviews arrive asynchronously — their comments will be picked up by **address-pr-comments** in Sub-step 2B1.

Expand Down Expand Up @@ -240,7 +246,7 @@ Check **all three** review sources for remaining issues:
| Any findings | Any | Any | **Continue** → go back to Sub-step 2A1 ∥ 2A2 |
| APPROVE | Unresolved threads | Any | **Continue** → go back to Sub-step 2A1 ∥ 2A2 (address-pr-comments will handle them) |
| APPROVE | None unresolved | Failing | **Continue** → go back to Sub-step 2A1 ∥ 2A2 (fix-ci-tests will handle it) |
| — | — | — | If `iteration > 10` → **STOP — iteration limit reached** |
| — | — | — | If `iteration > 30` → **STOP — iteration limit reached** |

Log the iteration result before continuing or stopping:
- Iteration number
Expand Down Expand Up @@ -331,9 +337,13 @@ Run a final verification regardless of how the loop exited:

Record the final state of each dimension (self-review, external reviews, CI, Codex response).

**If any verification fails** (CI failing, unresolved threads remain, unpushed commits that can't be pushed, or Codex hasn't responded to the latest review request), reset Step 2 and all its sub-steps to `pending`, and go back to **Step 2: Run the review-fix loop** for another iteration. Only proceed to Step 4 when all verifications pass.
Track how many times Step 3 has **succeeded** (all four verifications passed) across the entire run.

**If any verification fails** (CI failing, unresolved threads remain, unpushed commits that can't be pushed, or Codex hasn't responded to the latest review request), reset the success counter to 0, reset Step 2 and all its sub-steps to `pending`, and go back to **Step 2: Run the review-fix loop** for another iteration.

**If all verifications pass**, increment the success counter. If this is the **5th consecutive success** of Step 3 → proceed to **Step 4**. Otherwise → reset Step 2 and all its sub-steps to `pending`, and go back to **Step 2: Run the review-fix loop** for another iteration to re-confirm stability.

**Completion check:** All four verifications passed. Mark Step 3 as `completed`.
**Completion check:** Step 3 has succeeded 5 consecutive times. Mark Step 3 as `completed`.

---

Expand Down Expand Up @@ -385,5 +395,5 @@ gh pr comment <pr-number> --body "<the summary markdown above>"
- **Run address-pr-comments before fix-ci-tests** — 2B then 2C, sequentially, so CI fixes run on code that already incorporates review feedback.
- **Pull before fixing** — always `git pull --rebase` before launching fix agents to avoid working on stale code.
- **Stop early on APPROVE + CI green + no unresolved threads** — don't waste iterations if the PR is already clean.
- **Respect the iteration limit** — hard stop at 10 to prevent infinite loops. If issues persist after 10 iterations, report what's left for the user to handle.
- **Respect the iteration limit** — hard stop at 30 to prevent infinite loops. If issues persist after 30 iterations, report what's left for the user to handle.
- **Use gate checks** — always call TaskList and verify prerequisites before starting a step. This prevents out-of-order execution.
5 changes: 3 additions & 2 deletions .github/workflows/fuzz.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ name: Fuzz Tests

on:
push:
branches: ['**']
branches: [main]
pull_request:
branches: [main]

permissions:
contents: read
Expand Down Expand Up @@ -75,7 +76,7 @@ jobs:
fi
for FUNC in $FUZZ_FUNCS; do
echo "Fuzzing $FUNC..."
go test -fuzz="^${FUNC}$" -fuzztime=30s ${{ matrix.pkg }} -timeout 300s
go test -fuzz="^${FUNC}$" -fuzztime=60s ${{ matrix.pkg }} -timeout 300s
done

# Save corpus
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,14 @@ Every access path is default-deny:

| Resource | Default | Opt-in |
|----------------------|-------------------------------------|----------------------------------------------|
| Commands (builtins) | Blocked (exit code 1) | Allow with `AllowedCommands` list or `AllowAllCommands` |
| External commands | Blocked (exit code 127) | Provide an `ExecHandler` |
| Filesystem access | Blocked | Configure `AllowedPaths` with directory list |
| Environment variables| Empty (no host env inherited) | Pass variables via the `Env` option |
| Output redirections | Only `/dev/null` allowed (exit code 2 for other targets) | `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `2>&1` |

**AllowedCommands** restricts which commands (builtins and external) may execute. When set, only listed commands are allowed; disallowed commands return exit code 1 with `<name>: command not allowed` (distinct from exit code 127 used for unknown commands). Shell keywords and control flow (if/else, for, pipes, `&&`/`||`, variable assignment) are unaffected. Use the CLI flag `--allow-all-commands` to permit all commands, or `--allowed-command echo,cat,...` to allow specific commands.

**AllowedPaths** restricts all file operations to specified directories using Go's `os.Root` API (`openat` syscalls), making it immune to symlink traversal, TOCTOU races, and `..` escape attacks.

## Shell Features
Expand Down
1 change: 1 addition & 0 deletions SHELL_FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ Blocked features are rejected before execution with exit code 2.

## Execution

- ✅ AllowedCommands command restriction — by default all commands are blocked; use AllowedCommands to permit specific commands or AllowAllCommands to permit everything; disallowed commands return exit code 1 with `<name>: command not allowed` (distinct from exit code 127 used for unknown commands)
- ✅ AllowedPaths filesystem sandboxing — restricts all file access to specified directories
- ❌ External commands — blocked by default; requires an ExecHandler to be configured and the binary to be within AllowedPaths
- ❌ Background execution: `cmd &`
Expand Down
8 changes: 8 additions & 0 deletions allowedpaths/portable.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ func PortableErrMsg(err error) string {
if err == nil {
return ""
}
// Unwrap *os.PathError so we normalise the inner Err rather than
// returning the full "op path: msg" string. This handles the case
// where the error has already been wrapped by PortablePathError
// (which replaces the inner Err with a plain errors.New string).
var pe *os.PathError
if errors.As(err, &pe) {
return PortableErrMsg(pe.Err)
}
switch {
case errors.Is(err, fs.ErrNotExist):
return "no such file or directory"
Expand Down
17 changes: 17 additions & 0 deletions builtins/ls/ls.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,23 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt
// so no post-sort slicing is needed. The effectiveLimit is already
// enforced by readDir's maxRead parameter.

// Print total line for long format (matching GNU ls behaviour).
// GNU ls prints "total <blocks>" where blocks are 1024-byte (1K) blocks
// by default. Since syscall.Stat_t is not available (import restriction),
// we approximate using file sizes: blocks = ceil(size / 1024) per file,
// with directories counting as 0 blocks (matching GNU ls on most FSes
// where directory size varies).
if opts.longFmt {
var totalBlocks int64
for _, ei := range infoEntries {
if !ei.info.IsDir() {
sz := ei.info.Size()
totalBlocks += (sz + 1023) / 1024
}
Comment on lines +397 to +399
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Compute ls -l total from allocated blocks

The new long-format total uses ceil(size/1024) and skips directories, but GNU/bash ls -l totals are based on allocated filesystem blocks (st_blocks), not logical byte size, so this prints systematically incorrect totals (for example, a 1-byte file commonly shows total 4 in bash but total 1 here). Because this feature is meant to match ls semantics, the current calculation introduces observable bash-incompatibility.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member 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] This is an intentional simplification. The code comment (lines 388-392) already explains: syscall.Stat_t is not available because the syscall package is banned by the import allowlist. GNU ls uses st_blocks (allocated filesystem blocks), but without syscall we can only approximate using logical file sizes. This is a known trade-off documented in the long-format section of the ls package doc (line 49-52). Leaving unresolved for the reviewer.

Copy link
Copy Markdown
Member 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] Confirming this is an intentional limitation — the syscall package is banned by the import allowlist, so st_blocks (allocated filesystem blocks) is not accessible. The code uses ceil(size/1024) as the best available approximation. This is documented in the code comments. The test scenarios for ls -l use skip_assert_against_bash: true for this reason. Leaving unresolved for reviewer to confirm this trade-off is acceptable.

Comment on lines +395 to +399
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Compute ls -l total using allocated blocks

The new total calculation sums ceil(size/1024) per entry, but GNU/Bash-compatible ls -l uses filesystem-allocated blocks (st_blocks), so this undercounts common cases (e.g., a 1-byte file on ext4 shows total 4 in GNU ls but this code prints total 1) and also mishandles sparse files. Because this project compares behavior against bash/GNU tools, this introduces observable output regressions for long-format directory listings.

Useful? React with 👍 / 👎.

}
callCtx.Outf("total %d\n", totalBlocks)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

P3 Badge ls -l total line is an approximation

The comment already acknowledges this, and all ls -l scenario tests correctly set skip_assert_against_bash: true. The approximation (ceil(size/1024) per file, directories = 0) will diverge from GNU ls which uses the actual block count from stat(2). For example, sparse files or files with extended attributes will show different values. This is acceptable given the syscall.Stat_t import restriction — just noting for awareness.

}

// Print.
for _, ei := range infoEntries {
if ctx.Err() != nil {
Expand Down
2 changes: 1 addition & 1 deletion builtins/tests/cat/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts ..
t.Fatal(err)
}
var outBuf, errBuf bytes.Buffer
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf)}, opts...)
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf), interp.AllowAllCommands()}, opts...)
runner, err := interp.New(allOpts...)
if err != nil {
t.Fatal(err)
Expand Down
1 change: 1 addition & 0 deletions builtins/tests/cut/cut_gnu_compat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func cutRun(t *testing.T, script, dir string) (string, string, int) {
opts := []interp.RunnerOption{
interp.StdIO(nil, &outBuf, &errBuf),
interp.AllowedPaths([]string{dir}),
interp.AllowAllCommands(),
}

runner, err := interp.New(opts...)
Expand Down
1 change: 1 addition & 0 deletions builtins/tests/cut/cut_pentest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func cutPentestRunCtx(ctx context.Context, t *testing.T, script, dir string) (st
opts := []interp.RunnerOption{
interp.StdIO(nil, &outBuf, &errBuf),
interp.AllowedPaths([]string{dir}),
interp.AllowAllCommands(),
}

runner, err := interp.New(opts...)
Expand Down
2 changes: 1 addition & 1 deletion builtins/tests/cut/cut_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts ..
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)}, opts...)
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf), interp.AllowAllCommands()}, opts...)
runner, err := interp.New(allOpts...)
require.NoError(t, err)
defer runner.Close()
Expand Down
2 changes: 1 addition & 1 deletion builtins/tests/head/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts ..
t.Fatal(err)
}
var outBuf, errBuf bytes.Buffer
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf)}, opts...)
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf), interp.AllowAllCommands()}, opts...)
runner, err := interp.New(allOpts...)
if err != nil {
t.Fatal(err)
Expand Down
2 changes: 1 addition & 1 deletion builtins/tests/sed/sed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts ..
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)}, opts...)
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf), interp.AllowAllCommands()}, opts...)
runner, err := interp.New(allOpts...)
require.NoError(t, err)
defer runner.Close()
Expand Down
2 changes: 1 addition & 1 deletion builtins/tests/tail/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts ..
t.Fatal(err)
}
var outBuf, errBuf bytes.Buffer
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf)}, opts...)
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf), interp.AllowAllCommands()}, opts...)
runner, err := interp.New(allOpts...)
if err != nil {
t.Fatal(err)
Expand Down
2 changes: 1 addition & 1 deletion builtins/tests/wc/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func runScriptCtx(ctx context.Context, t *testing.T, script, dir string, opts ..
t.Fatal(err)
}
var outBuf, errBuf bytes.Buffer
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf)}, opts...)
allOpts := append([]interp.RunnerOption{interp.StdIO(nil, &outBuf, &errBuf), interp.AllowAllCommands()}, opts...)
runner, err := interp.New(allOpts...)
if err != nil {
t.Fatal(err)
Expand Down
1 change: 1 addition & 0 deletions builtins/tests/wc/wc_pentest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func wcRunCtx(ctx context.Context, t *testing.T, script, dir string) (string, st
opts := []interp.RunnerOption{
interp.StdIO(nil, &outBuf, &errBuf),
interp.AllowedPaths([]string{dir}),
interp.AllowAllCommands(),
}

runner, err := interp.New(opts...)
Expand Down
Loading
Loading