Skip to content

ci: add CodeQL + SonarQube Cloud security scanning - #562

Open
jl-0 wants to merge 2 commits into
masterfrom
feature/sonarcloud-security-scan
Open

ci: add CodeQL + SonarQube Cloud security scanning#562
jl-0 wants to merge 2 commits into
masterfrom
feature/sonarcloud-security-scan

Conversation

@jl-0

@jl-0 jl-0 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Adds workflow-based CodeQL + SonarQube Cloud scanning, following the AMMOS scanning guide and matching the setup on MMGIS and astria_ui. CI configuration only — no application code is touched.

Verified green on run 34556034583, published to master in SonarQube Cloud.

Pipeline

CodeQL → nasa-scrub translate_results → fix_sarif_paths.py → sonarqube-scan-action

nasa-scrub is the step the AMMOS guide prescribes, but it is lossy in two ways that break the SonarQube import:

  1. It rewrites CodeQL's workspace-relative URIs to absolute paths and writes the absolute source root into uriBaseId, so SonarQube cannot map findings to files — everything collapses to project-level "unable to resolve issue location".
  2. It deletes tool.extensions, where CodeQL keeps its real query metadata, and rebuilds tool.driver.rules as id-only stubs — so every finding imports as MEDIUM regardless of true severity, under a rule key named after the input file rather than CodeQL.

.github/scripts/fix_sarif_paths.py repairs both from the pre-scrub report. It keeps the guide's pipeline intact while making the import correct, and ships with a 24-case unit suite (python3 .github/scripts/test_fix_sarif_paths.py).

The identical pipeline is verified green on AIT-GUI (run 34523549184) and AIT-DSN (run 34529015091), where the imported findings carry external_CodeQL rule keys, real file paths and true severities.

Replaces the existing code-scanning.yml

This PR deletes .github/workflows/code-scanning.yml, which ran the AMMOS guide boilerplate unchanged as a three-leg language matrix with a SonarQube Scan step inside the matrix. That shape ran a separate SonarQube analysis per language against the same project key, so the three legs raced and whichever finished last decided what the project showed. It also ran nasa-scrub without the repair step, so its imports had the path and severity problems described above.

The new workflow analyzes all three languages (python, javascript-typescript, actions) in a single CodeQL run and imports their reports with one scanner invocation. GitHub code scanning coverage is preserved — same three languages, plus the security-extended query suite the old file left commented out.

Scope

In scope: the ait package — library modules, the CLI entry points in ait/core/bin, and the ZeroMQ server/plugin/handler stack in ait/core/server — plus the OpenMCT integration JavaScript and the example script. That is the surface worth scanning: dictionary parsing, PCAP handling, the command/telemetry paths and the message bus.

Skipped, with reasons:

path why
config/**, ait/data/**, ait/core/data/**, sequences/** mission dictionaries, packaged defaults and JSON schemas — data, not code, and large enough to matter for the org's LOC budget
doc/** ~94 Sphinx pages and a conf.py that is not shipped
tests/** the test suite (50 files)
poetry_cli/** two files of doc-build tooling invoked by poetry

.github/ is a Sonar source so CodeQL's actions findings have files to attach to. Resolved scope is 81 files.

A symlink that was silently dropping findings

Worth a look, because it is the kind of thing that would have gone unnoticed. openmct/example-server/ait_integration.js is a git-tracked symlink to openmct/ait_integration.js, so the analysis indexed one file under two paths. SonarQube's SARIF importer resolves a finding's file and expects exactly one match:

WARN  Failed to import an issue raised by tool CodeQL, error:
java.lang.IllegalArgumentException: expected one element but was:
<openmct/example-server/ait_integration.js, openmct/ait_integration.js>

It threw and dropped the finding — js/unvalidated-dynamic-method-call, security-severity 7.5, at openmct/ait_integration.js:376. GitHub code scanning reported it and the processed SARIF carried it correctly; only SonarQube lost it. The second commit excludes the symlink from sonar.sources.

The exclusion is in sonar-project.properties only, deliberately not in the CodeQL config. CodeQL resolves the symlink, so listing the link path in paths-ignore excludes the target too — run 34555766403 did exactly that and returned zero JavaScript results, suppressing the finding it was meant to preserve. CodeQL handles the duplicate correctly by itself.

Both findings now import cleanly:

external_CodeQL:actions/missing-workflow-permissions  MAJOR  (project level, see below)
external_CodeQL:js/unvalidated-dynamic-method-call    MAJOR  openmct/ait_integration.js:376

Duplicate basenames on their own are harmless — AIT-GUI imports four findings against ait/gui/__init__.py with ait/gui/bin/__init__.py also indexed, and AIT-DSN has five duplicated basenames and imports cleanly. AIT-Core is the only one of the four repos with a tracked symlink.

Known cosmetic issue

WARN  Unable to resolve Issue location from SARIF physical location
      .github/workflows/full_build.yaml. Falling back to the project location.

The actions/missing-workflow-permissions finding lands at project level rather than on the file. This is a SonarQube importer limitation, not a pipeline defect — the SARIF carries a correct relative path and startLine: 11, the file is indexed (as yaml), and Sonar's own analyzer anchors three githubactions:* issues to it at lines 32/39/40. The same finding anchors correctly in GitHub code scanning, so no information is lost overall.

Notes for the reviewer

  • sonar.branch.name is pinned to master. Leaving it unset does not publish into the project's main analysis — these projects are ALM-bound to GitHub, so the scanner auto-detects the branch from the Actions environment and creates a short-lived SonarQube branch (observed on AIT-GUI run 34522749731). Switch to ${{ github.ref_name }} when per-branch attribution is wanted.
  • CodeQL default setup was already not-configured here, so unlike the sibling repos nothing had to be turned off.
  • The workflow triggers on push to master and on manual dispatch.

https://claude.ai/code/session_01Bv21RHa8B3VAfPkBLU5M11

Adds the AMMOS-guide scanning pipeline, in the shape that was worked out
for MMGIS and astria_ui:

    CodeQL -> nasa-scrub translate_results -> fix_sarif_paths.py
           -> sonarqube-scan-action

scrub is the step the AMMOS guide prescribes, but it is lossy: it
rewrites CodeQL's relative URIs to absolute paths and puts the absolute
source root in uriBaseId, so SonarQube cannot map findings to files; and
it drops tool.extensions -- where CodeQL keeps its real query metadata --
rebuilding driver.rules as id-only stubs, so every finding imports as
MEDIUM. fix_sarif_paths.py repairs both from the pre-scrub report, which
keeps the guide's pipeline intact while making the import correct.

This replaces .github/workflows/code-scanning.yml, which ran the guide
boilerplate unchanged as a three-language matrix -- one SonarQube
analysis per matrix leg against the same project key, so the legs raced
and whichever finished last decided what the project showed. All three
languages (python, javascript-typescript, actions) are analyzed in a
single CodeQL run here and imported by one scanner invocation.

sonar-project.properties is scoped to code we actually own and ship: the
ait package (library, ait/core/bin CLI entry points, and the ZeroMQ
server/plugin/handler stack), the OpenMCT integration JavaScript, the
example script, and .github so CodeQL's `actions` findings have files to
attach to. Mission dictionaries and settings under config/, ait/data and
ait/core/data, the Sphinx tree in doc/, the test suite and the poetry
doc-build helpers are left out -- none of them is shipped code, and the
first three are large enough to matter for the organization's LOC budget.

sonar.branch.name is pinned to master while the pipeline is validated.
Leaving it unset does not publish into the project's main analysis: these
projects are ALM-bound to GitHub, so the scanner auto-detects the branch
from the Actions environment and creates a short-lived SonarQube Cloud
branch named after the git branch (measured on AIT-GUI run 34522749731).
Switch to ${{ github.ref_name }} once scans should be attributed to the
branch actually analyzed.

The workflow triggers on push to master and on manual dispatch. The
bootstrap trigger used to run the first scan from this branch has been
removed now that the pipeline is validated.

Claude-Session: https://claude.ai/code/session_01Bv21RHa8B3VAfPkBLU5M11
@jl-0
jl-0 requested review from a team as code owners September 10, 2026 21:05
@github-advanced-security

Copy link
Copy Markdown

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:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

openmct/example-server/ait_integration.js is a git-tracked symlink to
openmct/ait_integration.js, so the analysis indexed one file under two
paths.

That is not just redundant. SonarQube's SARIF importer resolves a
finding's file and then expects exactly one match; with the symlink and
its target both indexed it gets two and throws, dropping the finding:

    WARN  Failed to import an issue raised by tool CodeQL, error:
    java.lang.IllegalArgumentException: expected one element but was:
    <openmct/example-server/ait_integration.js, openmct/ait_integration.js>

Measured on run 34529017567, which silently lost
js/unvalidated-dynamic-method-call (security-severity 7.5) at
openmct/ait_integration.js:376 -- GitHub code scanning reported it, the
processed SARIF carried it with a correct relative path and line, and
SonarQube imported nothing.

Duplicate basenames on their own are fine: AIT-GUI imports four findings
against ait/gui/__init__.py while ait/gui/bin/__init__.py is also
indexed, and AIT-DSN has five duplicated basenames and imports cleanly.
The symlink is what makes two paths resolve to the same file. AIT-Core is
the only one of the four repositories with a tracked symlink.

Claude-Session: https://claude.ai/code/session_01Bv21RHa8B3VAfPkBLU5M11
@jl-0
jl-0 force-pushed the feature/sonarcloud-security-scan branch from 7df0579 to 83f051f Compare September 11, 2026 02:48
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