Skip to content

ci: no gate covers shell portability — a GNU-only \b nearly shipped a fail-open security predicate #1491

Description

@kyle-sexton

What is missing

No CI gate checks shell scripts for GNU-only constructs. This is a missing capability, not a defect in an existing gate.

To be explicit about what already exists and why neither covers this:

The concrete near-miss that motivated this

While hardening plugins/markdown-format/hooks/markdown-format.sh on #1097, a predicate deciding whether a markdownlint config's module specifiers can be pinned to an approval was written with \b word boundaries:

grep -Eq '\brequire\b' "$file"

\b is a GNU extension. POSIX leaves \<ordinary char> undefined, and BSD grep may read \b as a literal b — turning \brequire\b into brequireb, which matches nothing. The predicate would then have returned "nothing unpinnable found" for every input.

That is a fail-open in a security predicate, on macOS, that no test on a GNU runner could ever catch. markdown-format.sh's own header explicitly commits to macOS system bash, so the platform is in declared scope.

The fix, now on main, is POSIX-portable and is the reference implementation for what a gate should accept:

  • word starts/ends spelled as bracket expressions — (^|[^A-Za-z0-9_$])require($|[^A-Za-z0-9_$]) — instead of \b
  • the residue test uses grep -q rather than grep -o, so no boundary character is consumed (which is what made a count-based approach undercount a nested require(require("x")))

Suggested shape

A changed-file-scoped lane over **/*.sh flagging GNU-only constructs. Non-exhaustive starter set, all of which differ or are undefined under BSD userland:

  • ERE/BRE: \b, \<, \>, \s, \S, \w, \W, \+, \? in grep/sed patterns
  • grep -P (PCRE — absent in BSD grep and in some GNU builds)
  • sed -i without a backup suffix (BSD requires an argument)
  • sed \|, \+, \? and GNU-only addresses
  • readlink -f, date -d, stat -c, mktemp -p, head -c on some builds, sort -V
  • echo -e

Two design notes from the near-miss:

  1. The token list belongs outside the script, the way ci: portability lint lane — enforce declared agnosticism mechanically (19-issue coupling class + top review-churn source) #531's does, so a re-catch is a one-line data change rather than a gate edit.
  2. Direction matters. Flagging a GNU-ism that is actually safe costs a reviewer one line of justification; missing one in a security predicate costs a silent fail-open. The gate should over-flag and allow an inline opt-out with a stated reason, not under-flag.

An alternative or complement worth weighing: run the existing shell test suites once under a BSD-userland container so the behavioral difference surfaces as a test failure rather than a lint finding. That catches constructs no token list anticipated, at the cost of a slower lane.

Provenance

Found on #1097 by adversarially re-probing a predicate I had just written, not by any reviewer. It is the kind of defect that produces no symptom until it is exploited on the one platform CI does not cover.

Activity

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

Metadata

Metadata

Assignees

Labels

agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.priority: mediumReal value, no hard deadline; normal backlog flow.status: readyTriaged, unblocked, and fully specified; eligible to pick up.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions