Skip to content

Harden greenlight reviewer against instruction-file injection from the reviewed checkout - #8435

Merged
jeanschmidt merged 1 commit into
mainfrom
jeanschmidt/greenlight_hardening_fixes
Aug 5, 2026
Merged

Harden greenlight reviewer against instruction-file injection from the reviewed checkout#8435
jeanschmidt merged 1 commit into
mainfrom
jeanschmidt/greenlight_hardening_fixes

Conversation

@jeanschmidt

Copy link
Copy Markdown
Contributor

Impact: greenlight PR-reviewer workflow
Risk: medium

What

Adds two fail-closed controls to greenlight-pr-review.yml, both running before the model: a sanitize step that strips every in-repo AI-assistant instruction file (CLAUDE.md, CLAUDE.local.md, AGENTS.md, .claude/, .cursorrules, .github/copilot-instructions.md) from the untrusted ./pytorch checkout and restores only pytorch main's trusted .claude/skills/, plus a deny-root runtime detector that fails the review if any instruction file actually loaded from under ./pytorch (or if the sanitize/hook machinery never ran).

Why

The reviewer checks out the PR's pytorch/pytorch tree and points Claude Code at it. Any of those instruction files auto-loads as reviewer steering, so a reviewed PR could plant a CLAUDE.md (or equivalent) to override the reviewer's own task and coax a favorable verdict. Stripping them at the source plus a fail-closed detector that catches a poisoned-but-successful run closes that injection vector. This is the first hardening item from the post-MVP threat-model pass.

Notes

  • The detector depends on the SessionStart and InstructionsLoaded hooks firing under the pinned claude-code-action; it fails closed if they don't, but the first live dispatch should confirm they actually fire.
  • The forbidden-name set is single-sourced in the sanitize script and must be re-verified on any claude-code-action / Claude Code CLI bump (last verified CLI 2.1.169 / action v1.0.141). claudeMdExcludes in the settings is insurance only, a no-op on the pinned CLI — not a substitute for the sanitize step.
  • This does not close the higher-severity cred-exfil gap: the model keeps unrestricted Read and its verdict message is not secret-scrubbed, so a data-injection payload could still coax it to read a credential and emit it. Constraining Read and scrubbing the published message remain open follow-ups.
  • The new hooks live at .claude/hooks/greenlight/ and are excluded from test-infra's shared lintrunner; they're linted/typechecked/tested through greenlight's own just recipes, run by greenlight-tests.yml (which now also triggers on changes to the reviewer workflow).

…e reviewed checkout

**Impact:** greenlight PR-reviewer workflow (CI only); no change to the `review` scan or the CLI service
**Risk:** medium

## What
Adds two fail-closed controls to `greenlight-pr-review.yml`, both running before the model:
a sanitize step that strips every in-repo AI-assistant instruction file (`CLAUDE.md`,
`CLAUDE.local.md`, `AGENTS.md`, `.claude/`, `.cursorrules`,
`.github/copilot-instructions.md`) from the untrusted `./pytorch` checkout and restores
only pytorch `main`'s trusted `.claude/skills/`, plus a deny-root runtime detector that
fails the review if any instruction file actually loaded from under `./pytorch` (or if the
sanitize/hook machinery never ran).

## Why
The reviewer checks out the PR's `pytorch/pytorch` tree and points Claude Code at it. Any
of those instruction files auto-loads as reviewer steering, so a reviewed PR could plant a
`CLAUDE.md` (or equivalent) to override the reviewer's own task and coax a favorable
verdict. Stripping them at the source plus a fail-closed detector that catches a
poisoned-but-successful run closes that injection vector. This is the first hardening item
from the post-MVP threat-model pass.

# Notes
- The detector depends on the `SessionStart` and `InstructionsLoaded` hooks firing under
  the pinned `claude-code-action`; it fails closed if they don't, but the first live
  dispatch should confirm they actually fire.
- The forbidden-name set is single-sourced in the sanitize script and must be re-verified
  on any `claude-code-action` / Claude Code CLI bump (last verified CLI 2.1.169 / action
  v1.0.141). `claudeMdExcludes` in the settings is insurance only, a no-op on the pinned
  CLI — not a substitute for the sanitize step.
- This does **not** close the higher-severity cred-exfil gap: the model keeps unrestricted
  `Read` and its verdict `message` is not secret-scrubbed, so a data-injection payload
  could still coax it to read a credential and emit it. Constraining `Read` and scrubbing
  the published message remain open follow-ups.
- The new hooks live at `.claude/hooks/greenlight/` and are excluded from test-infra's
  shared lintrunner; they're linted/typechecked/tested through greenlight's own
  just recipes, run by `greenlight-tests.yml` (which now also triggers on changes to the
  reviewer workflow).

Signed-off-by: Jean Schmidt <contato@jschmidt.me>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 4, 2026
@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
torchci Ignored Ignored Aug 4, 2026 9:27pm

Request Review

@jeanschmidt
jeanschmidt temporarily deployed to greenlight-record August 5, 2026 00:35 — with GitHub Actions Inactive
@jeanschmidt
jeanschmidt temporarily deployed to greenlight-record August 5, 2026 00:36 — with GitHub Actions Inactive
@jeanschmidt
jeanschmidt merged commit f12f993 into main Aug 5, 2026
14 checks passed
@jeanschmidt
jeanschmidt deleted the jeanschmidt/greenlight_hardening_fixes branch August 5, 2026 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants