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

✨ Check for secrets in workflows run on pull requests #1615

Merged
merged 9 commits into from
Feb 10, 2022

Conversation

laurentsimon
Copy link
Contributor

@laurentsimon laurentsimon commented Feb 8, 2022

See point 5 in #426 (comment)

This PR looks for secrets used in job's env, jobs' with argument, job's run scripts and top-level env of the workflow.

We add the following constraints

  1. the workflow must run on pull_request
  2. jobs must have a call to action/checkout of the PR.
  3. jobs don't use an environment field which typically indicates some review before running - this can be refined later thru GitHub REST API calls

@laurentsimon laurentsimon requested a review from asraa February 8, 2022 22:48
@laurentsimon laurentsimon temporarily deployed to integration-test February 8, 2022 22:48 Inactive
@laurentsimon laurentsimon temporarily deployed to integration-test February 8, 2022 22:49 Inactive
@laurentsimon laurentsimon temporarily deployed to integration-test February 8, 2022 22:50 Inactive
@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Integration tests success for
[3f69524]
(https://github.com/ossf/scorecard/actions/runs/1815088209)

@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Integration tests success for
[ca5bd0d]
(https://github.com/ossf/scorecard/actions/runs/1815091568)

@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Integration tests success for
[a065589]
(https://github.com/ossf/scorecard/actions/runs/1815094358)

@laurentsimon laurentsimon temporarily deployed to integration-test February 9, 2022 00:03 Inactive
@github-actions
Copy link

github-actions bot commented Feb 9, 2022

Integration tests success for
[d5d78e4]
(https://github.com/ossf/scorecard/actions/runs/1815341376)

checks/dangerous_workflow.go Outdated Show resolved Hide resolved
checks/dangerous_workflow.go Outdated Show resolved Hide resolved
checks/dangerous_workflow.go Outdated Show resolved Hide resolved
checks/dangerous_workflow.go Outdated Show resolved Hide resolved
}

// Secrets used on jobs.
for _, job := range workflow.Jobs {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to check for secrets in every job here? Should we only check for secrets in the code checkout job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, the check is in checkJobForUsedSecrets()

Note: I'm going to update the code today to also verify whether an environment is declared or not, since this indicates a env secret and is typically gated by an approval, so we can discard such jobs. We can add further logic later to read the actual environment rules if we want

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the PR description to reflect my last comment.

@laurentsimon laurentsimon enabled auto-merge (squash) February 9, 2022 18:10
@laurentsimon laurentsimon temporarily deployed to integration-test February 9, 2022 18:10 Inactive
@laurentsimon laurentsimon disabled auto-merge February 9, 2022 18:10
@github-actions
Copy link

github-actions bot commented Feb 9, 2022

Integration tests success for
[95fc4c9]
(https://github.com/ossf/scorecard/actions/runs/1819688265)

@laurentsimon laurentsimon temporarily deployed to integration-test February 10, 2022 00:48 Inactive
@laurentsimon laurentsimon enabled auto-merge (squash) February 10, 2022 00:48
@laurentsimon laurentsimon temporarily deployed to integration-test February 10, 2022 00:48 Inactive
@laurentsimon laurentsimon temporarily deployed to integration-test February 10, 2022 00:50 Inactive
@github-actions
Copy link

Integration tests success for
[4d439d0]
(https://github.com/ossf/scorecard/actions/runs/1821246205)

@github-actions
Copy link

Integration tests success for
[b59c9a6]
(https://github.com/ossf/scorecard/actions/runs/1821249481)

@github-actions
Copy link

Integration tests success for
[8d67482]
(https://github.com/ossf/scorecard/actions/runs/1821255064)

@laurentsimon laurentsimon temporarily deployed to integration-test February 10, 2022 18:34 Inactive
@laurentsimon laurentsimon temporarily deployed to integration-test February 10, 2022 18:35 Inactive
@github-actions
Copy link

Integration tests success for
[e7e1d65]
(https://github.com/ossf/scorecard/actions/runs/1825498011)

@github-actions
Copy link

Integration tests success for
[4a65468]
(https://github.com/ossf/scorecard/actions/runs/1825501860)

@laurentsimon laurentsimon merged commit 7de151c into ossf:main Feb 10, 2022
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