diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 0d13c2ec0..543ebadf3 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,7 +1,14 @@ -## Checklist - + +## Description of changes + ---> +## Related issues + + +## Checklist + - [ ] I have reviewed the [contribution guide](../CONTRIBUTING.md). - [ ] My PR title and commits follow the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) convention. @@ -9,4 +16,3 @@ - [ ] I have signed my commits following the instructions provided by [GitHub](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits). Note that we run [GitHub's commit verification](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification) tool to check the commit signatures. A green `verified` label should appear next to **all** of your commits on GitHub. - [ ] I have updated the relevant documentation, if applicable. - [ ] I have tested my changes and verified they work as expected. -- [ ] I have referenced the issue(s) this pull request solves. diff --git a/.github/workflows/_build.yaml b/.github/workflows/_build.yaml index 69fe04523..98f31eb4a 100644 --- a/.github/workflows/_build.yaml +++ b/.github/workflows/_build.yaml @@ -57,7 +57,7 @@ jobs: fetch-depth: 0 - name: Set up Python - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 + uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0 with: python-version: ${{ matrix.python }} @@ -129,7 +129,7 @@ jobs: # Currently reusable workflows do not support setting strategy property from the caller workflow. - name: Upload the package artifact for debugging and release if: matrix.os == env.ARTIFACT_OS && matrix.python == env.ARTIFACT_PYTHON - uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: artifact-${{ matrix.os }}-python-${{ matrix.python }} path: dist diff --git a/.github/workflows/_build_docker.yaml b/.github/workflows/_build_docker.yaml index 20faa3c14..b16167ddb 100644 --- a/.github/workflows/_build_docker.yaml +++ b/.github/workflows/_build_docker.yaml @@ -33,7 +33,7 @@ jobs: # The Docker integration tests require Python 3.11. - name: Set up Python - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 + uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0 with: python-version: '3.11' diff --git a/.github/workflows/codeql-analysis.yaml b/.github/workflows/codeql-analysis.yaml index fdf820984..7b7f6230b 100644 --- a/.github/workflows/codeql-analysis.yaml +++ b/.github/workflows/codeql-analysis.yaml @@ -40,7 +40,7 @@ jobs: uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 - name: Set up Python - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 + uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0 with: python-version: '3.11' diff --git a/.github/workflows/pr-conventional-commits.yaml b/.github/workflows/pr-conventional-commits.yaml index 77d798fa0..72bcdb4b1 100644 --- a/.github/workflows/pr-conventional-commits.yaml +++ b/.github/workflows/pr-conventional-commits.yaml @@ -30,7 +30,7 @@ jobs: fetch-depth: 0 - name: Set up Python - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 + uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0 with: python-version: '3.11' diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 9702ce5f4..36d45d033 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -45,7 +45,7 @@ jobs: token: ${{ secrets.REPO_ACCESS_TOKEN }} - name: Set up Python - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 + uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0 with: python-version: '3.11' @@ -215,7 +215,7 @@ jobs: rm -f "$CHECKSUMS" - name: Set up Python - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 + uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0 with: python-version: '3.11' diff --git a/.github/workflows/scorecards-analysis.yaml b/.github/workflows/scorecards-analysis.yaml index e7370fea7..9647e1db8 100644 --- a/.github/workflows/scorecards-analysis.yaml +++ b/.github/workflows/scorecards-analysis.yaml @@ -49,7 +49,7 @@ jobs: # Upload the results as artifacts (optional). - name: Upload artifact - uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: SARIF file path: results.sarif diff --git a/src/macaron/malware_analyzer/README.md b/src/macaron/malware_analyzer/README.md index 7617e4156..d5d30a670 100644 --- a/src/macaron/malware_analyzer/README.md +++ b/src/macaron/malware_analyzer/README.md @@ -61,8 +61,9 @@ When contributing an analyzer, it must meet the following requirements: - The analyzer name must be added to [heuristics.py](./pypi_heuristics/heuristics.py) file so it can be used for rule combinations in [detect_malicious_metadata_check.py](../slsa_analyzer/checks/detect_malicious_metadata_check.py) - Update the `malware_rules_problog_model` in [detect_malicious_metadata_check.py](../slsa_analyzer/checks/detect_malicious_metadata_check.py) with logical statements where the heuristic should be included. When adding new rules, please follow the following guidelines: - Provide a [confidence value](../slsa_analyzer/checks/check_result.py) using the `Confidence` enum. - - Provide a name based on this confidence value (i.e. `high`, `medium`, or `low`) - - If it does not already exist, make sure to assign this to the result variable (`problog_result_access`) + - Ensure it is assigned to the `problog_result_access` string variable, otherwise it will not be queried and evaluated. + - Assign a rule ID to the rule. This will be used to backtrack to determine if it was triggered. + - Make sure to wrap pass/fail statements in `passed()` and `failed()`. Not doing so may result in undesirable behaviour, see the comments in the model for more details. - If there are commonly used combinations introduced by adding the heuristic, combine and justify them at the top of the static model (see `quickUndetailed` and `forceSetup` as current examples). ### Confidence Score Motivation diff --git a/src/macaron/provenance/provenance_verifier.py b/src/macaron/provenance/provenance_verifier.py index 18e090f0c..44b2193eb 100644 --- a/src/macaron/provenance/provenance_verifier.py +++ b/src/macaron/provenance/provenance_verifier.py @@ -334,7 +334,10 @@ def _verify_slsa( except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as error: logger.error(error) - errors.append(error.output.decode("utf-8")) + if error.output: + errors.append(error.output.decode("utf-8")) + else: + errors.append(f"Verification failed: {type(error)}") except OSError as error: logger.error(error) errors.append(str(error)) diff --git a/src/macaron/slsa_analyzer/checks/detect_malicious_metadata_check.py b/src/macaron/slsa_analyzer/checks/detect_malicious_metadata_check.py index 80439bb79..9d20e12bc 100644 --- a/src/macaron/slsa_analyzer/checks/detect_malicious_metadata_check.py +++ b/src/macaron/slsa_analyzer/checks/detect_malicious_metadata_check.py @@ -128,7 +128,9 @@ def validate_malware(self, pypi_package_json: PyPIPackageJsonAsset) -> tuple[boo is_malware, detail_info = sourcecode_analyzer.analyze() return is_malware, detail_info - def evaluate_heuristic_results(self, heuristic_results: dict[Heuristics, HeuristicResult]) -> float | None: + def evaluate_heuristic_results( + self, heuristic_results: dict[Heuristics, HeuristicResult] + ) -> tuple[float, JsonType]: """Analyse the heuristic results to determine the maliciousness of the package. Parameters @@ -138,18 +140,19 @@ def evaluate_heuristic_results(self, heuristic_results: dict[Heuristics, Heurist Returns ------- - float | None - Returns the confidence associated with the detected malicious combination, otherwise None if no associated - malicious combination was triggered. + tuple[float, JsonType] + Returns the confidence associated with the detected malicious combination, and associated rule IDs detailing + what rules were triggered and their confidence as a dict[str, float] type. """ facts_list: list[str] = [] + triggered_rules: dict[str, JsonType] = {} + for heuristic, result in heuristic_results.items(): - if result == HeuristicResult.SKIP: - facts_list.append(f"0.0::{heuristic.value}.") - elif result == HeuristicResult.PASS: + if result == HeuristicResult.PASS: facts_list.append(f"{heuristic.value} :- true.") - else: # HeuristicResult.FAIL + elif result == HeuristicResult.FAIL: facts_list.append(f"{heuristic.value} :- false.") + # Do not define for HeuristicResult.SKIP facts = "\n".join(facts_list) problog_code = f"{facts}\n\n{self.malware_rules_problog_model}" @@ -158,10 +161,13 @@ def evaluate_heuristic_results(self, heuristic_results: dict[Heuristics, Heurist problog_model = PrologString(problog_code) problog_results: dict[Term, float] = get_evaluatable().create_from(problog_model).evaluate() - confidence: float | None = problog_results.get(Term(self.problog_result_access)) - if confidence == 0.0: - return None # no rules were triggered - return confidence + confidence = problog_results.pop(Term(self.problog_result_access), 0.0) + if confidence > 0: # a rule was triggered + for term, conf in problog_results.items(): + if term.args: + triggered_rules[str(term.args[0])] = conf + + return confidence, triggered_rules def run_heuristics( self, pypi_package_json: PyPIPackageJsonAsset @@ -278,9 +284,10 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData: except HeuristicAnalyzerValueError: return CheckResultData(result_tables=[], result_type=CheckResultType.UNKNOWN) - confidence = self.evaluate_heuristic_results(result) + confidence, triggered_rules = self.evaluate_heuristic_results(result) + detail_info["triggered_rules"] = triggered_rules result_type = CheckResultType.FAILED - if confidence is None: + if not confidence: confidence = Confidence.HIGH result_type = CheckResultType.PASSED elif ctx.dynamic_data["validate_malware"]: @@ -321,51 +328,79 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData: AnomalousVersionAnalyzer, ] + # name used to query the result of all problog rules, so it can be accessed outside the model. problog_result_access = "result" malware_rules_problog_model = f""" - % Heuristic groupings + % ----- Wrappers ------ + % When a heuristic is skipped, it is ommitted from the problog model facts definition. This means that references in this + % static model must account for when they are not existent. These wrappers perform this function using the inbuilt try_call + % problog function. It will try to evaluate the provided logic, and return false if it encounters an error, such as the fact + % not being defined. For example, you are expecting A to pass, so we do: + % + % passed(A) + % + % If A was 'true', then this will return true, as A did pass. If A was 'false', then this will return false, as A did not pass. + % If A was not defined, then this will return false, as A did not pass. + % Please use these wrappers throughout the problog model for logic definitions. + + passed(H) :- try_call(H). + failed(H) :- try_call(not H). + + % ----- Heuristic groupings ----- % These are common combinations of heuristics that are used in many of the rules, thus themselves representing % certain behaviors. When changing or adding rules here, if there are frequent combinations of particular % heuristics, group them together here. % Maintainer has recently joined, publishing an undetailed page with no links. - quickUndetailed :- not {Heuristics.EMPTY_PROJECT_LINK.value}, not {Heuristics.CLOSER_RELEASE_JOIN_DATE.value}. + quickUndetailed :- failed({Heuristics.EMPTY_PROJECT_LINK.value}), failed({Heuristics.CLOSER_RELEASE_JOIN_DATE.value}). % Maintainer releases a suspicious setup.py and forces it to run by omitting a .whl file. - forceSetup :- not {Heuristics.SUSPICIOUS_SETUP.value}, not {Heuristics.WHEEL_ABSENCE.value}. + forceSetup :- failed({Heuristics.SUSPICIOUS_SETUP.value}), failed({Heuristics.WHEEL_ABSENCE.value}). - % Suspicious Combinations + % ----- Suspicious Combinations ----- % Package released recently with little detail, forcing the setup.py to run. - {Confidence.HIGH.value}::high :- quickUndetailed, forceSetup, not {Heuristics.ONE_RELEASE.value}. - {Confidence.HIGH.value}::high :- quickUndetailed, forceSetup, not {Heuristics.HIGH_RELEASE_FREQUENCY.value}. + {Confidence.HIGH.value}::trigger(malware_high_confidence_1) :- + quickUndetailed, forceSetup, failed({Heuristics.ONE_RELEASE.value}). + {Confidence.HIGH.value}::trigger(malware_high_confidence_2) :- + quickUndetailed, forceSetup, failed({Heuristics.HIGH_RELEASE_FREQUENCY.value}). % Package released recently with little detail, with some more refined trust markers introduced: project links, % multiple different releases, but there is no source code repository matching it and the setup is suspicious. - {Confidence.HIGH.value}::high :- not {Heuristics.SOURCE_CODE_REPO.value}, - not {Heuristics.HIGH_RELEASE_FREQUENCY.value}, - not {Heuristics.CLOSER_RELEASE_JOIN_DATE.value}, - {Heuristics.UNCHANGED_RELEASE.value}, + {Confidence.HIGH.value}::trigger(malware_high_confidence_3) :- + failed({Heuristics.SOURCE_CODE_REPO.value}), + failed({Heuristics.HIGH_RELEASE_FREQUENCY.value}), + passed({Heuristics.UNCHANGED_RELEASE.value}), + failed({Heuristics.CLOSER_RELEASE_JOIN_DATE.value}), forceSetup. % Package released recently with little detail, with multiple releases as a trust marker, but frequent and with % the same code. - {Confidence.MEDIUM.value}::medium :- quickUndetailed, - not {Heuristics.HIGH_RELEASE_FREQUENCY.value}, - not {Heuristics.UNCHANGED_RELEASE.value}, - {Heuristics.SUSPICIOUS_SETUP.value}. + {Confidence.MEDIUM.value}::trigger(malware_medium_confidence_1) :- + quickUndetailed, + failed({Heuristics.HIGH_RELEASE_FREQUENCY.value}), + failed({Heuristics.UNCHANGED_RELEASE.value}), + passed({Heuristics.SUSPICIOUS_SETUP.value}). % Package released recently with little detail and an anomalous version number for a single-release package. - {Confidence.MEDIUM.value}::medium :- quickUndetailed, - not {Heuristics.ONE_RELEASE.value}, - {Heuristics.WHEEL_ABSENCE.value}, - not {Heuristics.ANOMALOUS_VERSION.value}. - - {problog_result_access} :- high. - {problog_result_access} :- medium. - + {Confidence.MEDIUM.value}::trigger(malware_medium_confidence_2) :- + quickUndetailed, + failed({Heuristics.ONE_RELEASE.value}), + failed({Heuristics.ANOMALOUS_VERSION.value}). + + % ----- Evaluation ----- + + % Aggregate result + {problog_result_access} :- trigger(malware_high_confidence_1). + {problog_result_access} :- trigger(malware_high_confidence_2). + {problog_result_access} :- trigger(malware_high_confidence_3). + {problog_result_access} :- trigger(malware_medium_confidence_2). + {problog_result_access} :- trigger(malware_medium_confidence_1). query({problog_result_access}). + + % Explainability + query(trigger(_)). """ diff --git a/tests/slsa_analyzer/checks/test_detect_malicious_metadata_check.py b/tests/slsa_analyzer/checks/test_detect_malicious_metadata_check.py index c6ecb044d..ca4f17ddf 100644 --- a/tests/slsa_analyzer/checks/test_detect_malicious_metadata_check.py +++ b/tests/slsa_analyzer/checks/test_detect_malicious_metadata_check.py @@ -12,6 +12,7 @@ from pytest_httpserver import HTTPServer from macaron.config.defaults import load_defaults +from macaron.malware_analyzer.pypi_heuristics.heuristics import HeuristicResult, Heuristics from macaron.slsa_analyzer.build_tool.base_build_tool import BaseBuildTool from macaron.slsa_analyzer.checks.check_result import CheckResultType from macaron.slsa_analyzer.checks.detect_malicious_metadata_check import DetectMaliciousMetadataCheck @@ -98,3 +99,34 @@ def test_detect_malicious_metadata( ).respond_with_json({}) assert check.run_check(ctx).result_type == expected + + +@pytest.mark.parametrize( + ("combination"), + [ + pytest.param( + { + # similar to rule ID malware_high_confidence_1, but SUSPICIOUS_SETUP is skipped since the file does not exist, + # so the rule should not trigger. + Heuristics.EMPTY_PROJECT_LINK: HeuristicResult.FAIL, + Heuristics.SOURCE_CODE_REPO: HeuristicResult.SKIP, + Heuristics.ONE_RELEASE: HeuristicResult.FAIL, + Heuristics.HIGH_RELEASE_FREQUENCY: HeuristicResult.SKIP, + Heuristics.UNCHANGED_RELEASE: HeuristicResult.SKIP, + Heuristics.CLOSER_RELEASE_JOIN_DATE: HeuristicResult.FAIL, + Heuristics.SUSPICIOUS_SETUP: HeuristicResult.SKIP, + Heuristics.WHEEL_ABSENCE: HeuristicResult.FAIL, + Heuristics.ANOMALOUS_VERSION: HeuristicResult.PASS, + }, + id="test_skipped_evaluation", + ) + ], +) +def test_evaluations(combination: dict[Heuristics, HeuristicResult]) -> None: + """Test heuristic combinations to ensure they evaluate as expected.""" + check = DetectMaliciousMetadataCheck() + + confidence, triggered_rules = check.evaluate_heuristic_results(combination) + assert confidence == 0 + # Expecting this to be a dictionary, so we can ignore the type problems + assert len(dict(triggered_rules)) == 0 # type: ignore[arg-type]