Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Security Review of some workflows suggested #2363

Open
mcm1957 opened this issue Jun 13, 2023 · 0 comments
Open

Security Review of some workflows suggested #2363

mcm1957 opened this issue Jun 13, 2023 · 0 comments
Assignees

Comments

@mcm1957
Copy link
Collaborator

mcm1957 commented Jun 13, 2023

Das Problem dass ein neuer PR zwar den Check Workflow triggert (#2358) aber keinen Kommentar absetzen kann und keine Labels setzen kann liegt darin, dass Github beim Workflow Trigger PUSH_REQUEST keinerlei Zugriff auf die Secrets erlaubt. Daher kommen beim push auch kein Token in das Skript wobei dieses aber für den Rest-Api Zugriff benötigt wird. Beim RE-CHECK! zieht dagegen der ISSUE Trigger - und der bekommt Zugriff.

Abhilfe bringt die Verwendung des Trigegrs PUSH_REQUEST_TARGET. Damit funktioniert nun auch (gestestet mit unberechtigtem User mcm1957-test) der Worklflow incl. Kommentar / Labels.

Nachteil ist, dass Github in Bezug auf PUSH_REQUEST_TARGET Sicherheitshinweise verlautbart. So soll mit push_request_target keinerlei unsicherer Code ausgeführt werden.

Ich glaube, dass alles auch weiterhin sicher ist da:
-) der Workflow keine fremden Scripts ausführt
-) der Workflow den einzuliefernden Code nicht mal öffnet geschweige denn ausführt oder kompiliert
-) der idente Workflow schon jetzt mit identen Daten beim RE-CHECK läuft. Damit wäre zumindest sichergestellt, dass keine neue Türe geöffnet wird.

Da es sich aber um eine Frage der Sicherheit des Repos handelt ersuche ich dich (@Apollon77) das zu prüfen (oder jemand anderen mit Erfahrung darum zu ersuchen)

Betroffen sind:

  • Workflow check.yml mit mit Script lib/check.js
  • Worklfow setLabels.yml mit Script lib(setLabels.js

Background Infos von Github gibt es hier:
https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

Zitat:
These safeguards enable granting the pull_request_target additional permissions. The reason to introduce the pull_request_target trigger was to enable workflows to label PRs (e.g. needs review) or to comment on the PR. The intent is to use the trigger for PRs that do not require dangerous processing, say building or running the content of the PR.

@mcm1957 mcm1957 changed the title SECURITY REVIEW Security Review of some workflows suggested Jun 13, 2023
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

No branches or pull requests

2 participants