Skip to content

[security] - Split @claude into a read-only default and an owner-approved write mode - #1725

Merged
cgfixit merged 1 commit into
mainfrom
claude/claude-yml-allowlist
Oct 20, 2026
Merged

cgfixit merged 1 commit into
mainfrom
claude/claude-yml-allowlist

Conversation

@cgfixit

@cgfixit cgfixit commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Proposed changes

#1709 item 1. .github/workflows/claude.yml used to run a single job with contents: read and pull-requests/issues: write, with no claude_args. Claude got the action's default tool set. Pushes could not succeed, but nothing said so, and there was no deliberate way to grant write.

It is now three jobs:

Job Runs when Permissions Tools
gate the same owner-only if: as before: comment author, actor and triggering_actor are all the owner; open PR; @claude in the body pull-requests: read none (github-script)
review gate says read (the default) contents: read, PR/issues write action defaults minus Edit,MultiEdit,Write,NotebookEdit,Bash(git add/commit/rm:*),WebFetch,WebSearch
write the comment's last line is write access approved for this PR (case-insensitive, one trailing ./! allowed) contents: write, PR/issues write action defaults minus WebFetch,WebSearch

The fork-PR refusal (#1526 F8) moved from the old single job's first step into gate. A fork PR fails there, so neither Claude job starts, even when the comment carries the approval line.

Each mode also gets an --append-system-prompt framing line. Read mode is told to review only. Write mode is told to push only to the PR head, never force-push or rebase, follow CLAUDE.md, and leave locks/, soul.md and .githooks/ alone. The framing is a soft control. The hard controls are job selection, job permissions and the deny list.

Checked against the source of claude-code-action v1.0.252 (f27be6b), not guessed:

  • Setting the prompt: input switches a comment run from tag mode to agent mode (src/modes/detector.ts). The framing therefore goes through claude_args --append-system-prompt, which is the documented replacement for custom_instructions.
  • The tag-mode allow list and the git tool names come from src/modes/tag/index.ts: Bash(git add:*), Bash(git commit:*), Bash(git rm:*) and the push wrapper. Edits are allowed through --permission-mode acceptEdits, which is why Edit and Write are denied explicitly. Deny rules outrank allow rules.
  • The push wrapper, scripts/git-push.sh, refuses flags and any remote other than origin. It does not restrict the branch name. Staying off main therefore rests on the action's system prompt plus main's branch protection. The public branches API reports main as protected: true; I have not confirmed that the ruleset blocks a GITHUB_TOKEN push.

Invariant / Governance Impact: none. This is a workflow-only change. No core file, graph edge, gate route or soul path is touched.

Types of changes

  • Security (auth, privacy, injection, egress, redaction, or secrets)
  • CI / infrastructure (workflows, hooks, templates, or checkers)

Scope: Infrastructure / CI only.

Benefits / why

  • Read is the default, and permissions enforce it. A prompt-injected PR body cannot talk the read job into pushing, because its token cannot write contents.
  • Write needs a deliberate act. The phrase has to be on the comment's last line. Quoting it mid-comment ("what does … do?") stays read-only, and a test covers that case.
  • Web tools are denied in both modes. That closes the obvious outbound channel for anything the job can read. In write mode the action embeds the job token in the .git/config remote URL.
  • Easy to extend later. The phrase check is one regex. Adding a collaborator or a label trigger means changing gate only.

Risks to monitor

  • Write mode is still an LLM with push rights to the PR branch. It can push wrong code there. It cannot change .github/workflows/, because GITHUB_TOKEN cannot, and main protection should stop a push to main. Signal: an unexpected claude[bot] commit on the PR.
  • cancel-in-progress: true is unchanged. A second @claude comment on the same PR cancels a running write job, possibly mid-push.
  • The phrase is exact. "write access approved for this pr branch", with "branch" on the end, does not match and runs read-only.
  • It cannot be tested live before merge. issue_comment workflows always run the default branch's copy, so a comment on this PR still runs the old single job. The first real test happens on the first @claude comment after merge.

Checklist

  • I have read docs/THREAT_MODEL.md, INVARIANTS.md and SECURITY.md (and any relevant Phase docs)
  • This change preserves all 6 security invariants and I6 module isolation (no core file touched)
  • Full sandbox validation has been run (per CLAUDE.md §5, left to CI)
  • No new external network dependencies or mandatory online LLM assumptions were introduced
  • Commit messages follow the title prefix convention above

Further comments

Run directly:

  • The gate script, under Node, via the new tests/test_claude_workflow_security_contract.py. All 13 tests pass on Python 3.12. They cover:
    • read as the default: a plain request, the phrase quoted mid-line, the phrase not on the last line, a partial phrase
    • write: exact, uppercase, trailing period and padded variants, and a CRLF body
    • fork and deleted-fork refusal, even with approval
    • the closed-PR refusal
    • job permissions and the needs/if wiring
    • the read-mode deny list
  • actionlint: clean.
  • zizmor 1.28.0 --offline (the CI pin): no findings, 5 suppressed.
  • ruff: clean.

Only CI ran: the full suites.

Not run: a live @claude comment. That is only possible after merge, as noted in Risks above.

Post-merge smoke test:

  1. On any open same-repo PR, comment @claude explain what this PR changes. Expect the gate and review jobs to run and write to be skipped.
  2. On a throwaway PR, comment @claude fix the typo in X, with write access approved for this PR as the last line. Expect gate and write to run, and a commit on that PR's branch.

Suggested merge order of open PRs

Trial merge verified (git merge --no-commit --no-ff + abort) against main @ 770d09e on 2026-10-20.

This PR (#1725) · Blocked (draft)
Impact: @claude read/write split.
Action: Review, mark ready, merge, then run the two-comment smoke test above. Clean.

#1724 · Blocked (red by construction)
Impact: dependabot's 55-package bump; the lock records were not regenerated.
Action: Do not merge. Close or curate on the Mac.

ELI5

Before, @claude on a PR was a reviewer whose pen had no ink. It could not save changes, but nothing said so, and there was no way to hand it ink on purpose. Now a doorman reads your comment first: the gate job in .github/workflows/claude.yml.

Normally the doorman sends in the reviewer without ink. That is the review job: its contents: read token cannot push, and its edit and commit tools are switched off as well. If your comment's last line is exactly "write access approved for this PR", it sends in the write job instead, whose token can push to that PR's branch. Fork PRs are turned away at the door either way.

  • Enhances: a deliberate, one-line way to let Claude fix things.
  • Obsoletes: the old single job and its unstated limit.
  • Retrieves: the reviewer/poster split the 2026-07-21 audit (F7) asked for.
  • Reverses when pushed to extremes: the write job is still a model with push rights to one branch.

The safety comes from which job runs and what its token may do, not from asking the model nicely. The one soft spot is the action's push helper, which does not check the branch name, so main's protection is the real backstop. I ran the doorman logic, lint and zizmor directly. A live comment test is only possible after merge, because comment-triggered workflows run from main.

🤖 Generated with Claude Code

https://claude.ai/code/session_014FZ8JTF2v1eQd1L5CKQR8j

Last updated: 2026-10-20 09:35 ET

…oved write mode

claude.yml now runs a gate job that refuses fork PRs and picks a mode. Read
is the default: contents read, and the edit/commit/web tools are denied.
Write runs only when the owner's comment ends with the line "write access
approved for this PR": contents write, web tools still denied. The mode is
enforced by which job runs and its permissions, not by the prompt.

Adds tests/test_claude_workflow_security_contract.py, which runs the gate's
embedded script under Node against mocked PRs and comments. Refs #1709.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FZ8JTF2v1eQd1L5CKQR8j
@cgfixit
cgfixit marked this pull request as ready for review October 20, 2026 13:36
@cgfixit
cgfixit merged commit cdcef76 into main Oct 20, 2026
63 checks passed
@cgfixit
cgfixit deleted the claude/claude-yml-allowlist branch October 20, 2026 13:36
@cgfixit cgfixit mentioned this pull request Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants