Skip to content

Commit 01b89e4

Browse files
committed
Stop treating an unparseable .socket.facts.json as a blocking package
Scans with no supported manifest files uploaded a zero-byte `.socket.facts.json` placeholder. The API cannot parse that and responds by adding a synthetic `generic/invalid-socket-facts@1.0.0` artifact, which the CLI then reported as a new blocking package with no manifest file and no introducing dependency, failing the run and posting a pull request comment that could not be acted on. - Write an empty but well-formed facts document as the placeholder. - Give each placeholder its own temp directory, so concurrent runs cannot remove each other's file mid-upload. - Filter the `generic/invalid-socket-facts` marker out of full scan and diff artifacts, logging a warning instead. It is a diagnostic, not a dependency. Ref: CE-422
1 parent 5368c08 commit 01b89e4

7 files changed

Lines changed: 421 additions & 27 deletions

File tree

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,23 @@
11
# Changelog
22

3+
## 2.6.9
4+
5+
### Fixed: unreadable reachability facts no longer report a blocking package
6+
7+
- Scans with no supported manifest files uploaded a zero-byte `.socket.facts.json`
8+
placeholder. The API cannot parse that, and answers by adding a
9+
`generic/invalid-socket-facts@1.0.0` artifact to the scan, which the CLI then reported
10+
as a new blocking package with no manifest file and no introducing dependency —
11+
failing the run and, on pull requests, leaving a security comment that could not be
12+
acted on. The placeholder is now an empty but well-formed facts document.
13+
- When the API does report `generic/invalid-socket-facts` (a diagnostic for a facts file
14+
it could not parse, not a real dependency), the CLI now excludes it from scan results
15+
and logs a warning instead. It no longer blocks a run, appears in reports, or triggers
16+
a pull request comment.
17+
- Each placeholder is written to its own temporary directory. Two CLI runs sharing a
18+
temporary directory previously used the same path and could remove each other's
19+
placeholder mid-upload.
20+
321
## 2.6.8
422

523
### Changed: bump pinned @coana-tech/cli to 15.10.25

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.6.8"
9+
version = "2.6.9"
1010
requires-python = ">= 3.11"
1111
license = {"file" = "LICENSE"}
1212
dependencies = [

socketsecurity/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
__author__ = 'socket.dev'
2-
__version__ = '2.6.8'
2+
__version__ = '2.6.9'
33
USER_AGENT = f'SocketPythonCLI/{__version__}'

socketsecurity/core/__init__.py

Lines changed: 119 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import atexit
12
import copy
23
import fnmatch
34
import importlib
@@ -6,6 +7,7 @@
67
import os
78
import random
89
import re
10+
import shutil
911
import sys
1012
import tarfile
1113
import tempfile
@@ -69,6 +71,19 @@
6971
# Stream the facts file in 1 MiB chunks so large files aren't held fully in memory.
7072
SOCKET_FACTS_BROTLI_CHUNK_SIZE = 1024 * 1024
7173

74+
# Minimal well-formed facts document used for placeholder uploads (see empty_head_scan_file).
75+
# A zero-byte ``.socket.facts.json`` is not parseable, and the API answers an unparseable
76+
# facts file by synthesising the marker artifact below.
77+
SOCKET_FACTS_EMPTY_DOCUMENT = '{"components": []}'
78+
79+
# Synthetic artifact the Socket API adds to a full scan when an uploaded
80+
# ``.socket.facts.json`` could not be parsed. It is a diagnostic marker rather than a real
81+
# dependency: it has no manifest file and no introducing package, so the blocking alert it
82+
# carries is not actionable by a developer, and a PR comment about it is pure noise
83+
# (CE-422). Drop it from scan results and surface the parse failure as a warning instead.
84+
INVALID_FACTS_MARKER_TYPE = "generic"
85+
INVALID_FACTS_MARKER_NAME = "invalid-socket-facts"
86+
7287
# Full application reachability finalize retry policy. The finalize call links the reachability
7388
# scan to the full scan and can fail transiently (network/API blips); a few backoff retries make it robust.
7489
TIER1_FINALIZE_MAX_ATTEMPTS = 3
@@ -108,6 +123,17 @@
108123
DIFF_SCAN_POLL_BACKOFF_MULTIPLIER = 1.5
109124
DIFF_SCAN_POLL_TIMEOUT_SECONDS = 30 * 60.0
110125

126+
# Temp dirs holding placeholder facts files (see Core.empty_head_scan_file). Call sites unlink
127+
# the file itself once the upload finishes; the now-empty directory is removed at process exit
128+
# so a run that raises mid-scan doesn't leak one.
129+
_PLACEHOLDER_FACTS_DIRS: List[str] = []
130+
131+
132+
@atexit.register
133+
def _cleanup_placeholder_facts_dirs() -> None:
134+
for placeholder_dir in _PLACEHOLDER_FACTS_DIRS:
135+
shutil.rmtree(placeholder_dir, ignore_errors=True)
136+
111137

112138
def _humanize_alert_type(alert_type: str) -> str:
113139
"""Convert a camelCase/PascalCase alert type into a Title-Cased label.
@@ -209,13 +235,58 @@ def get_sbom_data(self, full_scan_id: str) -> Dict[str, SocketArtifact]:
209235
)
210236
if not hasattr(response, "artifacts") or not response.artifacts:
211237
return {}
212-
return response.artifacts
238+
artifacts = {
239+
artifact_id: artifact
240+
for artifact_id, artifact in response.artifacts.items()
241+
if not Core.is_invalid_facts_marker(artifact)
242+
}
243+
Core.warn_if_invalid_facts_marker(len(artifacts) != len(response.artifacts))
244+
return artifacts
213245

214246
def get_sbom_data_list(self, artifacts_dict: Dict[str, SocketArtifact]) -> list[SocketArtifact]:
215247
"""Converts artifacts dictionary to a list."""
216248
return list(artifacts_dict.values())
217249

250+
@staticmethod
251+
def is_invalid_facts_marker(artifact) -> bool:
252+
"""True for the API's ``generic/invalid-socket-facts`` unparseable-facts marker.
253+
254+
The marker is a signal that the uploaded ``.socket.facts.json`` failed to parse, not a
255+
dependency anyone added. Treating it as a package makes the CLI report a new blocking
256+
alert with an empty "Introduced by" and "Manifest File" and post a PR comment a
257+
developer has no way to act on (CE-422), so it is filtered out of scan results and
258+
reported through ``warn_if_invalid_facts_marker`` instead.
218259
260+
Matches on any version: the API currently pins it to 1.0.0, but the version carries no
261+
meaning here.
262+
263+
Args:
264+
artifact: A ``SocketArtifact`` or diff artifact (anything with ``type``/``name``).
265+
266+
Returns:
267+
True if the artifact is the marker rather than a real package.
268+
"""
269+
return (
270+
getattr(artifact, "type", None) == INVALID_FACTS_MARKER_TYPE
271+
and getattr(artifact, "name", None) == INVALID_FACTS_MARKER_NAME
272+
)
273+
274+
@staticmethod
275+
def warn_if_invalid_facts_marker(found: bool) -> None:
276+
"""Log the reachability-facts parse failure that ``is_invalid_facts_marker`` stands for.
277+
278+
Dropping the marker silently would hide a real (if non-blocking) problem: the scan ran
279+
without the reachability data it was supposed to carry.
280+
"""
281+
if not found:
282+
return
283+
log.warning(
284+
"Socket could not parse the uploaded .socket.facts.json, so reachability facts "
285+
"were not applied to this scan. Ignoring the "
286+
f"{INVALID_FACTS_MARKER_TYPE}/{INVALID_FACTS_MARKER_NAME} marker the API returns "
287+
"for this: it is a diagnostic, not a dependency, so it does not block the build "
288+
"or appear in reports. Scan results are otherwise unaffected."
289+
)
219290

220291
def create_sbom_output(self, diff: Diff) -> dict:
221292
"""Creates CycloneDX output for a given diff."""
@@ -809,20 +880,35 @@ def to_case_insensitive_regex(input_string: str) -> str:
809880
@staticmethod
810881
def empty_head_scan_file() -> List[str]:
811882
"""
812-
Creates a temporary empty file for baseline scans when no head scan exists.
813-
883+
Creates a temporary placeholder manifest for scans with no manifest files.
884+
885+
Used both for baseline scans when a repository has no head scan yet and for the new
886+
scan when no supported manifest files were found. The API rejects unsupported
887+
filenames, so the placeholder is named ``.socket.facts.json``; it must therefore also
888+
*parse* as a facts document. A zero-byte file does not, and the API answers an
889+
unparseable facts file by adding a blocking ``generic/invalid-socket-facts@1.0.0``
890+
artifact to the scan - which then surfaces as a new blocking package with no manifest
891+
and no introducer (CE-422). Writing an empty-but-well-formed document instead yields a
892+
genuinely empty scan.
893+
894+
Each call gets its own temp directory: the path used to be a fixed
895+
``$TMPDIR/.socket.facts.json``, so two CLI runs sharing a temp dir (back-to-back
896+
invocations in the same CI job, matrix jobs on one runner) could delete or truncate
897+
each other's placeholder mid-upload.
898+
814899
Returns:
815-
List containing path to a temporary empty file
900+
List containing path to a temporary placeholder facts file
816901
"""
817-
# Create a temporary directory and then create our specific filename
818-
temp_dir = tempfile.gettempdir()
819-
temp_path = os.path.join(temp_dir, '.socket.facts.json')
820-
821-
# Create the empty file
822-
with open(temp_path, 'w'):
823-
pass # Creates an empty file
824-
825-
log.debug(f"Created temporary empty file for baseline scan: {temp_path}")
902+
# Own directory per call so concurrent runs can't clobber each other's placeholder;
903+
# the basename must stay exactly SOCKET_FACTS_FILENAME to pass the API's validator.
904+
temp_dir = tempfile.mkdtemp(prefix='socket_baseline_')
905+
_PLACEHOLDER_FACTS_DIRS.append(temp_dir)
906+
temp_path = os.path.join(temp_dir, SOCKET_FACTS_FILENAME)
907+
908+
with open(temp_path, 'w') as f:
909+
f.write(SOCKET_FACTS_EMPTY_DOCUMENT)
910+
911+
log.debug(f"Created temporary placeholder facts file for baseline scan: {temp_path}")
826912
return [temp_path]
827913

828914
def finalize_tier1_scan(self, full_scan_id: str, facts_file_path: str) -> bool:
@@ -959,7 +1045,7 @@ def _compress_facts_files_for_upload(self, files: List[str]) -> Tuple[List[str],
9591045
exactly ``.socket.facts.json.br``, so compressing here keeps a large facts file under
9601046
the server's per-file size cap without changing the stored result. Files whose
9611047
basename is not exactly ``.socket.facts.json`` are left untouched (the server only
962-
matches that exact name), as are empty placeholder files (e.g. baseline scans).
1048+
matches that exact name), as are zero-byte files.
9631049
9641050
Compression never blocks an upload: if it fails for any reason (missing optional
9651051
``brotli`` dependency, unwritable directory, etc.) the original plain file is used.
@@ -1780,16 +1866,26 @@ def get_added_and_removed_packages(
17801866

17811867
diff_end = time.time()
17821868
log.info(f"Diff Report Gathered in {diff_end - diff_start:.2f} seconds")
1869+
1870+
# A scan whose facts file failed to parse carries the API's invalid-socket-facts
1871+
# marker. Left in, it reads as a newly added blocking package (CE-422), so drop it
1872+
# from every bucket - before the counts below, which should describe what the CLI
1873+
# actually reports on.
1874+
marker_found = False
1875+
buckets: Dict[str, List] = {}
1876+
for name in ("added", "removed", "unchanged", "replaced", "updated"):
1877+
bucket = getattr(diff_artifacts, name)
1878+
buckets[name] = [a for a in bucket if not Core.is_invalid_facts_marker(a)]
1879+
marker_found = marker_found or len(buckets[name]) != len(bucket)
1880+
Core.warn_if_invalid_facts_marker(marker_found)
1881+
17831882
log.info("Diff report artifact counts:")
1784-
log.info(f"Added: {len(diff_artifacts.added)}")
1785-
log.info(f"Removed: {len(diff_artifacts.removed)}")
1786-
log.info(f"Unchanged: {len(diff_artifacts.unchanged)}")
1787-
log.info(f"Replaced: {len(diff_artifacts.replaced)}")
1788-
log.info(f"Updated: {len(diff_artifacts.updated)}")
1789-
1790-
added_artifacts = diff_artifacts.added + diff_artifacts.updated
1791-
removed_artifacts = diff_artifacts.removed + diff_artifacts.replaced
1792-
unchanged_artifacts = diff_artifacts.unchanged
1883+
for name, bucket in buckets.items():
1884+
log.info(f"{name.capitalize()}: {len(bucket)}")
1885+
1886+
added_artifacts = buckets["added"] + buckets["updated"]
1887+
removed_artifacts = buckets["removed"] + buckets["replaced"]
1888+
unchanged_artifacts = buckets["unchanged"]
17931889

17941890
added_packages: Dict[str, Package] = {}
17951891
removed_packages: Dict[str, Package] = {}

tests/core/test_facts_compression.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ def test_compress_for_upload_preserves_directory_prefix(tmp_path):
9999

100100

101101
def test_empty_facts_file_is_not_compressed(tmp_path):
102-
"""Empty placeholder facts files (e.g. baseline scans) are uploaded as-is."""
102+
"""A zero-byte facts file has nothing to compress and is uploaded as-is."""
103103
core = Core.__new__(Core)
104104
empty_facts = _write(str(tmp_path / SOCKET_FACTS_FILENAME), b"")
105105

0 commit comments

Comments
 (0)