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

Collect mql-mimic-exempt Comments & Sent to MQL Mimic #1199

Merged
merged 26 commits into from
Dec 27, 2023

Conversation

cameron-dunn-sublime
Copy link
Member

@cameron-dunn-sublime cameron-dunn-sublime commented Dec 27, 2023

With some luck, this will be the last change on the sublime-rules side to support a system where we can make exceptions to MQL Mimic by commenting:

/mql-mimic-exempt: 12234, 556

That would cause 12234 & 556 to be skipped in the test suite. This will allow bypassing when the team decides that a apparent regression is actually a good thing, e.g. the EML shouldn't have matched the rule in the first place.

The behavior is that:

  1. Whenever a commit is pushed to a PR, or a comment with /mql-mimic-exempt is added by an org member, the test suite will be re-triggered and should update the existing checks.
    • Updating of the existing checks isn't tested yet, but I think it will work (I can't test till earlier issues are resolved on main)
  2. All comments on the PR are inspected for /mql-mimic-exempt, and any numbers that appear on the same line are considered.
    • To be clear, this means that the comments accumulate rather than overwrite.
    • The syntax is pretty flexible, e.g. all of these are allowed:
      • /mql-mimic-exempt :123;456
      • /mql-mimic-exempt: 123 456
      • /mql-mimic-exempt: 123,#456
      • /mql-mimic-exempt 123/456
      • etc.. # is ignored, ,, ;, :, / and any whitespace are allowed as separators

Downstream support in MQL Mimic has not been added yet! It's mostly done, but not merged.

Tested via #1202, resulting trigger request body is {"branch":"cd.test-2","repo":"sublime-security/sublime-rules","token":"***","sha":"de8c38262ed8cda7c55a087c9d0e802f7d462458","only_rule_ids":"2fc36c6d-86a2-5b12-b5a4-5d8744858381","skip_eml_ids":"123 456 789 123 456 789 444"} which is correct for the comments I've put on that PR.

@cameron-dunn-sublime
Copy link
Member Author

/mql-mimic-exempt

@cameron-dunn-sublime cameron-dunn-sublime changed the title testing Collect mql-mimic-exempt Comments & Sent to MQL Mimic Dec 27, 2023
@@ -39,8 +39,8 @@ jobs:
uses: actions/checkout@v3
if: github.event_name == 'issue_comment'
with:
repository: ${{ steps.comment-branch.outputs.head_owner }}/${{ steps.comment-branch.outputs.head_repo }}
ref: ${{ steps.comment-branch.outputs.head_ref }}
repository: ${{ steps.comment_branch.outputs.head_owner }}/${{ steps.comment_branch.outputs.head_repo }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Another follow up from #1198

Still not seeing the comment triggering behavior work as expected, and hopefully this is the last issue. I can't test this until it's merged to main, so no guarantees (but we have a near identical workflow for test rules, so it should be close if this doesn't fix it).

@@ -45,12 +45,12 @@ jobs:

- name: Get PR branch
uses: alessbell/[email protected] # Fork of xt0rted/pull-request-comment-branch, see https://github.com/xt0rted/pull-request-comment-branch/issues/322
id: comment-branch
id: comment_branch
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it here for consistency. _ preferred so that it's considered a single word in most contexts.

@cameron-dunn-sublime cameron-dunn-sublime requested review from a team December 27, 2023 19:34
@cameron-dunn-sublime cameron-dunn-sublime marked this pull request as ready for review December 27, 2023 19:34
@morriscode
Copy link
Member

This looks great! I'm excited to hear how the testing goes.

@cameron-dunn-sublime cameron-dunn-sublime enabled auto-merge (squash) December 27, 2023 19:49
@cameron-dunn-sublime cameron-dunn-sublime merged commit 797e0f5 into main Dec 27, 2023
3 checks passed
@cameron-dunn-sublime cameron-dunn-sublime deleted the cd.test branch December 27, 2023 19:53
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