Skip to content

Commit 0a4546f

Browse files
committed
feat: add --enable-diff flag and improve license policy violation handling
- Add --enable-diff flag to force differential scanning even when using --integration api - Improve license policy violation grouping and display in PR comments - Fix alert consolidation logic to prevent duplicate alerts based on manifest files - Enhance empty baseline scan creation with proper file cleanup - Add comprehensive test coverage for new enable_diff functionality - Update documentation with new scanning mode examples and usage patterns The --enable-diff flag enables differential mode without SCM integration, useful for getting diff reports while using the API integration type. License policy violations are now properly grouped by package and displayed with consistent formatting in GitHub PR comments.
1 parent 14a43da commit 0a4546f

File tree

8 files changed

+165
-19
lines changed

8 files changed

+165
-19
lines changed

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ If you don't want to provide the Socket API Token every time then you can use th
116116
|:-------------------------|:---------|:--------|:----------------------------------------------------------------------|
117117
| --ignore-commit-files | False | False | Ignore commit files |
118118
| --disable-blocking | False | False | Disable blocking mode |
119+
| --enable-diff | False | False | Enable diff mode even when using --integration api (forces diff mode without SCM integration) |
119120
| --scm | False | api | Source control management type |
120121
| --timeout | False | | Timeout in seconds for API requests |
121122
| --include-module-folders | False | False | If enabled will include manifest files from folders like node_modules |
@@ -205,13 +206,15 @@ The CLI determines which files to scan based on the following logic:
205206
- **Differential Mode**: When manifest files are detected in changes, performs a diff scan with PR/MR comment integration
206207
- **API Mode**: When no manifest files are in changes, creates a full scan report without PR comments but still scans the entire repository
207208
- **Force Mode**: With `--ignore-commit-files`, always performs a full scan regardless of changes
209+
- **Forced Diff Mode**: With `--enable-diff`, forces differential mode even when using `--integration api` (without SCM integration)
208210

209211
### Examples
210212

211213
- **Commit with manifest file**: If your commit includes changes to `package.json`, a differential scan will be triggered automatically with PR comment integration.
212214
- **Commit without manifest files**: If your commit only changes non-manifest files (like `.github/workflows/socket.yaml`), the CLI automatically switches to API mode and performs a full repository scan.
213215
- **Using `--files`**: If you specify `--files '["package.json"]'`, the CLI will check if this file exists and is a manifest file before determining scan type.
214216
- **Using `--ignore-commit-files`**: This forces a full scan of all manifest files in the target path, regardless of what's in your commit.
217+
- **Using `--enable-diff`**: Forces diff mode without SCM integration - useful when you want differential scanning but are using `--integration api`. For example: `socketcli --integration api --enable-diff --target-path /path/to/repo`
215218
- **Auto-detection**: Most CI/CD scenarios now work with just `socketcli --target-path /path/to/repo --scm github --pr-number $PR_NUM`
216219
217220
## Debugging and Troubleshooting

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
66

77
[project]
88
name = "socketsecurity"
9-
version = "2.1.36"
9+
version = "2.1.37"
1010
requires-python = ">= 3.10"
1111
license = {"file" = "LICENSE"}
1212
dependencies = [

socketsecurity/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
__author__ = 'socket.dev'
2-
__version__ = '2.1.36'
2+
__version__ = '2.1.37'

socketsecurity/config.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ class CliConfig:
4848
integration_type: IntegrationType = "api"
4949
integration_org_slug: Optional[str] = None
5050
pending_head: bool = False
51+
enable_diff: bool = False
5152
timeout: Optional[int] = 1200
5253
exclude_license_details: bool = False
5354
include_module_folders: bool = False
@@ -421,6 +422,12 @@ def create_argument_parser() -> argparse.ArgumentParser:
421422
action="store_true",
422423
help=argparse.SUPPRESS
423424
)
425+
advanced_group.add_argument(
426+
"--enable-diff",
427+
dest="enable_diff",
428+
action="store_true",
429+
help="Enable diff mode even when using --integration api (forces diff mode without SCM integration)"
430+
)
424431
advanced_group.add_argument(
425432
"--scm",
426433
metavar="<type>",

socketsecurity/core/__init__.py

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import os
33
import sys
44
import tarfile
5+
import tempfile
56
import time
67
import io
78
import json
@@ -433,12 +434,22 @@ def to_case_insensitive_regex(input_string: str) -> str:
433434
return ''.join(f'[{char.lower()}{char.upper()}]' if char.isalpha() else char for char in input_string)
434435

435436
@staticmethod
436-
def empty_head_scan_file() -> list[tuple[str, tuple[str, Union[BinaryIO, BytesIO]]]]:
437-
# Create an empty file for when no head full scan so that the diff endpoint can always be used
438-
empty_file_obj = io.BytesIO(b"")
439-
empty_filename = "initial_head_scan"
440-
empty_full_scan_file = [(empty_filename, (empty_filename, empty_file_obj))]
441-
return empty_full_scan_file
437+
def empty_head_scan_file() -> List[str]:
438+
"""
439+
Creates a temporary empty file for baseline scans when no head scan exists.
440+
441+
Returns:
442+
List containing path to a temporary empty file
443+
"""
444+
# Create a temporary empty file
445+
temp_fd, temp_path = tempfile.mkstemp(suffix='.empty', prefix='socket_baseline_')
446+
447+
# Close the file descriptor since we just need the path
448+
# The file is already created and empty
449+
os.close(temp_fd)
450+
451+
log.debug(f"Created temporary empty file for baseline scan: {temp_path}")
452+
return [temp_path]
442453

443454
def create_full_scan(self, files: List[str], params: FullScanParams) -> FullScan:
444455
"""
@@ -874,16 +885,39 @@ def create_new_diff(
874885
except APIResourceNotFound:
875886
head_full_scan_id = None
876887

888+
# If no head scan exists, create an empty baseline scan
877889
if head_full_scan_id is None:
890+
log.info("No previous scan found - creating empty baseline scan")
878891
new_params = copy.deepcopy(params.__dict__)
879892
new_params.pop('include_license_details')
880893
tmp_params = FullScanParams(**new_params)
881894
tmp_params.include_license_details = params.include_license_details
882895
tmp_params.tmp = True
883896
tmp_params.set_as_pending_head = False
884897
tmp_params.make_default_branch = False
885-
head_full_scan = self.create_full_scan(Core.empty_head_scan_file(), tmp_params)
886-
head_full_scan_id = head_full_scan.id
898+
899+
# Create baseline scan with empty file
900+
empty_files = Core.empty_head_scan_file()
901+
try:
902+
head_full_scan = self.create_full_scan(empty_files, tmp_params)
903+
head_full_scan_id = head_full_scan.id
904+
log.debug(f"Created empty baseline scan: {head_full_scan_id}")
905+
906+
# Clean up the temporary empty file
907+
for temp_file in empty_files:
908+
try:
909+
os.unlink(temp_file)
910+
log.debug(f"Cleaned up temporary file: {temp_file}")
911+
except OSError as e:
912+
log.warning(f"Failed to clean up temporary file {temp_file}: {e}")
913+
except Exception as e:
914+
# Clean up temp files even if scan creation fails
915+
for temp_file in empty_files:
916+
try:
917+
os.unlink(temp_file)
918+
except OSError:
919+
pass
920+
raise e
887921

888922
# Create new scan
889923
try:
@@ -900,6 +934,7 @@ def create_new_diff(
900934
log.error(f"Stack trace:\n{traceback.format_exc()}")
901935
raise
902936

937+
# Handle diff generation - now we always have both scans
903938
scans_ready = self.check_full_scans_status(head_full_scan_id, new_full_scan.id)
904939
if scans_ready is False:
905940
log.error(f"Full scans did not complete within {self.config.timeout} seconds")
@@ -1121,6 +1156,12 @@ def add_package_alerts_to_collection(self, package: Package, alerts_collection:
11211156
alert = Alert(**alert_item)
11221157
props = getattr(self.config.all_issues, alert.type, default_props)
11231158
introduced_by = self.get_source_data(package, packages)
1159+
1160+
# Handle special case for license policy violations
1161+
title = props.title
1162+
if alert.type == "licenseSpdxDisj" and not title:
1163+
title = "License Policy Violation"
1164+
11241165
issue_alert = Issue(
11251166
pkg_type=package.type,
11261167
pkg_name=package.name,
@@ -1131,7 +1172,7 @@ def add_package_alerts_to_collection(self, package: Package, alerts_collection:
11311172
type=alert.type,
11321173
severity=alert.severity,
11331174
description=props.description,
1134-
title=props.title,
1175+
title=title,
11351176
suggestion=props.suggestion,
11361177
next_step_title=props.nextStepTitle,
11371178
introduced_by=introduced_by,
@@ -1218,7 +1259,8 @@ def get_new_alerts(
12181259
if alert_key not in removed_package_alerts:
12191260
new_alerts = added_package_alerts[alert_key]
12201261
for alert in new_alerts:
1221-
alert_str = f"{alert.purl},{alert.manifests},{alert.type}"
1262+
# Consolidate by package and alert type, not by manifest details
1263+
alert_str = f"{alert.purl},{alert.type}"
12221264

12231265
if alert.error or alert.warn:
12241266
if alert_str not in consolidated_alerts:
@@ -1229,7 +1271,8 @@ def get_new_alerts(
12291271
removed_alerts = removed_package_alerts[alert_key]
12301272

12311273
for alert in new_alerts:
1232-
alert_str = f"{alert.purl},{alert.manifests},{alert.type}"
1274+
# Consolidate by package and alert type, not by manifest details
1275+
alert_str = f"{alert.purl},{alert.type}"
12331276

12341277
# Only add if:
12351278
# 1. Alert isn't in removed packages (or we're not ignoring readded alerts)

socketsecurity/core/messages.py

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,13 +309,26 @@ def security_comment_template(diff: Diff) -> str:
309309
:param diff: Diff - Contains the detected vulnerabilities and warnings.
310310
:return: str - The formatted Markdown/HTML string.
311311
"""
312+
# Group license policy violations by PURL (ecosystem/package@version)
313+
license_groups = {}
314+
security_alerts = []
315+
316+
for alert in diff.new_alerts:
317+
if alert.type == "licenseSpdxDisj":
318+
purl_key = f"{alert.pkg_type}/{alert.pkg_name}@{alert.pkg_version}"
319+
if purl_key not in license_groups:
320+
license_groups[purl_key] = []
321+
license_groups[purl_key].append(alert)
322+
else:
323+
security_alerts.append(alert)
324+
312325
# Start of the comment
313326
comment = """<!-- socket-security-comment-actions -->
314327
315328
> **❗️ Caution**
316329
> **Review the following alerts detected in dependencies.**
317330
>
318-
> According to your organizations Security Policy, you **must** resolve all **Block** alerts before proceeding. Its recommended to resolve **Warn** alerts too.
331+
> According to your organization's Security Policy, you **must** resolve all **"Block"** alerts before proceeding. It's recommended to resolve **"Warn"** alerts too.
319332
> Learn more about [Socket for GitHub](https://socket.dev?utm_medium=gh).
320333
321334
<!-- start-socket-updated-alerts-table -->
@@ -330,8 +343,8 @@ def security_comment_template(diff: Diff) -> str:
330343
<tbody>
331344
"""
332345

333-
# Loop through alerts, dynamically generating rows
334-
for alert in diff.new_alerts:
346+
# Loop through security alerts (non-license), dynamically generating rows
347+
for alert in security_alerts:
335348
severity_icon = Messages.get_severity_icon(alert.severity)
336349
action = "Block" if alert.error else "Warn"
337350
details_open = ""
@@ -365,7 +378,48 @@ def security_comment_template(diff: Diff) -> str:
365378
<!-- end-socket-alert-{alert.pkg_name}@{alert.pkg_version} -->
366379
"""
367380

368-
# Close table and comment
381+
# Add license policy violation entries grouped by PURL
382+
for purl_key, alerts in license_groups.items():
383+
action = "Block" if any(alert.error for alert in alerts) else "Warn"
384+
first_alert = alerts[0]
385+
386+
# Use orange diamond for license policy violations
387+
license_icon = "🔶"
388+
389+
# Build license findings list
390+
license_findings = []
391+
for alert in alerts:
392+
license_findings.append(alert.title)
393+
394+
comment += f"""
395+
<!-- start-socket-alert-{first_alert.pkg_name}@{first_alert.pkg_version} -->
396+
<tr>
397+
<td><strong>{action}</strong></td>
398+
<td align="center">{license_icon}</td>
399+
<td>
400+
<details>
401+
<summary>{first_alert.pkg_name}@{first_alert.pkg_version} has a License Policy Violation.</summary>
402+
<p><strong>License findings:</strong></p>
403+
<ul>
404+
"""
405+
for finding in license_findings:
406+
comment += f" <li>{finding}</li>\n"
407+
408+
comment += f""" </ul>
409+
<p><strong>From:</strong> {first_alert.manifests}</p>
410+
<p>ℹ️ Read more on: <a href="{first_alert.purl}">This package</a> | <a href="https://socket.dev/alerts/license">What is a license policy violation?</a></p>
411+
<blockquote>
412+
<p><em>Next steps:</em> Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at <strong>support@socket.dev</strong>.</p>
413+
<p><em>Suggestion:</em> Find a package that does not violate your license policy or adjust your policy to allow this package's license.</p>
414+
<p><em>Mark the package as acceptable risk:</em> To ignore this alert only in this pull request, reply with the comment <code>@SocketSecurity ignore {first_alert.pkg_name}@{first_alert.pkg_version}</code>. You can also ignore all packages with <code>@SocketSecurity ignore-all</code>. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.</p>
415+
</blockquote>
416+
</details>
417+
</td>
418+
</tr>
419+
<!-- end-socket-alert-{first_alert.pkg_name}@{first_alert.pkg_version} -->
420+
"""
421+
422+
# Close table
369423
comment += """
370424
</tbody>
371425
</table>

socketsecurity/socketcli.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,33 @@ def main_code():
320320
diff = core.create_new_diff(config.target_path, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar)
321321

322322
output_handler.handle_output(diff)
323+
324+
elif config.enable_diff and not force_api_mode:
325+
# New logic: --enable-diff forces diff mode even with --integration api (no SCM)
326+
log.info("Diff mode enabled without SCM integration")
327+
diff = core.create_new_diff(config.target_path, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar)
328+
output_handler.handle_output(diff)
329+
330+
elif config.enable_diff and force_api_mode:
331+
# User requested diff mode but no manifest files were detected
332+
log.warning("--enable-diff was specified but no supported manifest files were detected in the changed files. Falling back to full scan mode.")
333+
log.info("Creating Socket Report (full scan)")
334+
serializable_params = {
335+
key: value if isinstance(value, (int, float, str, list, dict, bool, type(None))) else str(value)
336+
for key, value in params.__dict__.items()
337+
}
338+
log.debug(f"params={serializable_params}")
339+
diff = core.create_full_scan_with_report_url(
340+
config.target_path,
341+
params,
342+
no_change=should_skip_scan,
343+
save_files_list_path=config.save_submitted_files_list,
344+
save_manifest_tar_path=config.save_manifest_tar
345+
)
346+
log.info(f"Full scan created with ID: {diff.id}")
347+
log.info(f"Full scan report URL: {diff.report_url}")
348+
output_handler.handle_output(diff)
349+
323350
else:
324351
if force_api_mode:
325352
log.info("No Manifest files changed, creating Socket Report")

tests/unit/test_cli_config.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,20 @@ def test_default_values(self):
2424
@pytest.mark.parametrize("flag,attr", [
2525
("--enable-debug", "enable_debug"),
2626
("--disable-blocking", "disable_blocking"),
27-
("--allow-unverified", "allow_unverified")
27+
("--allow-unverified", "allow_unverified"),
28+
("--enable-diff", "enable_diff")
2829
])
2930
def test_boolean_flags(self, flag, attr):
3031
config = CliConfig.from_args(["--api-token", "test", flag])
31-
assert getattr(config, attr) is True
32+
assert getattr(config, attr) is True
33+
34+
def test_enable_diff_default_false(self):
35+
"""Test that enable_diff defaults to False"""
36+
config = CliConfig.from_args(["--api-token", "test"])
37+
assert config.enable_diff is False
38+
39+
def test_enable_diff_with_integration_api(self):
40+
"""Test that enable_diff can be used with integration api"""
41+
config = CliConfig.from_args(["--api-token", "test", "--integration", "api", "--enable-diff"])
42+
assert config.enable_diff is True
43+
assert config.integration_type == "api"

0 commit comments

Comments
 (0)