Skip to content

Harden GitHub Workflow Against Supply Chain Attacks #15742

Closed

Description

@kevinjqliu

Recent supply chain attacks through github workflow (e.g. trivy) shows that we need to harden our github workflow implementation. This should apply to all subprojects of Iceberg.

I've already added a few improvements, including

I think we can do more by pinning ALL github actions to commit SHA (allowlisted by infrastructure-actions)
And enforce this for all github workflow definitions going forward.

Proposed Next Steps

  1. Pin all GitHub Actions to a full commit SHA rather than mutable tags (e.g., actions/checkout@v3), using only actions allowlisted by apache/infrastructure-actions. Pinning to a SHA ensures that a compromised or modified tag cannot silently swap in malicious code.

  2. Enforce SHA pinning going forward via CI checks or linting on all new and modified workflow files, so the policy is maintained consistently across contributions.

  3. Disable Dependabot auto-updates for GitHub Actions. Dependabot may automatically bump action versions that have not yet been reviewed and allowlisted by ASF Infrastructure. Until an action is on the allowlist, using it will cause workflows to silently fail with no notifications (see infrastructure-actions#574). Any action version updates should go through the ASF allowlist process first.

Activity

  1. changed the title [-]Harden GitHub Workflow Actions Against Supply Chain Attacks[/-] [+]Harden GitHub Workflow Against Supply Chain Attacks[/+] on Mar 23, 2026
  2. kevinjqliu commented on Mar 23, 2026

    @kevinjqliu
    ContributorAuthor

    I found CodeQL only alert on very specific security vulnerability in github workflow definition.

    zizmor is more particular and will flag much much more.
    For example, zizmor will flag the use of github actions without pinned hash commit.

    https://gist.github.com/kevinjqliu/2dd7a6fa646443aeffb75b42d8edf00e

  3. potiuk commented on Mar 23, 2026

    @potiuk
    Member

    Using dependabot or renovate are required by the ASF Policy [1] same as pin hashing. You should use cooldown [2] with dependabot - this is the best way to use it.

    Yes. I very much recommend Zizmor - it will detect all kind of issues with GH actions (including lack of pin-hashing but also using dangerous workflows and unsafe usages of templating. not using least priviledge permissions - and many more).

    Also dependabot will automatically upgrade such pin-hashed actions as of recently.

    You can also take a look at those infra docs:

    [1] https://infra.apache.org/github-actions-policy.html
    [2] https://infra.apache.org/dependabot.html#cooldown

  4. kevinjqliu commented on Mar 23, 2026

    @kevinjqliu
    ContributorAuthor

    @potiuk thanks for the callout on the ASF policy requiring Dependabot. I've already removed/disabled it from a few repos — I'll put it back.

    My main concern with Dependabot auto-updates is the silent CI failures I've observed recently (more context in apache/infrastructure-actions#574). When Dependabot updates to a version not on the ASF Infrastructure allowlist, the CI view still shows as passing — this is a footgun that will cause CI to fail silently.

    For example, I've reprod'ed with #15749

    • CI is green (shows 4 successful checks)
    • Go to the "checks" tab, some of the checks are black
    Image
    • that specific run is marked with "startup failure"
    • But the PR still can be merged...
    • After merging, the "Open-API" will continue to fail silently...
  5. potiuk commented on Mar 23, 2026

    @potiuk
    Member

    My main concern with Dependabot auto-updates is the silent CI failures I've observed recently (more context in apache/infrastructure-actions#574). When Dependabot updates to a version not on the ASF Infrastructure allowlist, the CI view still shows as passing — this is a footgun that will cause CI to fail silently.

    Dependabot cannot auto-update on it's own - you have to click merge, and there is a workflow in https://github.com/apache/infrastructure-actions that also uses dependabot to automatically bump such version (cooldown: 4 days of yesterday). I've also (yesterday) developed and submitted at PR with tooling for INFRA that should allow to review such updated actions and merge them easily: apache/infrastructure-actions#561 - so hopefully INFRA will review and merge them quickly - you can also always ping them to do so before merging new versions of actions.

    Generally it's a good idea to review such updates before merge anyway (you would not merge such new action without reviewing it - would you??), so checking if the action has already been merged in the infra repo and nugding in the PR if it's not, sounds like a good idea. You can also increase cooldown days - to be more cartain that the update in infra is merged before yours.

    And yes - this is known issue which I reported 3 years ago to GitHub - without fix yet - that workflow failure or not starting jobs at all makes PR "green". And you have very same issue when you make a typo in your wokrflow. There is a way to prevent that however. you just - in your protected_branch definition must define necessary job that must suceed for the branch to be mergable. https://github.com/apache/infrastructure-asfyaml?tab=readme-ov-file#branch-protection (contexts) -> in this case lack of the status check will make your PR non-mergable.

    I think there is always some room for improvement - and I think - similarly as I helped INFRA with the script development and testing (I was able to review and merge more than 10 prs in a matter of minutes yesterday) - improvements in the process are most welcome.

  6. potiuk commented on Mar 23, 2026

    @potiuk
    Member

    BTW. I also added the https://infra.apache.org/dependabot.html#cooldown page yesterday apache/infrastructure-website#269 trying to help with describing some of the Dependabot quirks (a bit in anticipation that people might have some questions) - and I think if you see some missing things, better description, more guidelines - especially as a learnign from this trivy issue - probably good idea to make PRs to infra website as well. That might also help others.

  7. potiuk commented on Mar 23, 2026

    @potiuk
    Member

    Infra team is really happy to see such improvement proposals from what I know.

  8. steveloughran commented on Mar 24, 2026

    @steveloughran
    Contributor

    this is good; especially pinning to SHA versions -last week's attack involved forced push of new tags to get target projects to run the malicious code.

    • the one the week before involved bash commands in the pr name and an over-eager AI action. The fix there is: don't put AI in your reviewing of external PRs.

    Dependabot has to be on ASF projects, but there's no requirement to listen to it. It is overoptimistic about backwards compatibility of older versions. At the same time, having every project say "let's just upgrade all our dependencies and let downstream deal with it" would be nice -but only possible with coordinated releases across projects alongside library shading.

  9. potiuk commented on Mar 24, 2026

    @potiuk
    Member

    Dependabot has to be on ASF projects, but there's no requirement to listen to it. It is overoptimistic about backwards compatibility of older versions. At the same time, having every project say "let's just upgrade all our dependencies and let downstream deal with it" would be nice -but only possible with coordinated releases across projects alongside library shading.

    Correct - how you deal with dependabot is up to you - and you can also configure it - for example - to only propose security patches where security vulnerability is found. I don't think no-one proposes that everyone upgrades to latest versions quickly.

    We do it in Airflow but only because we have extremely extensive test harness - covering test automation from basic unit tests - to end2end UI tests and everything in between - and often our tests detect unintende backwards incompatibilities that dependabot would not even have a chance of being aware of.

    We have canary builds run every 4 hours - and those canary build are cool with such extensive tests - because this means that we can deal with issues individually when they appear rather than "bunch of those" - but recently even we changed the frequency of ours "ubgrades" to max at 4 days - because it was quite a bit burdensome when several upgrades of the same dep happened in a quick succession - introducing and fixing breaking changes :)

  10. kevinjqliu commented on Mar 25, 2026

    @kevinjqliu
    ContributorAuthor

    A few updates:

    I'll port this to all the iceberg* sub-repos when we get to a better state; I'm still experimenting with a few things.

    For the "silent CI failure" (apache/infrastructure-actions#574, which led us to disable dependabot updates for github-actions), I found a relatively straight forward workaround. See apache/infrastructure-actions#574 (comment). With this workaround, we can add back the dependabot update without the fear of silent CI failures.

  11. kevinjqliu commented on Apr 16, 2026

    @kevinjqliu
    ContributorAuthor

    I consider this resolved.

    Added improvements to all apache/iceberg* repos:

    • apache/iceberg
    • apache/iceberg-python
    • apache/iceberg-rust
    • apache/iceberg-go
    • apache/iceberg-cpp
    • apache/iceberg-terraform

    Improvements:

  12. added a commit that references this issue on Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions