Skip to content

Conversation

@rwaight
Copy link

@rwaight rwaight commented Jan 7, 2025

The purpose of this PR is to improve the baseline security for using GitHub Actions with the docs-builder; mainly to improve user awareness as they use the README from this elastic/docs-builder repo to deploy documentation using GitHub Actions.

This PR pins the GitHub actions to the commit SHA in the README example, with a comment including the version.

This also adds notes to the example workflow with a link to the action in the GitHub Marketplace.

This is related to #147 and elastic/docs-builder-example#12

@Mpdreamz
Copy link
Member

Mpdreamz commented Jan 7, 2025

I updated our readme and index.md documentation to use our GitHub action for deploying to GitHub instead which greatly simplifies the steps:

environment:
  name: github-pages
  url: ${{ steps.deployment.outputs.page_url }}
steps:
  - uses: actions/checkout@v4
    
  - name: Publish Github
    uses: elastic/docs-builder/actions/publish@main
    id: deployment

This introduced the current conflict.

@rwaight
Copy link
Author

rwaight commented Jan 7, 2025

@Mpdreamz would you be okay with a new PR where the actions within elastic/docs-builder/actions/publish are pinned?

@Mpdreamz
Copy link
Member

Mpdreamz commented Jan 8, 2025

@rwaight discussed this today with the team:

According to our info sec guidelines:

https://elasticco.atlassian.net/wiki/spaces/EN/pages/48871919/Github+Workflows+Actions+-+Security+Best+Practice

It's indeed ok to depend on tags for trusted vendors (e.g GitHub and ourselves) :)

Since elastic/docs-builder/actions/publish only uses trusted vendors I rather rely on tags to ensure we get bug fixes.

elastic/docs-builder/actions/publish-vercel is the only GitHub action we use that uses an external vendor so we should depend on SHA there 💯 agree. We would welcome an equivalent PR of this there!

Thanks for bringing this topic forward! :)

@Mpdreamz Mpdreamz closed this Jan 8, 2025
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