feat(ci): let the issue lane run skill-quality's checks

[kyle-sexton]

Refs: #7840
Refs: #7375

Summary

The issue lane (work-resolve-issue) may now run skill-quality's two checks, check-skill.sh and
check-evals-quality.sh. This changes what the lane may do unattended; the repository owner chose
this design in the main session.

Run 39089376952
(issue #7840) stopped because the lane, under --permission-mode dontAsk, may run only the
scripts resolve-issue.verify-scripts lists, and check-skill.sh was left out on purpose.

Fix

• docs/conventions/pr-pipeline.yaml: resolve-issue.verify-scripts adds
  plugins/skill-quality/scripts/check-skill.sh and
  plugins/skill-quality/scripts/check-evals-quality.sh. The comment states the residual:
  check-skill.sh runs checkout code (each checked skill's scripts/*.test.sh and the checkout's
  markdownlint) with the network open, the same power scripts/run-test-file.sh has.
• .github/actions/collect-base-activity/action.yml: the generated verify-script wrapper runs
  unset GH_TOKEN GITHUB_TOKEN before the script, the line scripts/run-test-file.sh already
  runs before its test. One line in the generator covers every listed script, so no second
  token-stripping wrapper script is added.
• Schema description, README.md, pr-run-activity.md and the convention CHANGELOG.md (2.7.0)
  say the same.

Residual risk (the owner should confirm it): the unset clears the token from the script's own
environment only. Code the script runs, such as a skill test the lane wrote, can still read the
lane's push-capable token from the wrapper's or the Bash shell's /proc/<pid>/environ, and the
network is open. scripts/run-test-file.sh has exactly the same gap today. The owner approved this
design believing "no token", so the docs now state the gap plainly. Containment (a separate PID
namespace with a fresh /proc) is deferred to #7919.

Not added: scripts/sync-shared-copies.sh (#7375). It runs cd "$script_dir/.."
(scripts/sync-shared-copies.sh:18-19), so under the wrapper it works on the read-only base copy,
not the checkout. Its registry's first canonical, lib/html-escape.mjs, is not in that copy, which
holds only scripts/, so load_registry exits 2 (:42-45). --check-bump can call gh api
(scripts/lib/changelog-fragments.sh:279). The generator writes one Bash($verifier *) rule per
script, so it cannot limit the script to its read-only modes. To make it fit, the script would have
to resolve the checkout from its working directory and the wrapper would have to fix its flags.

Basis:

• Under dontAsk, a call that would prompt is denied and permissions.allow rules still run:
  permissions, permission modes table (dontAsk
  row), read 2026-10-17.
• Bash(<wrapper> *) matches the bare wrapper and the wrapper with arguments, and needs the space:
  same page, "Wildcard patterns". A rule must match each subcommand of a compound command, and an
  allow rule does not match past a leading assignment of a variable outside the known-safe set:
  same page, "Compound commands" and the environment-variable paragraph after it. So the lane
  cannot set GH_TOKEN=... or CHECK_SKILL_* in front of the wrapper, or chain it.
• The GitHub token is in the lane's Bash environment: CLAUDE_CODE_SUBPROCESS_ENV_SCRUB "leaves
  GitHub tokens and proxy settings in place"
  (env-vars, read 2026-10-17), set at
  .github/workflows/pr-run-activity-write.yml:966, with the lane token passed at :981.
• The generator writes one Bash($verifier *) rule per script, from a read-only base copy:
  .github/actions/collect-base-activity/action.yml:271-330.
• check-skill.sh runs each skill's scripts/*.test.sh (plugins/skill-quality/scripts/check-skill.sh:1298-1327)
  and markdownlint through npx --no-install or the nearest node_modules/.bin
  (:1249-1296).
• UNVERIFIED: whether other runner variables (for example ACTIONS_RUNTIME_TOKEN or
  ACTIONS_ID_TOKEN_REQUEST_TOKEN) reach the lane's Bash; the docs row names GitHub tokens and
  proxy settings only. This PR strips the same two names run-test-file.sh strips.

Verification

• Wrote each test first and saw it fail:
  • collect-base-activity.test.mjs: "the verify-script wrapper runs the script without GH_TOKEN
    or GITHUB_TOKEN and keeps the rest of the environment" failed with GH_TOKEN=fake-gh before
    the generator change.
  • "skill-quality's checks get wrapper rules, and check-skill runs a skill's test scripts without
    the GitHub token" stages the real plugins/skill-quality, runs check-skill.sh through the
    wrapper on a fixture skill whose test prints the tokens, and failed with
    seen=fake-gh,fake-github when the unset line was removed.
  • live-config.test.mjs: the resolve-issue verify-scripts list failed before the yaml edit.
• node --test .github/actions/resolve-config/*.test.mjs .github/actions/collect-base-activity/*.test.mjs:
  382 pass, 0 fail.
• bash scripts/write-runner-never-rules.test.sh: PASS=83 FAIL=0.
• actionlint .github/workflows/pr-run-activity-write.yml and zizmor --offline .github/actions/collect-base-activity/action.yml: no findings.
• markdownlint on the three changed docs: 0 issues.
• review:security-reviewer (opus): pass-with-findings. P2: the /proc/<pid>/environ gap above
  (reproduced locally; the docs are fixed and containment is deferred to Hide the lane token from checkout code run by verify-scripts #7919). P5: check-skill's
  markdownlint lookup walks above the checkout for node_modules/.bin. It needs code execution the
  run already has, so it is not fixed.

Related

• discovery:research: require inline source links when the parent presents findings #7840: the issue whose run stopped.
• work-items/work: guard the pull-request create reference #7598: where check-skill.sh was first left out.
• Hide the lane token from checkout code run by verify-scripts #7919: follow-up to hide the token from checkout code.
• Supersedes skill-quality: check-skill mode that skips bundled tests, for the issue lane #7649: the lane runs the full check-skill.sh with the token unset in its own
  environment, so no skip-tests mode is needed. Until Hide the lane token from checkout code run by verify-scripts #7919 lands, the skill tests it runs can
  still read the token from an ancestor process (see Residual risk).
• fix(prerequisites): check CLAUDE_PLUGIN_DATA belongs to the plugin before node-pkg and python-pkg read it #7375: needs sync-shared-copies.sh, which does not fit as written (see Fix).

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-18T02:59:58.457118Z	f95317d	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed f95317df13db3dd706246290f0b41859b5fea9f5: 0 finding(s), 0 as inline comments.

Security review: PR #7912 at f95317d

Findings: 0

The PR adds plugins/skill-quality/scripts/check-skill.sh and check-evals-quality.sh to the issue lane's verify-scripts. It also makes every verify-script wrapper unset GH_TOKEN and GITHUB_TOKEN before it runs the script.

What I checked

• New power: check-skill.sh runs each checked skill's scripts/*.test.sh and the checkout's markdownlint (plugins/skill-quality/scripts/check-skill.sh, checks 6 and 7). So the lane model can write a test script in the checkout and run it with the network open, and that script can read the lane token from an ancestor's /proc/<pid>/environ. This is the main security question in the diff.
• Existing grant: that power is not new. scripts/run-test-file.sh already runs checkout *.test.sh, *.test.mjs and vitest files for the same lane, with the same token-unset behaviour. The change adds a second route to a capability the lane already holds.
• Disclosure: the PR states the trade-off in pr-pipeline.yaml, the README, the schema description, pr-run-activity.md and the CHANGELOG. The test comment cites an owner decision dated 2026-10-17, which I did not independently verify.
• Wrapper change: the added unset GH_TOKEN GITHUB_TOKEN line only narrows what the script inherits. The new test checks that the token is unset and that the rest of the environment is kept. The existing refusal of path arguments outside the checkout is unchanged.
• check-evals-quality.sh: the PR describes it as bash and jq over evals.json. I did not trace its code, so I have no finding on it.
• Instruction-surface lens: the diff narrows no guardrail. The only text it removes is the comment explaining why check-skill.sh was left out. The control that comment described, the wrapper's path refusal, is still in place.

Residual note (not a finding): this lane reads untrusted issue text while holding push credentials, and /proc/<pid>/environ of an ancestor process stays readable. The PR documents that limit openly. Moving the token out of process environments would be a follow-up, not a defect in this diff.

Actions-hardening items such as permission changes and pins are zizmor's lane, and this diff has none of them. I did not consult docs/conventions/instruction-exception-register/README.md. The diff deletes no standing rule, so I did not need it.

[github-actions]

claude-review reviewed f95317df13db3dd706246290f0b41859b5fea9f5: 0 finding(s), 0 as inline comments.

Code review: PR #7912

No findings.

What I checked

• Generator change (collect-base-activity/action.yml): the unset GH_TOKEN GITHUB_TOKEN line runs after the outside-the-checkout path refusal and before the git-config exports and bash "$copy" "$@". Each wrapper runs in its own process, so the unset reaches only the script's own environment, as the PR says. The other listed scripts (cant-fail-scan.sh, git-mv-rm.sh, run-test-file.sh) are not documented as needing the token. I did not read them.
• Newly listed scripts: I grepped check-skill.sh and check-evals-quality.sh for GH_TOKEN, GITHUB_TOKEN and gh api. Neither matched, so stripping the token should not break them. check-evals-quality.sh matches its description as bash and jq over evals.json files; I read only part of it, by grep.
• Docs, yaml, schema and CHANGELOG: they agree with each other and with the generator change. They state the residual /proc/<pid>/environ gap and the open network instead of claiming the token is hidden. Containment is deferred to Hide the lane token from checkout code run by verify-scripts #7919.
• Tests: the new tests cover the unset, the kept environment, the wrapper rules for both scripts, and check-skill.sh running a skill's test without the token. The live-config list assertion is updated to match the yaml. I did not run them; the Verification section's pass counts are author-claimed and unverified by me.
• Closing issues: the PR uses Refs: only, so no closing-issue criteria apply.

Noted, not a finding

• This PR widens what the issue lane may do unattended, since check-skill.sh runs checkout code with the network open. The PR says the repository owner chose this design. Whether that approval holds, given the owner approved it believing the token was hidden, is for the human merger.