Skip to content

Make workflow run workflow in source repo context to prevent secrets … #814

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bj00rn
Copy link
Collaborator

@bj00rn bj00rn commented Jun 17, 2024

Make workflow run workflow in source repo context to prevent secrets exposure.

  • pull_request_target event will be run in target repo context and will have access to secrets
  • pull_request_target event SHA is latests commit in target base branch, and running ui-tests here does not make sense as ui-tests will be run on latest commit in target branch.
  • Remove file upload since workflow no longer has access to secrets.

…exposure. Remove file upload since it no longer has access to secrets
@bj00rn bj00rn requested a review from thomasnordquist as a code owner June 17, 2024 09:43
@thomasnordquist
Copy link
Owner

thomasnordquist commented Jul 5, 2024

Hey there,
sorry for taking my time to respond, and feel free to contradict me if you think I am wrong.
A proof-of-concept for an attack is also very welcome.

Securing secrets is hard

  • Using pull_request_target allows to limit the context of actions to "protected" branches (main, release). (workflows of target branches are used)
  • Workflows can only be updated via a PR with a Codeowner as reviewer (Me)
  • Workflow segregation: Secrets may only be used in steps where no code/tools maintained in this PR are used. (external actions)
    • this is what makes it safe to use pull_request_target
  • forks cannot run github actions in context of this repo without a maintainer approving the run. Workflow segregation still protects grabbing secrets

Given a good reason, I might consider using pull_request over pull_request_target, this will however weaken the security in regard to maintainers.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants