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

📖 Docs: clarify meaning of Dangerous Workflow #3484

Closed
wants to merge 1 commit into from

Conversation

lucasgonze
Copy link

@lucasgonze lucasgonze commented Sep 16, 2023

Changes documentation message per discussion in ticket #2831

What kind of change does this PR introduce?

Change type: docs update

Note: the only guideline there is including the 📖 icon

What is the current behavior?

See discussion in #2831

What is the new behavior (if this is a feature change)?**

  • Tests for the changes have been added (for bug fixes/features)

N/A

Which issue(s) this PR fixes

Fixes #2831

Special notes for your reviewer

Does this PR introduce a user-facing change?

The change is an improvement to user-facing documentation to clarify meaning of "Dangerous Workflow".

NONE

@@ -738,14 +738,15 @@ checks:
logging github context and secrets, or use of potentially untrusted inputs in scripts.
The following patterns are checked:

Untrusted Code Checkout: This is the misuse of potentially dangerous triggers.
This checks if a `pull_request_target` or `workflow_run` workflow trigger was used in conjunction
Untrusted Code Checkout: This is the misuse of potentially dangerous triggers.
Copy link
Member

Choose a reason for hiding this comment

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

probably should remove trailing whitespace, it'll make the diff less noisy too

@lucasgonze lucasgonze changed the title Docs: clarify meaning of Dangerous Workflow 📖 📖 Docs: clarify meaning of Dangerous Workflow Sep 18, 2023
@@ -746,6 +746,7 @@ checks:
example, by using build scripts controlled by the author of the PR or reading
token in memory. This check does not detect whether untrusted code checkouts are
used safely, for example, only on pull request that have been assigned a label.
See [Keeping your GitHub Actions and workflows secure Part 1](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/).
Copy link
Member

Choose a reason for hiding this comment

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

we already link to this a few lines below in the remediation section.

See this [post](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)
        for information on avoiding untrusted code checkouts.

Is this a matter of visibility? having the link in one section vs another?

Copy link
Author

Choose a reason for hiding this comment

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

Deleted duplicate link and re-pushed.

Copy link
Member

Choose a reason for hiding this comment

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

The PR now appears to be a single tab.
Can the PR and issue be closed? Or is there still some unaddressed ambiguity? I'm all for fixing ambiguity in the docs if needed.

Copy link
Author

Choose a reason for hiding this comment

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

We ended up using this pull request to complete the deferred conversation at #2831 (comment) . Let's close this.

Changes documentation message per discussion in ticket

Signed-off-by: Lucas Gonze <[email protected]>
@lucasgonze
Copy link
Author

We ended up using this pull request to complete the deferred conversation at #2831 (comment) . Let's close this.

@lucasgonze lucasgonze closed this Sep 28, 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

Successfully merging this pull request may close these issues.

Feature: clarify meaning of Dangerous Workflow
3 participants