Skip to content

ci: pin third-party actions to SHAs and gate PRs on an in-repo build - #206

Closed
shihyuho wants to merge 1 commit into
jakartafrom
ci/wp12-supply-chain-gating
Closed

ci: pin third-party actions to SHAs and gate PRs on an in-repo build#206
shihyuho wants to merge 1 commit into
jakartafrom
ci/wp12-supply-chain-gating

Conversation

@shihyuho

Copy link
Copy Markdown
Member

Closes #197

WP12 — CI/CD supply-chain & gating. Addresses findings DEP-02, TEST-01, COR-02.

DEP-02 — SHA-pin every third-party action

bump-spring.yml holds a repo-write App token and auto-merges the PRs it creates, so any third-party action it runs on a mutable tag was a supply-chain write path into the repo. All third-party actions across every workflow are now pinned to a full commit SHA with a version comment.

Each SHA is the commit the previously referenced tag already resolved to, so this pins current behaviour rather than upgrading anything.

Workflow Action Old New
bump-spring.yml madhead/semver-utils @latest @4cf918affe9106ea59f86c6250e5ec4570ac4389 # v5.0.0
bump-spring.yml shihyuho/go-spring-version @v1 @9f72980d26ef9ec6b1b4858724135a4dc65e409a # v1
bump-spring.yml peter-evans/create-pull-request @v8 @5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1
bump-spring.yml liskin/gh-workflow-keepalive @v1 @f72ff1a1336129f29bf0166c0fd0ca6cf1bcb38c # v1.2.1
bump-snapshot.yml WyriHaximus/github-action-next-semvers @v1 @d079934efaf011a4cf8912d4637097fe35d32b93 # v1
bump-snapshot.yml mukunku/tag-exists-action @v1.7.0 @5c39604fe8aef7e65acb6fbcf96ec580f7680313 # v1.7.0
bump-snapshot.yml shihyuho/go-jenkins-trigger @v2 @d6f3216fa650324688da24cbf081472e29d35d57 # v2
release-please.yml googleapis/release-please-action @v5.0.0 @45996ed1f6d02564a971a2fa1b5860e934307cf7 # v5.0.0
release-please.yml shihyuho/go-jenkins-trigger @v2 @d6f3216fa650324688da24cbf081472e29d35d57 # v2
commitlint.yml wagoid/commitlint-github-action @v6 @b948419dd99f3fd78a6548d48f94e3df7f6bf3ed # v6.2.1
site-build.yml peaceiris/actions-hugo @v3 @2752ce1d29631191ea3f27c23495fa06139a5b78 # v3.2.1
site-build.yml peaceiris/actions-gh-pages @v4 @84c30a85c19949d7eee79c4ff27748b70285e453 # v4.1.0

First-party actions/* (checkout@v7, setup-java@v5, setup-node@v6, github-script@v9, create-github-app-token@v3) are deliberately left on major tags, matching existing repo convention and the issue's scope ("third-party").

Note

WyriHaximus/github-action-next-semvers@v1 is a stale major alias — it points at a 2020 commit (~v1.0.0), 63 commits behind v1.2.1. Pinning it preserves today's behaviour exactly; upgrading to v1.2.1 would be a functional change, so it is left as a follow-up for the maintainer to decide.

TEST-01 — in-repo PR build gate

New .github/workflows/build.yml: runs make test on pull_request targeting jakarta (plus workflow_dispatch), on JDK 17 / temurin with maven caching — matching the JDK setup and trigger style of the existing workflows. Declares permissions: contents: read and a cancel-in-progress concurrency group.

Auto-merged bump PRs now get an in-repo build/test status instead of depending entirely on out-of-repo Jenkins branch protection. The full java/spring cross-version matrix stays in Jenkinsfile; this gate only covers the combination pom.xml currently declares.

Uses make test rather than a raw mvn verify because every other CI entry point in this repo goes through the Makefile, and the Makefile is owned by a sibling work package in this batch.

Important

Maintainer follow-up: mark the new Build / build check required in branch protection for jakarta. That is a repository settings change and is outside the scope of this PR — until it is done, the gate reports status but does not block auto-merge.

COR-02 — Jenkins failure notification guard

The failure notification was gated on BRANCH_NAME == 'main', which is never true on the active jakarta line, so build failures notified nobody. It now keys on BRANCH_IS_PRIMARY — set to 'true' by the multibranch branch-api plugin on the SCM default branch, so the guard survives a future default-branch rename — with a fallback to a literal jakarta name check for non-multibranch jobs where that variable is absent.

Verification

  • make testgreen (15 tests, JDK 17 / temurin). Note: the build does not work on JDK 25; JDK 17 is required, which is what the new workflow pins.
  • YAML parse of all 7 workflow files — OK.
  • actionlint 1.7.12 — no findings on build.yml and none on any pinned line. The two pre-existing findings it reports (stale bundled metadata for create-github-app-token's client-id input, and SC2086 in site-build.yml) are untouched by this PR.
  • Every SHA was resolved via gh api repos/<owner>/<repo>/commits/<tag> --jq .sha — none invented.

Not verifiable locally: the new workflow cannot be executed on this machine. Its first real run will be this PR itself, which is also the regression check the issue asks for.

Scope

Touches only .github/workflows/** and Jenkinsfile. Does not touch .github/dependabot.yml, the pom files, or the Makefile — those belong to sibling work packages. No repository settings were changed.

🤖 Generated with Claude Code

Closes #197.

DEP-02: every third-party action is now pinned to a full commit SHA with a
version comment. Notably madhead/semver-utils moved off the mutable @latest
tag; it ran in the same workflow that holds a repo-write App token and
auto-merges its own PRs, so a hijacked tag was a write path into the repo.
Each SHA is the one its previously referenced tag already resolved to, so
behaviour is unchanged. First-party actions/* are left on major tags, matching
existing convention.

TEST-01: adds a Build workflow running `make test` on pull requests targeting
jakarta, so auto-merged bump PRs have an in-repo build status instead of
relying solely on out-of-repo Jenkins branch protection. Marking the check
required in branch protection is a maintainer settings change.

COR-02: the Jenkins failure notification was gated on BRANCH_NAME == 'main',
never true on the active jakarta line, so failures notified nobody. It now
keys on BRANCH_IS_PRIMARY (set by branch-api on the SCM default branch) and
falls back to a jakarta name check for non-multibranch jobs.

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

@shihyuho shihyuho left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

APPROVE (advisory) — 9 個 axes 全數審查完畢,無 must-fix

三項 issue findings 都追得到對應改動:DEP-02(12 個 third-party action 全部 SHA pin)、TEST-01(build.ymlpull_request 上提供 in-repo 建置狀態)、COR-02(Jenkinsfile 的死分支守衛換成 BRANCH_IS_PRIMARY)。CI 五個 check 全綠。

Non-blocking (2)

  • FYI · Correctness — .github/workflows/build.yml:8 — 門檻的 branch filter 寫死 jakarta,日後預設分支再改名時 gate 會靜默停止執行,正是這個 PR 在 Jenkinsfile 修掉的同一類死守衛。
  • Nit · Readability — Jenkinsfile:158 — 註解替 BRANCH_NAME fallback 舉的理由是它涵蓋不到的情境;行為正確,錯的只有理由。
Coverage & checks
Axis Status
Correctness 1 finding
Spec clean — DEP-02 / TEST-01 / COR-02 三項驗收條件逐一追到對應程式路徑;mvn verify 改用 make test 的偏離已在 PR 內揭露並確認非覆蓋率退化
Scope clean — 7 個變更檔案全部對得回 #197 的三個 plan step,無 pom / src / Makefile 的順手改動
Convention clean — 對照 base ref 的 AGENTS.md(唯一規則為 commitlint),單一 commit ci: pin third-party actions… 合規;CONTRIBUTING.md 的「PR 對 main」為既有文件過時,不算此 PR 的違反
Security clean — build.ymlpull_request 而非 pull_request_targetpermissions: contents: read、全檔無 secrets.*,fork PR 拿不到寫入權;被 auto-merge 的 bump-spring.yml 寫入路徑已由 SHA pin 收斂
Readability 1 finding
Architecture clean — build.yml 沿用 site-build.yml 的 concurrency 與 setup-java 模式,且經由 make test 這道既有接縫呼叫,與 Jenkins matrix 不會漂移
Performance clean — 單 job、cache: mavencancel-in-progress: true,無 matrix 擴散;交叉版本測試仍留在 Jenkins
Test coverage clean — 純 CI 設定變更,無 src/ 行為可測;repo 內也無 workflow/Jenkinsfile lint job 可掛,且 build.yml 的首次執行即是本 PR
  • Verified — 5 個 check 全綠(build、commits、pr-title、Jenkins branch 與 pr-merge),以 gh pr checks 讀取平台狀態,未在本機執行任何 PR 程式
  • Out-of-band — 於 head SHA 讀取 7 個 workflow 全文,確認 12 處 pin 皆為純 uses: 置換、with: 區塊未動;label-issues.yml 雖不在 diff 內也已確認只用 first-party action,無漏網;base ref 的 pom.xml 確認 <java.version>17</java.version>;兩個 module pom 皆無 failsafe / integration-test 綁定,故 make testmvn verify 在此 repo 跑的是同一組測試;dependabot.yml 已含 github-actions weekly,SHA pin 不會鏽死
  • 未驗證 — 12 個 SHA 是否確實等於註解標示的 tag,需對外部 action repo 發出請求,超出本次審查邊界;PR 內「每個 SHA 都是原 tag 當下解析到的 commit」這項宣稱因此未經獨立確認

(self-review — same account; GitHub records this as event=COMMENT with no badge, so the verdict above is advisory.)

🤖 Reviewed by Claude Opus 4.8


on:
pull_request:
branches: [jakarta]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

FYI · Correctness

這個 branch filter 寫死 jakarta,日後預設分支再改名一次,gate 會靜默停止對 PR 執行,而不是大聲失敗——正是這個 PR 在 Jenkinsfile 修掉的同一類死守衛(env.BRANCH_NAME == 'main' 在 main 改名成 jakarta 後就永遠為 false)。

不算缺陷:site-build.yml 目前也是三處寫死 jakarta,這個寫法與既有慣例一致。若想讓它免疫改名,直接省略 branches: 讓所有 PR 都跑即可。

Comment thread Jenkinsfile
script {
if (env.BRANCH_NAME == 'main'
// BRANCH_IS_PRIMARY 由 multibranch 的 branch-api 在 SCM 預設分支上設為 'true',
// 不必隨預設分支改名而改這裡; 非 multibranch job 不會有這個變數, 故 fallback 比對分支名

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nit · Readability

「非 multibranch job 不會有這個變數, 故 fallback 比對分支名」——但 BRANCH_NAME 本身也是 multibranch 專有變數。單分支的 Pipeline job(Pipeline script from SCM)兩個變數都沒有,兩個條件都是 false,通知一樣不會發出,也就是 COR-02 原本的症狀。

fallback 真正有價值的情境是另一個:multibranch job 但 branch-api 版本較舊、或 SCM source 非 GitHub,導致 primary-branch metadata 取不到而 BRANCH_NAME 仍有值。

程式行為正確,只有註解寫的理由不成立(PR 描述裡也有同樣的說法)。

@shihyuho

Copy link
Copy Markdown
Member Author

經重新評估,本 PR 的三項內容決定拆分處理,此 PR 關閉。

項目 決定 去向
COR-02 Jenkinsfile 通知守衛 保留 已抽出為 #212
TEST-01 新增 build.yml 不做 與既有 Jenkins build 功能重疊,同一份測試跑兩次
DEP-02 第三方 action SHA 釘選 不做 維護成本相對於本 repo 的風險輪廓不划算

實作本身沒有問題——新增的 build gate 曾在本 PR 上實跑通過(51s),11 個 action 的 SHA 也都由當前 tag 實際解析而得、未做任何版本升級。關閉的理由是範圍取捨,不是品質問題。

順帶記錄一項本 PR 過程中發現、原審查未涵蓋的事實:WyriHaximus/github-action-next-semvers@v1 是停滯的 major alias,指向 2020 年的 commit,落後 v1.2.1 約 63 個 commit。即使不採用 SHA 釘選,這一項仍值得單獨評估。

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.

[WP12] CI/CD supply-chain & gating: unpinned actions, auto-merge without in-repo build gate, dead branch guard

1 participant