From fe1b6104ef9d565fa6397f49823b5595a70a85cb Mon Sep 17 00:00:00 2001 From: jdalton Date: Wed, 14 Oct 2026 20:03:16 -0400 Subject: [PATCH] Include API actions in JSON findings Expose the Socket API action on selected standard JSON findings while preserving legacy Issue serialization, equality, alert selection, and process exit behavior. Verify both real SDK transport paths with the local HTTP fixture. Assisted-by: Codex:gpt-6-luna --- docs/cli-reference.md | 7 + socketsecurity/core/__init__.py | 5 + socketsecurity/core/classes.py | 28 +- socketsecurity/core/report_output.py | 7 +- .../test_json_package_changes_transport.py | 284 +++++++++++++++++- tests/unit/test_report_output.py | 155 +++++++++- 6 files changed, 471 insertions(+), 15 deletions(-) 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