Skip to content

No lane runs the shell suites under a second awk, so awk -v escape divergence ships green #4143

Description

@kyle-sexton

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

  1. 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.
  2. 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.
  3. 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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions