Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
# Changelog

## 2.6.9

### Fixed: unreadable reachability facts no longer report a blocking package

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

## 2.6.8

### Changed: bump pinned @coana-tech/cli to 15.10.25
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ build-backend = "hatchling.build"

[project]
name = "socketsecurity"
version = "2.6.8"
version = "2.6.9"
requires-python = ">= 3.11"
license = {"file" = "LICENSE"}
dependencies = [
Expand Down
2 changes: 1 addition & 1 deletion socketsecurity/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
__author__ = 'socket.dev'
__version__ = '2.6.8'
__version__ = '2.6.9'
USER_AGENT = f'SocketPythonCLI/{__version__}'
142 changes: 119 additions & 23 deletions socketsecurity/core/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import atexit
import copy
import fnmatch
import importlib
Expand All @@ -6,6 +7,7 @@
import os
import random
import re
import shutil
import sys
import tarfile
import tempfile
Expand Down Expand Up @@ -69,6 +71,19 @@
# Stream the facts file in 1 MiB chunks so large files aren't held fully in memory.
SOCKET_FACTS_BROTLI_CHUNK_SIZE = 1024 * 1024

# Minimal well-formed facts document used for placeholder uploads (see empty_head_scan_file).
# A zero-byte ``.socket.facts.json`` is not parseable, and the API answers an unparseable
# facts file by synthesising the marker artifact below.
SOCKET_FACTS_EMPTY_DOCUMENT = '{"components": []}'

# Synthetic artifact the Socket API adds to a full scan when an uploaded
# ``.socket.facts.json`` could not be parsed. It is a diagnostic marker rather than a real
# dependency: it has no manifest file and no introducing package, so the blocking alert it
# carries is not actionable by a developer, and a PR comment about it is pure noise
# (CE-422). Drop it from scan results and surface the parse failure as a warning instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 8 spots where the ticket is directly mentioned in comments. That looks to me to be an LLM agent special, and I've got a skill to clear them out without having to pick through them all.

DM me if you disagree with this but I think that we just ship this version, and I'll do a PR tomorrow that adds the skill and scrubs these out of the main branch (assuming this is merged).

INVALID_FACTS_MARKER_TYPE = "generic"
INVALID_FACTS_MARKER_NAME = "invalid-socket-facts"

# Full application reachability finalize retry policy. The finalize call links the reachability
# scan to the full scan and can fail transiently (network/API blips); a few backoff retries make it robust.
TIER1_FINALIZE_MAX_ATTEMPTS = 3
Expand Down Expand Up @@ -108,6 +123,17 @@
DIFF_SCAN_POLL_BACKOFF_MULTIPLIER = 1.5
DIFF_SCAN_POLL_TIMEOUT_SECONDS = 30 * 60.0

# Temp dirs holding placeholder facts files (see Core.empty_head_scan_file). Call sites unlink
# the file itself once the upload finishes; the now-empty directory is removed at process exit
# so a run that raises mid-scan doesn't leak one.
_PLACEHOLDER_FACTS_DIRS: List[str] = []


@atexit.register
def _cleanup_placeholder_facts_dirs() -> None:
for placeholder_dir in _PLACEHOLDER_FACTS_DIRS:
shutil.rmtree(placeholder_dir, ignore_errors=True)


def _humanize_alert_type(alert_type: str) -> str:
"""Convert a camelCase/PascalCase alert type into a Title-Cased label.
Expand Down Expand Up @@ -209,13 +235,58 @@ def get_sbom_data(self, full_scan_id: str) -> Dict[str, SocketArtifact]:
)
if not hasattr(response, "artifacts") or not response.artifacts:
return {}
return response.artifacts
artifacts = {
artifact_id: artifact
for artifact_id, artifact in response.artifacts.items()
if not Core.is_invalid_facts_marker(artifact)
}
Core.warn_if_invalid_facts_marker(len(artifacts) != len(response.artifacts))
return artifacts

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

@staticmethod
def is_invalid_facts_marker(artifact) -> bool:
"""True for the API's ``generic/invalid-socket-facts`` unparseable-facts marker.

The marker is a signal that the uploaded ``.socket.facts.json`` failed to parse, not a
dependency anyone added. Treating it as a package makes the CLI report a new blocking
alert with an empty "Introduced by" and "Manifest File" and post a PR comment a
developer has no way to act on (CE-422), so it is filtered out of scan results and
reported through ``warn_if_invalid_facts_marker`` instead.

Matches on any version: the API currently pins it to 1.0.0, but the version carries no
meaning here.

Args:
artifact: A ``SocketArtifact`` or diff artifact (anything with ``type``/``name``).

Returns:
True if the artifact is the marker rather than a real package.
"""
return (
getattr(artifact, "type", None) == INVALID_FACTS_MARKER_TYPE
and getattr(artifact, "name", None) == INVALID_FACTS_MARKER_NAME
)

@staticmethod
def warn_if_invalid_facts_marker(found: bool) -> None:
"""Log the reachability-facts parse failure that ``is_invalid_facts_marker`` stands for.

Dropping the marker silently would hide a real (if non-blocking) problem: the scan ran
without the reachability data it was supposed to carry.
"""
if not found:
return
log.warning(
"Socket could not parse the uploaded .socket.facts.json, so reachability facts "
"were not applied to this scan. Ignoring the "
f"{INVALID_FACTS_MARKER_TYPE}/{INVALID_FACTS_MARKER_NAME} marker the API returns "
"for this: it is a diagnostic, not a dependency, so it does not block the build "
"or appear in reports. Scan results are otherwise unaffected."
)

def create_sbom_output(self, diff: Diff) -> dict:
"""Creates CycloneDX output for a given diff."""
Expand Down Expand Up @@ -809,20 +880,35 @@ def to_case_insensitive_regex(input_string: str) -> str:
@staticmethod
def empty_head_scan_file() -> List[str]:
"""
Creates a temporary empty file for baseline scans when no head scan exists.

Creates a temporary placeholder manifest for scans with no manifest files.

Used both for baseline scans when a repository has no head scan yet and for the new
scan when no supported manifest files were found. The API rejects unsupported
filenames, so the placeholder is named ``.socket.facts.json``; it must therefore also
*parse* as a facts document. A zero-byte file does not, and the API answers an
unparseable facts file by adding a blocking ``generic/invalid-socket-facts@1.0.0``
artifact to the scan - which then surfaces as a new blocking package with no manifest
and no introducer (CE-422). Writing an empty-but-well-formed document instead yields a
genuinely empty scan.

Each call gets its own temp directory: the path used to be a fixed
``$TMPDIR/.socket.facts.json``, so two CLI runs sharing a temp dir (back-to-back
invocations in the same CI job, matrix jobs on one runner) could delete or truncate
each other's placeholder mid-upload.

Returns:
List containing path to a temporary empty file
List containing path to a temporary placeholder facts file
"""
# Create a temporary directory and then create our specific filename
temp_dir = tempfile.gettempdir()
temp_path = os.path.join(temp_dir, '.socket.facts.json')

# Create the empty file
with open(temp_path, 'w'):
pass # Creates an empty file

log.debug(f"Created temporary empty file for baseline scan: {temp_path}")
# Own directory per call so concurrent runs can't clobber each other's placeholder;
# the basename must stay exactly SOCKET_FACTS_FILENAME to pass the API's validator.
temp_dir = tempfile.mkdtemp(prefix='socket_baseline_')
_PLACEHOLDER_FACTS_DIRS.append(temp_dir)
temp_path = os.path.join(temp_dir, SOCKET_FACTS_FILENAME)

with open(temp_path, 'w') as f:
f.write(SOCKET_FACTS_EMPTY_DOCUMENT)

log.debug(f"Created temporary placeholder facts file for baseline scan: {temp_path}")
return [temp_path]

def finalize_tier1_scan(self, full_scan_id: str, facts_file_path: str) -> bool:
Expand Down Expand Up @@ -959,7 +1045,7 @@ def _compress_facts_files_for_upload(self, files: List[str]) -> Tuple[List[str],
exactly ``.socket.facts.json.br``, so compressing here keeps a large facts file under
the server's per-file size cap without changing the stored result. Files whose
basename is not exactly ``.socket.facts.json`` are left untouched (the server only
matches that exact name), as are empty placeholder files (e.g. baseline scans).
matches that exact name), as are zero-byte files.

Compression never blocks an upload: if it fails for any reason (missing optional
``brotli`` dependency, unwritable directory, etc.) the original plain file is used.
Expand Down Expand Up @@ -1780,16 +1866,26 @@ def get_added_and_removed_packages(

diff_end = time.time()
log.info(f"Diff Report Gathered in {diff_end - diff_start:.2f} seconds")

# A scan whose facts file failed to parse carries the API's invalid-socket-facts
# marker. Left in, it reads as a newly added blocking package (CE-422), so drop it
# from every bucket - before the counts below, which should describe what the CLI
# actually reports on.
marker_found = False
buckets: Dict[str, List] = {}
for name in ("added", "removed", "unchanged", "replaced", "updated"):
bucket = getattr(diff_artifacts, name)
buckets[name] = [a for a in bucket if not Core.is_invalid_facts_marker(a)]
marker_found = marker_found or len(buckets[name]) != len(bucket)
Core.warn_if_invalid_facts_marker(marker_found)

log.info("Diff report artifact counts:")
log.info(f"Added: {len(diff_artifacts.added)}")
log.info(f"Removed: {len(diff_artifacts.removed)}")
log.info(f"Unchanged: {len(diff_artifacts.unchanged)}")
log.info(f"Replaced: {len(diff_artifacts.replaced)}")
log.info(f"Updated: {len(diff_artifacts.updated)}")

added_artifacts = diff_artifacts.added + diff_artifacts.updated
removed_artifacts = diff_artifacts.removed + diff_artifacts.replaced
unchanged_artifacts = diff_artifacts.unchanged
for name, bucket in buckets.items():
log.info(f"{name.capitalize()}: {len(bucket)}")

added_artifacts = buckets["added"] + buckets["updated"]
removed_artifacts = buckets["removed"] + buckets["replaced"]
unchanged_artifacts = buckets["unchanged"]

added_packages: Dict[str, Package] = {}
removed_packages: Dict[str, Package] = {}
Expand Down
2 changes: 1 addition & 1 deletion tests/core/test_facts_compression.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def test_compress_for_upload_preserves_directory_prefix(tmp_path):


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

Expand Down
Loading
Loading