fix: Windows/Git Bash compatibility (jq 1.7+ and CRLF)#3
fix: Windows/Git Bash compatibility (jq 1.7+ and CRLF)#3elquer wants to merge 1 commit intooryband:masterfrom
Conversation
Two issues when running on Windows with Git Bash: 1. jq 1.7+ treats \( inside strings as string interpolation, breaking the regex in sub() calls. Replaced \( with [(] character class syntax which is compatible with all jq versions. 2. shfmt and jq on Windows output CRLF line endings, causing \r to be appended to extracted command strings and prefixes, which breaks prefix matching. Added tr -d '\r' to strip carriage returns from jq/shfmt output. Tested on Windows 11 with Git Bash, jq 1.8.1, shfmt 3.13.0.
There was a problem hiding this comment.
Pull request overview
Improves approve-compound-bash.sh compatibility when running on Windows via Git Bash by avoiding jq 1.7+ string interpolation parsing issues in regex patterns and by stripping CRLF carriage returns that can break prefix matching.
Changes:
- Update jq
sub()regexes to avoid jq 1.7+\(string interpolation conflicts. - Strip
\rfrom jq/shfmt output in key pipelines to prevent CRLF-related matching failures.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| while IFS= read -r line; do | ||
| [[ -n "$line" ]] && allowed_prefixes+=("$line") | ||
| done < <(jq -r '.[] | sub("^Bash\\("; "") | sub("( \\*|\\*|:\\*)\\)$"; "") | sub("\\)$"; "")' <<< "$permissions_json" 2>/dev/null) | ||
| done < <(jq -r '.[] | sub("^Bash[(]"; "") | sub("( [*]|[*]|:[*])[)]$"; "") | sub("[)]$"; "")' <<< "$permissions_json" 2>/dev/null) |
There was a problem hiding this comment.
The --permissions parsing path still reads raw jq -r output directly into allowed_prefixes without stripping \r. On Windows jq may emit CRLF, leaving a trailing carriage return in each prefix (e.g., "ls\r") and causing prefix matching to fail. Pipe this process substitution through tr -d '\r' (or strip \r from line before appending).
| while IFS= read -r line; do | ||
| [[ -n "$line" ]] && denied_prefixes+=("$line") | ||
| done < <(jq -r '.[] | sub("^Bash\\("; "") | sub("( \\*|\\*|:\\*)\\)$"; "") | sub("\\)$"; "")' <<< "$deny_json" 2>/dev/null) | ||
| done < <(jq -r '.[] | sub("^Bash[(]"; "") | sub("( [*]|[*]|:[*])[)]$"; "") | sub("[)]$"; "")' <<< "$deny_json" 2>/dev/null) |
There was a problem hiding this comment.
Same CRLF issue as the allow list: the --deny parsing path appends jq -r output directly into denied_prefixes without removing \r, which can break deny matching on Windows Git Bash. Consider adding | tr -d '\r' to this process substitution as well.
| done < <(jq -r '.[] | sub("^Bash[(]"; "") | sub("( [*]|[*]|:[*])[)]$"; "") | sub("[)]$"; "")' <<< "$deny_json" 2>/dev/null) | |
| done < <(jq -r '.[] | sub("^Bash[(]"; "") | sub("( [*]|[*]|:[*])[)]$"; "") | sub("[)]$"; "")' <<< "$deny_json" 2>/dev/null | tr -d '\r') |
| local raw_commands | ||
| raw_commands=$(jq -r "$SHFMT_AST_FILTER" <<< "$ast" 2>/dev/null) || return 1 | ||
| raw_commands=$(jq -r "$SHFMT_AST_FILTER" <<< "$ast" 2>/dev/null | tr -d '\r') || return 1 | ||
|
|
There was a problem hiding this comment.
The new CRLF-stripping behavior (tr -d '\r') is not covered by tests. Since this repo uses BATS tests for permissions/parsing, consider adding a test that feeds permissions containing an embedded carriage return (e.g., a JSON string with \r) and asserts it still matches, to prevent regressions in Windows/Git Bash environments.
Summary
Two fixes for running on Windows with Git Bash:
1. jq 1.7+ string interpolation conflict
jq >= 1.7 treats
\(inside strings as string interpolation, which breaks the regex patterns insub()calls used to extract permission prefixes.Before:
sub("^Bash\("; "")→ jq 1.7+ sees\(as interpolation start → syntax errorAfter:
sub("^Bash[(]"; "")→ character class[(]is compatible with all jq versions2. CRLF line endings from Windows tools
shfmtandjqon Windows output\r\n(CRLF) line endings. The trailing\rgets appended to extracted command strings and prefixes, causing prefix matching to silently fail (e.g.,"find\r"never matches"find").Fix: Added
tr -d '\r'to strip carriage returns from jq/shfmt output pipelines.Test plan
php -l file.php→ approvedfind /path | head -20→ approvedcd /path && grep pattern | wc -l→ approvedrm -rf /→ falls through to prompt[(]syntax andtr -d '\r'are no-ops on systems without\r)