From 934546d6ce45abe2c0b8020c880e36f877a9c9e8 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 18:24:16 -0400 Subject: [PATCH 1/4] 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 " 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: ". 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". --- packages/cli/src/commands/fix/cmd-fix.mts | 27 +++++++++++++++++ .../test/unit/commands/fix/cmd-fix.test.mts | 29 +++++++++++++++++-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/fix/cmd-fix.mts b/packages/cli/src/commands/fix/cmd-fix.mts index 0cfbdf7d2..8ef7f840f 100644 --- a/packages/cli/src/commands/fix/cmd-fix.mts +++ b/packages/cli/src/commands/fix/cmd-fix.mts @@ -1,3 +1,4 @@ +import { existsSync } from 'node:fs' import path from 'node:path' import terminalLink from 'terminal-link' @@ -419,11 +420,37 @@ async function run( const orgSlug = orgSlugCResult.data + // Detect the common mistake of passing a vulnerability ID (GHSA / CVE / + // PURL) as a positional argument when the user meant to use `--id`. + // Without this guard we treat the ID as a directory path, resolve to cwd, + // and eventually fail with a confusing upload error. + const rawInput = cli.input[0] + if ( + rawInput && + (/^GHSA-/i.test(rawInput) || + /^CVE-/i.test(rawInput) || + rawInput.startsWith('pkg:')) + ) { + logger.fail( + `"${rawInput}" looks like a vulnerability identifier, not a directory path.\nDid you mean: socket fix ${FLAG_ID} ${rawInput}`, + ) + process.exitCode = 1 + return + } + let [cwd = '.'] = cli.input // Note: path.resolve vs .join: // If given path is absolute then cwd should not affect it. cwd = path.resolve(process.cwd(), cwd) + // Validate the target directory exists so we fail fast with a clear + // message instead of the API's "Need at least one file to be uploaded". + if (!existsSync(cwd)) { + logger.fail(`Target directory does not exist: ${cwd}`) + process.exitCode = 1 + return + } + const spinner = undefined const includePatterns = cmdFlagValueToArray(include) diff --git a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts index db55763e5..4f34b6801 100644 --- a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts +++ b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts @@ -332,15 +332,40 @@ describe('cmd-fix', () => { }) it('should support custom cwd argument', async () => { - await cmdFix.run(['./custom/path'], importMeta, context) + // Use a path that definitely exists (the test's own cwd) because + // cmd-fix now validates the target directory upfront. + const realDir = process.cwd() + await cmdFix.run([realDir], importMeta, context) expect(mockHandleFix).toHaveBeenCalledWith( expect.objectContaining({ - cwd: expect.stringContaining('custom/path'), + cwd: realDir, }), ) }) + it('should fail fast when target directory does not exist', async () => { + await cmdFix.run(['./this/path/does/not/exist'], importMeta, context) + + expect(process.exitCode).toBe(1) + expect(mockHandleFix).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('Target directory does not exist'), + ) + }) + + it('should fail fast when a GHSA is passed as a positional arg', async () => { + await cmdFix.run(['GHSA-xxxx-xxxx-xxxx'], importMeta, context) + + expect(process.exitCode).toBe(1) + expect(mockHandleFix).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining( + 'looks like a vulnerability identifier, not a directory path', + ), + ) + }) + it('should support --json output mode', async () => { await cmdFix.run(['--json'], importMeta, context) From e6a91f79634c7070bf39a8f076bd556fff839f4c Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 18:41:19 -0400 Subject: [PATCH 2/4] fix(fix): move input validation before org-slug resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/cli/src/commands/fix/cmd-fix.mts | 55 +++++++++++-------- .../test/unit/commands/fix/cmd-fix.test.mts | 27 +++++++++ 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/commands/fix/cmd-fix.mts b/packages/cli/src/commands/fix/cmd-fix.mts index 8ef7f840f..be3cf2a77 100644 --- a/packages/cli/src/commands/fix/cmd-fix.mts +++ b/packages/cli/src/commands/fix/cmd-fix.mts @@ -409,33 +409,29 @@ async function run( return } - const orgSlugCResult = await getDefaultOrgSlug() - if (!orgSlugCResult.ok) { - process.exitCode = orgSlugCResult.code ?? 1 - logger.fail( - `${ERROR_UNABLE_RESOLVE_ORG}.\nEnsure a Socket API token is specified for the organization using the SOCKET_CLI_API_TOKEN environment variable.`, - ) - return - } - - const orgSlug = orgSlugCResult.data - // Detect the common mistake of passing a vulnerability ID (GHSA / CVE / // PURL) as a positional argument when the user meant to use `--id`. // Without this guard we treat the ID as a directory path, resolve to cwd, - // and eventually fail with a confusing upload error. + // and eventually fail with a confusing upload error. Run this before + // `getDefaultOrgSlug()` so users still get the helpful message when no + // API token is configured. const rawInput = cli.input[0] - if ( - rawInput && - (/^GHSA-/i.test(rawInput) || - /^CVE-/i.test(rawInput) || - rawInput.startsWith('pkg:')) - ) { - logger.fail( - `"${rawInput}" looks like a vulnerability identifier, not a directory path.\nDid you mean: socket fix ${FLAG_ID} ${rawInput}`, - ) - process.exitCode = 1 - return + if (rawInput) { + const upperInput = rawInput.toUpperCase() + const isGhsa = upperInput.startsWith('GHSA-') + const isCve = upperInput.startsWith('CVE-') + const isPurl = rawInput.startsWith('pkg:') + if (isGhsa || isCve || isPurl) { + // `handle-fix.mts` matches GHSA/CVE prefixes case-sensitively, so + // echo GHSA/CVE in uppercase in the suggestion. PURLs keep their + // original case (they're intentionally lowercase). + const suggestion = isPurl ? rawInput : upperInput + logger.fail( + `"${rawInput}" looks like a vulnerability identifier, not a directory path.\nDid you mean: socket fix ${FLAG_ID} ${suggestion}`, + ) + process.exitCode = 1 + return + } } let [cwd = '.'] = cli.input @@ -445,12 +441,25 @@ async function run( // Validate the target directory exists so we fail fast with a clear // message instead of the API's "Need at least one file to be uploaded". + // Also runs before the org-slug resolution so the user sees a clearer + // error when pointing at a typo'd path without an API token set. if (!existsSync(cwd)) { logger.fail(`Target directory does not exist: ${cwd}`) process.exitCode = 1 return } + const orgSlugCResult = await getDefaultOrgSlug() + if (!orgSlugCResult.ok) { + process.exitCode = orgSlugCResult.code ?? 1 + logger.fail( + `${ERROR_UNABLE_RESOLVE_ORG}.\nEnsure a Socket API token is specified for the organization using the SOCKET_CLI_API_TOKEN environment variable.`, + ) + return + } + + const orgSlug = orgSlugCResult.data + const spinner = undefined const includePatterns = cmdFlagValueToArray(include) diff --git a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts index 4f34b6801..5a06a27b0 100644 --- a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts +++ b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts @@ -366,6 +366,33 @@ describe('cmd-fix', () => { ) }) + it('should uppercase a lowercase GHSA/CVE in the suggestion', async () => { + await cmdFix.run(['ghsa-abcd-efgh-ijkl'], importMeta, context) + + expect(process.exitCode).toBe(1) + expect(mockHandleFix).not.toHaveBeenCalled() + // handle-fix.mts matches GHSA/CVE prefixes case-sensitively, so the + // suggested command must use the uppercase form or it will fail + // downstream with "Unsupported ID format". + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('--id GHSA-ABCD-EFGH-IJKL'), + ) + }) + + it('should fail fast without calling getDefaultOrgSlug when input looks like an ID', async () => { + await cmdFix.run(['GHSA-xxxx-xxxx-xxxx'], importMeta, context) + + // The validation must run before the network-backed org-slug resolution, + // so users without a configured API token still see the helpful message. + expect(mockGetDefaultOrgSlug).not.toHaveBeenCalled() + }) + + it('should fail fast without calling getDefaultOrgSlug when target dir missing', async () => { + await cmdFix.run(['./this/path/does/not/exist'], importMeta, context) + + expect(mockGetDefaultOrgSlug).not.toHaveBeenCalled() + }) + it('should support --json output mode', async () => { await cmdFix.run(['--json'], importMeta, context) From 399789710b90853fbc8b9bbc6ab243f808c51304 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 18:53:51 -0400 Subject: [PATCH 3/4] 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. --- packages/cli/src/commands/fix/cmd-fix.mts | 16 +++++++--- .../test/unit/commands/fix/cmd-fix.test.mts | 29 +++++++++++++++---- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/commands/fix/cmd-fix.mts b/packages/cli/src/commands/fix/cmd-fix.mts index be3cf2a77..bd1ebb033 100644 --- a/packages/cli/src/commands/fix/cmd-fix.mts +++ b/packages/cli/src/commands/fix/cmd-fix.mts @@ -422,10 +422,18 @@ async function run( const isCve = upperInput.startsWith('CVE-') const isPurl = rawInput.startsWith('pkg:') if (isGhsa || isCve || isPurl) { - // `handle-fix.mts` matches GHSA/CVE prefixes case-sensitively, so - // echo GHSA/CVE in uppercase in the suggestion. PURLs keep their - // original case (they're intentionally lowercase). - const suggestion = isPurl ? rawInput : upperInput + // `handle-fix.mts` validates IDs with case-sensitive format regexes: + // * GHSA — prefix must be uppercase, body segments lowercase [a-z0-9] + // * CVE — prefix must be uppercase, body is all digits (case-free) + // PURLs are intentionally lowercase and validated separately. + let suggestion: string + if (isGhsa) { + suggestion = 'GHSA-' + rawInput.slice(5).toLowerCase() + } else if (isCve) { + suggestion = 'CVE-' + rawInput.slice(4) + } else { + suggestion = rawInput + } logger.fail( `"${rawInput}" looks like a vulnerability identifier, not a directory path.\nDid you mean: socket fix ${FLAG_ID} ${suggestion}`, ) diff --git a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts index 5a06a27b0..d91f6a82d 100644 --- a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts +++ b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts @@ -366,16 +366,35 @@ describe('cmd-fix', () => { ) }) - it('should uppercase a lowercase GHSA/CVE in the suggestion', async () => { + it('should normalize a lowercase GHSA suggestion to prefix-upper / body-lower', async () => { await cmdFix.run(['ghsa-abcd-efgh-ijkl'], importMeta, context) expect(process.exitCode).toBe(1) expect(mockHandleFix).not.toHaveBeenCalled() - // handle-fix.mts matches GHSA/CVE prefixes case-sensitively, so the - // suggested command must use the uppercase form or it will fail - // downstream with "Unsupported ID format". + // handle-fix.mts validates GHSA format as /^GHSA-[a-z0-9]{4}-...$/: + // prefix must be uppercase, body segments must stay lowercase. expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining('--id GHSA-ABCD-EFGH-IJKL'), + expect.stringContaining('--id GHSA-abcd-efgh-ijkl'), + ) + }) + + it('should normalize a lowercase CVE suggestion to uppercase CVE prefix', async () => { + await cmdFix.run(['cve-2021-23337'], importMeta, context) + + expect(process.exitCode).toBe(1) + expect(mockHandleFix).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('--id CVE-2021-23337'), + ) + }) + + it('should leave a PURL suggestion untouched', async () => { + await cmdFix.run(['pkg:npm/left-pad@1.3.0'], importMeta, context) + + expect(process.exitCode).toBe(1) + expect(mockHandleFix).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('--id pkg:npm/left-pad@1.3.0'), ) }) From e7a1bb1d7b931a4bb31e56dcd17254395fb68b32 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 19:19:13 -0400 Subject: [PATCH 4/4] test(fix): tighten coverage of input-validation branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ` 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. --- .../test/unit/commands/fix/cmd-fix.test.mts | 141 +++++++++--------- 1 file changed, 68 insertions(+), 73 deletions(-) diff --git a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts index d91f6a82d..e87346794 100644 --- a/packages/cli/test/unit/commands/fix/cmd-fix.test.mts +++ b/packages/cli/test/unit/commands/fix/cmd-fix.test.mts @@ -331,85 +331,80 @@ describe('cmd-fix', () => { ) }) - it('should support custom cwd argument', async () => { - // Use a path that definitely exists (the test's own cwd) because - // cmd-fix now validates the target directory upfront. - const realDir = process.cwd() - await cmdFix.run([realDir], importMeta, context) - - expect(mockHandleFix).toHaveBeenCalledWith( - expect.objectContaining({ - cwd: realDir, - }), - ) - }) - - it('should fail fast when target directory does not exist', async () => { - await cmdFix.run(['./this/path/does/not/exist'], importMeta, context) - - expect(process.exitCode).toBe(1) - expect(mockHandleFix).not.toHaveBeenCalled() - expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining('Target directory does not exist'), - ) - }) - - it('should fail fast when a GHSA is passed as a positional arg', async () => { - await cmdFix.run(['GHSA-xxxx-xxxx-xxxx'], importMeta, context) - - expect(process.exitCode).toBe(1) - expect(mockHandleFix).not.toHaveBeenCalled() - expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining( - 'looks like a vulnerability identifier, not a directory path', - ), - ) - }) - - it('should normalize a lowercase GHSA suggestion to prefix-upper / body-lower', async () => { - await cmdFix.run(['ghsa-abcd-efgh-ijkl'], importMeta, context) - - expect(process.exitCode).toBe(1) - expect(mockHandleFix).not.toHaveBeenCalled() - // handle-fix.mts validates GHSA format as /^GHSA-[a-z0-9]{4}-...$/: - // prefix must be uppercase, body segments must stay lowercase. - expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining('--id GHSA-abcd-efgh-ijkl'), - ) - }) - - it('should normalize a lowercase CVE suggestion to uppercase CVE prefix', async () => { - await cmdFix.run(['cve-2021-23337'], importMeta, context) - - expect(process.exitCode).toBe(1) - expect(mockHandleFix).not.toHaveBeenCalled() - expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining('--id CVE-2021-23337'), - ) + describe('misplaced vulnerability identifier detection', () => { + // The case matrix handle-fix.mts actually validates downstream: + // GHSA: /^GHSA-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}$/ + // CVE: /^CVE-\d{4}-\d{4,}$/ + // Suggestion must be exactly the form that passes those regexes, + // otherwise the user follows our advice and still gets an error. + it.each([ + // [label, input, expectedSuggestion] + // GHSA: prefix to upper, body to lower, regardless of input casing. + ['canonical GHSA', 'GHSA-abcd-efgh-ijkl', 'GHSA-abcd-efgh-ijkl'], + ['lowercase GHSA', 'ghsa-abcd-efgh-ijkl', 'GHSA-abcd-efgh-ijkl'], + ['mixed-case GHSA', 'GhSa-AbCd-EfGh-IjKl', 'GHSA-abcd-efgh-ijkl'], + // CVE: prefix to upper, body is digits so case is a no-op. + ['canonical CVE', 'CVE-2021-23337', 'CVE-2021-23337'], + ['lowercase CVE', 'cve-2021-23337', 'CVE-2021-23337'], + // PURL: always lowercase by spec, echo verbatim. + ['npm PURL', 'pkg:npm/left-pad@1.3.0', 'pkg:npm/left-pad@1.3.0'], + ])( + 'detects %s and suggests the downstream-valid form', + async (_label, input, expectedSuggestion) => { + await cmdFix.run([input], importMeta, context) + + expect(process.exitCode).toBe(1) + expect(mockHandleFix).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining( + 'looks like a vulnerability identifier, not a directory path', + ), + ) + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining(`--id ${expectedSuggestion}`), + ) + }, + ) + + it('validates IDs before resolving the org slug (no API token path)', async () => { + await cmdFix.run(['GHSA-xxxx-xxxx-xxxx'], importMeta, context) + + // The check must run *before* `getDefaultOrgSlug`, so users without + // a configured API token still see the helpful message instead of + // the generic "Unable to resolve org". + expect(mockGetDefaultOrgSlug).not.toHaveBeenCalled() + }) }) - it('should leave a PURL suggestion untouched', async () => { - await cmdFix.run(['pkg:npm/left-pad@1.3.0'], importMeta, context) + describe('target directory validation', () => { + it('should fail fast when target directory does not exist', async () => { + await cmdFix.run(['./this/path/does/not/exist'], importMeta, context) - expect(process.exitCode).toBe(1) - expect(mockHandleFix).not.toHaveBeenCalled() - expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining('--id pkg:npm/left-pad@1.3.0'), - ) - }) - - it('should fail fast without calling getDefaultOrgSlug when input looks like an ID', async () => { - await cmdFix.run(['GHSA-xxxx-xxxx-xxxx'], importMeta, context) + expect(process.exitCode).toBe(1) + expect(mockHandleFix).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('Target directory does not exist'), + ) + }) - // The validation must run before the network-backed org-slug resolution, - // so users without a configured API token still see the helpful message. - expect(mockGetDefaultOrgSlug).not.toHaveBeenCalled() - }) + it('validates the directory before resolving the org slug', async () => { + await cmdFix.run(['./this/path/does/not/exist'], importMeta, context) - it('should fail fast without calling getDefaultOrgSlug when target dir missing', async () => { - await cmdFix.run(['./this/path/does/not/exist'], importMeta, context) + expect(mockGetDefaultOrgSlug).not.toHaveBeenCalled() + }) - expect(mockGetDefaultOrgSlug).not.toHaveBeenCalled() + it('lets a real directory flow through to handleFix', async () => { + const realDir = process.cwd() + await cmdFix.run([realDir], importMeta, context) + + expect(mockHandleFix).toHaveBeenCalledWith( + expect.objectContaining({ cwd: realDir }), + ) + // Sanity: no bail on the happy path. + expect(mockLogger.fail).not.toHaveBeenCalledWith( + expect.stringContaining('Target directory does not exist'), + ) + }) }) it('should support --json output mode', async () => {