Skip to content

Commit 4797cb9

Browse files
committed
fix(comments): make per-alert ignores round trip
1 parent 2b23f28 commit 4797cb9

4 files changed

Lines changed: 72 additions & 17 deletions

File tree

socketsecurity/core/messages.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -936,7 +936,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
936936
> **Review the following alerts detected in dependencies.**
937937
>
938938
> According to your organization's policies, you **must** resolve all **"Block"** alerts before proceeding. It's recommended to resolve **"Warn"** alerts too.
939-
> Learn more about [Socket for GitHub](https://socket.dev?utm_medium=gh).
939+
> Learn more about [Socket](https://socket.dev).
940940
941941
<!-- start-socket-updated-alerts-table -->
942942
<table>
@@ -968,7 +968,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
968968
# Generate a table row for each alert
969969
ignore_html = (
970970
f"<p><em>Mark as acceptable risk:</em> To ignore this alert only in this pull request, reply with:<br/>"
971-
f"<code>@SocketSecurity ignore {alert.pkg_name}@{alert.pkg_version}</code><br/>"
971+
f"<code>@SocketSecurity ignore {alert.pkg_type}/{alert.pkg_name}@{alert.pkg_version}</code><br/>"
972972
f"Or ignore all future alerts with:<br/>"
973973
f"<code>@SocketSecurity ignore-all</code></p>"
974974
) if show_ignore else ""
@@ -1032,7 +1032,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
10321032

10331033
license_ignore_html = (
10341034
f"<p><em>Mark the package as acceptable risk:</em> To ignore this alert only in this pull request, reply with the comment "
1035-
f"<code>@SocketSecurity ignore {first_alert.pkg_name}@{first_alert.pkg_version}</code>. "
1035+
f"<code>@SocketSecurity ignore {first_alert.pkg_type}/{first_alert.pkg_name}@{first_alert.pkg_version}</code>. "
10361036
f"You can also ignore all packages with <code>@SocketSecurity ignore-all</code>. "
10371037
f"To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.</p>"
10381038
) if show_ignore else ""

socketsecurity/core/scm_comments.py

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ def remove_alerts(comments: dict, new_alerts: list) -> list:
3737
if ignore_all:
3838
break
3939
else:
40-
full_name = f"{alert.pkg_type}/{alert.pkg_name}"
41-
purl = (full_name, alert.pkg_version)
42-
purl_star = (full_name, "*")
43-
if purl in ignore_commands or purl_star in ignore_commands:
40+
if any(
41+
Comments.is_ignore(alert.pkg_name, alert.pkg_version, name, version, alert.pkg_type)
42+
for name, version in ignore_commands
43+
):
4444
log.info(f"Alerts for {alert.pkg_name}@{alert.pkg_version} ignored")
4545
else:
4646
log.info(f"Adding alert {alert.type} for {alert.pkg_name}@{alert.pkg_version}")
@@ -66,20 +66,28 @@ def get_ignore_options(comments: dict) -> [bool, list]:
6666
ignore_all = True
6767
else:
6868
command = command.lstrip("ignore").strip()
69-
name, version = command.split("@")
70-
data = (name, version)
69+
name, separator, version = command.rpartition("@")
70+
if not separator or not name or not version:
71+
raise ValueError("Expected package@version")
72+
data = (name.strip(), version.strip())
7173
ignore_commands.append(data)
7274
except Exception as error:
7375
log.error(f"Unable to process ignore command for {comment}")
7476
log.error(error)
7577
return ignore_all, ignore_commands
7678

7779
@staticmethod
78-
def is_ignore(pkg_name: str, pkg_version: str, name: str, version: str) -> bool:
79-
result = False
80-
if pkg_name == name and (pkg_version == version or version == "*"):
81-
result = True
82-
return result
80+
def is_ignore(
81+
pkg_name: str, pkg_version: str, name: str, version: str,
82+
pkg_type: str = ""
83+
) -> bool:
84+
package_names = {pkg_name}
85+
if pkg_type:
86+
package_names.add(f"{pkg_type}/{pkg_name}")
87+
target_names = {name}
88+
if not pkg_type and "/" in name:
89+
target_names.add(name.split("/", 1)[1])
90+
return bool(package_names & target_names) and (pkg_version == version or version == "*")
8391

8492
@staticmethod
8593
def is_heading_line(line) -> bool:
@@ -187,7 +195,7 @@ def process_updated_security_comment(
187195
# Extract package name and version from the comment
188196
try:
189197
start_marker = stripped[len("<!-- start-socket-alert-"):-4] # Strip the comment markers
190-
pkg_name, pkg_version = start_marker.split("@") # Extract pkg_name and pkg_version
198+
pkg_name, pkg_version = start_marker.rsplit("@", 1)
191199
except ValueError:
192200
pkg_name, pkg_version = "", ""
193201

tests/unit/test_disable_ignore.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
"""Tests for the --disable-ignore flag."""
22

3-
import pytest
43
from dataclasses import dataclass
54

65
from socketsecurity.config import CliConfig
76
from socketsecurity.core.classes import Comment, Diff, Issue
87
from socketsecurity.core.messages import Messages
98
from socketsecurity.core.scm_comments import Comments
109

11-
1210
# --- CLI flag parsing tests ---
1311

1412
class TestDisableIgnoreFlag:
@@ -84,6 +82,25 @@ def test_remove_alerts_suppresses_matching_alert(self):
8482
result = Comments.remove_alerts(comments, [alert])
8583
assert len(result) == 0
8684

85+
def test_bare_package_name_remains_supported(self):
86+
alert = _make_alert()
87+
ignore_comment = _make_comment("SocketSecurity ignore lodash@4.17.21")
88+
comments = Comments.check_for_socket_comments({ignore_comment.id: ignore_comment})
89+
90+
assert Comments.remove_alerts(comments, [alert]) == []
91+
92+
def test_scoped_package_name_is_parsed_from_the_right(self):
93+
alert = _make_alert(
94+
pkg_name="@socketsecurity/example",
95+
purl="pkg:npm/@socketsecurity/example@4.17.21",
96+
)
97+
ignore_comment = _make_comment(
98+
"SocketSecurity ignore npm/@socketsecurity/example@4.17.21"
99+
)
100+
comments = Comments.check_for_socket_comments({ignore_comment.id: ignore_comment})
101+
102+
assert Comments.remove_alerts(comments, [alert]) == []
103+
87104
def test_alerts_preserved_when_no_ignore_comments(self):
88105
"""With --disable-ignore the caller skips remove_alerts entirely,
89106
which is equivalent to passing empty comments."""
@@ -125,6 +142,12 @@ def test_ignore_instructions_shown_by_default(self):
125142
assert "@SocketSecurity ignore" in comment
126143
assert "Mark as acceptable risk" in comment
127144

145+
def test_ignore_instruction_uses_ecosystem_qualified_package(self):
146+
diff = self._make_diff_with_alert()
147+
comment = Messages.security_comment_template(diff, _FakeConfig())
148+
149+
assert "@SocketSecurity ignore npm/lodash@4.17.21" in comment
150+
128151
def test_ignore_instructions_hidden_when_disabled(self):
129152
diff = self._make_diff_with_alert()
130153
config = _FakeConfig(disable_ignore=True)

tests/unit/test_pr_comment_rendering.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,13 @@ def test_alert_markers_are_preserved(self):
155155
assert "<!-- start-socket-alert-lodash@4.17.21 -->" in body
156156
assert "<!-- end-socket-alert-lodash@4.17.21 -->" in body
157157

158+
def test_copy_is_provider_neutral(self):
159+
body = Messages.security_comment_template(
160+
_make_diff([_make_alert()]), _FakeConfig(scm="gitlab")
161+
)
162+
assert "Socket for GitHub" not in body
163+
assert "Learn more about [Socket]" in body
164+
158165

159166
class TestSecurityCommentTemplateWithNoAlerts:
160167
def test_no_alerts_omits_the_empty_table(self):
@@ -232,6 +239,23 @@ def test_ignoring_every_alert_individually_collapses_too(self):
232239

233240
assert "No dependency alerts to report" in new_body
234241

242+
def test_qualified_scoped_package_ignore_matches_comment_marker(self):
243+
security = _security_comment_with([
244+
_make_alert(
245+
pkg_name="@socketsecurity/example",
246+
purl="pkg:npm/@socketsecurity/example@4.17.21",
247+
)
248+
])
249+
comments = {
250+
"security": security,
251+
"ignore": [_make_comment(
252+
"SocketSecurity ignore npm/@socketsecurity/example@4.17.21",
253+
comment_id=2,
254+
)],
255+
}
256+
257+
assert "No dependency alerts to report" in Comments.process_security_comment(security, comments)
258+
235259
def test_no_ignore_commands_leaves_alerts_in_place(self):
236260
security = self._two_alert_comment()
237261
comments = {"security": security, "ignore": []}

0 commit comments

Comments
 (0)