Skip to content

refactor(builtins): BuiltinHelper trait, centralized limits, split ls/awk - #1788

Merged
chaliy merged 3 commits into
mainfrom
claude/charming-maxwell-eRZRU
May 26, 2026
Merged

chaliy merged 3 commits into
mainfrom
claude/charming-maxwell-eRZRU

Conversation

@chaliy

@chaliy chaliy commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Three behavior-preserving refactors that improve the structure of crates/bashkit/src/builtins/:

  • BuiltinHelper trait (builtins/helpers.rs): binds command name as an associated const and provides default check_help / err / err_path helpers. Removes per-builtin repetition of cmd: prefixes and cmd: file: msg path diagnostics. Migrated 12 builtins (echo, yes, sleep, base64, comm, fold, column, expand, unexpand, cut, tr, iconv).
  • Centralized DoS limits (builtins/limits.rs): gathers the security-relevant MAX_* / MKTEMP_* / TIMEOUT_* constants scattered across individual builtin files into a single, threat-model-reviewable location. Each builtin imports the shared constant under its existing local alias, so call sites are unchanged. Tunable runtime limits that are part of public config types (Python/TypeScript/Sqlite/SSH) are intentionally left alone.
  • Split ls.rs (3824 lines) into ls/: list.rs (Ls + column/long-format), find.rs (Find + recursion + printf), rmdir.rs, glob.rs, tests.rs.
  • Split awk.rs (4731 lines) into awk/: mod.rs (Awk + Builtin impl + AST + AwkState + AwkValue + preprocessing), parser.rs (1610 lines), interpreter.rs (1287 lines), tests.rs.

No behavioral changes. Internals exposed across sibling modules via pub(super). Test count and outcomes unchanged.

Test plan

  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • cargo test --features http_client — 2947 passed, 1 pre-existing failure (builtins::rg::tests::diff_rg_matches_real_rg_cases — baseline issue, rg/ files unmodified in this PR)
  • cargo test --features failpoints --test security_failpoint_tests -- --test-threads=1 — 14/14 passed
  • cargo vet --locked — passed

Generated by Claude Code

chaliy added 3 commits May 26, 2026 04:16
Introduces a small trait that binds the command name as an associated
const and provides default check_help / err / err_path helpers. Removes
the per-builtin repetition of 'cmd: ' prefixes and 'cmd: file: msg' path
diagnostics.

Migrated: echo, yes, sleep, base64, comm, fold, column, expand,
unexpand, cut, tr, iconv.
Gathers the security-relevant MAX_* / MKTEMP_* / TIMEOUT_* constants
that were scattered across individual builtin source files into
crates/bashkit/src/builtins/limits.rs. Each builtin now imports the
shared constant under its existing local alias, so the call sites
are unchanged.

Threat-model-referenced limits (TM-DOS-027/028/051/052/059, etc.) are
now reviewable in a single file. Tunable runtime limits that are part
of public config types (PythonLimits, TypeScriptLimits, SqliteLimits,
ssh defaults) are intentionally left alone.
ls/ (3824 lines → 5 production files + tests):
  - list.rs: Ls + column/long-format helpers
  - find.rs: Find + parse_find_args + find_recursive + printf
  - rmdir.rs: Rmdir
  - glob.rs: glob_match
  - tests.rs: all 73 tests

awk/ (4731 lines → 3 production files + tests):
  - mod.rs: Awk + Builtin impl + AST types + AwkState + AwkValue +
    preprocessing (everything except parser and interpreter)
  - parser.rs: AwkParser (1610 lines)
  - interpreter.rs: AwkInterpreter + AwkFlow (1287 lines)
  - tests.rs: all 87 tests

No behavioural changes. Internals exposed across sibling modules via
pub(super). Existing test count and outcomes are unchanged (2284 passed,
same pre-existing rg differential failure as baseline).
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 26, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit ad6f3e8 Commit Preview URL

Branch Preview URL
May 26 2026, 04:47 AM

@chaliy
chaliy merged commit a292535 into main May 26, 2026
34 checks passed
@chaliy
chaliy deleted the claude/charming-maxwell-eRZRU branch May 26, 2026 04:57
@chaliy chaliy mentioned this pull request May 27, 2026
6 tasks
chaliy added a commit that referenced this pull request May 27, 2026
## Summary

Maintenance release rolling up 50 commits since v0.7.1.

## Highlights

- **Maintenance release** — rolls up ~25 `rg` hardening fixes, a CoW
subshell-snapshot perf landmark (#1767), security tightening (FD cap
#1780, tool-hook enforcement for `command` #1781), a `BuiltinHelper`
refactor (#1788), and test/build hygiene work.

## Publish-readiness report

| Check | Result |
|-------|--------|
| `cargo fmt --check` | clean |
| `cargo clippy --all-targets -- -D warnings` | clean |
| `cargo test --features http_client` | 2965 pass (environmental rg-diff
failure resolved with pinned ripgrep 15.1) |
| `cargo publish --dry-run -p bashkit` (with CI's monty strip) |
succeeds |
| `cargo publish --dry-run -p bashkit-cli` | packaging succeeds;
resolution fails only because bashkit 0.7.2 isn't on crates.io yet
(chicken-and-egg, resolves post-merge per `publish.yml`) |
| Version sync (Cargo.toml, bashkit-cli/Cargo.toml,
bashkit-js/package.json, Cargo.lock) | all 0.7.2 |
| Registry version checks (crates.io / npm / PyPI) | all currently
0.7.1; 0.7.2 > 0.7.1 |

## Test plan

- [ ] CI green on this PR
- [ ] Merge → `release.yml` creates GitHub Release + tag `v0.7.2`
- [ ] `publish.yml` publishes bashkit + bashkit-cli to crates.io
- [ ] `publish-python.yml` publishes wheels to PyPI
- [ ] `publish-js.yml` publishes `@everruns/bashkit` to npm
- [ ] `cli-binaries.yml` ships prebuilt binaries and bumps Homebrew tap
chaliy added a commit that referenced this pull request May 30, 2026
…/awk (#1788)

- Add BuiltinHelper trait (helpers.rs) with command-name const and default check_help/err/err_path; migrate 12 builtins (echo, yes, sleep, base64, comm, fold, column, expand, unexpand, cut, tr, iconv)
- Centralize per-builtin DoS limits in limits.rs (threat-model-relevant MAX_*/MKTEMP_*/TIMEOUT_* constants)
- Split ls.rs (3824 lines) into ls/ (list.rs, find.rs, rmdir.rs, glob.rs, tests.rs)
- Split awk.rs (4731 lines) into awk/ (mod.rs, parser.rs, interpreter.rs, tests.rs)

No behavioral changes.
chaliy added a commit that referenced this pull request May 30, 2026
## Summary

Maintenance release rolling up 50 commits since v0.7.1.

## Highlights

- **Maintenance release** — rolls up ~25 `rg` hardening fixes, a CoW
subshell-snapshot perf landmark (#1767), security tightening (FD cap
#1780, tool-hook enforcement for `command` #1781), a `BuiltinHelper`
refactor (#1788), and test/build hygiene work.

## Publish-readiness report

| Check | Result |
|-------|--------|
| `cargo fmt --check` | clean |
| `cargo clippy --all-targets -- -D warnings` | clean |
| `cargo test --features http_client` | 2965 pass (environmental rg-diff
failure resolved with pinned ripgrep 15.1) |
| `cargo publish --dry-run -p bashkit` (with CI's monty strip) |
succeeds |
| `cargo publish --dry-run -p bashkit-cli` | packaging succeeds;
resolution fails only because bashkit 0.7.2 isn't on crates.io yet
(chicken-and-egg, resolves post-merge per `publish.yml`) |
| Version sync (Cargo.toml, bashkit-cli/Cargo.toml,
bashkit-js/package.json, Cargo.lock) | all 0.7.2 |
| Registry version checks (crates.io / npm / PyPI) | all currently
0.7.1; 0.7.2 > 0.7.1 |

## Test plan

- [ ] CI green on this PR
- [ ] Merge → `release.yml` creates GitHub Release + tag `v0.7.2`
- [ ] `publish.yml` publishes bashkit + bashkit-cli to crates.io
- [ ] `publish-python.yml` publishes wheels to PyPI
- [ ] `publish-js.yml` publishes `@everruns/bashkit` to npm
- [ ] `cli-binaries.yml` ships prebuilt binaries and bumps Homebrew tap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant