fix(parser): prevent unintended command execution via unmatched ')'

[scumfrog]

fix(parser): prevent unintended command execution via unmatched ')'

Summary

This change fixes a parser state machine flaw that allowed unintended command execution and parsing corruption when processing input containing unmatched ')' characters.

Root Cause

The parser previously toggled the internal dollarQuote state unconditionally upon encountering a ')' character, without verifying that a corresponding '$(' sequence had opened the state.

This created an asymmetry in the state machine:

• '(' handler → guarded (requires preceding '$')
• ')' handler → unguarded toggle

As a result, a bare ')' could incorrectly open a command substitution context.

Impact

Command Execution (when ParseBacktick=true)

Crafted input such as:

prefix)COMMAND)

could trigger execution via:

exec.Command($SHELL, "-c", COMMAND)

Denial of Service

Short inputs (e.g. ))) could trigger:

slice bounds out of range

Parsing Corruption (default configuration)

Unmatched ')' caused silent corruption of tokenization by incorrectly entering dollar-quote mode.

Fix

The fix enforces strict state transitions:

• A bare ')' can no longer open dollarQuote
• ')' only closes an already opened $( context
• Unmatched ')' is treated as a literal character
• Defensive bounds check added to prevent slice underflow

Changes

• Replace unconditional toggle:

dollarQuote = !dollarQuote

with guarded logic:

if !dollarQuote {
    // treat as literal
    continue
}
dollarQuote = false

• Add length validation before buffer slicing

Behavior After Fix

• $(...) → works as expected
• ) (unmatched) → treated as literal
• malformed input → returns error instead of panicking
• no unintended command execution paths

Tests

Added security-focused tests covering:

• No panic on malformed input ()))
• No command execution from unmatched ')'
• Correct parsing behavior in default configuration
• Valid $(...) substitution still works
• Proper error on unclosed $(

Security

This change addresses a command injection vulnerability caused by incorrect parser state handling (CWE-94), along with related robustness issues.

Notes

Upstream repository appears inactive; this patch is applied as part of a maintained fork with security fixes.

[mattn]

CI workflow has been updated on master (Go 1.25/1.26, latest GitHub Actions).

Please rebase your branch onto master to pick up the CI fix:

git fetch origin
git rebase origin/master
git push --force-with-lease

Also, after rebasing, CI will reveal two existing test failures caused by this PR:

• TestBacktick: When ParseBacktick=false, parsing $(echo "foo") — the closing ) is no longer added to the buffer, so the result is $(echo "foo" instead of $(echo "foo").
• TestBacktickError: echo )echo1 with ParseBacktick=true was previously an error (bare ) toggled dollarQuote), but now it's treated as a literal, so no error is returned.

Please fix these before the PR can be reviewed further.

[scumfrog]

Rebased onto latest master and updated the fix:

• Unmatched ')' now returns an error when ParseBacktick=true (preserves previous behavior)
• When ParseBacktick=false, ')' is treated as a literal and $(...) is preserved correctly
• Added bounds check to prevent panic

All tests pass locally.

[mattn]

One test failing

[scumfrog]

Fixed the failing test.

The previous CI run did not include the complete changes. Updated both parser logic and tests; behavior is now consistent with upstream expectations.

All tests pass locally. Thanks!

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.04%. Comparing base (f3bbb6f) to head (b074fa0).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
shellwords.go	88.88%	1 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
- Coverage   90.19%   90.04%   -0.16%     
==========================================
  Files           2        3       +1     
  Lines         204      241      +37     
==========================================
+ Hits          184      217      +33     
- Misses         14       16       +2     
- Partials        6        8       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[scumfrog]

All tests are passing now. Let me know if you'd like additional coverage for any specific branch.

[mattn]

Thank you for the fix! Merging this now.