fix(ci): shell-portability-lint's sed -Ei / --in-place tokens miss operator-terminated forms

[kyle-sexton]

This was generated by AI during work-loop execution.

Closes #1545

Summary

• ci: harden shell-portability-lint detection precision (sed -i spellings, portability-scope precision, awk operand edge case) #1513/fix(ci): harden shell-portability-lint detection precision #1534 added GNU-only sed -Ei (combined extended-regex + in-place short-flag cluster) and
  sed --in-place (long-form) tokens to scripts/shell-portability-tokens.txt, but both only
  accepted a trailing whitespace-or-end-of-line boundary — the same shape fix(ci): detect sort's --version-sort / --sort=version long forms #1530's original
  sort -V short-flag token had, and the same false negative fix(ci): shell-portability-lint's sort -V token misses operator-terminated forms #1537/fix(ci): shell-portability-lint's sort -V token misses operator-terminated forms #1546 fixed for sort -V /
  grep -P / echo -e: an operator-terminated form with no separating whitespace
  (x=$(sed -Ei), sed -Ei|cat, sed --in-place|cat, x=$(sed --in-place)) evaded detection
  entirely. Verified via repro before any fix: all four forms passed the gate clean on main.
• Widens both tokens' trailing boundary to a control operator, redirection, or subshell close —
  ([[:space:]|&;()<>]|$) (plus the pre-existing = alternative for --in-place=SUFFIX).
• Deliberately narrower than the sibling sort -V/grep -P/echo -e boundary (which also
  includes quote/backtick chars): a quote immediately following -Ei with no separating
  whitespace is exactly the attached-EMPTY-suffix shape (sed -Ei'') ci: harden shell-portability-lint detection precision (sed -i spellings, portability-scope precision, awk operand edge case) #1513/fix(ci): harden shell-portability-lint detection precision #1534 explicitly
  deferred as ambiguous sed-dialect territory (same as the -i'' case). Confirmed empirically —
  copying the sibling tokens' full quote-inclusive boundary verbatim broke the existing
  "sed -Ei'' must not be flagged" regression test, since a bare quote character then satisfied
  the widened boundary at the i+' junction. Excluding quotes/backtick from this pair's
  boundary keeps that deferred case correctly unflagged while still catching every
  operator-terminated form the issue named (none of which need a quote as the terminator).
• Verified before landing, per the issue's instruction. Ran check-shell-portability.sh --all
  against the corpus before and after the token edit: the hit sets are byte-for-byte identical
  (75 pre-existing findings, none sed-related), so the widened boundary surfaces no new corpus
  violations and needs no portability-ok: annotations.

Test plan

• bash scripts/check-shell-portability.test.sh — 75/75 passing (6 new regression cases: 3
  operator-terminated forms each for sed -Ei and sed --in-place at the isolated-token
  level, plus one shipped-list assertion proving all 4 forms are detected under the real
  shell-portability-tokens.txt).
• Verified the new tests are meaningful: reproduced the bug first (bash scripts/check-shell-portability.sh --paths t.sh against a file containing the four
  operator-terminated forms passed clean on main's shipped list); after the fix, all four
  are flagged.
• Verified the boundary is not over-widened: the full sibling (quote-inclusive) boundary was
  tried first and demonstrably broke the pre-existing sed -Ei'' deferred-ambiguous-suffix
  test (PASS=74 FAIL=1); narrowed to exclude quotes/backtick, re-ran — PASS=75 FAIL=0.
• scripts/check-shell-portability.sh --all — before/after hit sets identical (75 findings,
  unrelated to these tokens); see summary above.
• scripts/check-shell-portability.sh origin/main run directly against this branch's own
  diff — clean (no unexcused GNU-only constructs in the 2 changed files).
• shellcheck --rcfile=.shellcheckrc scripts/check-shell-portability.test.sh — clean.
• typos --config _typos.toml on both changed files — clean.

Related

• fix(ci): shell-portability-lint's sort -V token misses operator-terminated forms #1537 / fix(ci): shell-portability-lint's sort -V token misses operator-terminated forms #1546 (established the operator-terminated boundary shape this PR mirrors, and named
  sed -Ei / sed --in-place as the deferred-out sibling defect this PR fixes)
• ci: harden shell-portability-lint detection precision (sed -i spellings, portability-scope precision, awk operand edge case) #1513 / fix(ci): harden shell-portability-lint detection precision #1534 (introduced the two tokens this PR widens, and the attached-empty-suffix
  deferral this PR's narrower boundary deliberately preserves)

[claude]

Claude finished @kyle-sexton's task in 2m 57s —— View job

---

PR Review — fix(ci): sed -Ei / --in-place operator-terminated boundary

• Gather context and read changed files
• Review scripts/shell-portability-tokens.txt changes
• Review scripts/check-shell-portability.test.sh changes
• Verify regex correctness and boundary logic
• Check test coverage and regression cases
• Post final review

---

Summary

The fix is correct and well-reasoned. Two minor issues found; one very minor nit on a test comment.

---

Regex analysis

Both new boundary expressions use ([[:space:]|&;()<>]|$). This is valid POSIX ERE:

• Inside a bracket expression [...], | is a literal pipe character, not the alternation operator — so [[:space:]|&;()<>] matches one character from: whitespace, |, &, ;, (, ), <, >.
• The outer (…|$) alternates between "one boundary character" and "end of line."

This is the right set. Quotes and backtick are correctly absent, preserving the sed -Ei'' deferral the comment explains. The narrower boundary versus the sibling sort -V/grep -P/echo -e tokens is exactly right given the ambiguous empty-suffix constraint.

---

Findings

1. Misleading comment in test file — "or quote must still be detected"

check-shell-portability.test.sh lines 402–403:

# --- operator-terminated forms: no trailing whitespace before a control
# operator, redirection, subshell close or quote must still be detected

The phrase "or quote must still be detected" implies quote-terminated forms are included in the detection, but the whole point of this PR's narrower boundary is that they are not — sed -Ei'' stays deliberately undetected. The comment should not enumerate quotes among the terminators covered. A reader seeing this comment without the rest of the context would reasonably conclude sed -Ei'...' is flagged.

Suggested rewrite:

# --- operator-terminated forms: no trailing whitespace before a control
# operator, redirection, or subshell close must still be detected

Fix this →

2. Shipped-list test omits ;-terminated forms (minor coverage gap)

check-shell-portability.test.sh lines 754–758:

f="$(tmpsh "$(printf '%s\n' \
  'x=$(sed -Ei)' \
  'sed -Ei|cat' \
  'x=$(sed --in-place)' \
  'sed --in-place|cat')")"

The PR description names four forms as the bug's concrete scope, and the shipped-list check proves those four against the real token file. However, sed -Ei; echo done and sed --in-place; echo done — which the isolated-token loops do cover — are absent here. ; is the most common command separator and is also in the boundary. Since the isolated-token tests already exercise ;, this is not a correctness gap, but if the shipped-list test is meant to be the definitive "real-token-file proof," including the ; forms would make it exhaustive.

Not blocking — the isolated-token path already covers ; — but worth noting.

---

What's correct

• The two isolated-token while-loops (CASES heredocs) correctly check detection by expecting scan_paths to exit non-zero (violation found), consistent with every other detection test in the file.
• Cleanup is correct: rm -f "$f" inside the loop, rm -f "$tok" after the loop.
• Using case as the read variable name mirrors the pre-existing pattern at line 235 (already in origin/main), not a new smell.
• The grep -c "PORTABILITY: ${f}:" assertion in the shipped-list test is the established counting pattern (matches the sort long-form test above it).
• Token-file comment is thorough and accurate: explicitly documents the quote-exclusion rationale, the #1545 reference, and contrasts the boundary choice with the sibling tokens.

---

Findings: 1 comment accuracy issue (non-blocking), 1 minor coverage gap in the shipped-list test. Core fix is sound — approve when the comment wording is addressed or acknowledged.

[claude]

Claude finished @kyle-sexton's task in 2m 1s —— View job

---

Security Review — PR #1548

• Gather context and read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found.

This PR makes two narrow regex-widening changes to a developer linting tool (shell-portability-tokens.txt) and adds corresponding test cases. Here is what I checked:

Pattern consumption path — The ERE patterns in shell-portability-tokens.txt are consumed at check-shell-portability.sh:224 via awk's ~ operator (line ~ patterns[i]). This is awk's built-in regex matching, not a system() call, eval, or shell interpolation, so there is no code-execution surface here. A malformed or malicious pattern causes awk to exit non-zero, which the script already handles by failing closed (line 240–244).

ReDoS — The ([[:space:]][^\n]*)?[[:space:]] segment, which could in theory cause catastrophic backtracking, is not new to this PR — it is carried forward unchanged from the tokens these patterns widened. awk implementations (GNU awk uses a DFA/NFA hybrid) handle this class efficiently in practice, and the files being scanned are developer-controlled scripts in CI, not adversarial user input.

Bracket-expression correctness — [[:space:]|&;()<>] is syntactically valid: inside a POSIX bracket expression, | is a literal character, not ERE alternation. The intent (match pipe, ampersand, semicolon, parentheses, angle brackets, or whitespace) is correctly expressed. The |$ trailing alternation for end-of-line is correctly placed outside the brackets.

Test script injection — The new while IFS= read -r case; do loops in check-shell-portability.test.sh write test content to temp files via printf '%s\n' "$1" inside tmpsh() and then pass the file path to the checker — the content is scanned as text, never executed as shell. No injection risk.

No workflow files modified — No .github/workflows/ changes; no CI/CD injection surface (untrusted code with elevated permissions, pull_request_target, script injection through the github context, or permission widening).

No secrets, credentials, or supply-chain changes — None present.