Skip to content

Commit 63e938d

Browse files
leliaclaude
lelia
and
claude
committed
fix(comments): stop legacy comment updates crashing on scoped names
process_original_security_comment split the package cell on every "@", so a scoped name carrying its own "@" unpacked into three values and raised an uncaught ValueError. Same bug class this branch already fixed one function over in process_updated_security_comment, just left in its sibling. Split from the right, and pass the ecosystem through as pkg_type rather than pre-concatenating it onto the package name. That makes the two comment formats agree: both now accept an ignore command for a scoped package in either the ecosystem-qualified or the bare form, where the legacy path previously matched only the qualified one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent fc36c76 commit 63e938d

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

‎socketsecurity/core/scm_comments.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,14 @@ def process_original_security_comment(
152152
details, _ = package.split("](")
153153
ecosystem, details = details.split("/", 1)
154154
ecosystem = ecosystem.lstrip("[")
155-
pkg_name, pkg_version = details.split("@")
156-
pkg_name = f"{ecosystem}/{pkg_name}"
155+
# Split from the right: a scoped name carries its own "@", so
156+
# split("@") unpacks into three parts and raises.
157+
pkg_name, pkg_version = details.rsplit("@", 1)
157158
# ignore_all has to be checked outside the loop: an ignore-all
158159
# comment produces no ignore_commands, so a loop-internal check
159160
# never runs and every row was kept.
160161
ignore = ignore_all or any(
161-
Comments.is_ignore(pkg_name, pkg_version, name, version)
162+
Comments.is_ignore(pkg_name, pkg_version, name, version, ecosystem)
162163
for name, version in ignore_commands
163164
)
164165
if not ignore:

‎tests/unit/test_pr_comment_rendering.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,17 @@ def test_collapsed_body_is_stable_when_reprocessed(self):
289289
[View full report](https://socket.dev/report/legacy?action=error%2Cwarn)
290290
"""
291291

292+
SCOPED_LEGACY_COMMENT = """<!-- socket-security-comment-actions -->
293+
294+
<!-- start-socket-alerts-table -->
295+
|Alert|Package|Introduced by|Manifest File|CI|
296+
|:---|:---|:---|:---|:---|
297+
|Known Malware|[npm/@socketsecurity/example@1.0.0](https://socket.dev/z)|example|package.json|:no_entry_sign:|
298+
<!-- end-socket-alerts-table -->
299+
300+
[View full report](https://socket.dev/report/legacy?action=error%2Cwarn)
301+
"""
302+
292303

293304
class TestProcessOriginalSecurityComment:
294305
def test_partial_ignore_keeps_remaining_row(self):
@@ -316,6 +327,27 @@ def test_ignore_all_collapses_to_the_no_alerts_body(self):
316327
assert "No dependency alerts to report" in new_body
317328
assert "[View full report](https://socket.dev/report/legacy)" in new_body
318329

330+
def test_scoped_package_row_does_not_raise(self):
331+
"""A scoped name carries its own "@", so split("@") unpacked into three."""
332+
security = _make_comment(SCOPED_LEGACY_COMMENT)
333+
comments = {"security": security, "ignore": []}
334+
335+
new_body = Comments.process_security_comment(security, comments)
336+
337+
assert "npm/@socketsecurity/example@1.0.0" in new_body
338+
339+
def test_scoped_package_row_is_ignorable_both_ways(self):
340+
for command in (
341+
"SocketSecurity ignore npm/@socketsecurity/example@1.0.0",
342+
"SocketSecurity ignore @socketsecurity/example@1.0.0",
343+
):
344+
security = _make_comment(SCOPED_LEGACY_COMMENT)
345+
comments = {"security": security, "ignore": [_make_comment(command, comment_id=2)]}
346+
347+
new_body = Comments.process_security_comment(security, comments)
348+
349+
assert "No dependency alerts to report" in new_body, command
350+
319351

320352
class TestExtractReportUrl:
321353
def test_strips_the_action_filter(self):

0 commit comments

Comments
 (0)