Skip to content

feat(checkpoints): add GH-34/35/36 + reusable-workflow-pitfalls reference#71

Merged
CybotTM merged 2 commits intomainfrom
feat/gh-34-35-36-checkpoints
May 5, 2026
Merged

feat(checkpoints): add GH-34/35/36 + reusable-workflow-pitfalls reference#71
CybotTM merged 2 commits intomainfrom
feat/gh-34-35-36-checkpoints

Conversation

@CybotTM
Copy link
Copy Markdown
Member

@CybotTM CybotTM commented May 5, 2026

Motivation

A recent multi-day session shipping reusable GHA workflows in netresearch/typo3-ci-workflows and a consumer wiring in netresearch/t3x-rte_ckeditor_image hit three avoidable issues that this PR makes catchable mechanically:

  1. uses: …/preflight-gate@main self-reference inside reusable workflows broke SHA-pinned consumers. Composite-action references inside reusable workflows are resolved at the consumer's runner under the consumer's allow-list policy. PR #82 had to be followed by PR #83 (inline the bash, drop the composite action) once a SHA-pinned consumer rejected @main. feat: add GraphQL --input pattern for special characters #34 catches this.
  2. Five preflight jobs added without step-security/harden-runner. Caught only after Copilot flagged five separate workflow files in the same PR. The convention is consistent across the repo: every job's first step is the pinned harden-runner. Add GitHub Actions upgrade guide with breaking changes reference #35 catches this mechanically.
  3. PR review feedback declared "addressed" with two threads still unresolved. Twice in the session a "PR is ready" claim was made without verifying that all review threads were marked resolved — the user found the discrepancy when the PR stayed BLOCKED. feat: add fork workflow pattern and archived actions docs #36 is the process reminder.

A fourth, related observation made it into the new reference doc: gh run rerun caches uses: …@ref resolution from the original run trigger time, so re-running after an upstream fix lands picks up nothing — the workflow_ref has to change.

Changes

Three new checkpoints

# === COMPOSITE-ACTION SHA PINNING INSIDE WORKFLOWS ===
# Reusable-workflow refs like `uses: org/repo/.github/workflows/foo.yml@main`
# are exempt (see references/reusable-workflow-security.md — internal `@main`
# is allowed for full reusable workflows). Composite-action refs like
# `uses: org/repo/.github/actions/foo@main` are NOT exempt: they are resolved
# at the *consumer's* runner under the consumer's allow-list policy. A
# consumer that enforces "all actions must be SHA-pinned" will reject a
# reusable workflow that internally references a composite action by branch
# or tag, even though the workflow file itself is unchanged. SHA-pin
# composite-action refs to avoid breaking SHA-pinned consumers.
# PCRE negative lookahead matches refs that are NOT a 40-char hex SHA.
- id: GH-34
  type: regex_not
  target: ".github/workflows/*.{yml,yaml}"
  pattern: 'uses:\s+["'"'"']?[A-Za-z0-9._-]+/[A-Za-z0-9._-]+/\.github/actions/[^@]+@(?![0-9a-f]{40}\b)'
  severity: error
# === HARDEN-RUNNER FIRST-STEP CHECK ===
# Mechanical YAML-AST check via python yaml.safe_load (a regex-only check
# cannot reliably reason about "first step of each job" across multi-line
# YAML). Skipped if PyYAML is unavailable. Jobs that are pure
# reusable-workflow callers (job-level `uses:` instead of `steps:`) are
# exempt — they have no runner steps to harden.
- id: GH-35
  type: command
  target: |
    command -v python3 >/dev/null 2>&1 || exit 0
    # ... yaml.safe_load + first-step check, exits 1 if any job's
    # first non-uses step isn't step-security/harden-runner
  severity: warning
# === PR REVIEW COMPLETENESS PROCESS CHECK ===
- id: GH-36
  domain: pr-process
  prompt: |
    Before declaring PR review feedback addressed:
    1. Query unresolved threads via GraphQL (reviewThreads → isResolved=false)
    2. Count must be exactly zero.
    3. If non-zero, list each thread with first comment body + path/line.
    Common failure: addressing only top-of-discussion threads, missing
    inline-code or earlier-reviewer threads.
  severity: info

(See the diff for the full embedded python script in GH-35 and the full prompt in GH-36.)

New reference doc — references/reusable-workflow-pitfalls.md

This complements rather than overlaps references/reusable-workflow-security.md:

  • reusable-workflow-security.md — supply-chain trust, SHA-pinning external actions, transitive-dependency risks, audit checklist for adopting third-party workflows.
  • reusable-workflow-pitfalls.md (new) — operational/structural traps when authoring internal reusable workflows:
    1. ./ does NOT resolve to the reusable workflow's repo (linked to GitHub docs)
    2. Composite-action SHA-pinning rule (the rule feat: add GraphQL --input pattern for special characters #34 enforces)
    3. gh run rerun caches @ref resolution at original trigger time
    4. Permissions ceiling — caller's job-level permissions clamp the token

SKILL.md

  • description: broadened to activate on reusable-workflow, composite-action, and harden-runner authoring tasks (329 chars total, under the ~350 ceiling).
  • References table gains rows for both reusable-workflow-security.md (previously not listed) and the new reusable-workflow-pitfalls.md.

Self-test

Test plan

…ence

GH-34 (mechanical, error): Composite-action references inside
.github/workflows/*.yml must be SHA-pinned. Reusable-workflow refs
(uses: org/repo/.github/workflows/foo.yml@main) remain exempt per the
existing reusable-workflow-security.md doc, but composite-action refs
(uses: org/repo/.github/actions/foo@main) break consumers that enforce
SHA-pinning when this workflow is called as a reusable workflow.
Implemented via PCRE negative lookahead in regex_not.

GH-35 (mechanical, warning): Every workflow job with `steps:` should
start with step-security/harden-runner. YAML-AST check via embedded
python yaml.safe_load — a regex-only check cannot reliably reason about
"first step of each job" across multi-line YAML. Pure reusable-workflow
callers (job-level `uses:` instead of `steps:`) are exempt. No-ops if
python3 or PyYAML are unavailable.

GH-36 (llm_review, info): Before declaring PR review feedback addressed,
verify zero unresolved review threads via GraphQL. Process check that
catches the failure mode where only top-of-discussion threads are
addressed and inline-code or earlier-reviewer threads stay open.

New reference: references/reusable-workflow-pitfalls.md covers four
operational traps distinct from the supply-chain focus of
reusable-workflow-security.md:
- ./ does NOT resolve to the reusable workflow's repo
- composite-action SHA-pinning rule (paired with GH-34)
- gh run rerun caches @ref resolution at original trigger time
- permissions ceiling: caller's job-level permissions clamp the token

SKILL.md description broadened to activate on reusable-workflow,
composite-action, and harden-runner authoring tasks (329 chars total).
References table gains rows for both reusable-workflow-* docs.

Self-tested: skill repo's own workflows pass GH-34 and GH-35.

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
Copilot AI review requested due to automatic review settings May 5, 2026 18:26
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the github-project skill by expanding its description and adding new checkpoints for GitHub Actions security and process management. Specifically, it introduces checks for SHA-pinning composite actions (GH-34), ensuring harden-runner is the first step in workflow jobs (GH-35), and verifying that all PR review threads are resolved via GraphQL (GH-36). Additionally, a new reference document, reusable-workflow-pitfalls.md, has been added to provide guidance on common issues with reusable workflows and composite actions. I have no feedback to provide.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds new mechanical/process checkpoints and supporting documentation to make common reusable-workflow authoring pitfalls and PR-review completeness issues detectable and repeatable within the github-project skill.

Changes:

  • Add three new checkpoints: GH-34 (composite-action SHA pinning), GH-35 (harden-runner first-step enforcement), GH-36 (verify all PR review threads resolved via GraphQL).
  • Add a new reference doc: references/reusable-workflow-pitfalls.md.
  • Update SKILL.md description and references table to include reusable-workflow content.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
skills/github-project/checkpoints.yaml Adds GH-34/35/36 checkpoint definitions (regex + embedded Python + process prompt).
skills/github-project/references/reusable-workflow-pitfalls.md New reference documenting reusable-workflow authoring pitfalls (local action resolution, SHA pinning, rerun ref caching, permissions ceiling).
skills/github-project/SKILL.md Expands activation description and links in new reusable-workflow reference docs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skills/github-project/checkpoints.yaml Outdated
Comment thread skills/github-project/checkpoints.yaml Outdated
Comment thread skills/github-project/checkpoints.yaml Outdated
- SKILL.md: trim to 495 words (under 500 limit) — tightened
  description, removed redundant "See ..." pointers already covered
  by the references table.
- SKILL.md / checkpoints.yaml GH-36: bump reviewThreads(first:50) to
  first:100 (GraphQL max) and document pageInfo.hasNextPage
  pagination so PRs with >100 threads aren't silently mis-reported as
  "all resolved".
- checkpoints.yaml GH-34: tighten the 40-hex-SHA negative lookahead
  so refs like @<40hex>-tag are correctly flagged as non-SHA-pinned;
  the previous \b boundary let any 40-hex prefix pass.
- checkpoints.yaml GH-35: drop the trailing "|| exit 0" and the
  "2>/dev/null" wrapper on the Python harden-runner check so real
  failures (and the "Jobs missing harden-runner..." message) actually
  surface; missing python3/PyYAML cases are still handled explicitly
  upstream.

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 5, 2026

@CybotTM CybotTM merged commit 0560a7d into main May 5, 2026
15 checks passed
@CybotTM CybotTM deleted the feat/gh-34-35-36-checkpoints branch May 5, 2026 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants