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

MQL Mimic Exemption Follow Ups #1206

Merged
merged 11 commits into from
Dec 28, 2023
100 changes: 63 additions & 37 deletions .github/workflows/rule-validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,43 @@ jobs:
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

- name: Checkout (from comment)
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 }}
fetch-depth: 0
- name: Get Refs
id: get_refs
Copy link
Member Author

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)

run: |
head_ref="${{ github.head_ref }}"
repo="${{ github.repository }}"
# Either of these must be set in block below
run_all=""
base_ref=""

if [[ "${{ github.event_name }}" == 'pull_request_target' ]]; then
# Detect changes based on whatever we're merging into.
base_ref="${{ github.base_ref }}"
repo="${{ github.event.pull_request.head.repo.full_name }}"
elif [[ "${{ github.event_name }}" == 'push' ]]; then
# Detect changes based on the previous commit
base_ref="$(git rev-parse HEAD^)"
elif [[ "${{ github.event_name }}" == 'workflow_dispatch' ]]; then
# Run on a target, so run for all rules.
run_all="true"
elif [[ "${{ github.event_name }}" == 'issue_comment' ]]; then
# Rely on comment_branch to figure out the head and base
base_ref="${{ steps.comment_branch.outputs.base_ref }}"
head_ref="${{ steps.comment_branch.outputs.head_ref }}"
repo="${{ steps.comment_branch.outputs.head_owner }}/${{ steps.comment_branch.outputs.head_repo }}"
fi

echo "##[set-output name=head_ref;]$head_ref"
echo "##[set-output name=repo;]$repo"
echo "##[set-output name=run_all;]$run_all"
echo "##[set-output name=base_ref;]$base_ref"

- name: Checkout
uses: actions/checkout@v4
if: github.event_name != 'issue_comment'
uses: actions/checkout@v3
with:
ref: ${{ github.head_ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
depth: 0
repository: ${{ steps.get_refs.outputs.repo }}
ref: ${{ steps.get_refs.outputs.head_ref }}
fetch-depth: 0

- uses: actions/setup-python@v4
with:
Expand Down Expand Up @@ -112,7 +134,9 @@ jobs:
git config user.email '[email protected]'
git add **/*.yml
git commit -m "Auto add rule ID"
git push origin ${{ github.head_ref }}
# This will only work when running for a pull_request_target, but rather than filter we'll let this expose
# any issues.
git push origin ${{ steps.get_refs.outputs.head_ref }}

- name: Get the head SHA
id: get_head
Expand Down Expand Up @@ -162,33 +186,17 @@ jobs:
files: "detection-rules/**"
recover_deleted_files: true

- name: Get base ref
id: get_base_ref
run: |
if [[ "${{ github.event_name }}" == 'pull_request_target' ]]; then
# Detect changes based on whatever we're merging into.
echo "##[set-output name=ref;]${{ github.base_ref }}"
elif [[ "${{ github.event_name }}" == 'push' ]]; then
# Detect changes based on the previous commit
echo "##[set-output name=ref;]$(git rev-parse HEAD^)"
elif [[ "${{ github.event_name }}" == 'workflow_dispatch' ]]; then
# Run on a target, so run for all rules.
echo "##[set-output name=run_all;]true"
elif [[ "${{ github.event_name }}" == 'issue_comment' ]]; then
echo "##[set-output name=ref;]${{ steps.comment_branch.outputs.base_ref }}"
fi

- name: Checkout base
uses: actions/checkout@v4
if: ${{ steps.get_base_ref.outputs.run_all != 'true' }}
if: ${{ steps.get_refs.outputs.run_all != 'true' }}
with:
ref: ${{ steps.get_base_ref.outputs.ref }}
ref: ${{ steps.get_refs.outputs.head_ref }}
repository: sublime-security/sublime-rules
depth: 0
path: sr-main

- name: Rename files in sr-main based on rule id
if: ${{ steps.get_base_ref.outputs.run_all != 'true' }}
if: ${{ steps.get_refs.outputs.run_all != 'true' }}
run: |
cd sr-main/detection-rules

Expand All @@ -205,7 +213,7 @@ jobs:
for file in detection-rules/*.yml; do
rule_id=$(yq '.id' $file)

if [[ "${{ steps.get_base_ref.outputs.run_all }}" == "true" ]]; then
if [[ "${{ steps.get_refs.outputs.run_all }}" == "true" ]]; then
altered_rule_ids=$(echo "$rule_id"" ""$altered_rule_ids")
continue
fi
Expand Down Expand Up @@ -295,12 +303,30 @@ jobs:
// MQL Mimic will handle duplicates gracefully, no need to handle here.
return allEMLsToSkip.join(" ")

- name: "Find Existing MQL Mimic Test Results"
uses: actions/github-script@v6
id: find_mql_mimic_results
env:
sha: '${{ steps.get_head.outputs.HEAD }}'
with:
debug: ${{ secrets.ACTIONS_STEP_DEBUG || false }}
script: |
const result = await github.rest.checks.listForRef({
check_name: "MQL Mimic Tests",
owner: "sublime-security",
repo: "sublime-rules",
Comment on lines +316 to +317
Copy link
Member Author

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: process.env.sha
})

let existingRuns = result.data.check_runs.map((r) => r.id)
console.log(existingRuns)
return existingRuns

- name: "Trigger MQL Mimic Tests"
env:
trigger_url: '${{ secrets.MQL_MOCK_TRIGGER }}'
branch: ${{ github.event_name == 'pull_request_target' && github.head_ref || github.ref }}
repo: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name || github.repository }}
branch: '${{ steps.get_refs.outputs.head_ref }}'
repo: '${{ steps.get_refs.outputs.repo }}'
token: '${{ secrets.GITHUB_TOKEN }}'
sha: '${{ steps.get_head.outputs.HEAD }}'
only_rule_ids: '${{ steps.find_ids.outputs.rule_ids }}'
Expand All @@ -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
Copy link
Member Author

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.

# Wait for results so that the token remains valid while the test suite is executing and posting a check here.
with:
token: ${{ secrets.GITHUB_TOKEN }}
checkName: "MQL Mimic Tests"
ref: ${{ steps.get_head.outputs.HEAD }}
timeoutSeconds: 3600
ignoreIDs: ${{ steps.find_mql_mimic_results.outputs.result }}

1 change: 1 addition & 0 deletions discovery-rules/attachment_suspicious_macro.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ source: |
"xlt",
"xltm"
)

and ml.macro_classifier(.).malicious
and ml.macro_classifier(.).confidence in ("low", "medium", "high")
)
Expand Down
Loading