[kyle-sexton]

No related issue: owner decisions Q52/Q53 (2026-10-24)

Summary

Owner rule: only a real security issue or the build no longer matching agreed intent stops a pull request for the owner. Two sandbox pull requests stopped today against that rule: pr-pipeline-sandbox#203 (a byte-identical check-trusted-trigger, 0 security findings, held as trust-gate) and pr-pipeline-sandbox#210 (restoring default turn and time limits, held as cost).

Fix

• Q52, trusted paths. hold-paths no longer holds. The compose step lists its globs in the prompt as TRUSTED PATHS for extra security scrutiny. The hold step no longer reads hold-paths or lists the changed files. A model trust-gate criterion holds nothing. trust-gate stays in the schema and in the extract step's allowlist, so a review skill that still returns it does not get the whole review dropped as malformed. A trusted-path change stops only through the security review's critical-security (CRITICAL) path.
• Q53, cost. The prompt's hold classes now replace the review command's own "Hold for a person" classes. cost holds only when the change can cause a real charge: enabling or changing a paid plan, extra usage, auto-reload or a payment method; Azure resources or deploys that bill; or GitHub billing (paid seats, paid Actions minutes, larger runners, Codespaces, storage). A change that only spends prepaid API credits or subscription usage (turn limits, timeouts, model or effort choice, more or longer lane runs, extra review runs, schedule frequency) is reported as one summary line. It is not a hold, an inline comment or a finding.
• The cost-decision carry logic (v0.65.4 to v0.65.6, including the deleted and edited marker rules) is unchanged.
• hold-paths stays an input with the same name. Its consumers, pr-review-hosted.yml and pr-review-security-hosted.yml in claude-code-plugins, need no change.

Verification

• nice node --test .github/scripts/*.test.cjs: 689 pass, 0 fail.
• New or rewritten tests failed before the fix: 7 in apply-review-hold.test.cjs (a hold-paths match holds nothing and reads no file list; trust-gate alone holds nothing while cost and critical-security still hold; a short file list holds nothing; the cost-decision call orders no longer include the file listing) and 1 in pr-review-cli.test.cjs (the prompt carries TRUSTED PATHS and the real-charge rule, identically in both legs). pr-review-cli-api-calls.test.cjs pins a hold-paths change at 8 requests with no hold and a critical-security hold at 11.
• actionlint and biome check pass on the changed files.

Related

• Follow-up outside this repository: claude-code-plugins plugins/review/skills/code-review/SKILL.md and security-review/SKILL.md still define the old cost and trust-gate hold classes. This release meets their recheck trigger ("a ci-workflows release changes the hold schema or its criteria"). The wrapper prompt overrides them in the meantime.

🤖 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-24T16:19:05.504624Z	03007c0	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.

[claude]

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

---

Code review complete

• Load review criteria (/review:code-review skill, REVIEW.md, path-scoped rules)
• Read full PR diff (.claude-lane/pr.diff)
• Check PR context / closing issue (none declared — PR body opens with No related issue: owner decisions Q52/Q53 (2026-10-24), satisfies pr-body-contract.md)
• Review .github/actions/apply-review-hold/action.yml and hold.cjs
• Review test changes (apply-review-hold.test.cjs, pr-review-cli.test.cjs, pr-review-cli-api-calls.test.cjs)
• Review pr-review-cli.yml workflow changes
• Review README.md changes
• Post findings

Scope note: this repository runs a dedicated security lane (pr-review-security.yml), so per REVIEW.md's code-review/security split, security findings are out of scope here and left to that lane.

Findings: 0. This is a tightly scoped, internally consistent change:

• hold.cjs's CRITERIA now correctly excludes trust-gate (cost, critical-security only), and pathHold/globToRegExp/PR_FILES-for-hold-paths are fully removed along with their tests, matching the PR's stated intent.
• The workflow's compose-step schema and extract-step allowlist (pr-review-cli.yml:966,1228,2038,2300) deliberately keep trust-gate in the enum/allowlist so a review that still returns it is not dropped as malformed — consistent with the PR description, and verified by reading both call sites in the duplicated review-key/review-subscription blocks, which are kept in lockstep.
• HOLD_PATHS is removed from the hold step's env (apply-review-hold step) and added to the compose step's env in both jobs, feeding the new TRUSTED PATHS prompt block instead — verified no other workflow in this repo references HOLD_PATHS/trust-gate, so no other caller needed updating.
• README.md and the action/workflow header comments were updated in step with the behavior change (no stale prose referencing path-based holding).
• Test rewrites (apply-review-hold.test.cjs, pr-review-cli-api-calls.test.cjs, pr-review-cli.test.cjs) match the new behavior: trust-gate-alone holds nothing, cost/critical-security still hold, no file-listing calls happen for hold purposes, and the new prompt text (real-charge cost classes vs. prepaid-spend summary line) is asserted in both lanes.

Not independently verified: I could not execute node --test, actionlint, or biome check in this session (the sandbox requires interactive approval for node --test, unavailable here), so the PR's "689 pass, 0 fail" and lint results are author-claimed, not independently confirmed. Static read-through of the diff found no logic inconsistency between the implementation, its tests, and the documentation.

claude-lane-summary: lane=claude-review sha=03007c0a40ca5f5783a5a9e3f2a7263f8c049628 run=40110834025

[claude]

Claude finished @kyle-sexton's task in 4m 10s —— View job

---

Security review complete

• Gather context (diff, PR description, changed files)
• Review apply-review-hold/action.yml and hold.cjs changes
• Review pr-review-cli.yml workflow changes (hold schema, prompt)
• Review test file changes for coverage of new behavior
• Review README changes
• Check instruction-surface deletions (hold-paths removal) against protected classes
• Verify candidate finding (adversarial verifier: CONFIRMED) and report

Findings

1. CRITICAL — hold-paths deterministic trust-gate backstop is removed with no non-model fallback

Before this PR, hold.cjs held a pull request with criterion trust-gate whenever a changed file (or a rename's old name) matched a hold-paths glob — regardless of what the LLM review said. Downstream consumers (pr-review-hosted.yml / pr-review-security-hosted.yml in claude-code-plugins, per this PR's "Related" section) use hold-paths to cover trust-boundary surfaces such as check-trusted-trigger/**, trusted-actors/**, CODEOWNERS, .gitmodules.

This PR deletes that mechanism entirely:

• hold.cjs#L14: CRITERIA drops trust-gate, now ["cost", "critical-security"].
• hold.cjs#L23: modelHold() explicitly discards a model-returned trust-gate instead of honoring it.
• pathHold/globToRegExp/patterns (the glob-matching code itself) are deleted outright, and HOLD_PATHS is no longer passed into the hold step's env in pr-review-cli.yml (both legs).
• hold-paths is now only injected as advisory prompt text (pr-review-cli.yml#L990, #L2062) telling the model to apply "extra scrutiny," and the prompt tells the model outright not to return trust-gate (#L1021, #L2093).

Net effect: whether a change to trust-boundary code (e.g. weakening check-trusted-trigger's verification, or widening a trusted-actors allowlist) gets held now depends entirely on the LLM reviewer independently reaching a critical-security (CRITICAL) finding on its own. There is no remaining deterministic, non-model backstop in this code path — exactly the attack surface the old mechanism closed: a trust-boundary change crafted to read as benign to the model (obfuscation, misdirection, or injected text elsewhere in the diff/PR aimed at the reviewer) can now pass with no hold at all, where before it would have been held mechanically no matter what the model concluded. This repo's own CODEOWNERS does not substitute for it — it only requires owner review for .github/scripts/** and test files, not for check-trusted-trigger, trusted-actors, root CODEOWNERS, or .gitmodules. This was checked against the instruction-exception register's "removing a guardrail" lens (the register file itself is not present in this checkout, so the protected-class list was not directly consulted — this is recognition by consequence).

Note: the PR body frames this explicitly as a deliberate, reasoned owner decision ("Q52, trusted paths," with a concrete motivating incident). It is disclosed, not hidden, and the new tests (apply-review-hold.test.cjs) exercise the new behavior directly. Flagging it here so the trade-off — no deterministic backstop remains for trust-gate paths, only model judgment — is visible to the owner for a final go/no-go, per this lane's role of surfacing exactly this class of change.

severity: CRITICAL

Manifest check

No .claude-plugin/plugin.json or .claude-plugin/marketplace.json changes in this diff — not applicable.

Summary

One CRITICAL finding: the hold-paths mechanism's deterministic, code-enforced trust-gate hold is removed with no non-model fallback; a trust-boundary change can now evade a hold solely by not reading as CRITICAL to the LLM reviewer. This is the PR's stated, deliberate intent (owner decision Q52) and the rest of the diff (prompt wording, schema, tests, docs) is internally consistent with that intent. No other security-relevant issues found in the Actions-security-adjacent (zizmor-lane) sense, and no GitHub Actions hardening findings are reported here per this lane's scope.

critical-findings: 1

claude-lane-summary: lane=claude-security-review sha=03007c0a40ca5f5783a5a9e3f2a7263f8c049628 run=40110834025

[github-actions]

claude-review has reviewed this pull request through 03007c0; a later push is reviewed from there.

[github-actions]

claude-security-review has reviewed this pull request through 03007c0; a later push is reviewed from there.

[kyle-sexton]

Security review finding 1 (CRITICAL, no deterministic trust-gate backstop): this is the owner's decision, so it is not a defect to fix in this PR.

Finding	Classification	Basis
hold-paths no longer holds deterministically	INCORRECT as a defect: this is the intended design	Owner decision Q52 (2026-10-24): a trusted-path match must not by itself apply the hold. Those paths get extra security scrutiny, and a stop happens only when the security review reports a real finding. That is the behavior this PR implements.

What still stops a trusted-path change after this PR:

• The security review's own critical-security hold, with the hold-paths globs now named in its prompt as TRUSTED PATHS for extra scrutiny.
• In claude-code-plugins, two checks outside this lane:
  • babysit's trust-gate:trusted-actors HIGH floor, which still waits for the owner on a change that widens the trusted-actors list (plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py:40).
  • The pr-require-checks.yml gate for an author no review lane reviews (lines 3886 to 3956).

The finding text and this classification go to the owner in the session report. Nothing here changes code.