diff --git a/docs/cli-reference.md b/docs/cli-reference.md index aa75ff6..cb32af5 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -349,6 +349,13 @@ Either field is JSON null when its source value is unavailable. The existing are distinct from the scan-reference URLs above and do not identify an individual alert or triage page. +Each selected alert also includes `action`, the scalar action returned by the +Socket API, such as `error`, `warn`, `monitor`, or `ignore`. It is JSON null +when no action is available. Existing alert selection is unchanged: standard +output includes new error and warning findings, and `--strict-blocking` also +includes unchanged error and warning findings. The action field does not +change the CLI process exit code. + Standard Socket JSON also includes four package-change arrays: `new_packages`, `updated_packages`, `removed_packages`, and `replaced_packages`. Rows contain `id`, `name`, `version`, `ecosystem`, `direct`, `author`, `size`, diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index da08c04..7f4276e 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -2836,6 +2836,11 @@ def add_package_alerts_to_collection(self, package: Package, alerts_collection: pkg_type=package.type, pkg_name=package.name, pkg_version=package.version, + api_action=( + alert_item.get("action") + if isinstance(alert_item.get("action"), str) + else None + ), pkg_id=package.id, props=alert.props, key=alert.key, diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index 3eabd41..af476c7 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -322,12 +322,15 @@ class Issue: # introduced_by chain, but its manifest is still known. manifest_files: list direct: bool + api_action: Optional[str] def __init__(self, **kwargs): if kwargs: for key, value in kwargs.items(): setattr(self, key, value) + if not hasattr(self, "api_action"): + self.api_action = None if hasattr(self, "created_at"): self.created_at = self.created_at.strip(" (Coordinated Universal Time)") if not hasattr(self, "manifest_files"): @@ -355,13 +358,22 @@ def __init__(self, **kwargs): self.ignore = False def __str__(self): - return json.dumps(self.__dict__) + return json.dumps(self.to_dict()) + + def to_dict(self) -> dict[str, object]: + result = dict(self.__dict__) + result.pop("api_action", None) + return result def __eq__(self, other): - return self.__dict__ == other.__dict__ + other_dict = dict(other.__dict__) + other_dict.pop("api_action", None) + return self.to_dict() == other_dict def __ne__(self, other): - return self.__dict__ != other.__dict__ + other_dict = dict(other.__dict__) + other_dict.pop("api_action", None) + return self.to_dict() != other_dict class YamlFile: @@ -642,9 +654,13 @@ def to_dict(self) -> dict: "new_capabilities": self.new_capabilities, "removed_packages": [p.to_dict() for p in self.removed_packages], "replaced_packages": [p.to_dict() for p in self.replaced_packages], - "new_alerts": [alert.__dict__ for alert in self.new_alerts], - "unchanged_alerts": [alert.__dict__ for alert in self.unchanged_alerts] if hasattr(self, "unchanged_alerts") else [], - "removed_alerts": [alert.__dict__ for alert in self.removed_alerts] if hasattr(self, "removed_alerts") else [], + "new_alerts": [alert.to_dict() for alert in self.new_alerts], + "unchanged_alerts": [alert.to_dict() for alert in self.unchanged_alerts] + if hasattr(self, "unchanged_alerts") + else [], + "removed_alerts": [alert.to_dict() for alert in self.removed_alerts] + if hasattr(self, "removed_alerts") + else [], "id": self.id, "sbom": self.sbom if hasattr(self, "sbom") else [], "packages": {k: v.to_dict() for k, v in self.packages.items()} if hasattr(self, "packages") else {}, diff --git a/socketsecurity/core/report_output.py b/socketsecurity/core/report_output.py index b5b9b45..71326a0 100644 --- a/socketsecurity/core/report_output.py +++ b/socketsecurity/core/report_output.py @@ -142,7 +142,12 @@ def build_socket_json_report( ) selected_diff = clone_diff_with_selected_alerts(diff_report, selected_alerts) report = Messages.create_security_comment_json(selected_diff) - for alert_row in report["new_alerts"]: + for issue, alert_row in zip( + selected_alerts, + report["new_alerts"], + strict=True, + ): + alert_row["action"] = issue.api_action package_url = alert_row.get("url") alert_type = alert_row.get("type") links = build_alert_links( diff --git a/tests/core/test_json_package_changes_transport.py b/tests/core/test_json_package_changes_transport.py index 2171fba..e176fbc 100644 --- a/tests/core/test_json_package_changes_transport.py +++ b/tests/core/test_json_package_changes_transport.py @@ -2,6 +2,7 @@ from __future__ import annotations +import copy from collections.abc import Iterator from dataclasses import replace @@ -11,10 +12,11 @@ from socketsecurity.config import CliConfig from socketsecurity.core import Core -from socketsecurity.core.classes import Package +from socketsecurity.core.classes import Diff, Package from socketsecurity.core.messages import Messages from socketsecurity.core.report_output import build_socket_json_report from socketsecurity.core.socket_config import SocketConfig +from socketsecurity.output import OutputHandler from tests.support.socket_api_fixture import ( SocketApiFixture, SocketResponse, @@ -665,6 +667,286 @@ def test_json_contains_real_classified_packages_without_extra_transport( ] +@pytest.mark.parametrize("streaming_fallback", [False, True]) +def test_json_alert_actions_follow_real_sdk_transport_and_selection( + socket_api: SocketApiFixture, + streaming_fallback: bool, +) -> None: + core = _core(socket_api) + + def api_alert( + key: str, + action: str | None, + policy_index: int, + *, + include_action: bool = True, + ) -> dict[str, object]: + result: dict[str, object] = { + "key": key, + "type": "envVars", + "severity": "high", + "category": "supplyChainRisk", + "props": {"source": "local-fixture"}, + "actionPolicyIndex": policy_index, + } + if include_action: + result["action"] = action + return result + + def artifact( + artifact_id: str, + change_type: str, + name: str, + *, + action: str | None = None, + policy_index: int = 0, + alert_key: str | None = None, + include_action: bool = True, + ) -> dict[str, object]: + return _artifact( + artifact_id, + change_type, + name, + "1.0.0", + score=_score(), + alerts=( + [ + api_alert( + alert_key or f"{artifact_id}-alert", + action, + policy_index, + include_action=include_action, + ) + ] + ), + ) + + artifacts = { + "added": [ + artifact("added-error", "added", "error-lib", action="error"), + artifact("added-warning", "added", "warning-lib", action="warn"), + artifact("added-monitor", "added", "monitor-lib", action="monitor"), + artifact("added-ignore", "added", "ignore-lib", action="ignore"), + artifact("added-absent", "added", "absent-lib", include_action=False), + artifact("added-null", "added", "null-lib", action=None), + artifact("added-empty", "added", "empty-lib", action=""), + artifact( + "same-artifact", + "added", + "same-lib", + action="warn", + policy_index=1, + alert_key="same-alert-key", + ), + artifact( + "changed-artifact", + "added", + "changed-lib", + action="error", + policy_index=4, + alert_key="changed-alert-key", + ), + ], + "updated": [], + "removed": [ + artifact( + "same-artifact", + "removed", + "same-lib", + action="warn", + policy_index=99, + alert_key="same-alert-key", + ), + artifact( + "changed-artifact", + "removed", + "changed-lib", + action="warn", + policy_index=8, + alert_key="changed-alert-key", + ), + ], + "replaced": [], + "unchanged": [ + artifact( + "unchanged-error", + "unchanged", + "unchanged-error-lib", + action="error", + ), + artifact( + "unchanged-warning", + "unchanged", + "unchanged-warning-lib", + action="warn", + ), + ], + } + artifacts_before = copy.deepcopy(artifacts) + if streaming_fallback: + socket_api.route( + "POST", + "orgs/fixture-org/diff-scans/from-ids", + SocketResponse(403, {"error": {"message": "forbidden"}}), + ) + socket_api.route( + "GET", + "orgs/fixture-org/full-scans/diff", + SocketResponse(200, _stream_response(artifacts)), + ) + else: + socket_api.route( + "POST", + "orgs/fixture-org/diff-scans/from-ids", + SocketResponse(201, {"diff_scan": {"id": "diff-action-fixture"}}), + ) + socket_api.route( + "GET", + "orgs/fixture-org/diff-scans/diff-action-fixture", + SocketResponse( + 200, + { + "diff_scan": { + "artifacts": artifacts, + "before_full_scan": {"id": "baseline-id"}, + "after_full_scan": {"id": "new-scan-id"}, + } + }, + ), + ) + + comparison = core._get_package_comparison( + "baseline-id", "new-scan-id", include_license_details=False + ) + requests_after_comparison = len(socket_api.requests) + unchanged_packages = { + package.id: package + for package in comparison.packages.values() + if package.id in {"unchanged-error", "unchanged-warning"} + } + diff = core.create_diff_report( + comparison.added_packages, + comparison.removed_packages, + unchanged_packages, + ) + report = build_socket_json_report( + diff, + CliConfig(api_token="fictional-local-only", repo="fixture-repo"), + ) + strict_report = build_socket_json_report( + diff, + CliConfig( + api_token="fictional-local-only", + repo="fixture-repo", + strict_blocking=True, + ), + ) + + assert [issue.key for issue in diff.new_alerts] == [ + "added-error-alert", + "added-warning-alert", + "changed-alert-key", + ] + assert [issue.api_action for issue in diff.new_alerts] == [ + "error", + "warn", + "error", + ] + assert diff.new_alerts[0].error is True + assert diff.new_alerts[0].warn is False + assert diff.new_alerts[1].error is False + assert diff.new_alerts[1].warn is True + assert diff.new_alerts[2].error is True + assert diff.new_alerts[2].warn is False + assert [issue.api_action for issue in diff.unchanged_alerts] == ["error", "warn"] + assert [issue.key for issue in diff.unchanged_alerts] == [ + "unchanged-error-alert", + "unchanged-warning-alert", + ] + assert [row["action"] for row in report["new_alerts"]] == [ + "error", + "warn", + "error", + ] + assert [row["action"] for row in strict_report["new_alerts"]] == [ + "error", + "warn", + "error", + "error", + "warn", + ] + assert report["scan_failed"] is True + assert report["new_alerts"][0]["introduced_by"] == [["direct", "requirements.txt"]] + assert report["new_alerts"][0]["package_url"] == ( + "https://socket.dev/pypi/package/error-lib/overview/1.0.0" + ) + assert report["new_alerts"][0]["info_url"] == ("https://socket.dev/alerts/envVars") + + inspected_actions = {} + for package_id in ( + "added-error", + "added-warning", + "added-monitor", + "added-ignore", + "added-absent", + "added-null", + "added-empty", + "same-artifact", + "changed-artifact", + ): + package = comparison.added_packages[package_id] + collection = core.add_package_alerts_to_collection( + package, + {}, + comparison.added_packages, + ) + issues = [issue for rows in collection.values() for issue in rows] + assert len(issues) == 1 + issue = issues[0] + inspected_actions[package_id] = issue.api_action + assert not hasattr(issue, "actionPolicyIndex") + if package_id == "added-monitor": + assert issue.monitor is True + if package_id == "added-ignore": + assert issue.ignore is True + assert inspected_actions == { + "added-error": "error", + "added-warning": "warn", + "added-monitor": "monitor", + "added-ignore": "ignore", + "added-absent": None, + "added-null": None, + "added-empty": "", + "same-artifact": "warn", + "changed-artifact": "error", + } + removed_collection = core.add_package_alerts_to_collection( + comparison.removed_packages["changed-artifact"], + {}, + comparison.removed_packages, + ) + removed_changed_issue = next( + issue for rows in removed_collection.values() for issue in rows + ) + assert removed_changed_issue.api_action == "warn" + assert removed_changed_issue.error is False + assert removed_changed_issue.warn is True + + warning_issue = next(issue for issue in diff.new_alerts if issue.warn) + error_config = CliConfig(api_token="fictional-local-only", repo="fixture-repo") + disable_blocking_config = CliConfig( + api_token="fictional-local-only", + repo="fixture-repo", + disable_blocking=True, + ) + warning_diff = Diff(new_alerts=[warning_issue]) + assert OutputHandler(error_config, core.sdk).return_exit_code(diff) == 1 + assert OutputHandler(disable_blocking_config, core.sdk).return_exit_code(diff) == 0 + assert OutputHandler(error_config, core.sdk).return_exit_code(warning_diff) == 0 + assert len(socket_api.requests) == requests_after_comparison + assert artifacts == artifacts_before + + def test_no_run_keeps_change_lists_empty_without_scan_requests( socket_api: SocketApiFixture, ) -> None: diff --git a/tests/unit/test_report_output.py b/tests/unit/test_report_output.py index ff2d4a9..f33acc1 100644 --- a/tests/unit/test_report_output.py +++ b/tests/unit/test_report_output.py @@ -3,6 +3,7 @@ from socketsecurity.config import CliConfig from socketsecurity.core.classes import Diff, Issue, Package, Purl +from socketsecurity.core.messages import Messages from socketsecurity.core.report_output import ( build_console_summary, build_report_references, @@ -23,8 +24,9 @@ def _issue( *, error: bool = False, warn: bool = False, + api_action: str | None = None, ) -> Issue: - return Issue( + issue = Issue( pkg_type="pypi", pkg_name="fictional-package", pkg_version="1.2.3", @@ -48,6 +50,9 @@ def _issue( url="https://socket.dev/pypi/package/fictional-package/alerts/1.2.3", purl="pkg:pypi/fictional-package@1.2.3", ) + if api_action is not None: + issue.api_action = api_action + return issue def _purl( @@ -187,18 +192,20 @@ def test_strict_blocking_adds_unchanged_alerts_without_changing_references(): def test_full_scan_uses_report_url_and_non_new_finding_labels(): diff = _diff( - new_alerts=[_issue("Full scan warning", warn=True)], + new_alerts=[_issue("Full scan warning", error=True, api_action="error")], is_full_scan=True, ) summary = build_console_summary(diff, _config()) + report = build_socket_json_report(diff, _config()) - assert "Blocking issues:" not in summary - assert "Warning issues: 1" in summary + assert "Blocking issues: 1" in summary assert "NEW warning issues" not in summary assert "whole repository, with no baseline" in summary assert "Report Url: https://socket.dev/orgs/fictional/reports/full-123" in summary assert "Diff Url:" not in summary + assert report["scan_failed"] is True + assert report["new_alerts"][0]["action"] == "error" def test_clean_full_scan_uses_diff_url_as_report_url_fallback(): @@ -674,26 +681,32 @@ def test_full_scan_package_inventory_does_not_become_change_lists(): def test_alert_json_links_are_additive_and_do_not_mutate_real_models(): - issue = _issue("envVars", warn=True) + issue = _issue("envVars", warn=True, api_action="warn") issue.url = "https://packages.example.test/@scope/pkg?view=a&mode=b#source" issue.purl = "pkg:npm/@scope/pkg@1.2.3" issue.props = {"nested": {"value": ["kept"]}} - diff = _diff(new_alerts=[issue]) + unchanged = _issue("unchanged", warn=True, api_action="monitor") + unchanged.props = {"nested": {"value": ["strict row"]}} + diff = _diff(new_alerts=[issue], unchanged_alerts=[unchanged]) diff_before = copy.deepcopy(diff.__dict__) issue_before = copy.deepcopy(issue.__dict__) + unchanged_before = copy.deepcopy(unchanged.__dict__) original_alert = json.loads(str(issue)) - report = build_socket_json_report(diff, _config()) + report = build_socket_json_report(diff, _config(strict_blocking=True)) serialized = json.loads(json.dumps(report)) alert_row = serialized["new_alerts"][0] assert alert_row == { **original_alert, + "action": "warn", "package_url": "https://packages.example.test/@scope/pkg?view=a&mode=b#source", "info_url": "https://socket.dev/alerts/envVars", } assert alert_row["url"] == original_alert["url"] assert alert_row["purl"] == "pkg:npm/@scope/pkg@1.2.3" + assert alert_row["action"] == "warn" + assert serialized["new_alerts"][1]["action"] == "monitor" assert set(report) == { "scan_failed", "new_alerts", @@ -717,6 +730,7 @@ def test_alert_json_links_are_additive_and_do_not_mutate_real_models(): assert len(report) == 18 assert diff.__dict__ == diff_before assert issue.__dict__ == issue_before + assert unchanged.__dict__ == unchanged_before assert "package_url" not in issue.__dict__ assert "info_url" not in issue.__dict__ @@ -753,3 +767,130 @@ def test_alert_json_links_follow_existing_selection_and_emit_nulls(): assert ordinary["scan_failed"] is False assert strict["scan_failed"] is False assert build_socket_json_report(_diff(), _config())["new_alerts"] == [] + + +def test_selected_json_alerts_preserve_nullable_raw_api_actions(): + issues = [ + _issue("error", error=True, api_action="error"), + _issue("warning", warn=True, api_action="warn"), + _issue("inconsistent", error=True), + _issue("future", warn=True, api_action="future-api-action"), + _issue("empty", warn=True, api_action=""), + ] + + report = build_socket_json_report(_diff(new_alerts=issues), _config()) + + assert [row["action"] for row in report["new_alerts"]] == [ + "error", + "warn", + None, + "future-api-action", + "", + ] + assert set(report) == { + "scan_failed", + "new_alerts", + "full_scan_id", + "diff_url", + "report_url", + "diff_scan_id", + "old_scan_id", + "new_scan_id", + "old_scan_url", + "new_scan_url", + "repo", + "branch", + "commit_sha", + "legal_mode", + "new_packages", + "updated_packages", + "removed_packages", + "replaced_packages", + } + + +def test_strict_json_action_pairing_preserves_selected_order_for_duplicate_keys(): + new_alerts = [ + _issue("same-key", error=True, api_action="new-error"), + _issue("same-key", warn=True, api_action="new-warning"), + ] + unchanged_alerts = [ + _issue("same-key", error=True, api_action="old-error"), + _issue("same-key", warn=True, api_action="old-warning"), + ] + diff = _diff(new_alerts=new_alerts, unchanged_alerts=unchanged_alerts) + + ordinary = build_socket_json_report(diff, _config()) + strict = build_socket_json_report(diff, _config(strict_blocking=True)) + + assert [row["action"] for row in ordinary["new_alerts"]] == [ + "new-error", + "new-warning", + ] + assert [row["action"] for row in strict["new_alerts"]] == [ + "new-error", + "new-warning", + "old-error", + "old-warning", + ] + assert [row["type"] for row in strict["new_alerts"]] == [ + "same-key", + "same-key", + "same-key", + "same-key", + ] + + +def test_issue_action_is_informational_for_legacy_identity_and_serializers(): + issue_without_action = _issue("identity", warn=True) + issue_with_action = _issue("identity", warn=True, api_action="warn") + issue_with_other_action = _issue("identity", warn=True, api_action="ignore") + explicit_none = _issue("identity", warn=True) + explicit_none.api_action = None + + assert issue_without_action.api_action is None + assert issue_without_action == issue_with_action + assert (issue_without_action != issue_with_action) is False + assert issue_with_action == issue_with_other_action + assert (issue_with_action != issue_with_other_action) is False + assert issue_without_action == explicit_none + assert (issue_without_action != explicit_none) is False + + changed_error = _issue("identity", error=True, api_action="warn") + changed_props = _issue("identity", warn=True, api_action="warn") + changed_props.props = {"nested": ["changed"]} + changed_version = _issue("identity", warn=True, api_action="warn") + changed_version.pkg_version = "9.9.9" + assert issue_with_action != changed_error + assert issue_with_action != changed_props + assert issue_with_action != changed_version + + issue_with_action.props = {"nested": {"value": ["kept"]}} + issue_with_action.existing_dynamic_field = {"kept": ["value"]} + same_dynamic_field = _issue("identity", warn=True) + same_dynamic_field.props = {"nested": {"value": ["kept"]}} + same_dynamic_field.existing_dynamic_field = {"kept": ["value"]} + assert issue_with_action == same_dynamic_field + same_dynamic_field.existing_dynamic_field = {"kept": ["changed"]} + assert issue_with_action != same_dynamic_field + projection = issue_with_action.to_dict() + assert projection is not issue_with_action.__dict__ + assert projection["props"] is issue_with_action.props + assert ( + projection["existing_dynamic_field"] is issue_with_action.existing_dynamic_field + ) + assert "api_action" not in projection + assert "api_action" not in json.loads(str(issue_with_action)) + + diff = _diff( + new_alerts=[issue_with_action], + unchanged_alerts=[issue_with_action], + ) + diff.removed_alerts = [issue_with_action] + message_json = Messages.create_security_comment_json(diff) + diff_json = diff.to_dict() + assert "api_action" not in message_json["new_alerts"][0] + assert "action" not in message_json["new_alerts"][0] + for field in ("new_alerts", "unchanged_alerts", "removed_alerts"): + assert "api_action" not in diff_json[field][0] + assert diff_json[field][0] == projection