fix(scripts): scan skill markdown in the shell-portability gate

[kyle-sexton]

Summary

• Extends scripts/check-shell-portability.sh so CI-facing modes (--all and <base-ref>) also select skill markdown under plugins/*/skills/**/*.md (excluding vendor/ and evals/), matching the executable surface agents actually run.
• Stages today's measured backlog (69 hits / 33 files at measure time — mostly Windows-path prose colliding with bare \\b/\\s/\\w, intentional docs-hygiene regex docs, and markdown-structure false positives) in scripts/shell-portability-skill-md-baseline.txt so new/changed skill markdown is gated first; the list is shrink-only (stale entries fail).
• --paths remains the unfiltered audit tool (never consults the baseline), preserving the reproduction path from fix(scripts): shell-portability gate never scans skill markdown, where the shell is actually executed #2704.

Test plan

• bash scripts/check-shell-portability.test.sh (PASS=331)
• --paths on a synthetic skill md with stat -c fails (detector + selection)
• --paths on a baselined skill md still reports hits (audit mode)
• --all does not report baselined skill-md PORTABILITY lines; no STALE BASELINE
• Unit coverage for skill-md --all / diff-mode selection, baseline grandfathering, stale baseline, and --paths bypass

Closes #2704

Related

• Baseline staging is shrink-only; residual skill-md portability hits remain grandfathered until cleaned (tracked by the baseline file itself, not a separate issue).

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

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

---

Security Review — PR #2780

• Gather context (PR diff, changed files)
• Invoke /review:security-review and follow its instructions
• Post findings / summary

Verdict: No security vulnerabilities found.

This PR widens scripts/check-shell-portability.sh's CI-facing selection (--all and <base-ref> diff mode) to also scan skill markdown under plugins/*/skills/**/*.md, and adds a shrink-only grandfather list (scripts/shell-portability-skill-md-baseline.txt) for the pre-existing backlog. I reviewed it against the standard trust-boundary / injection / CI-Actions-security lens:

• Command/path injection — All new/changed variable usages ($f, $file, $entry, $path) are quoted everywhere they're referenced (check-shell-portability.sh#L172-L258, #L1739-L1791). Baseline entries are read from a static repo file and only ever compared with [[ == ]] string equality or passed to [[ -f ]] — never eval'd, globbed, or interpolated into a command. No new exposure to attacker-controlled filenames beyond what git diff/find already sanitize into NUL-delimited records (existing pattern, correctly extended).
• No code from scanned files is executed — confirmed there's no eval/source/backtick execution of skill-markdown or .sh file contents anywhere in the script; matching is grep/awk-based only, so a malicious construct in a scanned file can't achieve RCE inside the gate itself.
• GitHub Actions surface (.github/workflows/ci.yml) — the diff is comment/label-only plus a renamed step; the job still triggers on pull_request (not pull_request_target), keeps permissions: contents: read, and persist-credentials: false on checkout. No secrets exposure, no privilege change.
• Baseline mechanism (informational, not a finding) — the new baseline file is shrink-only enforced (a stale/cleaned-up entry fails the gate), matching the existing convention already used by scripts/orphaned-fixtures-baseline.txt / hook-userconfig-argv-allowlist.txt. Nothing in the script stops a future PR from adding a new entry to self-exempt a newly-introduced problematic skill-md file from the gate — same as those existing baselines. This is a lint-completeness/gate-integrity observation, not a security vulnerability (the underlying construct-class this gate hunts is portability breakage, not an exploitable defect), and it mirrors an accepted repo-wide pattern rather than something new to this PR.

No changes to authentication, authorization, secrets handling, or externally-reachable surfaces are involved — this is a self-contained CI linting/test change.

[github-actions]

Last security-reviewed head: 6913383883e2a56c8cc526430e4282e83deb8163. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[chatgpt-codex-connector]

💡 Codex Review

claude-code-plugins/scripts/check-shell-portability.sh

Lines 263 to 265 in 6913383

	if ((${#files[@]} == 0)); then
	echo "No shell files in scope — nothing to gate."
	exit 0

Validate the baseline before returning on an empty diff

When a PR only deletes a baselined skill Markdown file, git diff --diff-filter=d produces no scoped files, so this return occurs before the stale-baseline guard. The changed-file invocation in ci.yml therefore accepts the dangling entry, while the next unrelated PR that touches a shell or skill Markdown file fails because the baseline names a missing file. Baseline-only edits similarly bypass all validation; run the baseline checks before this early return.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 6m 16s —— View job

---

Code review — /review:code-review

• Gather context (PR diff, CLAUDE.md/REVIEW.md, changed files)
• Understand the request (CI code-review lane via /review:code-review)
• Review scripts/check-shell-portability.sh changes
• Review scripts/check-shell-portability.test.sh changes
• Review scripts/shell-portability-skill-md-baseline.txt
• Review scripts/shell-portability-tokens.txt and .github/workflows/ci.yml
• Post findings

I read the full diff (gh pr diff) plus the surrounding, unchanged parts of check-shell-portability.sh for context, and confirmed via mcp__github_ci__get_ci_status that this PR's ci workflow run is green (so the 30 baseline entries and new unit tests all pass as claimed — I did not re-run the suite myself, only observed the CI result).

Two findings posted as inline comments:

1. 🔴 Important — scripts/check-shell-portability.sh#L1774: the new stale-baseline "missing file" check isn't scoped to the current run's changed files, unlike the two branches right below it. Since diff mode (the mode CI runs on every PR) applies this check unconditionally, a baseline entry going stale via an unrelated file deletion/rename would fail every subsequent, unrelated PR's shell-portability-lint step — the exact "red-lines main for unrelated changes" failure mode this gate's own design principle (changed-file scoping) is built to avoid.
2. 🟡 Suggestion — scripts/check-shell-portability.test.sh#L1591-L1602: only 1 of the 3 new STALE BASELINE branches (cleaned-up file) has a unit test; the missing-file and outside-scannable-set branches are untested.

Everything else — is_scannable's new plugins/*/skills/*.md selection (verified bash case glob matches /, consistent with the nested-path test fixtures), the --all file discovery, the diff-mode plugins/ pathspec addition, the baseline-loading/baselined() helper, and the ci.yml/tokens-file comment-only changes — reads correct and matches its extensive inline rationale. No security-relevant findings (this repo has a separate claude-security-review lane, so security is out of scope here per REVIEW.md).
· Branch

[github-actions]

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 31s —— View job

---

Code review — /review:code-review

• Gather context (PR diff, CLAUDE.md/REVIEW.md, changed files)
• Understand the request (CI code-review lane via /review:code-review)
• Invoke /review:code-review and follow its instructions
• Post findings

Skipped: PR is merged.

Per the skip gate in /review:code-review ("stop early when the PR is closed or not open, say so in the summary, and post nothing else"), this PR is already MERGED, so no new review was performed and no inline comments were posted for HEAD SHA 42bf235966033fec054799b8e720115912d0cf9c.

For reference, this lane already reviewed this branch twice while it was open (last at commit 6913383883e2a56c8cc526430e4282e83deb8163), flagging the unscoped stale-baseline check and missing test coverage — both of which the author (kyle-sexton) addressed per their reply comments, and the PR has since merged to main.