Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion .claude/skills/analyze-action-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

---
name: analyze-action-pr
description: Triage a PR that adds or bumps an action in this repo's allowlist. Runs verify-action-build, classifies each failing action (clean / pipe-to-shell / unverified-download / nested-action-issue / verify-script-bug), and proposes concrete next actions — recommend approval, open an upstream issue + ping the PR author, or fix verify-action-build itself with a regression test. Use when the user says "analyze PR <N>", "triage PR <N>", "verify PR <N>", or otherwise asks to review an action-allowlist PR in this repo.
description: Triage a PR that adds or bumps an action in this repo's allowlist. Runs verify-action-build, classifies each failing action (clean / pipe-to-shell / unverified-download / nested-action-issue / metadata-only / verify-script gap / in-tree native binaries) and proposes concrete next actions — recommend approval, open an upstream issue + ping the PR author, or fix verify-action-build itself with a regression test. Use when the user says "analyze PR <N>", "triage PR <N>", "verify PR <N>", or otherwise asks to review an action-allowlist PR in this repo.
---

# Analyze an apache/infrastructure-actions PR
Expand Down Expand Up @@ -69,6 +69,7 @@ under "Classify".
| **C** | nested-action issue | Top-level action passes but a `uses:` dependency (e.g. `install/foo`) hits A or B |
| **D** | metadata-only | `No LICENSE`, input interpolation in `run:` blocks, `GITHUB_PATH` writes — soft warnings, mention but don't block |
| **E** | verify-script gap | The script gets the wrong answer for a reason unrelated to the action's actual security: false positive (regex hole, missing pattern), missing capability (new action type / build flow / verification mechanism it doesn't yet recognize), bad attribution (extractor drops an action that's clearly in the diff), or a check that misreads a legitimate input shape |
| **F** | unverified in-tree binaries | The action ships pre-compiled native binaries directly in the repo (Go cross-compile, `.exe` / `.dll` / `.so` / `.dylib`, `.jar`, `.wasm`, etc.) and exec's them from a small launcher. `verify-action-build`'s `In-tree binary check` tries to reconcile each binary with verifiable upstream provenance — first via `gh attestation verify --owner <org>` (SLSA attestation transparency log), then by comparing SHA256 against the release's `SHA256SUMS` asset. Binaries that pass either check are ✓; those that pass neither are case **F** and should be rejected until upstream adds provenance (`actions/attest-build-provenance` or a signed `SHA256SUMS`). Successful verification is a hard pass: the binary's bytes are tied back to the workflow run that produced them or to a release-time checksum |

### 4. Look up upstream verification material (for A/B/C)

Expand Down Expand Up @@ -116,6 +117,50 @@ Two messages, both held until the user OKs:

Do **not** approve the action.

#### F — unverified in-tree native binaries

`verify-action-build`'s `In-tree binary check` reports a per-binary
result. Three outcomes:

- ✓ **Verified via `gh attestation verify`** — the binary's bytes are
in GitHub's attestation transparency log under the action's owner.
Pass; recommend approval.
- ✓ **Verified against `SHA256SUMS` release asset** — the binary's
SHA256 matches the release's checksum file. Slightly weaker than
attestation (no signature on the checksum file itself yet) but a
strong cryptographic chain from release to artifact. Pass.
- ✗ **Unverified** — neither mechanism worked: no attestation, no
matching SHA256SUMS entry, or the hash didn't match. Hard reject.

For the unverified case, two messages, both held until the user OKs:

1. **Upstream issue** asking the action's maintainers to add either
`actions/attest-build-provenance` to their release workflow (so the
binaries get a SLSA attestation verifiable via `gh attestation
verify`) or a `SHA256SUMS` file shipped as a release asset. Quote
the offending paths and explain the downstream-review impact. See
the runs-on/action#36 / runs-on/action#37 thread as a worked
example: the maintainer accepted a one-line PR that added
`actions/attest-build-provenance` and started shipping `SHA256SUMS`
on each release; once that landed, future bumps verified
automatically.
2. **Comment** on the ASF PR pinging the PR author, summarising why
approval is held: the JS-rebuild check only verifies the launcher,
not the binary, and we have no way to tie what runs on the runner
back to the published source. Link the upstream issue.

Before recommending retroactive rejection of an already-approved version
in this shape, **check downstream impact** with code search:

```
gh api 'search/code?q=<org>%2F<action>+org%3Aapache+language%3AYAML' \
--jq '.items[] | "\(.repository.full_name) \(.path)"'
```

If multiple ASF projects depend on the action, loop in their maintainers
(e.g. `@apache/<project>-committers`) before any retroactive decision —
they pay the cost of pinning back to an older version or migrating off.

#### D only (passing verification but with metadata warnings)

Note the warnings and recommend approval — the user typically approves
Expand Down Expand Up @@ -229,3 +274,6 @@ future runs can cite a precedent instead of re-deriving the analysis.
| #802 | carabiner-dev nested `install/{ampel,bnd}` do `curl + chmod 0755`; upstream ships SLSA `provenance.json` / `attestations.jsonl` | C | Upstream issue carabiner-dev/actions#51 + PR comment |
| #803 | 3 actions in one diff; extractor only got the wholly-new key | E | Fix landed in PR #804 |
| #806 | `jbangdev/setup-jbang` does `curl ... \| bash`; upstream ships SHA256/GPG | A | Upstream issue jbangdev/setup-jbang#16 + PR comment |
| #813 | `browser-actions/setup-firefox@v1.7.2` ships a minimal `{"type":"module"}` package.json with no deps; lock-file check too strict | E | Fix landed in PR #816 |
| #809 | `runs-on/action@v2.1.1` ships ~10 MB of UPX-packed Go binaries (`main-linux-amd64`, `main-linux-arm64`, `main-windows-amd64.exe`); launcher exec's them as root; no SLSA, no SHA256SUMS | F | Upstream issue runs-on/action#36; deferred until upstream adds provenance |
| #825 | `runs-on/action@v2.1.2` — same in-tree binaries as v2.1.1, but upstream now ships SLSA attestations (`actions/attest-build-provenance` was wired in via runs-on/action#37) plus a `SHA256SUMS` release asset | F (verified) | Pass — `In-tree binary check` reports all 3 binaries verified via `gh attestation verify` |
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ When reviewing an action (new or updated), watch for these potential issues in t
- **Obfuscated code**: hex-encoded strings, base64 blobs, or intentionally unreadable code in source files (not in compiled `dist/`).
- **File-system tampering**: writing to locations outside the workspace (`$GITHUB_WORKSPACE`), modifying `$GITHUB_ENV`, `$GITHUB_PATH`, or `$GITHUB_OUTPUT` in unexpected ways to influence subsequent workflow steps.
- **Compiled JS mismatch**: any unexplained diff between the published `dist/` and a clean rebuild — this is the primary check the verification script performs.
- **Pre-compiled native binaries shipped in-tree**: actions that commit Go/Rust/C-style binaries (`main-linux-amd64`, `*.exe`, `*.dll`, `*.so`, `*.dylib`, `*.jar`, `*.wasm`, etc.) directly in the repo and exec them from a small launcher are running opaque executable code on the runner. The JS-rebuild check verifies the launcher but **cannot** reconcile the binaries with source on its own. `verify-action-build`'s **In-tree binary check** tries to close the gap automatically: each detected binary is verified first via `gh attestation verify --owner <org>` (the SLSA attestation transparency log populated by [`actions/attest-build-provenance`](https://github.com/actions/attest-build-provenance)), then by SHA256-comparing each binary against the release's `SHA256SUMS` asset. Binaries that pass either check are ✓; binaries that pass neither are a hard reject. Push back on actions in this shape until upstream adds attestation or `SHA256SUMS` so the chain from release to artifact can be verified.

For the full approval policy and requirements, see the [ASF GitHub Actions Policy](https://infra.apache.org/github-actions-policy.html).

Expand Down
Loading