fix: improve socket fix error messages for misplaced IDs and missing directories#1199
Merged
Martin Torp (mtorp) merged 2 commits intov1.xfrom Apr 14, 2026
Merged
Conversation
…directories Detect when a GHSA/CVE/PURL identifier is passed as a positional argument instead of with --id and show a helpful suggestion. Also validate the target directory exists before making API calls. Bump version to 1.1.83.
Benjamin Barslev Nielsen (barslev)
approved these changes
Apr 14, 2026
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Apr 17, 2026
Backport of v1.x #1199 to main. Two quality-of-life fixes for \`socket fix\` that surface clear error messages instead of the confusing API error the user hits today: 1. **Misplaced vulnerability ID**: if the first positional arg looks like a GHSA, CVE, or PURL, we treat it as a directory path and eventually fail with "Need at least one file to be uploaded" from the API. Now we detect the pattern early, fail fast with a "Did you mean: socket fix --id <that value>" hint. 2. **Nonexistent target directory**: previously the directory wasn't validated upfront, so we'd happily resolve it, glob zero files, upload nothing, and the API would reject with the same confusing "Need at least one file to be uploaded" message. Now we \`existsSync()\` the resolved path and fail with "Target directory does not exist: <path>". Tests: * Reworked the existing "should support custom cwd argument" test to pass \`process.cwd()\` (guaranteed to exist) now that we validate the dir. * Added "should fail fast when target directory does not exist". * Added "should fail fast when a GHSA is passed as a positional arg".
4 tasks
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Apr 17, 2026
* fix(fix): validate target directory and detect misplaced IDs Backport of v1.x #1199 to main. Two quality-of-life fixes for \`socket fix\` that surface clear error messages instead of the confusing API error the user hits today: 1. **Misplaced vulnerability ID**: if the first positional arg looks like a GHSA, CVE, or PURL, we treat it as a directory path and eventually fail with "Need at least one file to be uploaded" from the API. Now we detect the pattern early, fail fast with a "Did you mean: socket fix --id <that value>" hint. 2. **Nonexistent target directory**: previously the directory wasn't validated upfront, so we'd happily resolve it, glob zero files, upload nothing, and the API would reject with the same confusing "Need at least one file to be uploaded" message. Now we \`existsSync()\` the resolved path and fail with "Target directory does not exist: <path>". Tests: * Reworked the existing "should support custom cwd argument" test to pass \`process.cwd()\` (guaranteed to exist) now that we validate the dir. * Added "should fail fast when target directory does not exist". * Added "should fail fast when a GHSA is passed as a positional arg". * fix(fix): move input validation before org-slug resolution Addresses both Cursor bugbot findings on #1227: 1. **[Medium] Misplaced-ID check was behind getDefaultOrgSlug()**. If a user had no API token configured, `getDefaultOrgSlug()` would fail first and the helpful "looks like a vulnerability identifier" / "Target directory does not exist" messages never reached them. Moved both validation blocks ahead of the org-slug resolution — they only depend on `cli.input[0]`, no token required. 2. **[Low] Case-insensitive detection with verbatim echo**. The regex matched `ghsa-abcd-…` but `handle-fix.mts` matches the GHSA/CVE prefix case-sensitively, so the suggested `socket fix --id ghsa-…` would fail downstream with "Unsupported ID format". Now we normalize GHSA/CVE to uppercase in the suggestion while keeping PURLs unchanged (they're intentionally lowercase). Added tests: - uppercase normalization of a lowercase GHSA in the suggestion. - validation short-circuits before `getDefaultOrgSlug` for both the misplaced-ID path and the missing-target-dir path. * fix(fix): preserve GHSA body case in uppercased suggestion Cursor bugbot caught a second-order bug in my earlier case fix on #1227. The previous commit uppercased the *entire* rawInput, but handle-fix.mts's format validator is GHSA_FORMAT_REGEXP = /^GHSA-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}$/ which requires the body segments to be lowercase. A user typing `ghsa-abcd-efgh-ijkl` would have been told to run `socket fix --id GHSA-ABCD-EFGH-IJKL`, and that command would fail downstream with "Invalid GHSA format". Fix: only uppercase the prefix. * GHSA: `'GHSA-' + body.toLowerCase()` * CVE: `'CVE-' + rest` (body is digits + dashes, case-free) * PURL: leave verbatim. Added tests covering all three branches. * test(fix): tighten coverage of input-validation branches Tightens the cmd-fix tests introduced with the input-validation work: * Consolidates four separate ID-detection tests into an `it.each` table that exercises the real case matrix against handle-fix.mts's downstream format regexes: - canonical / lowercase / mixed-case GHSA - canonical / lowercase CVE - PURL (unchanged) Each row asserts both the "looks like a vulnerability identifier" message and the exact downstream-valid `--id <suggestion>` form. Previously a mixed-case GHSA and the canonical CVE/GHSA cases weren't covered, so a future regression to the case-normalization logic could slip by. * Splits the directory-validation tests into a dedicated `describe('target directory validation', ...)` block and adds a happy-path assertion ("lets a real directory flow through to handleFix") so we can tell the difference between "validation passed" and "validation silently skipped". * Folded the redundant "should support custom cwd argument" test into the happy-path assertion — same setup, one test instead of two with overlapping mocks. * Order-of-operations assertions (that validation runs before `getDefaultOrgSlug`) now live next to the related bail tests instead of floating at the end of the outer describe. Net: 6 cases in the ID-detection table (up from 3 hardcoded), plus 3 directory-validation tests including the happy path. 47 tests total.
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
2 participants
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.
Summary
socket fixinstead of with--id, and show a helpful "Did you mean" suggestionTest plan
./sd fix GHSA-xhpv-hc6g-r9c6→ shows "Did you mean: socket fix --id GHSA-xhpv-hc6g-r9c6"./sd fix CVE-2021-23337→ shows "Did you mean: socket fix --id CVE-2021-23337"./sd fix pkg:npm/lodash@4.17.20→ shows "Did you mean: socket fix --id pkg:npm/lodash@4.17.20"./sd fix /nonexistent/directory→ shows "Target directory does not exist: /nonexistent/directory"Note
High Risk
High risk because this PR replaces the package/build/CI tooling (new Rollup configs, new bins/exports, new lint/typecheck setup, and new GitHub workflows), which can easily affect distribution artifacts and publishing. It also changes
socket fixargument validation/behavior, so regressions could block fix flows in CI and local usage.Overview
Major toolchain + packaging overhaul. Replaces the previous JS-based CLI packaging with a pnpm + Rollup-based build system (new
.config/rollup.*, Babel/TS configs,bin/*entrypoints, updatedpackage.jsonexports/files/engines, pnpm overrides/patches, and new formatter/linter configs likeeslint.config.js,biome.json,oxlint).CI/CD changes. Removes the old Dependabot + reusable workflows and adds explicit GitHub Actions workflows for lint/typecheck/test, E2E, and npm publishing with provenance and Socket firewall shims.
User-facing
socket fiximprovements. Adds early validation that rejects positional GHSA/CVE/PURL arguments with a “Did you mean: socket fix --id …” hint, and checks that the target directory exists before making API calls; updates tests and bumps version to1.1.83with corresponding changelog entry.Reviewed by Cursor Bugbot for commit 7f8d472. Configure here.