-
Notifications
You must be signed in to change notification settings - Fork 172
feat(workflows): add weekly GitHub code scanning automation #1495
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
base: main
Are you sure you want to change the base?
Changes from all commits
7c4baf8
1206f8f
5c764c2
9882e45
96f79d8
2dc5520
24c5303
8e1cce4
01413f1
bc030f7
30d556c
9d1e51e
cf56d7e
902d65f
ab37e93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,13 +85,23 @@ if ($MyInvocation.InvocationName -ne '.') { | |
| $Grouped = $Alerts | | ||
| Group-Object { $_.rule.description } | | ||
| ForEach-Object { | ||
| $paths = @( | ||
| $_.Group | | ||
| ForEach-Object { $_.most_recent_instance.location.path } | | ||
| Where-Object { $_ -and $_ -notmatch '(?i)no file' } | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 M3. Path filter regex is too loose.github/skills/github/gh-code-scanning/scripts/Get-CodeScanningAlerts.ps1 Where-Object { $_ -and $_ -notmatch '(?i)no file' }
Where-Object { $_ -and $_ -ne 'no file associated with this alert' } |
||
| Sort-Object -Unique | ||
| ) | ||
| [PSCustomObject]@{ | ||
| RuleDescription = $_.Name | ||
| RuleId = $_.Group[0].rule.id | ||
| Tool = $_.Group[0].tool.name | ||
| SecuritySeverity = $_.Group[0].rule.security_severity_level | ||
| Count = $_.Count | ||
| SamplePaths = @($_.Group | ForEach-Object { $_.most_recent_instance.location.path } | Sort-Object -Unique) | ||
| RuleDescription = $_.Name | ||
| RuleId = $_.Group[0].rule.id | ||
| Tool = $_.Group[0].tool.name | ||
| SecuritySeverity = $_.Group[0].rule.security_severity_level | ||
| Severity = $_.Group[0].rule.severity | ||
| Count = $_.Count | ||
| AffectedPaths = $paths | ||
| HasFilePaths = ($paths.Count -gt 0) | ||
| AlertUrl = $_.Group[0].html_url | ||
| FindingDescription = $_.Group[0].most_recent_instance.message.text | ||
| } | ||
| } | | ||
| Sort-Object -Property Count -Descending | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,31 +84,31 @@ Describe 'Get-CodeScanningAlerts' -Tag 'Unit' { | |
| $parsed.Count | Should -BeGreaterThan 0 | ||
| } | ||
|
|
||
| It 'Serializes SamplePaths as a JSON array even when only one path exists' { | ||
| It 'Serializes AffectedPaths as a JSON array even when only one path exists' { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 M6. New skill output fields are not under unit test.github/skills/github/gh-code-scanning/tests/Get-CodeScanningAlerts.Tests.ps1 The PR adds |
||
| # js/xss has a single occurrence; verify the raw JSON uses bracket notation, | ||
| # not a bare string (ConvertFrom-Json re-unwraps single-element arrays so | ||
| # the raw string is the authoritative check) | ||
| $result = & $script:ScriptPath -Owner 'testorg' -Repo 'testrepo' -OutputFormat Json | ||
| $rawJson = $result | Out-String | ||
|
|
||
| $rawJson | Should -Match '"SamplePaths":\s*\[' | ||
| $rawJson | Should -Match '"AffectedPaths":\s*\[' | ||
| } | ||
|
|
||
| It 'Serializes SamplePaths as a JSON array when alert has no associated file path' { | ||
| It 'Serializes AffectedPaths as empty array and sets HasFilePaths false when alert has no associated file path' { | ||
| $noPathJson = '[{"number":10,"rule":{"id":"BranchProtectionID","description":"Branch-Protection","security_severity_level":"high"},"tool":{"name":"Scorecard"},"most_recent_instance":{"location":{"path":"no file associated with this alert"}}}]' | ||
| ${Function:gh} = { | ||
| $global:LASTEXITCODE = 0 | ||
| return $noPathJson | ||
| }.GetNewClosure() | ||
|
|
||
| $result = & $script:ScriptPath -Owner 'testorg' -Repo 'testrepo' -OutputFormat Json | ||
| $rawJson = $result | Out-String | ||
| $parsed = $result | ConvertFrom-Json | ||
|
|
||
| $rawJson | Should -Match '"SamplePaths":\s*\[' | ||
| $rawJson | Should -Match 'no file associated with this alert' | ||
| $parsed[0].AffectedPaths | Should -HaveCount 0 | ||
| $parsed[0].HasFilePaths | Should -BeFalse | ||
| } | ||
|
|
||
| It 'Deduplicates and sorts SamplePaths across multiple occurrences of the same rule' { | ||
| It 'Deduplicates and sorts AffectedPaths across multiple occurrences of the same rule' { | ||
| $multiPathJson = '[{"number":1,"rule":{"id":"py/empty-except","description":"Empty except","security_severity_level":null},"tool":{"name":"CodeQL"},"most_recent_instance":{"location":{"path":"scripts/b.py"}}},{"number":2,"rule":{"id":"py/empty-except","description":"Empty except","security_severity_level":null},"tool":{"name":"CodeQL"},"most_recent_instance":{"location":{"path":"scripts/a.py"}}},{"number":3,"rule":{"id":"py/empty-except","description":"Empty except","security_severity_level":null},"tool":{"name":"CodeQL"},"most_recent_instance":{"location":{"path":"scripts/a.py"}}}]' | ||
| ${Function:gh} = { | ||
| $global:LASTEXITCODE = 0 | ||
|
|
@@ -118,9 +118,9 @@ Describe 'Get-CodeScanningAlerts' -Tag 'Unit' { | |
| $result = & $script:ScriptPath -Owner 'testorg' -Repo 'testrepo' -OutputFormat Json | ||
| $parsed = $result | ConvertFrom-Json | ||
|
|
||
| $parsed[0].SamplePaths | Should -HaveCount 2 | ||
| $parsed[0].SamplePaths[0] | Should -Be 'scripts/a.py' | ||
| $parsed[0].SamplePaths[1] | Should -Be 'scripts/b.py' | ||
| $parsed[0].AffectedPaths | Should -HaveCount 2 | ||
| $parsed[0].AffectedPaths[0] | Should -Be 'scripts/a.py' | ||
| $parsed[0].AffectedPaths[1] | Should -Be 'scripts/b.py' | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| name: Create GitHub Code Scanning Issues | ||
|
|
||
| on: | ||
| workflow_call: | ||
| inputs: | ||
| artifact-name: | ||
| description: 'Name of the artifact containing code scanning alerts' | ||
| required: false | ||
| type: string | ||
| default: gh-code-scanning-alerts | ||
|
|
||
| permissions: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 M5. Reusable workflow does not declare
|
||
| issues: write | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 M4. Unused
|
||
| security-events: read | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 This workflow downloads a pre-generated artifact and calls permissions:
issues: write
# Remove: security-events: readApply the same change to the job-level
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 This workflow only (1) downloads a pre-uploaded artifact and (2) creates/updates GitHub issues via Per the repository's workflow convention, permissions follow the principle of least privilege. Suggested fix: permissions:
issues: write |
||
|
|
||
| jobs: | ||
| create-gh-code-scanning-issues: | ||
| name: Create GitHub Code Scanning Issues | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| issues: write | ||
| security-events: read | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| OWNER: ${{ github.repository_owner }} | ||
| REPO: ${{ github.event.repository.name }} | ||
| GITHUB_SERVER_URL: ${{ github.server_url }} | ||
| GITHUB_REPOSITORY: ${{ github.repository }} | ||
| GITHUB_RUN_ID: ${{ github.run_id }} | ||
| steps: | ||
| - name: Download alerts artifact | ||
| uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 | ||
| with: | ||
| name: ${{ inputs.artifact-name }} | ||
|
|
||
| - name: Create backlog issues for new findings | ||
| shell: bash | ||
| run: | | ||
| while IFS= read -r alert; do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ Missing strict error handling — Per the bash script conventions for this repository ( - name: Create backlog issues for new findings
shell: bash
run: |
set -euo pipefail
while IFS= read -r alert; do |
||
| RULE_ID=$(echo "$alert" | jq -r '.RuleId') | ||
| RULE_DESC=$(echo "$alert" | jq -r '.RuleDescription') | ||
| SEVERITY=$(echo "$alert" | jq -r '.SecuritySeverity // .Severity // empty') | ||
| TOOL=$(echo "$alert" | jq -r '.Tool') | ||
| COUNT=$(echo "$alert" | jq -r '.Count') | ||
| HAS_FILE_PATHS=$(echo "$alert" | jq -r '.HasFilePaths') | ||
| ALERT_URL=$(echo "$alert" | jq -r '.AlertUrl // ""') | ||
| FINDING_DESC=$(echo "$alert" | jq -r '.FindingDescription // ""') | ||
| MARKER="automation:security-scan:${RULE_ID}" | ||
| if [[ -n "$SEVERITY" ]]; then | ||
| ISSUE_TITLE="[Security][${SEVERITY}] ${RULE_DESC}" | ||
| else | ||
| ISSUE_TITLE="[Security] ${RULE_DESC}" | ||
| fi | ||
| OCCURRENCE_WORD="$([ "$COUNT" = "1" ] && echo "occurrence" || echo "occurrences")" | ||
| DETECTION_DATE=$(date -u '+%Y-%m-%d') | ||
| RUN_URL="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" | ||
| REPO_BASE="https://github.com/${OWNER}/${REPO}/blob/main" | ||
|
|
||
| if [[ "$HAS_FILE_PATHS" == "true" ]]; then | ||
| AFFECTED_LIST=$(echo "$alert" | jq -r --arg base "$REPO_BASE" '.AffectedPaths[] | "- [\(.)](\($base + "/" + .))"') | ||
| PATHS_SECTION=$'### Affected paths\n\n'"${AFFECTED_LIST}" | ||
| else | ||
| PATHS_SECTION=$'### Repository configuration finding\n\nThis alert refers to a repository-level configuration setting with no associated source file.' | ||
| fi | ||
|
|
||
| existing=$(gh issue list \ | ||
| --repo "${OWNER}/${REPO}" \ | ||
| --search "\"${MARKER}\" in:body" \ | ||
| --state open --json number --jq '.[0].number // empty') | ||
|
|
||
| ISSUE_BODY="<!-- ${MARKER} --> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 M1. Bash here-doc style is fragile and non-idiomatic.github/workflows/create-gh-code-scanning-issues.yml The body is built with a multi-line double-quoted assignment whose internal indentation (10 spaces) only happens to render correctly because YAML strips the block-scalar indent first. Any future edit that nudges indentation by one column will silently turn the issue body into a code block. Use a Suggested rewrite (heredoc): ISSUE_BODY=$(cat <<EOF
<!-- ${MARKER} -->
## Code Scanning Alert: ${RULE_DESC}
**Rule:** \`${RULE_ID}\`
$([ -n "${SEVERITY}" ] && echo "**Severity:** ${SEVERITY}")
**Tool:** ${TOOL}
...
EOF
)This also removes the trailing-quote-on-its-own-line pattern that is easy to misalign. |
||
| ## Code Scanning Alert: ${RULE_DESC} | ||
|
|
||
| **Rule:** \`${RULE_ID}\` | ||
| $([ -n "${SEVERITY}" ] && echo "**Severity:** ${SEVERITY}") | ||
| **Tool:** ${TOOL} | ||
| **Occurrences:** ${COUNT} ${OCCURRENCE_WORD} | ||
| $([ -n "${ALERT_URL}" ] && echo "**Alert:** ${ALERT_URL}") | ||
|
|
||
| ### What was found | ||
| $([ -n "${FINDING_DESC}" ] && echo "${FINDING_DESC}" || echo "See the linked alert for details.") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
While the risk is low in practice because GitHub API values are controlled, defense-in-depth recommends preventing expansion by writing the body to a temporary file instead of a shell variable, or by using ISSUE_BODY=$(cat <<'BODY_EOF'
<!-- \$\{MARKER} -->
BODY_EOF
printf "**Rule:** \`%s\`\n**Tool:** %s\n...\n" "\$\{RULE_ID}" "\$\{TOOL}" >> /tmp/issue_body_$$.md)Alternatively, build the body with |
||
|
|
||
| ${PATHS_SECTION} | ||
|
|
||
| --- | ||
| **Detection Date:** ${DETECTION_DATE} | ||
| **Workflow Run:** ${RUN_URL} | ||
|
|
||
| ### Action Required | ||
| - [ ] Review the alert and confirm it is not a false positive | ||
| - [ ] Remediate or dismiss the finding with a documented reason | ||
| - [ ] Close this issue after the fix is merged | ||
| " | ||
|
|
||
| if [[ -z "$existing" ]]; then | ||
| gh issue create \ | ||
| --repo "${OWNER}/${REPO}" \ | ||
| --title "${ISSUE_TITLE}" \ | ||
| --label "security,automated,needs-triage" \ | ||
| --body "${ISSUE_BODY}" | ||
| else | ||
| gh issue edit "${existing}" \ | ||
| --repo "${OWNER}/${REPO}" \ | ||
| --title "${ISSUE_TITLE}" \ | ||
| --body "${ISSUE_BODY}" | ||
| gh issue edit "${existing}" \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 M2.
|
||
| --repo "${OWNER}/${REPO}" \ | ||
| --add-label "automated,needs-triage" | ||
| gh issue comment "${existing}" \ | ||
| --repo "${OWNER}/${REPO}" \ | ||
| --body "Weekly scan update: ${COUNT} ${OCCURRENCE_WORD} as of ${DETECTION_DATE}." | ||
| fi | ||
| done < <(jq -c '.[]' alerts.json) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| name: Create GitHub Security Scanning Issues | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 B1. Orphan/dead workflow file:
|
||
|
|
||
| on: | ||
| workflow_call: | ||
| inputs: | ||
| artifact-name: | ||
| description: 'Name of the artifact containing code scanning alerts' | ||
| required: false | ||
| type: string | ||
| default: gh-security-scanning-alerts | ||
|
|
||
| permissions: | ||
| issues: write | ||
| security-events: read | ||
|
|
||
| jobs: | ||
| create-gh-security-scanning-issues: | ||
| name: Create GitHub Security Scanning Issues | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| issues: write | ||
| security-events: read | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| OWNER: ${{ github.repository_owner }} | ||
| REPO: ${{ github.event.repository.name }} | ||
| steps: | ||
| - name: Download alerts artifact | ||
| uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 | ||
| with: | ||
| name: ${{ inputs.artifact-name }} | ||
|
|
||
| - name: Create backlog issues for new findings | ||
| shell: bash | ||
| run: | | ||
| while IFS= read -r alert; do | ||
| RULE_ID=$(echo "$alert" | jq -r '.RuleId') | ||
| RULE_DESC=$(echo "$alert" | jq -r '.RuleDescription') | ||
| SEVERITY=$(echo "$alert" | jq -r '.SecuritySeverity // "unspecified"') | ||
| TOOL=$(echo "$alert" | jq -r '.Tool') | ||
| COUNT=$(echo "$alert" | jq -r '.Count') | ||
| PATHS=$(echo "$alert" | jq -r '.SamplePaths | join(", ")') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line references Beyond the field name, this file has several additional problems that suggest it is an accidental inclusion of an early draft:
Recommended fix: Remove |
||
| MARKER="automation:security-scan:${RULE_ID}" | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This line reads # Fix: use the new field name
PATHS=$(echo "$alert" | jq -r '.AffectedPaths | join(", ")')More broadly, this entire file appears to be a leftover from an earlier development iteration. It is not referenced by any orchestrator workflow in this PR, and its deduplication strategy (title search on line 55) diverges from the body-marker approach adopted by |
||
| existing=$(gh issue list \ | ||
| --repo "${OWNER}/${REPO}" \ | ||
| --search "\"[Security] ${RULE_DESC}\" in:title" \ | ||
| --state open --json number --jq '.[0].number // empty') | ||
|
|
||
| if [[ -z "$existing" ]]; then | ||
| gh issue create \ | ||
| --repo "${OWNER}/${REPO}" \ | ||
| --title "[Security] ${RULE_DESC}" \ | ||
| --label "security" \ | ||
| --body "<!-- ${MARKER} --> | ||
| ## Code Scanning Alert: ${RULE_DESC} | ||
|
|
||
| **Rule:** \`${RULE_ID}\` | ||
| **Severity:** ${SEVERITY} | ||
| **Tool:** ${TOOL} | ||
| **Affected files:** ${COUNT} occurrences | ||
|
|
||
| ### Sample affected paths | ||
| ${PATHS} | ||
| " | ||
| fi | ||
| done < <(jq -c '.[]' alerts.json) | ||
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.
💡 Overly broad sentinel filter
The regex
-notmatch '(?i)no file'will silently drop any path containing the substringno file(case-insensitive), including legitimate paths such as/services/no-file-cache/handler.ps1orscripts/nofile-cleanup.py.Suggested fix: Match the exact sentinel string the GitHub API returns for repo-level alerts:
This is unambiguous, doesn't rely on a substring heuristic, and matches the existing test fixture string verbatim.