Skip to content

fix(sed): enforce grouped command depth at parse time - #1460

Merged
chaliy merged 1 commit into
mainfrom
2026-04-24-fix-sed-nesting-validator-bypass
Apr 25, 2026
Merged

chaliy merged 1 commit into
mainfrom
2026-04-24-fix-sed-nesting-validator-bypass

Conversation

@chaliy

@chaliy chaliy commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • The flat precheck validate_group_nesting can misinterpret an s in non-substitution contexts and undercount {/} causing the parser to recurse past MAX_GROUP_NESTING_DEPTH.
  • Prevent unbounded recursion/stack overflow by ensuring the parser itself never recurses past the max depth even if the precheck is bypassed.

Description

  • Add an explicit recursion depth guard in parse_sed_command_with_depth for the '{' branch that returns an error when depth + 1 > MAX_GROUP_NESTING_DEPTH (file: crates/bashkit/src/builtins/sed.rs).
  • Keep the existing validate_group_nesting precheck but add the parser-level hard guard so bypasses cannot trigger unbounded recursion.
  • Add regression test test_sed_grouped_command_nesting_depth_limit_with_address_bypass_shape that exercises the /s<delim>/-shaped address bypass and asserts a depth-limit error (file: crates/bashkit/src/builtins/sed.rs tests).

Testing

  • Ran cargo fmt --all which completed successfully.
  • Ran cargo test -p bashkit test_sed_grouped_command_nesting_depth_limit -- --nocapture which executed the existing depth-limit test and the new regression test and both passed.
  • Ran the crate test run (unit test suite invoked during the command above) and observed the two modified/added tests pass with no regressions in the executed test set.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Apr 24, 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 909528f Commit Preview URL Apr 25 2026, 01:31 AM

@chaliy
chaliy force-pushed the 2026-04-24-fix-sed-nesting-validator-bypass branch from 757caea to 909528f Compare April 25, 2026 01:30
@chaliy
chaliy merged commit 62e8439 into main Apr 25, 2026
34 checks passed
@chaliy
chaliy deleted the 2026-04-24-fix-sed-nesting-validator-bypass branch April 25, 2026 01:41
@codecov

codecov Bot commented Apr 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

chaliy added a commit that referenced this pull request May 30, 2026
### Motivation
- The flat precheck `validate_group_nesting` can misinterpret an `s` in
non-substitution contexts and undercount `{`/`}` causing the parser to
recurse past `MAX_GROUP_NESTING_DEPTH`.
- Prevent unbounded recursion/stack overflow by ensuring the parser
itself never recurses past the max depth even if the precheck is
bypassed.

### Description
- Add an explicit recursion depth guard in
`parse_sed_command_with_depth` for the `'{`' branch that returns an
error when `depth + 1 > MAX_GROUP_NESTING_DEPTH` (file:
`crates/bashkit/src/builtins/sed.rs`).
- Keep the existing `validate_group_nesting` precheck but add the
parser-level hard guard so bypasses cannot trigger unbounded recursion.
- Add regression test
`test_sed_grouped_command_nesting_depth_limit_with_address_bypass_shape`
that exercises the `/s<delim>/`-shaped address bypass and asserts a
depth-limit error (file: `crates/bashkit/src/builtins/sed.rs` tests).

### Testing
- Ran `cargo fmt --all` which completed successfully.
- Ran `cargo test -p bashkit
test_sed_grouped_command_nesting_depth_limit -- --nocapture` which
executed the existing depth-limit test and the new regression test and
both passed.
- Ran the crate test run (unit test suite invoked during the command
above) and observed the two modified/added tests pass with no
regressions in the executed test set.

------
[Codex
Task](https://chatgpt.com/codex/cloud/tasks/task_e_69eb99e1c918832bbae9de27f473a00b)
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.

1 participant