Repository navigation
ci: add CodeQL + SonarQube Cloud security scanning - #2
Merged
Merged
Conversation
Adds a manually-dispatched Security-Scan workflow following the AMMOS
guide, "Code Vulnerability Scanning Set Up for AMMOS Open Source
Software" (wiki pageId 1029935039):
CodeQL -> nasa-scrub -> fix_sarif_paths.py -> sonarqube-scan-action
The scrub step is the one the guide prescribes. It is lossy, so a repair
step is chained after it. Measured against nasa-scrub 3.0.2 with a
CodeQL-shaped report, translate_results:
- rewrites CodeQL's workspace-relative URIs to absolute paths and puts
the absolute source root in uriBaseId, where SARIF expects a symbolic
key such as %SRCROOT%. SonarQube then cannot map findings to files.
- deletes tool.extensions -- where CodeQL keeps its real query metadata
-- and rebuilds tool.driver.rules as id-only stubs, dropping every
severity, so all findings import as MEDIUM.
- sets driver.name to the input filename rather than CodeQL, changing
the external rule namespace to external_javascript:*.
This is not an ordering problem: scrub rebuilds driver.rules from scratch
even when handed a report whose driver already carries rich rules, so
running the repair first does not help. fix_sarif_paths.py instead takes
the pre-scrub report via --reference and restores rule metadata, tool
name and severities, and relativizes paths. Covered by 24 tests.
Residual, documented in the script: scrub also discards relatedLocations
and codeFlows, which are not recoverable by this route.
Scope is limited to first-party code. Excluded are dependencies and build
output, static assets under public/ (including the vendored minified
pdf.js worker we only host), docs/, fonts and images, and tests.
src/externals/ is deliberately kept: those vendored React components are
compiled into our bundle and absent from package.json, so Dependabot does
not track them and nothing else would cover them. Their own bundled
dependencies and test suite are excluded.
sonar.branch.name is intentionally not set for the first runs, so a
manual dispatch is attributed to the project's main branch instead of
creating a second branch in SonarQube Cloud.
Claude-Session: https://claude.ai/code/session_01KKwLzyhbDQ2MyUuofXE25y
workflow_dispatch alone cannot start this workflow yet. GitHub only exposes the "Run workflow" button for workflows that exist on the repository's default branch, and main has no workflows at all -- the Actions API reports zero workflows for the repo. So there is nothing to dispatch until this merges. Adding this branch to the push trigger runs the workflow from the branch's own copy, which is how the first scan can happen before the file reaches main. Mirrors the sonarcloud_action branch trigger in MMGIS. The entry comes out once this is on main. Claude-Session: https://claude.ai/code/session_01KKwLzyhbDQ2MyUuofXE25y
Sets -Dsonar.branch.name=main so runs from this branch publish into
main's analysis instead of creating a second branch in SonarQube Cloud.
The branch carries the same code as main, so that is where the numbers
belong.
Leaving the property unset would likely behave the same way -- an unset
branch name published to the main analysis on MMGIS in this org -- but
that relies on the scanner's CI branch auto-detection staying inactive.
Pinning does not rely on it: the failure mode becomes a failed step
rather than an unwanted branch appearing in the project.
Switch to ${{ github.ref_name }} once the pipeline is trusted.
Claude-Session: https://claude.ai/code/session_01KKwLzyhbDQ2MyUuofXE25y
The first run warned on both counts:
- actions/setup-python, actions/upload-artifact and both codeql-action
steps target Node 20, which the runners now force onto Node 24.
https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
- CodeQL Action v3 is deprecated in December 2026.
https://github.blog/changelog/2025-10-28-upcoming-deprecation-of-codeql-action-v3/
Bumped, keeping the SHA pinning:
actions/setup-python v5.6.0 -> v7.0.0
actions/upload-artifact v4.6.2 -> v7.0.1
github/codeql-action v3 -> v4.38.0 (init and analyze)
Each SHA was resolved from its release tag and the action manifest
checked to confirm runs.using is node24. actions/checkout was already on
v7.0.1/node24 and was not in the warning list, so it is unchanged.
Checked for breakage rather than assuming: codeql-action v4.38.0 still
defaults analyze `output` to ../results, which the Post-Process Output
step depends on, and upload-artifact v7 still accepts name, path,
if-no-files-found and retention-days.
Claude-Session: https://claude.ai/code/session_01KKwLzyhbDQ2MyUuofXE25y
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
flynnplatt
reviewed
Sep 10, 2026
Comment on lines
+28
to
+41
| push: | ||
| branches: | ||
| # Bootstrap trigger. GitHub only offers a "Run workflow" button for | ||
| # workflow_dispatch once this file exists on the repository's default | ||
| # branch, and main currently has no workflows at all -- so until this | ||
| # merges, a dispatch cannot be started at all. Pushing this branch runs | ||
| # the workflow from the branch's own copy instead, which is how the | ||
| # first scan gets done before anything lands on main. | ||
| # | ||
| # Remove this entry once the workflow is on main. | ||
| - feature/sonarcloud-security-scan | ||
|
|
||
| # Enable once the first runs look right, so merges to main are scanned. | ||
| # - main |
Contributor
There was a problem hiding this comment.
this PR seems to be behaving as expected, so should we uncomment this line and remove the line above?
Brings this workflow in line with the MMGIS security-scan setup and retires the bootstrap triggers used to validate the pipeline: - push now triggers on main only, not on the feature branch - pull_request triggers on any PR targeting main, from any source branch - concurrency group per ref, cancelling superseded in-progress PR runs but never cancelling a run on main - the job is skipped for PRs from forks, which have no SONAR_TOKEN and would otherwise fail red for a reason the contributor cannot fix Branch attribution is now left to the scanner's GitHub Actions auto-detection: a push to main analyzes as main, and a PR analyzes as a SonarQube pull request, which expires with the PR rather than becoming a long-lived branch in the project. Only workflow_dispatch still pins -Dsonar.branch.name=main, so validating the pipeline by hand cannot leave a stray branch behind. Claude-Session: https://claude.ai/code/session_011WeGYwtRQbXf1wJE7YzvYW
|
flynnplatt
approved these changes
Sep 11, 2026
flynnplatt
left a comment
flynnplatt
left a comment
Contributor
There was a problem hiding this comment.
this seems ok to me, if scans start to seem erroneous or lacking we can revisit
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
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.
Purpose
Adds CodeQL + SonarQube Cloud static analysis for
astria_ui, following the AMMOS guide Code Vulnerability Scanning Set Up for AMMOS Open Source Software.Pipeline: CodeQL → nasa-scrub →
fix_sarif_paths.py→ sonarqube-scan-action.The scrub step is the one the AMMOS guide prescribes, so we keep it. It is also lossy, which is why a repair step is chained after it — see below.
Proposed Changes
.github/workflows/security-scan.yml— the scan workflow. Triggers on push tomain, on any PR intomain, and onworkflow_dispatch..github/scripts/fix_sarif_paths.py— repairs the CodeQL SARIF after scrub so SonarQube can attribute findings..github/scripts/test_fix_sarif_paths.py— 24 tests covering the repair..github/codeql/codeql-config.yml— CodeQL query suite and scan scope.sonar-project.properties— project keyNASA-AMMOS_astria_ui, sources and exclusions.Why the repair step exists
Measured against
nasa-scrub3.0.2 using the real CodeQL report from this repo (artifacts attached to run 34392036795),translate_resultsdegrades the report on four axes:driver.namejavascriptdriver.rules(with severity)tool.extensions[].rulesrelatedLocationsCodeQL publishes its query metadata under
tool.extensions[].rulesand leavestool.driver.rulesempty; SonarQube reads only the driver. So scrub is the only thing populating the field SonarQube reads — but it populates it with metadata-free stubs and deletes the source of the real metadata, which would flatten every finding to a default severity.This is not an ordering problem: scrub rebuilds
driver.rulesfrom scratch even when handed a report whose driver already carries rich rules, so running the repair first does not help.fix_sarif_paths.pytakes the pre-scrub report via--referenceand restores rule metadata, tool name and severities, and relativizes the paths.Known residual: scrub also discards
relatedLocations(2 → 0). Recovering those would mean heuristically matching results across two reports, so it is documented in the script rather than bodged. Dropping scrub entirely would preserve them, but that deviates from the AMMOS guide and is a conversation with the security office rather than a code change.Scan scope
Limited to first-party code. Excluded: dependencies and build output,
public/**(static assets and the vendored minified pdf.js worker we only host),docs/, fonts and images, and tests.src/externals/is deliberately kept in scope. Those are vendored React components compiled into our bundle and absent frompackage.json, so Dependabot does not track them — if we don't scan them, nothing does. Their own bundled dependencies and test suite are excluded.Triggers and branch attribution
Brought in line with the MMGIS
security-scan.ymlsetup; the bootstrap triggers used to validate the pipeline are gone.mainmainbranchmain, from any source branchworkflow_dispatchmainFork PRs are skipped deliberately: they run without repository secrets, so
SONAR_TOKENwould be empty and the scan would fail red for a reason the contributor cannot fix. Maintainers still get a scan when the change lands onmain.Push and PR runs pass no
sonar.branch.nameand let the scanner's GitHub Actions auto-detection do the work. Onlyworkflow_dispatchstill pins it tomain, so validating the pipeline by hand cannot leave a stray branch behind in the project. The tradeoff, noted in the file: dispatching from a branch that carries real code changes publishes those numbers ontomain.A concurrency group per ref supersedes an in-progress PR run when new commits arrive, and never cancels a run on
main.Notes for the reviewer
sonar-project.propertiesand the CodeQL scope.fix_sarif_paths.pyis shared verbatim withmis_rest_serviceandtile_service(ci: add CodeQL + SonarQube Cloud security scanning mis_rest_service#1, ci: add CodeQL + SonarQube Cloud security scanning tile_service#1) — fix it in one, copy to all.Issues
Testing
Unit: 24 tests pass —
python3 .github/scripts/test_fix_sarif_paths.py. They cover both pipelines (with and without scrub), path relativization including nested code-flow locations, symbolic%SRCROOT%preservation, rule promotion, and malformed input.End-to-end: two green runs against this repo, verified from the downloaded artifacts rather than the logs alone.
Confirmed in SonarQube Cloud after both runs:
mainis the only branch — the pin held, no stray branch was created.8.7.0frompackage.json, distinguishing them from the earlier Automatic Analysis run.external_CodeQL:js/missing-rate-limitingatserver.js:234andserver.js:329, MAJOR / VULNERABILITY. Theexternal_CodeQL:prefix (rather thanexternal_javascript:) confirms the tool-name restoration, and the line mapping confirms the path relativization.Automatic Analysis has been disabled for the project, as the guide requires for GitHub Actions analysis.