Skip to content

Don't render .socket.facts.json placeholder as blocking package - #321

Open
lelia wants to merge 1 commit into
mainfrom
lelia/ce-422-python-cli-treats-invalid-socketfactsjson-error-as-blocking
Open

Don't render .socket.facts.json placeholder as blocking package#321
lelia wants to merge 1 commit into
mainfrom
lelia/ce-422-python-cli-treats-invalid-socketfactsjson-error-as-blocking

Conversation

@lelia

@lelia lelia commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Problem

When a scan finds no supported manifest files, the CLI uploads a placeholder named .socket.facts.json so the upload passes the API's supported-filename validator. That placeholder was zero bytes, which the API cannot parse as a facts document — it responds by adding a synthetic generic/invalid-socket-facts@1.0.0 artifact carrying a blocking alert.

The CLI read that back as a real dependency, so the run reported a new blocking package with no manifest file and no introducing dependency, exited non-zero, and on pull requests posted a security comment that a developer had no way to act on.

Changes

  • Placeholder is now valid. It contains an empty but well-formed facts document, so the scan is genuinely empty instead of triggering a parse error.
  • Placeholders no longer share a path. Each gets its own temp directory, removed at exit. The previous fixed $TMPDIR/.socket.facts.json meant two runs sharing a temp directory could remove each other's file mid-upload.
  • The marker is filtered out regardless of origin. generic/invalid-socket-facts is dropped from full scan artifacts and from every diff bucket (added/removed/unchanged/replaced/updated), with a warning logged in its place. Filtering happens before the artifact-count log lines so the counts describe what the CLI actually reports on. With no new alerts, the existing gate in socketcli.py suppresses the security comment on its own.

Testing

tests/core/test_invalid_facts_marker.py (13 tests). The eight behavioral ones were confirmed to fail against the pre-fix code. Full suite: 492 passed.

Note

That the backend accepts the new placeholder payload is not verified against a live API — there is no no-manifest scenario in the e2e workflow. If the schema wants more, the marker filtering still prevents the block and the comment, but the placeholder would keep tripping the parser. Worth a preview build against a repository with no manifests to confirm the resulting scan carries no invalid-socket-facts artifact.

Fixes CE-422


Note

Medium Risk
Changes core scan/diff artifact handling and empty-scan upload behavior, which directly affects blocking exits and PR comments, though the logic is narrow and well-tested.

Overview
Fixes CE-422, where scans with no manifest files could fail CI and post unusable PR security comments.

Placeholder uploads no longer write a zero-byte .socket.facts.json (which the API could not parse). They now upload a minimal valid document ({"components": []}), and each run gets its own temp directory (with exit-time cleanup) instead of sharing a fixed $TMPDIR/.socket.facts.json path that concurrent jobs could clobber.

API diagnostic artifact generic/invalid-socket-facts is treated as a parse-failure signal, not a dependency: it is stripped from full-scan SBOM artifacts and from every diff bucket before package/alert assembly and count logging, with a warning when present. Runs no longer treat it as a new blocking package or PR comment fodder.

Release 2.6.9 with changelog and focused tests in test_invalid_facts_marker.py.

Reviewed by Cursor Bugbot for commit 01b89e4. Configure here.

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
@lelia
lelia requested a review from a team as a code owner August 28, 2026 00:00
@lelia
lelia deployed to socket-firewall August 28, 2026 00:00 — with GitHub Actions Active
@lelia lelia changed the title Stop treating an unparseable .socket.facts.json as a blocking package Don't render .socket.facts.json placeholder file as blocking package Aug 28, 2026
@lelia lelia changed the title Don't render .socket.facts.json placeholder file as blocking package Don't render .socket.facts.json placeholder as blocking package Aug 28, 2026
@lelia lelia added publish-preview Publish a CLI preview to TestPyPI. publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub labels Aug 28, 2026
@github-actions

Copy link
Copy Markdown

🐳 Docker preview published: socketdev/cli:pr-321

This mutable tag is only created when a Docker preview is explicitly requested.

@github-actions

Copy link
Copy Markdown

🚀 CLI preview published: socketsecurity==2.6.9.dev3312870820701

pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketsecurity==2.6.9.dev3312870820701

TestPyPI's package index can take several minutes to expose a newly uploaded version.

# ``.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).

@flowstate Eric Hibbs (flowstate) left a comment

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.

See my note about comment nits, but the important stuff lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

publish-docker-preview Publish `socketdev/cli:pr-<number>` to DockerHub publish-preview Publish a CLI preview to TestPyPI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants