Add Fix for bot-conditions audit rule #921
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2eebc62 to
1bb7633
Compare
woodruffw
reviewed
Jun 12, 2025
woodruffw
reviewed
Jun 12, 2025
|
Thanks @mostafa! I'll do another review tomorrow most likely. (I'm going to look into improving the expression APIs more generally -- they should be returning spanned AST nodes so that we don't ever have to do any offset guesswork.) |
woodruffw
reviewed
Jun 23, 2025
woodruffw
reviewed
Jun 23, 2025
|
Hey @mostafa, sorry for the delay here! I've been working on some expression spanning APIs that will hopefully make future interactions with the expression APIs + getting the "raw" expression from a parsed AST a bit easier. |
woodruffw
reviewed
Jun 23, 2025
37606ca to
44dcb65
Compare
|
@woodruffw Thanks for the reviews! 🙏 I rebased from main and addressed your comments. |
Signed-off-by: William Woodruff <william@yossarian.net>
woodruffw
reviewed
Jun 25, 2025
woodruffw
reviewed
Jun 25, 2025
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
...for now. We might want to re-add this, but the more I think about it the more I think that we probably always want to flag these kinds of spoofable checks. Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
|
Thanks a ton @mostafa! I deduped the patch generation a bit, but otherwise this is good to go 🙂 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds Fixes to bot-conditions audit rule, as suggested by the documentation. The bot-conditions audit rule provides three fixes that work together to secure the workflow:
RewriteFragmetfor actor context: fine-grained rewrites ofgithub.actortogithub.event.pull_request.user.loginin conditions to ensure we check the PR author rather than the last actor.Replacefor trigger: Changespull_request_targettopull_requestto prevent running with elevated permissions.Removefor conditions: Optionally removes bot conditions entirely if they're not needed.These operations are applied in sequence to transform the workflow from using spoofable bot checks to a more secure configuration.
xref #876