diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 00000000..a4fa0b08 --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,50 @@ +name: "CodeQL Security Scan Configuration" + +# Keep the default query suite and add the extended security queries. +disable-default-queries: false + +queries: + - uses: security-extended + +# Scope note: AIT-Core is a Python ground data system toolkit -- the ait +# package (library, CLI entry points under ait/core/bin, and the ZeroMQ +# server/plugin/handler stack under ait/core/server) -- plus a small OpenMCT +# JavaScript integration. What is worth analyzing is our own source. Everything +# below is either not ours, not code, or not shipped as code. +paths-ignore: + # Dependencies and build output -- covered by Dependabot, not our source. + - '**/node_modules/**' + - '**/__pycache__/**' + - '**/*.pyc' + - '**/.tox/**' + - '**/.venv/**' + - '**/dist/**' + - '**/build/**' + - '**/*.egg-info/**' + + # Documentation. doc/ is Sphinx: ~90 .rst pages plus a conf.py that only + # configures the doc build and is not shipped in the package. + - 'doc/**' + + # Configuration and dictionary data, not code. config/ holds the example + # mission dictionaries (cmd/tlm/evr/limits/table YAML) and leapseconds.dat; + # ait/data and ait/core/data hold packaged defaults and JSON schemas. + # Scanning them produces no security signal and they are large. + - 'config/**' + - 'ait/data/**' + - 'ait/core/data/**' + - 'sequences/**' + + # Tests. AIT-Core keeps its suite outside the package, in tests/. + - 'tests/**' + - '**/test_*.py' + - '**/*.test.js' + + # Build tooling for the docs (invoked by poetry, not shipped). + - 'poetry_cli/**' + +query-filters: + # Style and maintainability suggestions add volume without adding security + # signal; SonarQube already covers that ground. + - exclude: + problem.severity: recommendation diff --git a/.github/scripts/fix_sarif_paths.py b/.github/scripts/fix_sarif_paths.py new file mode 100755 index 00000000..3b4074eb --- /dev/null +++ b/.github/scripts/fix_sarif_paths.py @@ -0,0 +1,372 @@ +#!/usr/bin/env python3 +""" +Repair a CodeQL SARIF report so SonarQube Cloud can import it correctly. + +This runs *after* nasa-scrub's ``translate_results``, which the AMMOS scanning +guide prescribes: + + https://wiki.jpl.nasa.gov/display/AmmosArch/Code+Vulnerability+Scanning+Set+Up+for+AMMOS+Open+Source+Software + +scrub is kept in the pipeline for compliance with that guide, but it is +lossy. Measured against a CodeQL-shaped report with nasa-scrub 3.0.2 (the +current release), ``translate_results``: + +1. Rewrites workspace-relative URIs to **absolute** filesystem paths, and + writes the absolute source root into ``uriBaseId`` -- where SARIF 2.1.0 + expects a symbolic key into ``originalUriBaseIds`` such as ``%SRCROOT%``. + SonarQube cannot map issues to files with either, so findings collapse to + project-level "Unable to resolve issue location" warnings. + +2. Deletes ``runs[].tool.extensions`` and rebuilds ``runs[].tool.driver.rules`` + as id-only stubs:: + + {"id": "js/redos", "shortDescription": {"text": "js/redos"}} + + CodeQL publishes its real query metadata -- descriptions, ``help``, + ``defaultConfiguration.level`` and ``properties.security-severity`` -- under + ``tool.extensions[].rules``, leaving ``driver.rules`` empty. SonarQube reads + only ``driver.rules``. So scrub is the only thing populating the field + SonarQube reads, but it populates it with metadata-free stubs, and every + finding imports as MEDIUM regardless of real severity. + + This is not an ordering problem: scrub rebuilds ``driver.rules`` from + scratch even when handed a report whose driver already carries rich rules, + so it cannot be fixed by running this script first. + +3. Sets ``driver.name`` to the input *filename* (e.g. ``javascript``) rather + than ``CodeQL``, which is what SonarQube uses to namespace external rule + keys (``external_javascript:js/redos`` instead of ``external_CodeQL:...``). + +Passing ``--reference`` with the original, pre-scrub CodeQL SARIF lets this +script restore (2) and (3) from the authoritative source, and it always fixes +(1). The result is a report that satisfies the AMMOS guide's pipeline while +still importing with correct paths, rule names, descriptions and severities. + +Known residual loss: scrub also discards ``relatedLocations`` and +``codeFlows``. Those are not restored here -- re-associating them would mean +matching results across two reports by heuristic, which is fragile. If the +security office is open to it, dropping the scrub step entirely (feeding this +script the raw CodeQL SARIF, where ``--reference`` is unnecessary because the +extensions are still present) preserves them. + +Usage: + fix_sarif_paths.py + [--reference ] +""" +import json +import os +import sys + +# The valid SARIF 2.1.0 schema URL. Some tools emit the old master-branch URL +# (which 404s); rewrite it. Mirrors nasa/scrub PR #121. +GOOD_SCHEMA_URL = ( + 'https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/' + 'sarif-2.1/schema/sarif-schema-2.1.0.json' +) +BAD_SCHEMA_URLS = ( + 'https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/' + 'Schemata/sarif-schema-2.1.0.json', +) + + +def collect_rules(run): + """Gather every rule a run publishes, richest-first, de-duplicated by id. + + CodeQL puts its query metadata under ``tool.extensions[].rules`` (one tool + component per query pack) and leaves ``tool.driver.rules`` empty, so both + locations have to be consulted. Driver rules win on conflict. + """ + tool = run.get('tool') + if not isinstance(tool, dict): + return [] + + rules = [] + seen = set() + driver = tool.get('driver') + sources = [] + if isinstance(driver, dict): + sources.append(driver.get('rules') or []) + for extension in tool.get('extensions') or []: + if isinstance(extension, dict): + sources.append(extension.get('rules') or []) + + for source in sources: + for rule in source: + if not isinstance(rule, dict): + continue + rule_id = rule.get('id') + if rule_id is None or rule_id in seen: + continue + # An id-only stub carries no metadata worth keeping; let a richer + # definition of the same id from a later source replace it. + seen.add(rule_id) + rules.append(rule) + return rules + + +def is_stub(rule): + """True if a rule carries no metadata beyond its id. + + scrub emits ``{"id": X, "shortDescription": {"text": X}}`` -- a description + identical to the id and nothing else. Treated as empty so a reference + report can replace it. + """ + if not isinstance(rule, dict): + return True + rule_id = rule.get('id') + has_severity = bool( + (rule.get('defaultConfiguration') or {}).get('level') + or (rule.get('properties') or {}).get('security-severity') + ) + short = (rule.get('shortDescription') or {}).get('text') + has_text = bool(short and short != rule_id) or bool(rule.get('fullDescription')) + return not (has_severity or has_text) + + +def apply_rules(run, rules, tool_name=None, force_level=False): + """Install ``rules`` on the run's driver and re-point every result at them. + + Results are matched by ``ruleId``; ``ruleIndex`` and ``result.rule`` are + rewritten to index into the driver's array, since any previous index + referred to an extension that may no longer exist. ``result.level`` is made + explicit from the rule's ``defaultConfiguration`` so SonarQube's importer + cannot miss it and fall back to MEDIUM. + + ``force_level`` overwrites a level that is already present. It is set when + restoring from a reference report, because scrub materializes a flat + ``"warning"`` onto every result -- SARIF says an absent level is inherited + from the rule's ``defaultConfiguration``, and scrub writes that inherited + value out after having already discarded the real one. Left off, a genuine + per-result level from the tool is preserved. + """ + if not rules: + return False + + tool = run.setdefault('tool', {}) + driver = tool.setdefault('driver', {}) + driver['rules'] = rules + if tool_name: + driver['name'] = tool_name + + index_of = {rule.get('id'): i for i, rule in enumerate(rules)} + + for result in run.get('results') or []: + rule_id = result.get('ruleId') or (result.get('rule') or {}).get('id') + index = index_of.get(rule_id) + if index is None: + continue + result['ruleId'] = rule_id + result['ruleIndex'] = index + result['rule'] = {'id': rule_id, 'index': index} + if force_level or 'level' not in result: + level = (rules[index].get('defaultConfiguration') or {}).get('level') + if level: + result['level'] = level + + return True + + +def restore_rules(run, reference_run): + """Replace scrub's stub rules with the real metadata from the pre-scrub report. + + No-op when the run already carries non-stub rules, so a report that was + never degraded is left alone. + """ + # Only the driver's own rules decide whether the report is already good -- + # SonarQube reads nothing else. Rules sitting in tool.extensions[] (where + # CodeQL actually puts them) still need promoting onto the driver. + driver_rules = ((run.get('tool') or {}).get('driver') or {}).get('rules') or [] + if driver_rules and not all(is_stub(rule) for rule in driver_rules): + return False + + existing = collect_rules(run) + if reference_run is None: + # Nothing authoritative to restore from; fall back to whatever the run + # itself publishes (this is the no-scrub path, where CodeQL's + # extensions are still intact). + return apply_rules(run, existing) + + rules = collect_rules(reference_run) + if not rules: + return apply_rules(run, existing) + + reference_name = ( + (reference_run.get('tool') or {}).get('driver') or {} + ).get('name') + return apply_rules(run, rules, tool_name=reference_name, force_level=True) + + +def harden_attribution(sarif): + """Normalize the document so SonarQube can attribute imported issues. + + SonarQube reads the tool name from ``runs[].tool.driver.name`` (mandatory) + and rule metadata from ``runs[].tool.driver.rules[]``. Anything under + ``runs[].tool.rules`` is ignored on import. + """ + modified = False + + if sarif.get('$schema') in BAD_SCHEMA_URLS: + sarif['$schema'] = GOOD_SCHEMA_URL + modified = True + + for run in sarif.get('runs') or []: + tool = run.get('tool') + if not isinstance(tool, dict): + continue + driver = tool.setdefault('driver', {}) + if not isinstance(driver, dict): + continue + + # Move misplaced rules onto the driver, without clobbering richer ones. + if 'rules' in tool: + if not driver.get('rules'): + driver['rules'] = tool['rules'] + del tool['rules'] + modified = True + + # SonarQube drops the whole report if driver.name is missing. + if not driver.get('name'): + driver['name'] = 'CodeQL' + modified = True + + return modified + + +def strip_absolute_uri_base_id(artifact_loc): + """Drop a ``uriBaseId`` holding a filesystem path rather than a symbolic name. + + SARIF 2.1.0 defines ``uriBaseId`` as a key into ``run.originalUriBaseIds`` + (e.g. ``%SRCROOT%``). scrub writes the absolute source root, which + SonarQube may prepend to an already-relative ``uri``. Symbolic ids are left + untouched. + """ + base_id = artifact_loc.get('uriBaseId') + if isinstance(base_id, str) and (base_id.startswith('/') or base_id.startswith('file://')): + del artifact_loc['uriBaseId'] + return True + return False + + +def relativize(artifact_loc, workspace_path): + """Make a single artifactLocation's URI relative to the workspace root.""" + modified = False + uri = artifact_loc.get('uri') + if isinstance(uri, str): + if uri.startswith('file://'): + uri = uri[len('file://'):] + if uri.startswith(workspace_path + '/'): + uri = uri[len(workspace_path) + 1:] + if uri != artifact_loc.get('uri'): + artifact_loc['uri'] = uri + modified = True + if strip_absolute_uri_base_id(artifact_loc): + modified = True + return modified + + +def iter_artifact_locations(run): + """Yield every artifactLocation in a run, including nested flow locations. + + scrub currently discards relatedLocations and codeFlows, but the raw + CodeQL report carries both, so this also covers the no-scrub pipeline. + """ + for artifact in run.get('artifacts') or []: + location = artifact.get('location') + if isinstance(location, dict): + yield location + + def from_location(location): + physical = (location or {}).get('physicalLocation') or {} + artifact_loc = physical.get('artifactLocation') + if isinstance(artifact_loc, dict): + yield artifact_loc + + for result in run.get('results') or []: + for key in ('locations', 'relatedLocations'): + for location in result.get(key) or []: + yield from from_location(location) + for flow in result.get('codeFlows') or []: + for thread_flow in flow.get('threadFlows') or []: + for entry in thread_flow.get('locations') or []: + yield from from_location(entry.get('location')) + + +def fix_sarif(sarif_file, output_file, workspace_path, reference_file=None): + """Repair paths and rule attribution, writing the result to ``output_file``.""" + try: + with open(sarif_file, 'r', encoding='utf-8') as handle: + sarif = json.load(handle) + + reference = None + if reference_file: + try: + with open(reference_file, 'r', encoding='utf-8') as handle: + reference = json.load(handle) + except (OSError, ValueError) as exc: + print( + f"WARNING: could not read reference {reference_file}: {exc}; " + "continuing without rule restoration", + file=sys.stderr, + ) + + reference_runs = (reference or {}).get('runs') or [] + modified = False + + for index, run in enumerate(sarif.get('runs') or []): + reference_run = None + if index < len(reference_runs): + reference_run = reference_runs[index] + elif len(reference_runs) == 1: + reference_run = reference_runs[0] + + if restore_rules(run, reference_run): + modified = True + + if harden_attribution(sarif): + modified = True + + workspace_path = workspace_path.rstrip('/') + for run in sarif.get('runs') or []: + for artifact_loc in iter_artifact_locations(run): + if relativize(artifact_loc, workspace_path): + modified = True + + with open(output_file, 'w', encoding='utf-8') as handle: + json.dump(sarif, handle, ensure_ascii=False, indent=2) + + name = os.path.basename(output_file) + if modified: + print(f"[ok] Normalized paths/attribution in {name}") + else: + print(f"[info] No changes needed for {name}") + return 0 + + except Exception as exc: # noqa: BLE001 - surface any failure to the runner + print(f"ERROR processing {sarif_file}: {exc}", file=sys.stderr) + return 1 + + +def main(argv): + args = list(argv) + reference_file = None + if '--reference' in args: + position = args.index('--reference') + if position + 1 >= len(args): + print("ERROR: --reference requires a file path", file=sys.stderr) + return 2 + reference_file = args[position + 1] + del args[position:position + 2] + + if len(args) != 3: + print( + "Usage: fix_sarif_paths.py " + " [--reference ]", + file=sys.stderr, + ) + return 2 + + return fix_sarif(args[0], args[1], args[2], reference_file) + + +if __name__ == '__main__': + sys.exit(main(sys.argv[1:])) diff --git a/.github/scripts/test_fix_sarif_paths.py b/.github/scripts/test_fix_sarif_paths.py new file mode 100644 index 00000000..9e804f5b --- /dev/null +++ b/.github/scripts/test_fix_sarif_paths.py @@ -0,0 +1,295 @@ +#!/usr/bin/env python3 +"""Tests for fix_sarif_paths.py. + +Run with: python3 .github/scripts/test_fix_sarif_paths.py +""" +import copy +import json +import os +import sys +import tempfile +import unittest + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +from fix_sarif_paths import ( # noqa: E402 + GOOD_SCHEMA_URL, + collect_rules, + fix_sarif, + is_stub, +) + +WORKSPACE = '/home/runner/work/AIT-Core/AIT-Core' + +# A CodeQL report as github/codeql-action actually emits it: rules live under +# tool.extensions[], driver.rules is empty, URIs are relative with a symbolic +# %SRCROOT% uriBaseId, and results carry no explicit level. +RAW_CODEQL = { + "$schema": ("https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/" + "Schemata/sarif-schema-2.1.0.json"), + "version": "2.1.0", + "runs": [{ + "tool": { + "driver": {"name": "CodeQL", "rules": []}, + "extensions": [{ + "name": "codeql/javascript-queries", + "rules": [ + {"id": "js/redos", + "shortDescription": {"text": "Inefficient regular expression"}, + "fullDescription": {"text": "Exponential backtracking."}, + "defaultConfiguration": {"level": "error"}, + "properties": {"security-severity": "7.5"}}, + {"id": "js/unused-local-variable", + "shortDescription": {"text": "Unused variable"}, + "defaultConfiguration": {"level": "note"}}, + ], + }], + }, + "originalUriBaseIds": {"%SRCROOT%": {"uri": "file://" + WORKSPACE + "/"}}, + "artifacts": [{"location": {"uri": "src/app.js", "uriBaseId": "%SRCROOT%"}}], + "results": [{ + "ruleId": "js/redos", + "rule": {"id": "js/redos", "index": 0, "toolComponent": {"index": 0}}, + "message": {"text": "Vulnerable regex."}, + "locations": [{"physicalLocation": { + "artifactLocation": {"uri": "src/app.js", "uriBaseId": "%SRCROOT%"}, + "region": {"startLine": 1}}}], + "relatedLocations": [{"id": 1, "physicalLocation": { + "artifactLocation": {"uri": "src/util.js", "uriBaseId": "%SRCROOT%"}, + "region": {"startLine": 9}}}], + "codeFlows": [{"threadFlows": [{"locations": [{"location": { + "physicalLocation": { + "artifactLocation": {"uri": "src/flow.js", "uriBaseId": "%SRCROOT%"}, + "region": {"startLine": 3}}}}]}]}], + }], + }], +} + +# The same report after nasa-scrub's translate_results: absolute URIs, an +# absolute uriBaseId, extensions deleted, driver.rules rebuilt as id-only +# stubs, driver.name set to the filename, and a flat "warning" level. +SCRUBBED = { + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "version": "2.1.0", + "runs": [{ + "tool": {"driver": { + "name": "javascript", + "rules": [{"id": "js/redos", "shortDescription": {"text": "js/redos"}}], + }}, + "artifacts": [{"location": {"uri": WORKSPACE + "/src/app.js"}}], + "results": [{ + "ruleId": "js/redos", + "level": "warning", + "message": {"text": "Vulnerable regex."}, + "locations": [{"physicalLocation": { + "artifactLocation": {"uri": WORKSPACE + "/src/app.js", + "uriBaseId": WORKSPACE}, + "region": {"startLine": 1}}}], + }], + }], +} + + +def run_fix(document, reference=None, workspace=WORKSPACE): + """Run fix_sarif on an in-memory document, returning the parsed output.""" + with tempfile.TemporaryDirectory() as tmp: + src = os.path.join(tmp, 'in.sarif') + dst = os.path.join(tmp, 'out.sarif') + with open(src, 'w', encoding='utf-8') as handle: + json.dump(document, handle) + + ref_path = None + if reference is not None: + ref_path = os.path.join(tmp, 'ref.sarif') + with open(ref_path, 'w', encoding='utf-8') as handle: + json.dump(reference, handle) + + code = fix_sarif(src, dst, workspace, ref_path) + assert code == 0, f"fix_sarif returned {code}" + with open(dst, encoding='utf-8') as handle: + return json.load(handle) + + +def first_location(result): + return result['locations'][0]['physicalLocation']['artifactLocation'] + + +class TestScrubRepair(unittest.TestCase): + """The wiki-prescribed pipeline: CodeQL -> scrub -> this script.""" + + def setUp(self): + self.out = run_fix(copy.deepcopy(SCRUBBED), reference=copy.deepcopy(RAW_CODEQL)) + self.run = self.out['runs'][0] + + def test_rule_metadata_restored_from_reference(self): + rules = self.run['tool']['driver']['rules'] + self.assertEqual(len(rules), 2) + redos = rules[0] + self.assertEqual(redos['shortDescription']['text'], + 'Inefficient regular expression') + self.assertEqual(redos['defaultConfiguration']['level'], 'error') + self.assertEqual(redos['properties']['security-severity'], '7.5') + + def test_tool_name_restored(self): + # Drives SonarQube's external rule namespace: external_CodeQL:js/redos + # rather than external_javascript:js/redos. + self.assertEqual(self.run['tool']['driver']['name'], 'CodeQL') + + def test_scrub_flat_warning_is_overridden(self): + # scrub flattens every finding to "warning"; the real rule is "error". + self.assertEqual(self.run['results'][0]['level'], 'error') + + def test_paths_relativized(self): + self.assertEqual(first_location(self.run['results'][0])['uri'], 'src/app.js') + self.assertEqual(self.run['artifacts'][0]['location']['uri'], 'src/app.js') + + def test_absolute_uri_base_id_removed(self): + self.assertNotIn('uriBaseId', first_location(self.run['results'][0])) + + def test_results_repointed_at_driver_rules(self): + result = self.run['results'][0] + self.assertEqual(result['ruleIndex'], 0) + self.assertEqual(result['rule'], {'id': 'js/redos', 'index': 0}) + + +class TestRawCodeQL(unittest.TestCase): + """The no-scrub pipeline: CodeQL -> this script.""" + + def setUp(self): + self.out = run_fix(copy.deepcopy(RAW_CODEQL)) + self.run = self.out['runs'][0] + + def test_extension_rules_promoted_to_driver(self): + rules = self.run['tool']['driver']['rules'] + self.assertEqual([r['id'] for r in rules], + ['js/redos', 'js/unused-local-variable']) + self.assertEqual(rules[0]['properties']['security-severity'], '7.5') + + def test_level_derived_from_rule(self): + self.assertEqual(self.run['results'][0]['level'], 'error') + + def test_symbolic_uri_base_id_preserved(self): + # %SRCROOT% is valid SARIF and must survive; only filesystem paths go. + self.assertEqual(first_location(self.run['results'][0])['uriBaseId'], + '%SRCROOT%') + + def test_relative_paths_untouched(self): + self.assertEqual(first_location(self.run['results'][0])['uri'], 'src/app.js') + + def test_related_locations_and_code_flows_preserved(self): + result = self.run['results'][0] + self.assertEqual(len(result['relatedLocations']), 1) + self.assertEqual(len(result['codeFlows']), 1) + + def test_broken_schema_url_corrected(self): + self.assertEqual(self.out['$schema'], GOOD_SCHEMA_URL) + + +class TestPathHandling(unittest.TestCase): + def test_absolute_paths_in_nested_locations_relativized(self): + doc = copy.deepcopy(RAW_CODEQL) + result = doc['runs'][0]['results'][0] + result['relatedLocations'][0]['physicalLocation']['artifactLocation'] = { + 'uri': 'file://' + WORKSPACE + '/src/util.js'} + flow = result['codeFlows'][0]['threadFlows'][0]['locations'][0] + flow['location']['physicalLocation']['artifactLocation'] = { + 'uri': WORKSPACE + '/src/flow.js'} + + out = run_fix(doc)['runs'][0]['results'][0] + self.assertEqual( + out['relatedLocations'][0]['physicalLocation']['artifactLocation']['uri'], + 'src/util.js') + self.assertEqual( + out['codeFlows'][0]['threadFlows'][0]['locations'][0]['location'] + ['physicalLocation']['artifactLocation']['uri'], + 'src/flow.js') + + def test_trailing_slash_workspace_is_tolerated(self): + out = run_fix(copy.deepcopy(SCRUBBED), reference=copy.deepcopy(RAW_CODEQL), + workspace=WORKSPACE + '/') + self.assertEqual(first_location(out['runs'][0]['results'][0])['uri'], + 'src/app.js') + + def test_paths_outside_workspace_left_alone(self): + doc = copy.deepcopy(SCRUBBED) + first_location(doc['runs'][0]['results'][0])['uri'] = '/usr/lib/node/thing.js' + out = run_fix(doc, reference=copy.deepcopy(RAW_CODEQL)) + self.assertEqual(first_location(out['runs'][0]['results'][0])['uri'], + '/usr/lib/node/thing.js') + + +class TestAttributionHardening(unittest.TestCase): + def test_misplaced_tool_rules_moved_to_driver(self): + doc = copy.deepcopy(SCRUBBED) + tool = doc['runs'][0]['tool'] + tool['driver'] = {'name': 'javascript'} + tool['rules'] = [{'id': 'js/redos', + 'shortDescription': {'text': 'Inefficient regex'}, + 'defaultConfiguration': {'level': 'error'}}] + out = run_fix(doc)['runs'][0]['tool'] + self.assertNotIn('rules', out) + self.assertEqual(out['driver']['rules'][0]['id'], 'js/redos') + + def test_missing_driver_name_defaults(self): + doc = copy.deepcopy(SCRUBBED) + del doc['runs'][0]['tool']['driver']['name'] + out = run_fix(doc) + self.assertEqual(out['runs'][0]['tool']['driver']['name'], 'CodeQL') + + def test_rich_rules_not_clobbered_by_reference(self): + # A report that was never degraded must be left alone. + doc = copy.deepcopy(SCRUBBED) + doc['runs'][0]['tool']['driver']['rules'] = [{ + 'id': 'js/redos', + 'shortDescription': {'text': 'Real description'}, + 'defaultConfiguration': {'level': 'note'}}] + out = run_fix(doc, reference=copy.deepcopy(RAW_CODEQL)) + rules = out['runs'][0]['tool']['driver']['rules'] + self.assertEqual(len(rules), 1) + self.assertEqual(rules[0]['shortDescription']['text'], 'Real description') + # And a genuine per-result level survives. + self.assertEqual(out['runs'][0]['results'][0]['level'], 'warning') + + +class TestRobustness(unittest.TestCase): + def test_missing_reference_file_does_not_fail_the_build(self): + with tempfile.TemporaryDirectory() as tmp: + src = os.path.join(tmp, 'in.sarif') + dst = os.path.join(tmp, 'out.sarif') + with open(src, 'w', encoding='utf-8') as handle: + json.dump(SCRUBBED, handle) + code = fix_sarif(src, dst, WORKSPACE, os.path.join(tmp, 'nope.sarif')) + self.assertEqual(code, 0) + + def test_malformed_input_reports_error(self): + with tempfile.TemporaryDirectory() as tmp: + src = os.path.join(tmp, 'in.sarif') + with open(src, 'w', encoding='utf-8') as handle: + handle.write('{not json') + code = fix_sarif(src, os.path.join(tmp, 'out.sarif'), WORKSPACE) + self.assertEqual(code, 1) + + def test_empty_runs_is_a_no_op(self): + out = run_fix({"version": "2.1.0", "runs": []}) + self.assertEqual(out['runs'], []) + + +class TestHelpers(unittest.TestCase): + def test_is_stub_detects_scrub_output(self): + self.assertTrue(is_stub({'id': 'js/redos', + 'shortDescription': {'text': 'js/redos'}})) + + def test_is_stub_rejects_real_rule(self): + self.assertFalse(is_stub({'id': 'js/redos', + 'shortDescription': {'text': 'Inefficient regex'}})) + self.assertFalse(is_stub({'id': 'js/redos', + 'defaultConfiguration': {'level': 'error'}})) + + def test_collect_rules_merges_driver_and_extensions(self): + rules = collect_rules(RAW_CODEQL['runs'][0]) + self.assertEqual([r['id'] for r in rules], + ['js/redos', 'js/unused-local-variable']) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/.github/workflows/code-scanning.yml b/.github/workflows/code-scanning.yml deleted file mode 100644 index 3ce98c13..00000000 --- a/.github/workflows/code-scanning.yml +++ /dev/null @@ -1,134 +0,0 @@ -# For most projects, this workflow file will not need changing; you simply need -# to commit it to your repository. -# -# You may wish to alter this file to override the set of languages analyzed, -# or to provide custom queries or build logic. -# -# ******** NOTE ******** -# We have attempted to detect the languages in your repository. Please check -# the `language` matrix defined below to confirm you have the correct set of -# supported CodeQL languages. -# -name: "CodeQL Advanced" - -on: - push: - branches: [ "master" ] - pull_request: - branches: [ "master" ] - schedule: - - cron: '16 18 * * 4' - -jobs: - analyze: - name: Analyze (${{ matrix.language }}) - # Runner size impacts CodeQL analysis time. To learn more, please see: - # - https://gh.io/recommended-hardware-resources-for-running-codeql - # - https://gh.io/supported-runners-and-hardware-resources - # - https://gh.io/using-larger-runners (GitHub.com only) - # Consider using larger runners or machines with greater resources for possible analysis time improvements. - runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }} - permissions: - # required for all workflows - security-events: write - - # required to fetch internal or private CodeQL packs - packages: read - - # only required for workflows in private repositories - actions: read - contents: read - - strategy: - fail-fast: false - matrix: - include: - - language: actions - build-mode: none - - language: javascript-typescript - build-mode: none - - language: python - build-mode: none - # CodeQL supports the following values keywords for 'language': 'actions', 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'rust', 'swift' - # Use `c-cpp` to analyze code written in C, C++ or both - # Use 'java-kotlin' to analyze code written in Java, Kotlin or both - # Use 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both - # To learn more about changing the languages that are analyzed or customizing the build mode for your analysis, - # see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning. - # If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how - # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - # Add any setup steps before running the `github/codeql-action/init` action. - # This includes steps like installing compilers or runtimes (`actions/setup-node` - # or others). This is typically only required for manual builds. - # - name: Setup runtime (example) - # uses: actions/setup-example@v1 - - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v4 - with: - languages: ${{ matrix.language }} - build-mode: ${{ matrix.build-mode }} - # If you wish to specify custom queries, you can do so here or in a config file. - # By default, queries listed here will override any specified in a config file. - # Prefix the list here with "+" to use these queries and those in the config file. - - # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs - # queries: security-extended,security-and-quality - - # If the analyze step fails for one of the languages you are analyzing with - # "We were unable to automatically build your code", modify the matrix above - # to set the build mode to "manual" for that language. Then modify this step - # to build your code. - # â„šī¸ Command-line programs to run using the OS shell. - # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun - - name: Run manual build steps - if: matrix.build-mode == 'manual' - shell: bash - run: | - echo 'If you are using a "manual" build mode for one or more of the' \ - 'languages you are analyzing, replace this with the commands to build' \ - 'your code, for example:' - echo ' make bootstrap' - echo ' make release' - exit 1 - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4 - with: - category: "/language:${{matrix.language}}" - - # This step is required to remove malformed data from the CodeQL output - # It may be possible to remove in future iterations - - name: Post-Process Output - run: | - python3 -m pip install nasa-scrub - - results_dir=`realpath ${{ github.workspace }}/../results` - sarif_files=`find $results_dir -name '*.sarif'` - - for sarif_file in $sarif_files - do - output_file="$results_dir/$(basename $sarif_file .sarif)_stripped.sarif" - - python3 -m scrub.tools.parsers.translate_results $sarif_file $output_file ${{ github.workspace }} sarifv2.1.0 - done - - echo "RESULTS_DIR=$results_dir" >> $GITHUB_ENV - - # This step makes the analysis artifacts available for download - # This can be helpful for debugging and archive purposes - - name: Upload Artifacts - uses: actions/upload-artifact@v4.3.1 - with: - name: codeql-artifacts-${{ matrix.language }} - path: ${{ env.RESULTS_DIR }} - - - name: SonarQube Scan - uses: SonarSource/sonarqube-scan-action@v6 - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml new file mode 100644 index 00000000..a5d9ea00 --- /dev/null +++ b/.github/workflows/security-scan.yml @@ -0,0 +1,176 @@ +name: Security-Scan + +# Static analysis for AIT-Core, following the AMMOS scanning guide: +# "Code Vulnerability Scanning Set Up for AMMOS Open Source Software" +# https://wiki.jpl.nasa.gov/display/AmmosArch/Code+Vulnerability+Scanning+Set+Up+for+AMMOS+Open+Source+Software +# +# Pipeline: +# CodeQL scan (python, javascript-typescript, actions) +# -> nasa-scrub translate_results (the step the AMMOS guide prescribes) +# -> fix_sarif_paths.py (repairs what scrub degrades) +# -> sonarqube-scan-action (imports into SonarQube Cloud) +# +# Why the extra repair step: scrub rewrites CodeQL's workspace-relative URIs to +# absolute paths and writes the absolute source root into uriBaseId, which stops +# SonarQube mapping findings to files. It also deletes tool.extensions -- where +# CodeQL keeps its real rule metadata -- and rebuilds tool.driver.rules as +# id-only stubs, so every finding would otherwise import as MEDIUM regardless of +# its true severity. fix_sarif_paths.py restores both from the pre-scrub report. +# See .github/scripts/fix_sarif_paths.py for the measurements. +# +# This replaces the previous .github/workflows/code-scanning.yml, which ran the +# unmodified guide boilerplate as a three-leg language matrix. That shape ran a +# separate SonarQube analysis per language against the same project key, so the +# legs raced and the last one to finish decided what the project showed. All +# three languages are analyzed here in a single CodeQL run instead, and their +# reports are imported together by one scanner invocation. +# +# Manual-first: dispatch this workflow to validate a change to the scan +# pipeline. workflow_dispatch checks out the ref you select, so it scans that +# branch's code. + +on: + workflow_dispatch: + + push: + branches: + - master + +permissions: + contents: read + security-events: write + +jobs: + sonarqube: + name: SonarQube + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # Shallow clones give SonarQube poorer relevancy on issue dating. + fetch-depth: 0 + + # Pinned interpreter for the scan tooling itself, so `pip install + # nasa-scrub` lands in a clean, non-externally-managed environment on the + # runner. Unrelated to the Python level AIT-Core targets, which is + # declared to the analyzer as sonar.python.version in + # sonar-project.properties. + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.12' + + - name: Extract version from pyproject.toml + run: | + set -euo pipefail + VERSION=$(python3 -c "import tomllib, pathlib; print(tomllib.loads(pathlib.Path('pyproject.toml').read_text())['tool']['poetry']['version'])") + echo "VERSION=$VERSION" >> $GITHUB_ENV + echo "Extracted version: $VERSION" + + # All three languages in one init. Every one of them is interpreted, so + # none needs a build; build-mode is left unset because the input is only + # accepted when a single language is analyzed, and `none` is already the + # default for these. + - name: Initialize CodeQL + uses: github/codeql-action/init@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0 + with: + languages: python,javascript-typescript,actions + config-file: ./.github/codeql/codeql-config.yml + + # category is left unset: with several languages in one run the action + # derives a distinct category per language, which is what keeps the + # GitHub code scanning alert sets from overwriting each other. + - name: CodeQL Scan + uses: github/codeql-action/analyze@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0 + + - name: Post-Process Output + run: | + set -euo pipefail + python3 -m pip install --disable-pip-version-check nasa-scrub==3.0.2 + + results_dir=$(realpath "${{ github.workspace }}/../results") + workspace_path="${{ github.workspace }}" + + # Exported before any processing so the Upload Artifacts step still + # has a path to publish if a step below fails -- a broken run is + # exactly when you want the raw reports to look at. + echo "RESULTS_DIR=$results_dir" >> $GITHUB_ENV + + echo "SARIF files directory: $results_dir" + echo "Workspace path: $workspace_path" + echo "Available SARIF files:" + ls -lh "$results_dir"/*.sarif + + # Captured before the loop so the files written below are not picked + # up as inputs on a later iteration. + sarif_files=$(find "$results_dir" -name '*.sarif') + + for sarif_file in $sarif_files + do + base=$(basename "$sarif_file" .sarif) + stripped_file="$results_dir/${base}_stripped.sarif" + processed_file="$results_dir/${base}_processed.sarif" + echo "Processing: $(basename "$sarif_file")" + + # The AMMOS guide's step: reformat CodeQL output for SonarQube. + python3 -m scrub.tools.parsers.translate_results \ + "$sarif_file" "$stripped_file" "$workspace_path" sarifv2.1.0 + + # Repair the paths and rule metadata scrub degrades, using the + # original CodeQL report as the source of truth. + python3 .github/scripts/fix_sarif_paths.py \ + "$stripped_file" "$processed_file" "$workspace_path" \ + --reference "$sarif_file" + done + + sarif_list=$(find "$results_dir" -name '*_processed.sarif' | paste -sd, -) + + echo "SARIF_FILES=$sarif_list" >> $GITHUB_ENV + + echo "" + echo "SARIF files for SonarQube:" + echo "$sarif_list" + + # Makes the raw, scrubbed and repaired reports downloadable, which is how + # you check attribution after a run without re-running the scan. + # Guarded on RESULTS_DIR rather than a bare always(): if the CodeQL step + # fails, Post-Process never runs, the env var is unset, and the upload + # action aborts the job a second time with "Input required and not + # supplied: path" -- burying the real error under a spurious one. + - name: Upload Artifacts + if: always() && env.RESULTS_DIR != '' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: codeql-artifacts + path: ${{ env.RESULTS_DIR }} + if-no-files-found: warn + retention-days: 14 + + # sonar.branch.name is pinned to master on purpose while we validate the + # pipeline. This branch carries master's code plus CI config, so the + # findings belong on master's analysis -- and pinning keeps a validation + # run from creating a throwaway branch in SonarQube Cloud. + # + # Leaving it unset does NOT do the same thing. Measured on AIT-GUI run + # 34522749731: these projects are ALM-bound to GitHub, so the scanner + # auto-detects the branch from the Actions environment and published to + # a new SHORT-lived branch named after the git branch. Pinning is what + # actually puts the results on master. + # + # Once the results look right, change this to: + # -Dsonar.branch.name=${{ github.ref_name }} + # so scans are attributed to the branch actually being analyzed. Until + # then, be aware that a run from a branch with real code changes will + # publish those numbers onto master. + - name: SonarQube Scan + uses: SonarSource/sonarqube-scan-action@22918119ff8e1ca75a623e15c8296b6ea4fbe28f # v8.2.1 + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + with: + projectBaseDir: . + args: > + -Dsonar.sarifReportPaths=${{ env.SARIF_FILES }} + -Dsonar.projectVersion=${{ env.VERSION }} + -Dsonar.branch.name=master diff --git a/sonar-project.properties b/sonar-project.properties index d5cad02e..9f6067a0 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -1,19 +1,117 @@ +# SonarQube Cloud configuration for AIT-Core +# +# Set up per the AMMOS scanning guide: +# https://wiki.jpl.nasa.gov/display/AmmosArch/Code+Vulnerability+Scanning+Set+Up+for+AMMOS+Open+Source+Software sonar.projectKey=NASA-AMMOS_AIT-Core sonar.organization=nasa-ammos - -# This is the name and version displayed in the SonarCloud UI. +# Project metadata. projectVersion is overridden from pyproject.toml by the +# Security-Scan workflow, so the value here is only a fallback for a scan run +# outside that workflow. sonar.projectName=AIT-Core -sonar.projectVersion=3.0.0 +sonar.projectVersion=3.1.1 +sonar.projectDescription=AMMOS Instrument Toolkit (AIT) Core - NASA JPL's ground data system toolkit for instrument and CubeSat missions. +# Project links - shown in the SonarQube Cloud UI +sonar.links.homepage=https://ait-core.readthedocs.io/en/latest +sonar.links.scm=https://github.com/NASA-AMMOS/AIT-Core +sonar.links.issue=https://github.com/NASA-AMMOS/AIT-Core/issues +sonar.links.ci=https://github.com/NASA-AMMOS/AIT-Core/actions -# Path is relative to the sonar-project.properties file. Replace "\" by "/" on Windows. -sonar.sources=. +# ========================================== +# Analysis scope +# ========================================== +# Our own code only: +# ait/ the Python package -- library modules, the CLI entry points in +# ait/core/bin, and the ZeroMQ server, plugins and handlers in +# ait/core/server. This is the whole attack surface worth +# scanning: dictionary parsing, PCAP handling, the command/ +# telemetry paths and the message bus. +# openmct/ the OpenMCT integration JavaScript we ship, plus the Node +# example server that goes with it. +# scripts/ the example user script shipped in the repo. +# .github/ workflow definitions, so the CodeQL `actions` findings have +# source files to attach to, and the SARIF import can map them. +# +# Deliberately not listed: +# config/, ait/data/, ait/core/data/, sequences/ -- dictionary and settings +# data, not code (see sonar.exclusions note below). +# doc/ -- ~90 Sphinx .rst pages and a conf.py that is not shipped. +# tests/ -- excluded rather than declared via sonar.tests; no coverage +# report is published today, so indexing them would only add LOC. +# poetry_cli/ -- two files of doc-build tooling invoked by poetry. +# +# Keep this in step with .github/codeql/codeql-config.yml: SonarQube can only +# attach an imported CodeQL finding to a file that is inside its own analysis +# scope, so anything CodeQL scans must be a source here too. +sonar.sources=ait,openmct,scripts,.github +# Exclusions. These are the only lever that keeps a path out of analysis -- +# sonar.test.exclusions is deliberately not used, because it filters the file +# set named by sonar.tests, and sonar.tests is unset here (see the scope note +# above). Test paths therefore belong in this list. +# +# openmct/example-server/ait_integration.js is a git-tracked SYMLINK to +# openmct/ait_integration.js, so indexing it puts the same file in the analysis +# under two paths. That is not merely redundant: SonarQube's SARIF importer +# resolves a finding's file by name, and two candidates make it throw +# IllegalArgumentException: expected one element but was: +# +# and drop the finding entirely. Measured on run 34529017567, where it silently +# lost the js/unvalidated-dynamic-method-call finding (security-severity 7.5) +# that GitHub code scanning reported at openmct/ait_integration.js:376. +# Excluding the symlink leaves the real file scanned and the import intact. +# +# Note this exclusion belongs HERE and NOT in the CodeQL config. CodeQL +# resolves the symlink, so listing the link path in codeql-config.yml +# paths-ignore excludes the target as well: run 34555766403 did exactly that +# and returned zero JavaScript results, suppressing the very finding this is +# meant to preserve. CodeQL handles the duplicate correctly on its own, +# reporting once against the real path; only SonarQube's importer needs help. +sonar.exclusions=\ + **/node_modules/**,\ + **/__pycache__/**,\ + **/*.pyc,\ + **/.tox/**,\ + **/.venv/**,\ + **/dist/**,\ + **/build/**,\ + **/*.egg-info/**,\ + ait/data/**,\ + ait/core/data/**,\ + **/package-lock.json,\ + openmct/example-server/ait_integration.js,\ + **/*.min.js,\ + **/*.png,\ + tests/**,\ + **/test_*.py,\ + **/*.test.js -# Guessing the value based on the example of Git Action script... -sonar.sarifReportPaths=../results/*_stripped.sarif +# Coverage exclusions - no coverage report is published today, but these +# should never count against it when one is. +sonar.coverage.exclusions=\ + tests/**,\ + **/test_*.py,\ + ait/core/bin/**,\ + openmct/example-server/**,\ + scripts/**,\ + .github/** +# Duplication exclusions. The CLI entry points in ait/core/bin share a common +# argparse/main skeleton by design; flagging that as duplication is noise. +sonar.cpd.exclusions=\ + ait/core/bin/**,\ + **/*.min.js -# Encoding of the source code. Default is default system encoding +# Language settings. AIT-Core supports Python >= 3.10 < 3.12 (pyproject.toml). +sonar.python.version=3.10,3.11 +sonar.javascript.file.suffixes=.js,.mjs sonar.sourceEncoding=UTF-8 + +sonar.tags=nasa,ammos,ait,ground-data-system,telemetry,python,cubesat + +# New Code: anything added in the last 30 days counts as new. +sonar.leak.period=30 + +# sonar.sarifReportPaths is supplied by the Security-Scan workflow, which +# passes the repaired CodeQL reports on the scanner command line.