-
Notifications
You must be signed in to change notification settings - Fork 50
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
MQL Mimic Exemption Follow Ups #1206
Conversation
/mql-mimic-exempt: 10000 |
@@ -314,12 +340,12 @@ jobs: | |||
-d "$body" | |||
|
|||
- name: Wait for MQL Mimic check to be completed | |||
uses: fountainhead/[email protected] | |||
id: wait-for-build | |||
uses: sublime-security/action-wait-for-check@master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forked this to add support for ignoreIDs
. This is pretty simple so I'm hopeful it'll work.
The issue I saw was that we triggered the new MQL Mimic run, but immediately saw the previous result and finished the workflow (it needs to stay running for MQL Mimic to post back results).
Ignoring the existing check ID should avoid this issue. I validated that new checks show up with a new ID, even when the same name is posted to the same ref/commit.
I'd probably do this differently if I was starting from scratch -- have the pending check be created here (vs. from MQL mimic) and pass along the check ID to be updated, but that would require juggling changes in a couple spots at once.
owner: "sublime-security", | ||
repo: "sublime-rules", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct -- the checks are always on sublime-security/sublime-rules
. Sending along the repo from forks just supports checking out the rule code.
ref: ${{ steps.comment_branch.outputs.head_ref }} | ||
fetch-depth: 0 | ||
- name: Get Refs | ||
id: get_refs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored this to handle some more logic. When a comment was triggering, we were running MQL Mimic with a ref of main
, not the PR branch (see line 302 of the old code)
Described more in line. These issues only affected comment triggers (what I added earlier today).