PR-G 7a: deprecate Tier 1 third-party actions + zizmor provider safelist#1221
Merged
PR-G 7a: deprecate Tier 1 third-party actions + zizmor provider safelist#1221
Conversation
Drops the third-party `styfle/cancel-workflow-action@0.11.0` and the `cancel_outstanding` job that wrapped it. Replaced by a workflow-level `concurrency:` block which is GitHub's modern equivalent (the styfle action's own README recommends switching). Behavior preserved: - Cancels in-progress runs of the same workflow on the same PR/ref - Honors the `skip-cancel` PR label via expression in `cancel-in-progress` The `cancel_outstanding` job was already gated by `GRAPHISTRY_ENABLE_GPU_PUBLIC == 'true'` (disabled by default per PR-D lockdown) so its removal does not affect any currently-running path. Part of PR-G 7a (#1215) — Tier 1 third-party action deprecation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the third-party `dorny/paths-filter@v3` and replaces the `changes` job's filter step with an inline `git diff --name-only` shell that emits the same set of outputs (`infra`, `python`, `gfql`, `cypher_frontend_ci`, `benchmarks`, `docs`). Behavior preserved: - Uses `git merge-base` to match dorny's default base resolution - Each filter dimension keeps its full pattern set (translated from YAML globs to anchored regex) - Same `$GITHUB_OUTPUT` keys, so all downstream `if:` conditions resolve unchanged - Non-diff events (workflow_dispatch / schedule) emit `false`; the existing OR-condition `event_name == 'workflow_dispatch'` etc. in consumers handles those cases independently - New-branch/zero-SHA pushes conservatively emit `true` (run all) Tradeoff: lose YAML-style filter expressions in exchange for zero third-party dep + consistency with the existing inline `docs_only_latest` detection step in the same job. Requires `fetch-depth: 0` on checkout for full history. Part of PR-G 7a (#1215) — Tier 1 third-party action deprecation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ckouts Five `actions/checkout@v4` call sites with `lfs: true` or `fetch-depth: 0` did not get the `persist-credentials: false` treatment during PR-B (#1130). Closes those gaps: - ci.yml: cypher-frontend-strict-typing job (line 401) - ci.yml: cypher-frontend-differential-parity job (line 451) - ci.yml: 3.14 compat job (line 1248) - codeql-analysis.yml: CodeQL scan checkout (line 43) - publish-pypi.yml: versioneer fetch-depth checkout (line 39) None of these jobs push back to the repo; the persisted token is unused and removing it shrinks the credential window. Surfaced by enabling the medium-severity zizmor gate in the next commit. Part of PR-G 7a (#1215); also completes PR-B (#1130) coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the blanket `unpinned-uses: disable: true` with a policy configuration that requires hash-pinning for any third-party action outside the safelist of trusted providers: - actions/* (GitHub-owned core) - github/* (GitHub-owned extended, e.g., codeql-action) - pypa/* (PyPA-maintained, including gh-action-pypi-publish) These three providers may use ref-pin (tags or branches like release/v1) without triggering a finding. Anything else must pin to a full commit SHA. Workflow-security gate dropped from `--min-severity high` to `--min-severity medium` so `unpinned-uses` policy violations actually fail CI (zizmor emits the rule at medium). Decision context (durable; do not relitigate): The team explicitly chose not to SHA-pin trusted-org actions. SHA- pinning trades fast CVE patching for marginal mutable-tag protection that the orgs' account security already provides; with 78 first-party call sites the dependabot churn would not be triaged carefully on a small team, leading to stale-pin-with-CVE which is worse than floating-with-fast-patch. Full rationale in #1215 body. Part of PR-G 7a (#1215). Follow-up: 7b enables a matching repo-level Actions allowlist as defense-in-depth at the runner level. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Self-review findings F1 + F6 from plans/1130-pr-g-7a/review/findings.md: - ci-gpu.yml: explain why `!contains(null, …)` works for non-PR events (the cancel-in-progress expression looks surprising in isolation). - ci.yml: rephrase the merge-base comment so it explains the *behavior* (diff diverging commits, not unrelated ones picked up on base) rather than naming the now-removed dorny dependency. Cosmetic only; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… diffs Wave-1 review findings (plans/1130-pr-g-7a/waves/wave-1/): - C1 [BLOCKER] force-push to a branch silently skipped all gated jobs. `github.event.before` after a force-push is an orphaned SHA; with `fetch-depth: 0` the orphan is not in the local clone, so `git merge-base "$base" "$head"` exits 128. The prior fallback `|| echo "$base"` re-assigned the unreachable SHA back to `merge_base`, and `git diff --name-only ... 2>/dev/null || true` swallowed the subsequent failure to produce empty `$changed`, so every `emit()` returned `false` and every gated job (python tests, gfql tests, etc.) was skipped on a code-bearing commit. PR showed green CI without running tests. - C2 [BLOCKER] same root cause when a PR base ref is rebased between PR open and CI re-run. Same code path, same fix. Both BLOCKERs reproduced live in an isolated git repo; see plans/1130-pr-g-7a/waves/wave-1/adversarial/report.md. Fix: treat merge-base / diff failure as "couldn't compute, run everything" via emit_all true — same conservative stance already applied to zero-SHA new-branch pushes. - C3 [IMPORTANT] `printf '%s\n' "$changed" | grep -qE "$pattern"` races SIGPIPE on diffs exceeding the Linux pipe buffer (~64 KB, thousands of files). With `set -o pipefail`, a SIGPIPE'd printf flips the pipeline non-zero and silently inverts the match. Replaced pipeline with here-string `<<<` which is fully buffered before grep starts; structurally eliminates the race. - Q1 [SUGGESTION] Unicode `→` in ci-gpu.yml concurrency comment replaced with ASCII `->` for consistency with sibling workflow comment style. Verification: - Local actionlint + zizmor (--min-severity medium): 0 findings. - Test harness in plans/1130-pr-g-7a/waves/wave-2/test-filter.sh drives the post-fix shell across 9 scenarios (PR/python, PR/docs, PR/gfql, PR/orphaned-base, push-zero-SHA, schedule, empty-diff, push-valid, ~234 KB SIGPIPE stress) — 28/28 PASS. Evidence in plans/1130-pr-g-7a/waves/wave-2/test-evidence.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wave-2 review (full ceremony) findings (plans/1130-pr-g-7a/waves/wave-2/): - W2-S2 [SUGGESTION] Operability: when the conservative `emit_all true; exit 0` fallback fires (force-pushed branch with orphaned event.before, or rebased PR base), no log signal was emitted. Operators had to infer the fail-open from the all-true outputs. Added a `::warning::` GHA annotation that names the failure and the underlying SHA pair. - W2-S3 [SUGGESTION] Observability: dropped the `2>/dev/null` on the failing `git merge-base` and `git diff` calls so the underlying git error message (e.g., "fatal: Not a valid object name") shows up in the runner log alongside the warning. - W2-S4 [SUGGESTION] Quality: expanded the `emit` SIGPIPE comment from one dense clause into a step-by-step explanation of how `set -o pipefail` + early `grep -q` exit + SIGPIPE'd printf produces a wrong `false` result. Self-evident now without requiring the reader to recall the pipefail/SIGPIPE interaction from memory. All cosmetic / observability; no behavior change to the success paths. Test harness re-run: 28/28 PASS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Records the merged scope: Tier 1 third-party action deprecation, zizmor unpinned-uses provider safelist, gate threshold lowered to medium, 5 PR-B credential-persistence gaps closed, BLOCKER fix on the changes job's fail-open behavior for force-push / rebased-base edge cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 6 `emit` call sites previously squeezed each filter dimension into
a single regex blob — `'\.py$|^graphistry/|^setup\.py$|...'` — which
was hard to scan and didn't diff cleanly when adding/removing one
pattern. Refactored `emit` to take variadic regex args and join them
with `|` internally (`local IFS='|'; local pat="$*"`). Each call site
now lists patterns one per line, matching the original dorny YAML
shape:
emit python \
'\.py$' \
'^graphistry/' \
...
Same regex semantics, no behavior change. Test harness updated to
match; 28/28 PASS unchanged.
Suggested by review: the joined-blob form was hard to read.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1215. Part of umbrella #1130.
Summary
styfle/cancel-workflow-action@0.11.0→ workflow-levelconcurrency:block inci-gpu.ymldorny/paths-filter@v3→ inlinegit diff --name-onlyshell filter inci.yml(same outputs, merge-base semantics)unpinned-usesprovider safelist in.github/zizmor.yml:actions/*,github/*,pypa/*→ ref-pin allowed (floating tags + branches OK)--min-severity hightomediuminworkflow-security.ymlso policy violations actually fail CI.persist-credentials: falseto LFS / fetch-depth checkouts inci.yml(×3),codeql-analysis.yml,publish-pypi.yml. Surfaced when enabling the medium gate; in scope for the security-hardening theme.changesjob (caught by multi-wave review): whengit merge-basecannot resolve (force-pushed branch with orphanedevent.before, or PR base rebased mid-flight), the job previously emittedfalsefor every filter output and silently skipped all gated tests. Now emitstruefor every output (conservative "couldn't compute, run everything") with a::warning::annotation in the runner log. Test harness covering 9 scenarios (incl. the orphaned-base BLOCKER repro) underplans/1130-pr-g-7a/waves/wave-2/test-filter.sh.After this PR: 0 third-party
uses:references in.github/workflows/. Trusted-org actions remain on floating refs by explicit team decision (see below).Why we are NOT SHA-pinning trusted orgs
Durable rationale — please do not relitigate:
The team explicitly chose not to SHA-pin
actions/*,github/*, orpypa/*. The tradeoff:@v4/release/v1gets CVE patches automatically. SHA-pinning 78 first-party call sites generates dependabot churn that nobody on a small team will triage carefully — leading to stale-pin-with-CVE, which is worse than floating-with-fast-patch.Full discussion: #1130 (PR-G section + descope comment) and #1215 body.
Validation
actionlintclean on all touched workflows.zizmor --min-severity medium .github/workflows/*.ymlagainst the new config: 0 findings.infra,python,gfql,cypher_frontend_ci,benchmarks,docs); downstreamneeds.changes.outputs.*references unchanged.concurrency:block inci-gpu.ymlhonors the existingskip-cancelPR label semantics via expression incancel-in-progress.Out of scope (follow-ups)
Settings → Actions → General → Allow specified actionsallowlist matching this safelist (admin action; defense-in-depth at runner level).Test plan
changesjob emits expected outputs for: docs-only PR, python-code PR, gfql PR (verify by inspecting the job summary on this PR's run)needs.changes.outputs.*🤖 Generated with Claude Code