fix(threatcrush-scan): pin the CLI install spec instead of @latest - #952
Merged
Conversation
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
ThreatCrush Security Scan310 finding(s) HIGH/CRITICAL: 24 | MEDIUM: 50 | LOW: 236
…and 260 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
The
threatcrush-scanaction pack installs the CLI withthreatcrushPackageSpecdefaulting to@profullstack/threatcrush@latest, and that spec is baked into every consumer's workflow at install time. So a scanner that runs on every PR is an unpinned dependency shared across the whole fleet — one bad publish breaks CI in every repo that installed the pack, simultaneously.That is not hypothetical. A
workspace:protocol slip shipped in@profullstack/threatcrush0.7.0 and 0.7.1 madenpm installfail withEUNSUPPORTEDPROTOCOL, and every@latestconsumer's scan errored on it — ~28 repos plus the testbed's own CI — on a registry problem that had nothing to do with their diff. The retry loop in the workflow can't help: a broken publish fails deterministically, not transiently.The fix
Pin the default install spec to a known-good version (
@0.11.0) instead of@latest, and bump the pack1.1.0 → 1.2.0so the fleet re-syncs consumers off the unpinned spec.@latestpublish can no longer cascade — consumers are on a version that was checked first.Consumers that want to track a different version still override
threatcrushPackageSpecper-repo.Two files: the pack default + description, and the README's input table.
Follow-up for whoever runs the fleet: existing consumers keep the
@latestvalue baked in until they re-sync to pack 1.2.0 — a fleet re-apply propagates the pin to the ~28 repos already installed.