What happened
PR #4133 shipped apply-rename.sh with three regexes handed to awk through -v:
prefix_re='\]\([^)]*$'
awk -v prefix_re="$prefix_re" '...'
POSIX has a -v assignment undergo escape processing. gawk therefore reads that value as ]([^)]*$ and dies:
awk: warning: escape sequence `\]' treated as plain `]'
awk: cmd. line:15: fatal: invalid regexp: Unmatched ( or \(: /]([^)]*$/
mawk passes unknown escapes through untouched, so the same script works there.
The development machine had mawk 1.3.4 and no gawk. The suite reported 89 cases, 0 failures against a script that could not rewrite a single markdown link on any CI runner. Three lanes went red on push — test-linux, test-windows, and lint (which runs the skill's own script test) — all on the one cause. Fixed in 2be7033b by moving the values to ENVIRON[].
Why this is worth a guard
This is the second time the same defect class landed in this repository. emit-gate.sh already carries the lesson in a comment, from a Phase 4 review finding in the same PR:
awk -v is not a transparent channel (escape processing halves a doubled backslash and turns a literal backslash-t into a TAB)
Knowing the rule did not stop the next caller from breaking it, and nothing mechanical was watching.
Why grep is not the guard
The dangerous shape is -v var="$shellvar" where the shell variable holds a regex. The value is not in the source line, so a static scan cannot see it. A repo-wide scan for a literal backslash in a -v value finds two sites, both the intended -v OFS='\t':
plugins/repo-hygiene/skills/clean/scripts/git-branch-delete.test.sh:247
plugins/repo-hygiene/skills/clean/scripts/git-branch-delete.test.sh:261
So a lint rule would be all false positives and still miss the real bug.
What would have caught it
Running the suites under a second awk. Locally this is the whole reproduction:
mkdir -p /tmp/gawkbin && ln -sf "$(command -v gawk)" /tmp/gawkbin/awk
PATH=/tmp/gawkbin:$PATH bash <suite>
Against the parent commit that yields 34 failures; against 2be7033b, 0 under both gawk 5.2.1 and mawk 1.3.4.
Options
- A second-awk CI lane. Re-run the shell suites with gawk forced onto
PATH as awk (or mawk, whichever the runner does not already have). Highest confidence, costs one more shard.
- Fold it into
check-shell-portability.sh as a documented rule plus a runtime probe, rather than a source scan: the checker already owns the GNU-only-construct vocabulary, and "pass arbitrary values through ENVIRON[], never -v" belongs in the same place.
- Document only, in the portability convention, and accept that the next caller learns it from a red CI run.
Worth deciding which; (2) plus a note in the convention looks like the cheapest thing that actually changes behaviour, with (1) as the belt if the shell surface keeps growing.
Related
What happened
PR #4133 shipped
apply-rename.shwith three regexes handed to awk through-v:POSIX has a
-vassignment undergo escape processing. gawk therefore reads that value as]([^)]*$and dies:mawk passes unknown escapes through untouched, so the same script works there.
The development machine had mawk 1.3.4 and no gawk. The suite reported 89 cases, 0 failures against a script that could not rewrite a single markdown link on any CI runner. Three lanes went red on push —
test-linux,test-windows, andlint(which runs the skill's own script test) — all on the one cause. Fixed in2be7033bby moving the values toENVIRON[].Why this is worth a guard
This is the second time the same defect class landed in this repository.
emit-gate.shalready carries the lesson in a comment, from a Phase 4 review finding in the same PR:Knowing the rule did not stop the next caller from breaking it, and nothing mechanical was watching.
Why grep is not the guard
The dangerous shape is
-v var="$shellvar"where the shell variable holds a regex. The value is not in the source line, so a static scan cannot see it. A repo-wide scan for a literal backslash in a-vvalue finds two sites, both the intended-v OFS='\t':So a lint rule would be all false positives and still miss the real bug.
What would have caught it
Running the suites under a second awk. Locally this is the whole reproduction:
Against the parent commit that yields 34 failures; against
2be7033b, 0 under both gawk 5.2.1 and mawk 1.3.4.Options
PATHasawk(or mawk, whichever the runner does not already have). Highest confidence, costs one more shard.check-shell-portability.shas a documented rule plus a runtime probe, rather than a source scan: the checker already owns the GNU-only-construct vocabulary, and "pass arbitrary values throughENVIRON[], never-v" belongs in the same place.Worth deciding which; (2) plus a note in the convention looks like the cheapest thing that actually changes behaviour, with (1) as the belt if the shell surface keeps growing.
Related
2be7033bis the fix)emit-gate.shcomment recording the same lesson from the earlier finding