-
Notifications
You must be signed in to change notification settings - Fork 0
docs: document OIDC immutability constraint and exempt claude.yml from SHA pinning #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -235,6 +235,22 @@ | |
| AI-assisted code review on PRs and issue automation via Claude Code Action. | ||
| A copy-paste ready template is available at [`standards/workflows/claude.yml`](workflows/claude.yml). | ||
|
|
||
| > **OIDC security constraint — `claude.yml` is immutable on PR branches.** | ||
| > Anthropic's token endpoint validates that `.github/workflows/claude.yml` on | ||
| > a PR branch is byte-for-byte identical to the same file on the default branch. | ||
| > Any diff — including SHA-pinning the `uses:` line, adding a trigger, or | ||
| > changing a comment — causes the OIDC token exchange to fail: | ||
| > ``` | ||
|
Check failure on line 243 in standards/ci-standards.md
|
||
| > App token exchange failed: 401 Unauthorized — Workflow validation failed. | ||
| > The workflow file must exist and have identical content to the version | ||
| > on the repository's default branch. | ||
| > ``` | ||
|
Check failure on line 247 in standards/ci-standards.md
|
||
| > Claude Code will not run on that PR. Agents must not open PRs that modify | ||
| > `.github/workflows/claude.yml`. The caller stub template now includes a | ||
| > `paths-ignore` guard that prevents this workflow from triggering on PRs that | ||
| > only change this file. See also [Action Pinning Policy](#action-pinning-policy) | ||
| > for the reusable workflow ref exemption. | ||
|
|
||
|
Check failure on line 253 in standards/ci-standards.md
|
||
|
Comment on lines
+238
to
+253
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix markdownlint failures in the OIDC callout — CI is red. The fenced block inside the blockquote violates MD031 (no surrounding blank lines inside the blockquote), MD040 (missing language), and MD028 (blank line at 253 between this blockquote and the next breaks them into two). Pipeline is failing on MD031. 🛠️ Proposed fix > **OIDC security constraint — `claude.yml` is immutable on PR branches.**
> Anthropic's token endpoint validates that `.github/workflows/claude.yml` on
> a PR branch is byte-for-byte identical to the same file on the default branch.
> Any diff — including SHA-pinning the `uses:` line, adding a trigger, or
> changing a comment — causes the OIDC token exchange to fail:
-> ```
+>
+> ```text
> App token exchange failed: 401 Unauthorized — Workflow validation failed.
> The workflow file must exist and have identical content to the version
> on the repository's default branch.
> ```
+>
> Claude Code will not run on that PR. Agents must not open PRs that modify
> `.github/workflows/claude.yml`. The caller stub template now includes a
> `paths-ignore` guard that prevents this workflow from triggering on PRs that
> only change this file. See also [Action Pinning Policy](`#action-pinning-policy`)
> for the reusable workflow ref exemption.
-
> **All three jobs require a checkout step.** The `claude` job (PR reviews), theIf the blockquote separation at line 253 is intentional (two distinct callouts), keep the blank line but ensure markdownlint config accepts MD028 — the current default does not. 🧰 Tools🪛 GitHub Actions: CI[error] 243-243: markdownlint-cli2 reported MD031/blanks-around-fences: Fenced code blocks should be surrounded by blank lines [Context: "> ```"]. 🪛 GitHub Check: Lint[failure] 253-253: Blank line inside blockquote [failure] 247-247: Fenced code blocks should be surrounded by blank lines [failure] 243-243: Fenced code blocks should have a language specified [failure] 243-243: Fenced code blocks should be surrounded by blank lines 🪛 LanguageTool[uncategorized] ~239-~239: The official name of this software platform is spelled with a capital “H”. (GITHUB) [uncategorized] ~248-~248: The official name of this software platform is spelled with a capital “H”. (GITHUB) 🤖 Prompt for AI Agents |
||
| > **All three jobs require a checkout step.** The `claude` job (PR reviews), the | ||
| > `claude-issue` job (issue automation), and the `claude-ci-fix` job (CI failure | ||
| > response) each need `actions/checkout` **before** the `claude-code-action` step. | ||
|
|
@@ -737,6 +753,30 @@ | |
| > example to a repository, always look up the current SHA for each action and | ||
| > pin to it with a version comment. | ||
|
|
||
| ### Exception: Internal Reusable Workflow References | ||
|
|
||
| Calls to `petry-projects/.github` reusable workflows use tag references | ||
| (`@v1`, `@main`) — **not SHA pins** — and are exempt from this policy. | ||
|
|
||
| ```yaml | ||
| # CORRECT — tag ref for internal reusable workflow | ||
| uses: petry-projects/.github/.github/workflows/claude-code-reusable.yml@v1 | ||
|
|
||
| # WRONG — do not SHA-pin internal reusable workflow refs | ||
| uses: petry-projects/.github/.github/workflows/claude-code-reusable.yml@ee22b427cbce9ecadcf2b436acb57c3adf0cb63d | ||
| ``` | ||
|
|
||
| **Why:** Pinning the `uses:` line in a Tier 1 caller stub creates a diff from | ||
| the default branch. Anthropic's OIDC token endpoint validates that | ||
| `.github/workflows/claude.yml` on a PR branch is identical to the default | ||
| branch — any diff causes `401 Workflow validation failed` and Claude Code | ||
| cannot run on that PR. | ||
|
|
||
| The `@v1` tag on `petry-projects/.github` is managed deliberately (bumped only | ||
| on backward-compatible releases) and is not subject to tag-force-push risk | ||
| because the org controls the tag. **Do not open compliance PRs to pin these | ||
| references.** | ||
|
|
||
|
Comment on lines
+756
to
+779
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
rg -nP -C3 'check_action_pinning|unpinned-actions' scripts/compliance-audit.shRepository: petry-projects/.github Length of output: 998 🏁 Script executed: sed -n '186,250p' scripts/compliance-audit.shRepository: petry-projects/.github Length of output: 2532 🏁 Script executed: rg -n 'petry-projects.*\.github' scripts/compliance-audit.sh | head -20Repository: petry-projects/.github Length of output: 2479 🏁 Script executed: rg -n '@v1|@main' scripts/compliance-audit.sh | grep -i 'filter\|exempt\|exclude'Repository: petry-projects/.github Length of output: 48 🏁 Script executed: sed -n '756,779p' standards/ci-standards.mdRepository: petry-projects/.github Length of output: 1137 Add whitelist for The 🧰 Tools🪛 LanguageTool[uncategorized] ~758-~758: The official name of this software platform is spelled with a capital “H”. (GITHUB) [uncategorized] ~770-~770: The official name of this software platform is spelled with a capital “H”. (GITHUB) [uncategorized] ~775-~775: The official name of this software platform is spelled with a capital “H”. (GITHUB) 🤖 Prompt for AI Agents |
||
| --- | ||
|
|
||
| ## Permissions Policy | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
LGTM — exemption section is well-scoped.
Clear rationale per file, and the "adopt verbatim / updated only via standards PR" instruction lines up with the Tier 1 stub contract in
ci-standards.md. The LanguageTool "GitHub" capitalization hints on lines 25/26/29 are false positives — these are literal path/repo identifiers.Minor suggestion: consider adding a machine-readable signal (e.g., a known list consumed by
scripts/compliance-audit.sh) so the exemption is enforced at PR-open time rather than relying on agents reading this doc. Otherwise an agent that skips the doc will still attempt a compliance PR againstclaude.yml.🧰 Tools
🪛 LanguageTool
[uncategorized] ~25-~25: The official name of this software platform is spelled with a capital “H”.
Context: ... | File | Reason | |------|--------| |
.github/workflows/claude.yml| Anthropic OIDC ...(GITHUB)
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...; Claude Code cannot run on that PR | |
.github/workflows/agent-shield.yml| Security ...(GITHUB)
[uncategorized] ~29-~29: The official name of this software platform is spelled with a capital “H”.
Context: ...ted only by merging a standards PR from
petry-projects/.github, which propagates to all repos via the...(GITHUB)
🤖 Prompt for AI Agents