Skip to content
Merged
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
22 changes: 21 additions & 1 deletion .claude/skills/analyze-action-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ GITHUB_TOKEN=$(gh auth token) uv run python -m verify_action_build --from-pr <N>

Exit 0 = all actions pass. Exit 1 = at least one failed.

For a PR whose `verify` check has already run in CI, read that job's log
instead — it is the same tool with the same output, costs no Docker
rebuild, and is the only option when the local environment can't run one:

```
gh pr checks <N> --json name,link --jq '.[] | select(.name=="verify") | .link'
gh run view <run-id> --log --job <job-id> | sed -e 's/\x1b\[[0-9;]*[a-zA-Z]//g'
```

Strip the ANSI escapes (as above) or the summary table is unreadable. Re-run
locally only when you need to test a change to `verify-action-build` itself.

If the count of `Extracted action reference` lines is lower than the
number of new hashes in the diff, the extractor is buggy → see case **E**
under "Classify".
Expand All @@ -83,7 +95,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 |
| **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. **Not every unverified binary is case F**: one a bundler copied into the output directory from a lockfile-pinned dependency (a `.wasm`, `.node`, native lib) has npm's provenance chain, not the action's, so demanding a GitHub release attestation for it is case **E** — check whether the clean rebuild reproduces it before treating it as F |

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

Expand Down Expand Up @@ -210,6 +222,10 @@ as templates when proposing your own extension):
| Extraction shape | Hash added under an existing `actions.yml` key | [#804](https://github.com/apache/infrastructure-actions/pull/804) |
| Rebuild emits intermediate output the action doesn't ship | Two-stage `tsc` → `esbuild` build where OUT_DIR resolves to the sub-project, so the gitignored stage-one `lib/` appears only in the rebuild | see the #1123 row under "Recent precedents" |
| Per-ecosystem exemption | Lock-file rule too strict for library-first projects | [#770](https://github.com/apache/infrastructure-actions/pull/770) |
| New lock-file format | `aube-lock.yaml` (pnpm lockfileVersion 9) unrecognised | [#1175](https://github.com/apache/infrastructure-actions/pull/1175) |
| Registry tarball root assumed | DefinitelyTyped roots `@types/*` at the bare package name, not `package/` | [#1174](https://github.com/apache/infrastructure-actions/pull/1174) |
| Executable source not diffed or scanned | `.sh` / `Dockerfile*` outside the source-diff extension set, and script analysis limited to composite/docker actions | see the #1196 row under "Recent precedents" |
| Binary verifiable by rebuild, not by release | Bundler-copied dependency asset in `dist/` has npm provenance, not GitHub-release provenance | see the #1195 row under "Recent precedents" |

When you hit a case that's clearly one of these — or a new kind not
in the table — propose the fix. The cost of leaving a false positive
Expand Down Expand Up @@ -296,3 +312,7 @@ future runs can cite a precedent instead of re-deriving the analysis.
| #1123 | `JetBrains/qodana-action@v2026.2.0` — `JS build verification ✗` whose entire content was 4 files **only in the rebuild** (`lib/{annotations,main,output,utils}.js`). Two-stage build: `tsc --build` (`outDir: ./lib`) → `esbuild lib/main.js --bundle --outfile=dist/index.js`. `main: scan/dist/index.js` makes the Dockerfile's `cut -d'/' -f1` resolve OUT_DIR to the whole `scan/` sub-project, so stage-one output — gitignored upstream via `**/lib`, never committed — lands in the compared tree. **#960 hit this too and was merged over it; only the gradle-jar half got fixed in #951.** Also burns a pointless approved-lock-file retry rebuild | E | Fixed by treating only-in-rebuilt files as informational (not published → never runs on a consumer's runner → not a supply-chain vector), keeping only-in-original a hard failure and guarding the no-published-JS case. Action itself clean: dist bundle maps 1:1 to the TS source diff, `common/cli.json` checksums bumped in lockstep |
| #1013 | `jdx/mise-action@5228313` (v3.6.3, node20) — clean all-pass: dist matches rebuild, LICENSE + lockfile present, and its `curl \| tar` fetch of the mise binary is **not** flagged (verifier reports "no binary downloads detected" — it's the tool pulling its own tarball from its official pinned release). Checksum verification is opt-in only (`sha256` input, from mise-action#185), off by default | clean (soft D note) | Approved. Gentle hardening issue mise-action#547: verify by default via a **signature against a pinned key** (mise's `minisign.pub`, key `64113EDF160FDEC2`, stable since Dec 2024), not a same-source `SHASUMS256.txt` — a checksum fetched from the same release as the binary is integrity, not authenticity, and can't cover the `mise.jdx.dev` CDN path. **Outcome: upstream accepted and fixed it same-day in mise-action#548 (signed-checksum verification); #547 closed completed** |
| #1133 / #1134 | `pypa/gh-action-pypi-publish` v1.13.0→v1.14.2 (composite) and `hadolint/hadolint-action` v3.3.0→v3.4.0 (docker) — both clean all-pass, each with the single warning `Dockerfile FROM <image> is tag-pinned, not digest-pinned`. In both cases the **already-approved prior version carries the identical shape**, so the warning is a standing property of the action, not something the bump introduced. pypi-publish's `oidc-exchange.py` looks alarming at +83/-15 but is ~90% type annotations; the substantive edits are hardening (`os.getenv()`→`os.environ[]`, explicit `IdentityError` when `detect_credential()` returns `None`, `format_map(locals())`→explicit kwargs), with no new endpoints and no change to the token-exchange flow | clean (soft D note) | Approved and merged. **Rule of thumb: for a tag-pinned-base-image warning, diff against the approved version before treating it as a finding — carried-over warnings are not regressions.** Worth tracking as a general hardening theme (digest-pinning base images) rather than blocking individual bumps |
| #1167 | `jdx/mise-action@7e36c90` (v4.2.4) — single red check `Lock file presence ✗`, everything else green including `JS build verification ✓`. The repo *does* have a lock file: v4.2.4 moved the build to [aube](https://github.com/jdx/aube), which writes `aube-lock.yaml` (pnpm lockfileVersion 9, 460 deps pinned with sha512 integrity). The presence check only knew the five established filenames | E | Fix in PR #1175. **Confirm a "missing lock file" verdict by listing the repo root before believing it** — `gh api 'repos/<org>/<repo>/contents/?ref=<sha>'` |
| #1171 | `JamesIves/github-pages-deploy-action@fa24774` (v4.9.0) — `Vendored npm registry check ✗ 13 extra`, but 0 modified, 0 errors, 116 packages integrity-verified, `JS build verification ✓`, and the vendored `node_modules` comparison matching all 3008 files across 75 packages. The 13 "extra" files were *every* file of exactly three packages (`@types/esrecurse`, `@types/estree`, `@types/json-schema`) — DefinitelyTyped roots its tarballs at the bare package name, so nothing lined up with `node_modules/@types/<pkg>/<rel>`. Switched on for the first time by this version's yarn → npm migration adding `node_modules/.package-lock.json` | E | Fix in PR #1174. **Heuristic: when *all* files of a package are reported extra, that is tarball resolution failing, not tampering. Injected code shows up as a few extra files among many accounted-for ones** |
| #1196 | `uraimo/run-on-arch-action@460cb8e` (v3.2.0) — two findings. (a) `JS build verification ✗` is entirely vendored-`node_modules` drift (87 files / 3 packages committed vs 192 / 6 rebuilt); the upstream compare touches no `node_modules` file, so the already-approved v3.1.0 carries the identical shape — carried over, not a regression, and the stricter registry check never engages because there is no `node_modules/.package-lock.json`. (b) The tool's "source changes" section listed only `action.yml`, hiding `src/run-on-arch.sh` **+12/-1** and three new `Dockerfiles/Dockerfile.*` — and `src/run-on-arch.js:127` `exec()`s that very script, so the shell payload *is* the action. `.sh` was outside `diff_source.py`'s extension set, and `analyze_scripts` ran only for composite/docker actions and only for scripts named in `action.yml` or a Dockerfile | E (+ carried-over) | Fix in branch `verify-diff-shell-sources`. The hidden `.sh` change was benign (bind-mounts `GITHUB_OUTPUT`/`ENV`/`PATH`/`STEP_SUMMARY`/`STATE` into the container to replace deprecated `set-output`). **For a node action, always check the upstream `compare` API for changed non-JS files — the tool's source diff is not the whole diff** |
| #1195 | `1Password/load-secrets-action@70062d7` (v5.0.1) — two findings, both carried over from the approved v5.0.0. (a) `Binary download verification ✗ 3 unverified` — `tc.downloadTool()` pulls the `op` CLI from the `cache.agilebits.com` CDN (not GitHub releases) with no checksum, then `core.addPath()`s it; the three installer `.ts` files are **untouched** by the bump. (b) `In-tree binary check ✗ dist/core_bg.wasm` — but the build is `ncc build ./src/index.ts`, and ncc copies dependency assets verbatim, so that 14 MB wasm comes from `@1password/sdk-core@0.5.0`, pinned by sha512 integrity in `package-lock.json`. Its provenance chain is npm's; asking for a GitHub-release attestation was the wrong question — case E, not F | E (+ carried-over B) | Fix in branch `verify-intree-rebuild-credit`: delete bundler-copied binaries before the rebuild as the minified JS already is, and credit any that come back byte-identical. **Both findings were already filed upstream by @potiuk — 1Password/load-secrets-action#168 (CLI download) and #186 (wasm provenance). Search the upstream repo before drafting anything** |