Skip to content

[ci][gh][workflow] explicit action hash and permissions - #12581

Open
rmannibucau wants to merge 1 commit into
apache:masterfrom
rmannibucau:fix/ci-gh-workflows
Open

[ci][gh][workflow] explicit action hash and permissions#12581
rmannibucau wants to merge 1 commit into
apache:masterfrom
rmannibucau:fix/ci-gh-workflows

Conversation

@rmannibucau

Copy link
Copy Markdown
Contributor

Trying to harness more our workflows - but maven shared is designed to be too opened.
Audit mainly done by zizmor.

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean security-hardening PR that adds top-level permissions: {} and explicit job-level permissions to three workflow files, pins reusable workflow references to a verified commit hash, and adds a justified zizmor annotation. All changes are correct and consistent with the already-hardened maven.yml workflow.

Verified that:

  • The commit hash 8599b638c78a2bec146a98d75d4c8b4e8458324f matches the current HEAD of the v5 branch of apache/maven-gh-actions-shared
  • Job-level permissions in all three workflows match what the corresponding reusable workflows require
  • The zizmor: ignore[dangerous-triggers] annotation on the pull_request_target trigger in pr-automation.yml is appropriate since the workflow only triggers on closed events
  • All four workflow files in the repository are now hardened

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

permissions:
issues: write
pull-requests: write
uses: apache/maven-gh-actions-shared/.github/workflows/pr-automation.yml@8599b638c78a2bec146a98d75d4c8b4e8458324f # v5

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is intensionally here.

We assume our repo with shared actions is secure, we introduce every changes to it.

When we pin to hash we need update about 100+ our repos in every changes in shared actions.

We also manage permissions in shared action.
https://github.com/apache/maven-gh-actions-shared/blob/v5/.github/workflows/pr-automation.yml

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I understand but I dont think we should auto upgrade all repos like that, this is a good idea today which will likely blow up tomorrow so better to invest in automating it with a workflow in shared repo (on demand/manual) than relying on it by default IMHO

permissions being per workflow/job it is always better to explicit it in leaves than in parents IMHO, in particular when there it does have some serious impact on the run of the workflow (not having write perms will fail and you can wonder why - would be ok only if read only for PR IMHO)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for permissions I'm ok,

but for hash version I afraid that will be more maintenance work for us - who will be take care about it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what I had in mind was something like dependabot and we do approve after review as any "jar" dependency, wdyt?

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.

3 participants