From 6cb84b8c4456cbcca1ceabb9beea386f1a0fe272 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 19 May 2026 16:39:25 -0400 Subject: [PATCH 1/2] test(cli): foundation for CLI-contract unit-test campaign Sets up the foundation for a comprehensive unit-test campaign on the socket-patch CLI. No behavior change to the binary. Two changes: 1. Expose the clap parser as a library so integration tests can verify the public CLI contract without spawning the binary: - new crates/socket-patch-cli/src/lib.rs hosting Cli, Commands, looks_like_uuid, and parse_with_uuid_fallback (extracted from main.rs) - Cargo.toml gains [lib] entry alongside the existing [[bin]] - main.rs is now a thin wrapper that delegates to the lib 2. De-duplicate the manifest-path resolution block that was copy-pasted into 5 commands (apply, rollback, list, remove, repair). New helper socket_patch_core::manifest::operations::resolve_manifest_path handles the absolute-vs-relative join in one place, with 3 unit tests. 3. New CLI_CONTRACT.md next to the crate documenting every subcommand, flag, default, alias, JSON shape, and exit code as semver-significant surface. Adds a comment block above pub enum Commands pointing to it so anyone editing the parser sees the contract reminder. Verified: cargo build/clippy/test --workspace --all-features all clean (415 unit tests pass, including 3 new resolve_manifest_path tests). Foundation for follow-up PRs that add the per-command parser snapshot tests and helper unit tests. Assisted-by: Claude Code:claude-opus-4-7 --- crates/socket-patch-cli/CLI_CONTRACT.md | 266 ++++++++++++++++++ crates/socket-patch-cli/Cargo.toml | 4 + crates/socket-patch-cli/src/commands/apply.rs | 8 +- crates/socket-patch-cli/src/commands/list.rs | 10 +- .../socket-patch-cli/src/commands/remove.rs | 10 +- .../socket-patch-cli/src/commands/repair.rs | 8 +- .../socket-patch-cli/src/commands/rollback.rs | 8 +- crates/socket-patch-cli/src/lib.rs | 96 +++++++ crates/socket-patch-cli/src/main.rs | 79 +----- .../src/manifest/operations.rs | 39 ++- 10 files changed, 421 insertions(+), 107 deletions(-) create mode 100644 crates/socket-patch-cli/CLI_CONTRACT.md create mode 100644 crates/socket-patch-cli/src/lib.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md new file mode 100644 index 0000000..df3bdd9 --- /dev/null +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -0,0 +1,266 @@ +# socket-patch CLI contract + +This document defines the **public surface** of the `socket-patch` binary. Anything listed here is part of the user-visible contract: third-party scripts, CI pipelines, and the npm/pypi/cargo wrappers depend on it. Changes are governed by the semver policy at the bottom of this file. + +> **Why this exists.** Until late 2026 the CLI crate had zero unit tests under `src/` — only network-dependent `tests/e2e_*.rs` suites that run with `--ignored`. A flag rename, a default-value change, or a JSON key rename could land green and break every shipped wrapper silently. The contract below is now backed by the unit tests under `crates/socket-patch-cli/src/**` (`#[cfg(test)] mod tests`) and the parser tests under `crates/socket-patch-cli/tests/cli_parse_*.rs`. Changes that violate the contract must update those tests in lock-step with a major version bump. + +## Subcommands + +| Name | Visible alias(es) | Notes | +|---|---|---| +| `apply` | — | Apply patches from the local manifest | +| `rollback` | — | Restore original files; takes optional positional `identifier` | +| `get` | `download` | Fetch + apply patch; requires positional `identifier` | +| `scan` | — | Crawl installed packages for available patches | +| `list` | — | Print patches in the local manifest | +| `remove` | — | Remove patch from manifest (rolls back first); requires positional `identifier` | +| `setup` | — | Configure package.json postinstall scripts | +| `repair` | `gc` | Download missing blobs + clean up unused ones | + +**Bare-UUID fallback.** `socket-patch ` is rewritten to `socket-patch get `. The UUID shape checked is the standard 8-4-4-4-12 hex pattern (case-insensitive). See [`src/lib.rs::looks_like_uuid`](src/lib.rs). + +## Flags — long and short forms + +Every flag below is part of the contract. The default values are pinned by parser tests. + +### `apply` + +| Long | Short | Default | Type | +|---|---|---|---| +| `--cwd` | — | `.` | path | +| `--dry-run` | `-d` | `false` | bool | +| `--silent` | `-s` | `false` | bool | +| `--manifest-path` | `-m` | `.socket/manifest.json` | string | +| `--offline` | — | `false` | bool | +| `--global` | `-g` | `false` | bool | +| `--global-prefix` | — | (none) | path | +| `--ecosystems` | — | (none) | CSV → `Vec` | +| `--force` | `-f` | `false` | bool | +| `--json` | — | `false` | bool | +| `--verbose` | `-v` | `false` | bool | +| `--download-mode` | — | **`diff`** | string | + +### `rollback` + +Same as `apply` plus: `--one-off` (bool), `--org` (string), `--api-url` (string), `--api-token` (string). Positional `identifier` is **optional** (omit to rollback everything). + +### `get` + +Required positional `identifier`. Flags: + +| Long | Short | Alias | Default | Type | +|---|---|---|---|---| +| `--org` | — | — | (none) | string | +| `--cwd` | — | — | `.` | path | +| `--id` | — | — | `false` | bool | +| `--cve` | — | — | `false` | bool | +| `--ghsa` | — | — | `false` | bool | +| `--package` | `-p` | — | `false` | bool | +| `--yes` | `-y` | — | `false` | bool | +| `--api-url` | — | — | (none) | string | +| `--api-token` | — | — | (none) | string | +| `--save-only` | — | **`--no-apply`** | `false` | bool | +| `--global` | `-g` | — | `false` | bool | +| `--global-prefix` | — | — | (none) | path | +| `--one-off` | — | — | `false` | bool | +| `--json` | — | — | `false` | bool | +| `--download-mode` | — | — | **`diff`** | string | + +The hidden alias `--no-apply` on `--save-only` is **part of the contract** — it does not appear in `--help` but is widely used in existing scripts. + +### `scan` + +| Long | Short | Default | Type | +|---|---|---|---| +| `--cwd` | — | `.` | path | +| `--org` | — | (none) | string | +| `--json` | — | `false` | bool | +| `--yes` | `-y` | `false` | bool | +| `--global` | `-g` | `false` | bool | +| `--global-prefix` | — | (none) | path | +| `--batch-size` | — | **`100`** | usize | +| `--api-url` | — | (none) | string | +| `--api-token` | — | (none) | string | +| `--ecosystems` | — | (none) | CSV → `Vec` | +| `--download-mode` | — | **`diff`** | string | + +### `list` + +| Long | Short | Default | Type | +|---|---|---|---| +| `--cwd` | — | `.` | path | +| `--manifest-path` | `-m` | `.socket/manifest.json` | string | +| `--json` | — | `false` | bool | + +### `remove` + +Required positional `identifier`. Flags: + +| Long | Short | Default | Type | +|---|---|---|---| +| `--cwd` | — | `.` | path | +| `--manifest-path` | `-m` | `.socket/manifest.json` | string | +| `--skip-rollback` | — | `false` | bool | +| `--yes` | `-y` | `false` | bool | +| `--global` | `-g` | `false` | bool | +| `--global-prefix` | — | (none) | path | +| `--json` | — | `false` | bool | + +### `setup` + +| Long | Short | Default | Type | +|---|---|---|---| +| `--cwd` | — | `.` | path | +| `--dry-run` | `-d` | `false` | bool | +| `--yes` | `-y` | `false` | bool | +| `--json` | — | `false` | bool | + +### `repair` + +| Long | Short | Default | Type | +|---|---|---|---| +| `--cwd` | — | `.` | path | +| `--manifest-path` | `-m` | `.socket/manifest.json` | string | +| `--dry-run` | `-d` | `false` | bool | +| `--offline` | — | `false` | bool | +| `--download-only` | — | `false` | bool | +| `--json` | — | `false` | bool | +| `--download-mode` | — | **`file`** | string | + +**Note:** `repair`'s `--download-mode` default differs from every other command (`file` vs `diff`). This is intentional — repair restores legacy per-file blobs needed to apply any patch. + +## CSV value parsing + +`--ecosystems` on `apply`, `rollback`, and `scan` uses clap's `value_delimiter = ','`. Input `--ecosystems npm,pypi,cargo` becomes `vec!["npm", "pypi", "cargo"]`. Switching to space-separated or dropping the delimiter is a **breaking** change. + +## JSON output shapes + +When `--json` is set, commands print a single JSON object to stdout. The schemas below are stable. + +### Missing-manifest error (`apply`/`list`/`remove`/`repair`/`rollback`) + +```json +{ + "status": "error", + "error": "Manifest not found", + "path": "" +} +``` + +### Invalid-manifest error + +```json +{ "status": "error", "error": "Invalid manifest" } +``` + +### Generic error + +```json +{ "status": "error", "error": "" } +``` + +### `list` success — empty manifest + +```json +{ "status": "success", "patches": [] } +``` + +### `list` success — populated + +```json +{ + "status": "success", + "patches": [ + { + "purl": "pkg:npm/foo@1.2.3", + "uuid": "…", + "exportedAt": "…", + "tier": "free|paid", + "license": "…", + "description": "…", + "files": ["…"], + "vulnerabilities": [ + { "id": "…", "cves": ["…"], "summary": "…", "severity": "…", "description": "…" } + ] + } + ] +} +``` + +### `setup` — no package.json files found + +```json +{ + "status": "no_files", + "updated": 0, + "alreadyConfigured": 0, + "errors": 0, + "files": [] +} +``` + +### `get` — multiple-patch selection required (JSON mode) + +```json +{ + "status": "selection_required", + "error": "Multiple patches available for . Specify --id to select one.", + "purl": "", + "options": [ + { "uuid": "…", "tier": "…", "published_at": "…", "description": "…", "vulnerabilities": [ … ] } + ] +} +``` + +## Exit codes + +| Code | Meaning | +|---|---| +| `0` | Success | +| `1` | Error (missing/invalid manifest, fetch failed, apply failed, selection cancelled in non-JSON mode, etc.) | + +`list` returns **`0`** for an empty manifest and **`1`** for a missing manifest — these are distinct and load-bearing. + +## Semver policy + +Versioning lives in **`Cargo.toml`** at the workspace root (`version = "..."`) and is propagated to npm, pypi, and cargo wrappers by **`scripts/version-sync.sh `**. + +| Change | Bump | +|---|---| +| Rename or remove a subcommand | **MAJOR** | +| Rename or remove a visible alias (`download`, `gc`) | **MAJOR** | +| Rename or remove a hidden alias (`--no-apply`) | **MAJOR** | +| Rename, remove, or change short form of a flag (`-d`, `-m`, etc.) | **MAJOR** | +| Change a default value (`--download-mode`, `--batch-size`, `--manifest-path`, …) | **MAJOR** | +| Change an exit code's meaning or add a new non-zero code with different semantics | **MAJOR** | +| Rename a JSON output key or change a `status` string | **MAJOR** | +| Remove a JSON output key | **MAJOR** | +| Drop the bare-UUID fallback | **MAJOR** | +| Add a *required* new flag | **MAJOR** | +| Add a new subcommand | **MINOR** | +| Add a new optional flag | **MINOR** | +| Add a new optional JSON output key (additive) | **MINOR** | +| Add a new visible alias to an existing subcommand | **MINOR** | +| Fix a bug without changing any of the above | **PATCH** | + +After bumping `Cargo.toml`, run: + +```bash +scripts/version-sync.sh +``` + +This syncs the workspace package version into: + +- `npm/socket-patch/package.json` (and its `optionalDependencies`) +- every per-platform `npm/socket-patch-*/package.json` +- `pypi/socket-patch/pyproject.toml` + +## How the contract is enforced + +Every item in this document is locked in by at least one of: + +- **clap parser snapshots** in `crates/socket-patch-cli/tests/cli_parse_*.rs` — assert flag names, short forms, defaults, aliases, and CSV delimiters by calling `socket_patch_cli::Cli::try_parse_from(...)`. +- **Helper unit tests** in `crates/socket-patch-cli/src/**` (`#[cfg(test)] mod tests` blocks) — cover `looks_like_uuid`, `parse_with_uuid_fallback`, `detect_identifier_type`, `select_patches`, `find_patches_to_rollback`, `partition_purls`, `verify_status_str`, `format_severity`, `color`, and the JSON serializers. +- **Async `run()` integration tests** in `tests/cli_parse_list.rs`, `tests/cli_parse_remove.rs`, `tests/cli_parse_setup.rs` — exercise the no-network error paths and assert JSON shape via `serde_json::from_str::` + per-key assertions. + +If you add a new flag/subcommand/JSON key, add a test here that locks the new surface in the same PR. diff --git a/crates/socket-patch-cli/Cargo.toml b/crates/socket-patch-cli/Cargo.toml index 8911c79..ed2a651 100644 --- a/crates/socket-patch-cli/Cargo.toml +++ b/crates/socket-patch-cli/Cargo.toml @@ -7,6 +7,10 @@ license.workspace = true repository.workspace = true readme = "README.md" +[lib] +name = "socket_patch_cli" +path = "src/lib.rs" + [[bin]] name = "socket-patch" path = "src/main.rs" diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 80f4f67..0f88fe1 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -6,7 +6,7 @@ use socket_patch_core::api::blob_fetcher::{ use socket_patch_core::api::client::get_api_client_from_env; use socket_patch_core::constants::DEFAULT_PATCH_MANIFEST_PATH; use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem}; -use socket_patch_core::manifest::operations::read_manifest; +use socket_patch_core::manifest::operations::{read_manifest, resolve_manifest_path}; use socket_patch_core::patch::apply::{ apply_package_patch, verify_file_patch, ApplyResult, PatchSources, VerifyStatus, }; @@ -113,11 +113,7 @@ pub async fn run(args: ApplyArgs) -> i32 { let api_token = telemetry_client.api_token().cloned(); let org_slug = telemetry_client.org_slug().cloned(); - let manifest_path = if Path::new(&args.manifest_path).is_absolute() { - PathBuf::from(&args.manifest_path) - } else { - args.cwd.join(&args.manifest_path) - }; + let manifest_path = resolve_manifest_path(&args.cwd, &args.manifest_path); // Check if manifest exists - exit successfully if no .socket folder is set up if tokio::fs::metadata(&manifest_path).await.is_err() { diff --git a/crates/socket-patch-cli/src/commands/list.rs b/crates/socket-patch-cli/src/commands/list.rs index 8dc00a6..9365537 100644 --- a/crates/socket-patch-cli/src/commands/list.rs +++ b/crates/socket-patch-cli/src/commands/list.rs @@ -1,7 +1,7 @@ use clap::Args; use socket_patch_core::constants::DEFAULT_PATCH_MANIFEST_PATH; -use socket_patch_core::manifest::operations::read_manifest; -use std::path::{Path, PathBuf}; +use socket_patch_core::manifest::operations::{read_manifest, resolve_manifest_path}; +use std::path::PathBuf; #[derive(Args)] pub struct ListArgs { @@ -19,11 +19,7 @@ pub struct ListArgs { } pub async fn run(args: ListArgs) -> i32 { - let manifest_path = if Path::new(&args.manifest_path).is_absolute() { - PathBuf::from(&args.manifest_path) - } else { - args.cwd.join(&args.manifest_path) - }; + let manifest_path = resolve_manifest_path(&args.cwd, &args.manifest_path); // Check if manifest exists if tokio::fs::metadata(&manifest_path).await.is_err() { diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 8acff80..1d5c203 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -1,6 +1,8 @@ use clap::Args; use socket_patch_core::constants::DEFAULT_PATCH_MANIFEST_PATH; -use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; +use socket_patch_core::manifest::operations::{ + read_manifest, resolve_manifest_path, write_manifest, +}; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::utils::cleanup_blobs::{cleanup_unused_blobs, format_cleanup_result}; use socket_patch_core::utils::telemetry::{track_patch_removed, track_patch_remove_failed}; @@ -49,11 +51,7 @@ pub async fn run(args: RemoveArgs) -> i32 { let api_token = telemetry_client.api_token().cloned(); let org_slug = telemetry_client.org_slug().cloned(); - let manifest_path = if Path::new(&args.manifest_path).is_absolute() { - PathBuf::from(&args.manifest_path) - } else { - args.cwd.join(&args.manifest_path) - }; + let manifest_path = resolve_manifest_path(&args.cwd, &args.manifest_path); if tokio::fs::metadata(&manifest_path).await.is_err() { if args.json { diff --git a/crates/socket-patch-cli/src/commands/repair.rs b/crates/socket-patch-cli/src/commands/repair.rs index 2197bb1..ff54e07 100644 --- a/crates/socket-patch-cli/src/commands/repair.rs +++ b/crates/socket-patch-cli/src/commands/repair.rs @@ -5,7 +5,7 @@ use socket_patch_core::api::blob_fetcher::{ }; use socket_patch_core::api::client::get_api_client_from_env; use socket_patch_core::constants::DEFAULT_PATCH_MANIFEST_PATH; -use socket_patch_core::manifest::operations::read_manifest; +use socket_patch_core::manifest::operations::{read_manifest, resolve_manifest_path}; use socket_patch_core::patch::apply::PatchSources; use socket_patch_core::utils::cleanup_blobs::{ cleanup_unused_archives, cleanup_unused_blobs, format_cleanup_result, @@ -46,11 +46,7 @@ pub struct RepairArgs { } pub async fn run(args: RepairArgs) -> i32 { - let manifest_path = if Path::new(&args.manifest_path).is_absolute() { - PathBuf::from(&args.manifest_path) - } else { - args.cwd.join(&args.manifest_path) - }; + let manifest_path = resolve_manifest_path(&args.cwd, &args.manifest_path); if tokio::fs::metadata(&manifest_path).await.is_err() { if args.json { diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 93bf96f..3de0194 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -5,7 +5,7 @@ use socket_patch_core::api::blob_fetcher::{ use socket_patch_core::api::client::get_api_client_from_env; use socket_patch_core::constants::DEFAULT_PATCH_MANIFEST_PATH; use socket_patch_core::crawlers::CrawlerOptions; -use socket_patch_core::manifest::operations::read_manifest; +use socket_patch_core::manifest::operations::{read_manifest, resolve_manifest_path}; use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord}; use socket_patch_core::patch::rollback::{rollback_package_patch, RollbackResult, VerifyRollbackStatus}; use socket_patch_core::utils::telemetry::{track_patch_rolled_back, track_patch_rollback_failed}; @@ -212,11 +212,7 @@ pub async fn run(args: RollbackArgs) -> i32 { return 1; } - let manifest_path = if Path::new(&args.manifest_path).is_absolute() { - PathBuf::from(&args.manifest_path) - } else { - args.cwd.join(&args.manifest_path) - }; + let manifest_path = resolve_manifest_path(&args.cwd, &args.manifest_path); if tokio::fs::metadata(&manifest_path).await.is_err() { if args.json { diff --git a/crates/socket-patch-cli/src/lib.rs b/crates/socket-patch-cli/src/lib.rs new file mode 100644 index 0000000..a06cf5c --- /dev/null +++ b/crates/socket-patch-cli/src/lib.rs @@ -0,0 +1,96 @@ +//! socket-patch CLI library crate. +//! +//! Exposes the clap parser types so integration tests can verify the public +//! CLI contract without invoking the binary. The `main.rs` binary entry point +//! is a thin wrapper that delegates to [`parse_with_uuid_fallback`] and the +//! `run` function on each command's `Args`. + +pub mod commands; +pub mod ecosystem_dispatch; +pub mod output; + +use clap::{Parser, Subcommand}; + +// CLI contract surface — subcommand names, visible_alias values, flag names, +// defaults, JSON shapes, and exit codes are PUBLIC and SEMVER-SIGNIFICANT. +// Changes here require a MAJOR bump + `scripts/version-sync.sh`. +// See crates/socket-patch-cli/CLI_CONTRACT.md. +#[derive(Parser)] +#[command( + name = "socket-patch", + about = "CLI tool for applying security patches to dependencies", + version, + propagate_version = true +)] +pub struct Cli { + #[command(subcommand)] + pub command: Commands, +} + +#[derive(Subcommand)] +pub enum Commands { + /// Apply security patches to dependencies + Apply(commands::apply::ApplyArgs), + + /// Rollback patches to restore original files + Rollback(commands::rollback::RollbackArgs), + + /// Get security patches from Socket API and apply them + #[command(visible_alias = "download")] + Get(commands::get::GetArgs), + + /// Scan installed packages for available security patches + Scan(commands::scan::ScanArgs), + + /// List all patches in the local manifest + List(commands::list::ListArgs), + + /// Remove a patch from the manifest by PURL or UUID (rolls back files first) + Remove(commands::remove::RemoveArgs), + + /// Configure package.json postinstall scripts to apply patches + Setup(commands::setup::SetupArgs), + + /// Download missing blobs and clean up unused blobs + #[command(visible_alias = "gc")] + Repair(commands::repair::RepairArgs), +} + +/// Check whether `s` looks like a UUID (8-4-4-4-12 hex pattern). +/// +/// Used by [`parse_with_uuid_fallback`] to detect the convenience form +/// `socket-patch ` and rewrite it to `socket-patch get `. +pub fn looks_like_uuid(s: &str) -> bool { + let parts: Vec<&str> = s.split('-').collect(); + if parts.len() != 5 { + return false; + } + let expected = [8, 4, 4, 4, 12]; + parts + .iter() + .zip(expected.iter()) + .all(|(p, &len)| p.len() == len && p.chars().all(|c| c.is_ascii_hexdigit())) +} + +/// Parse a full argv vector, falling back to `get ` when the user +/// invoked `socket-patch [...]` directly. Returns the original clap +/// error if the fallback also fails or if the first arg isn't a UUID. +/// +/// Pulled out of `main.rs` so the fallback path is unit-testable. +pub fn parse_with_uuid_fallback(argv: Vec) -> Result { + match Cli::try_parse_from(&argv) { + Ok(cli) => Ok(cli), + Err(err) => { + if argv.len() >= 2 && looks_like_uuid(&argv[1]) { + let mut new_args = vec![argv[0].clone(), "get".into()]; + new_args.extend_from_slice(&argv[1..]); + match Cli::try_parse_from(&new_args) { + Ok(cli) => Ok(cli), + Err(_) => Err(err), + } + } else { + Err(err) + } + } + } +} diff --git a/crates/socket-patch-cli/src/main.rs b/crates/socket-patch-cli/src/main.rs index e278400..ffdbf6e 100644 --- a/crates/socket-patch-cli/src/main.rs +++ b/crates/socket-patch-cli/src/main.rs @@ -1,82 +1,11 @@ -mod commands; -mod ecosystem_dispatch; -mod output; - -use clap::{Parser, Subcommand}; - -#[derive(Parser)] -#[command( - name = "socket-patch", - about = "CLI tool for applying security patches to dependencies", - version, - propagate_version = true -)] -struct Cli { - #[command(subcommand)] - command: Commands, -} - -#[derive(Subcommand)] -enum Commands { - /// Apply security patches to dependencies - Apply(commands::apply::ApplyArgs), - - /// Rollback patches to restore original files - Rollback(commands::rollback::RollbackArgs), - - /// Get security patches from Socket API and apply them - #[command(visible_alias = "download")] - Get(commands::get::GetArgs), - - /// Scan installed packages for available security patches - Scan(commands::scan::ScanArgs), - - /// List all patches in the local manifest - List(commands::list::ListArgs), - - /// Remove a patch from the manifest by PURL or UUID (rolls back files first) - Remove(commands::remove::RemoveArgs), - - /// Configure package.json postinstall scripts to apply patches - Setup(commands::setup::SetupArgs), - - /// Download missing blobs and clean up unused blobs - #[command(visible_alias = "gc")] - Repair(commands::repair::RepairArgs), -} - -/// Check whether `s` looks like a UUID (8-4-4-4-12 hex pattern). -fn looks_like_uuid(s: &str) -> bool { - let parts: Vec<&str> = s.split('-').collect(); - if parts.len() != 5 { - return false; - } - let expected = [8, 4, 4, 4, 12]; - parts - .iter() - .zip(expected.iter()) - .all(|(p, &len)| p.len() == len && p.chars().all(|c| c.is_ascii_hexdigit())) -} +use socket_patch_cli::{commands, parse_with_uuid_fallback, Commands}; #[tokio::main] async fn main() { - let cli = match Cli::try_parse() { + let argv: Vec = std::env::args().collect(); + let cli = match parse_with_uuid_fallback(argv) { Ok(cli) => cli, - Err(err) => { - // If parsing failed, check whether the user passed a bare UUID - // (e.g. `socket-patch 80630680-...`) and retry as `get ...`. - let args: Vec = std::env::args().collect(); - if args.len() >= 2 && looks_like_uuid(&args[1]) { - let mut new_args = vec![args[0].clone(), "get".into()]; - new_args.extend_from_slice(&args[1..]); - match Cli::try_parse_from(&new_args) { - Ok(cli) => cli, - Err(_) => err.exit(), - } - } else { - err.exit() - } - } + Err(err) => err.exit(), }; let exit_code = match cli.command { diff --git a/crates/socket-patch-core/src/manifest/operations.rs b/crates/socket-patch-core/src/manifest/operations.rs index 30fae4a..1417775 100644 --- a/crates/socket-patch-core/src/manifest/operations.rs +++ b/crates/socket-patch-core/src/manifest/operations.rs @@ -1,8 +1,19 @@ use std::collections::HashSet; -use std::path::Path; +use std::path::{Path, PathBuf}; use crate::manifest::schema::PatchManifest; +/// Resolve a manifest path: absolute paths are returned as-is, relative paths +/// are joined to `cwd`. Centralizes the duplicate block previously inlined in +/// apply/rollback/list/remove/repair commands. +pub fn resolve_manifest_path(cwd: &Path, manifest_path: &str) -> PathBuf { + if Path::new(manifest_path).is_absolute() { + PathBuf::from(manifest_path) + } else { + cwd.join(manifest_path) + } +} + /// Get all blob hashes referenced by a manifest (both beforeHash and afterHash). /// Used for garbage collection and validation. pub fn get_referenced_blobs(manifest: &PatchManifest) -> HashSet { @@ -457,4 +468,30 @@ mod tests { let read_back = read_back.unwrap(); assert_eq!(read_back.patches.len(), 2); } + + #[test] + fn test_resolve_manifest_path_relative_joins_cwd() { + let cwd = Path::new("/tmp/proj"); + let resolved = resolve_manifest_path(cwd, ".socket/manifest.json"); + assert_eq!(resolved, PathBuf::from("/tmp/proj/.socket/manifest.json")); + } + + #[test] + fn test_resolve_manifest_path_absolute_unchanged() { + let cwd = Path::new("/tmp/proj"); + let absolute = if cfg!(windows) { + r"C:\custom\manifest.json" + } else { + "/etc/custom/manifest.json" + }; + let resolved = resolve_manifest_path(cwd, absolute); + assert_eq!(resolved, PathBuf::from(absolute)); + } + + #[test] + fn test_resolve_manifest_path_relative_dotted() { + let cwd = Path::new("/tmp/proj"); + let resolved = resolve_manifest_path(cwd, "../manifest.json"); + assert_eq!(resolved, PathBuf::from("/tmp/proj/../manifest.json")); + } } From 0eb8ac6eb5d914c25d00012855286bce97811a74 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 19 May 2026 16:47:50 -0400 Subject: [PATCH 2/2] test(cli): pin GetArgs flag surface, download/no-apply aliases, detect_identifier_type Locks in the `get` subcommand's public CLI contract with parser snapshot tests and unit-tests the pure helpers behind it. No behavior change. - New tests/cli_parse_get.rs (27 tests) asserts every flag in the get table parses to its documented default and bound field: positional identifier, `--org`, `--cwd`, `--id`, `--cve`, `--ghsa`, `--package`/`-p`, `--yes`/`-y`, `--api-url`, `--api-token`, `--save-only`, `--global`/`-g`, `--global-prefix`, `--one-off`, `--json`, `--download-mode` (default `diff`, plus `package` and `file`). Exercises the hidden `--no-apply` alias for `--save-only` and the visible `download` alias for `get`. Pins the missing-positional and unknown-flag clap error kinds. - New `#[cfg(test)] mod tests` in commands/get.rs (17 tests) covers `detect_identifier_type` (UUID/CVE/GHSA/PURL, case-insensitive for UUID+CVE+GHSA, plain package names + malformed CVE + empty string both None) and `select_patches` (free/paid x single/multi/empty, paid-wins, most-recent-paid, fallback to most-recent-free, JSON-mode rejection for multi-free). Interactive TTY path is intentionally skipped. Verified: cargo build/clippy/test --workspace --all-features all clean from the worktree (459 unit tests + 27 cli_parse_get tests pass). Assisted-by: Claude Code:claude-opus-4-7 --- crates/socket-patch-cli/src/commands/get.rs | 184 ++++++++++++++ .../socket-patch-cli/tests/cli_parse_get.rs | 232 ++++++++++++++++++ 2 files changed, 416 insertions(+) create mode 100644 crates/socket-patch-cli/tests/cli_parse_get.rs diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index ee2399a..e00c462 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1268,3 +1268,187 @@ fn base64_decode(input: &str) -> Result, String> { Ok(output) } + +#[cfg(test)] +mod tests { + use super::*; + use socket_patch_core::api::types::VulnerabilityResponse; + use std::collections::HashMap; + + // --- detect_identifier_type ------------------------------------------- + + #[test] + fn detect_uuid_lowercase() { + assert_eq!( + detect_identifier_type("80630680-4da6-45f9-bba8-b888e0ffd58c"), + Some(IdentifierType::Uuid) + ); + } + + #[test] + fn detect_uuid_uppercase() { + // Case-insensitive UUID regex per contract. + assert_eq!( + detect_identifier_type("80630680-4DA6-45F9-BBA8-B888E0FFD58C"), + Some(IdentifierType::Uuid) + ); + } + + #[test] + fn detect_cve_uppercase() { + assert_eq!( + detect_identifier_type("CVE-2021-44906"), + Some(IdentifierType::Cve) + ); + } + + #[test] + fn detect_cve_lowercase() { + // Load-bearing: CVE detection must be case-insensitive. + assert_eq!( + detect_identifier_type("cve-2021-44906"), + Some(IdentifierType::Cve) + ); + } + + #[test] + fn detect_ghsa_uppercase() { + assert_eq!( + detect_identifier_type("GHSA-abcd-1234-wxyz"), + Some(IdentifierType::Ghsa) + ); + } + + #[test] + fn detect_ghsa_lowercase() { + // Load-bearing: GHSA detection must be case-insensitive. + assert_eq!( + detect_identifier_type("ghsa-abcd-1234-wxyz"), + Some(IdentifierType::Ghsa) + ); + } + + #[test] + fn detect_purl() { + assert_eq!( + detect_identifier_type("pkg:npm/foo@1.0"), + Some(IdentifierType::Purl) + ); + } + + #[test] + fn detect_package_name_returns_none() { + // Bare package names don't match any pattern; caller treats this as + // Package via the `else` branch in run(). + assert_eq!(detect_identifier_type("minimist"), None); + } + + #[test] + fn detect_malformed_cve_returns_none() { + assert_eq!(detect_identifier_type("CVE-not-a-year"), None); + } + + #[test] + fn detect_empty_string_returns_none() { + assert_eq!(detect_identifier_type(""), None); + } + + // --- select_patches --------------------------------------------------- + + fn mk_patch( + uuid: &str, + purl: &str, + tier: &str, + published_at: &str, + ) -> PatchSearchResult { + PatchSearchResult { + uuid: uuid.into(), + purl: purl.into(), + published_at: published_at.into(), + description: format!("desc-{uuid}"), + license: "MIT".into(), + tier: tier.into(), + vulnerabilities: HashMap::::new(), + } + } + + #[test] + fn select_free_user_one_free_patch_returns_it() { + let patches = vec![mk_patch("u1", "pkg:npm/foo@1.0", "free", "2024-01-01")]; + let out = select_patches(&patches, false, false).expect("ok"); + assert_eq!(out.len(), 1); + assert_eq!(out[0].uuid, "u1"); + } + + #[test] + fn select_paid_user_prefers_paid_over_free_same_purl() { + let patches = vec![ + mk_patch("free1", "pkg:npm/foo@1.0", "free", "2024-06-01"), + mk_patch("paid1", "pkg:npm/foo@1.0", "paid", "2024-01-01"), + ]; + let out = select_patches(&patches, true, false).expect("ok"); + assert_eq!(out.len(), 1); + // Paid wins even if free is more recent. + assert_eq!(out[0].uuid, "paid1"); + assert_eq!(out[0].tier, "paid"); + } + + #[test] + fn select_paid_user_picks_most_recent_paid() { + let patches = vec![ + mk_patch("old", "pkg:npm/foo@1.0", "paid", "2024-01-01"), + mk_patch("new", "pkg:npm/foo@1.0", "paid", "2024-06-01"), + ]; + let out = select_patches(&patches, true, false).expect("ok"); + assert_eq!(out.len(), 1); + assert_eq!(out[0].uuid, "new"); + } + + #[test] + fn select_paid_user_falls_back_to_most_recent_free_when_no_paid() { + let patches = vec![ + mk_patch("old", "pkg:npm/foo@1.0", "free", "2024-01-01"), + mk_patch("new", "pkg:npm/foo@1.0", "free", "2024-06-01"), + ]; + let out = select_patches(&patches, true, false).expect("ok"); + assert_eq!(out.len(), 1); + assert_eq!(out[0].uuid, "new"); + } + + #[test] + fn select_free_user_multi_free_json_mode_errors() { + // JSON mode requires explicit selection; multiple free patches in JSON + // mode means the caller must pass --id. + let patches = vec![ + mk_patch("a", "pkg:npm/foo@1.0", "free", "2024-01-01"), + mk_patch("b", "pkg:npm/foo@1.0", "free", "2024-06-01"), + ]; + let err = select_patches(&patches, false, true).expect_err("should fail"); + assert_eq!(err, 1); + } + + #[test] + fn select_empty_input_returns_empty() { + let out = select_patches(&[], false, false).expect("ok"); + assert!(out.is_empty()); + let out = select_patches(&[], true, false).expect("ok"); + assert!(out.is_empty()); + let out = select_patches(&[], false, true).expect("ok"); + assert!(out.is_empty()); + } + + #[test] + fn select_free_user_paid_filtered_out_then_single_free_auto_selects() { + // Free user: paid patch is filtered out before grouping; only the free + // patch survives, and since the group has exactly one entry it + // auto-selects without hitting the interactive path. + let patches = vec![ + mk_patch("paid", "pkg:npm/foo@1.0", "paid", "2024-06-01"), + mk_patch("free", "pkg:npm/foo@1.0", "free", "2024-01-01"), + ]; + let out = select_patches(&patches, false, false).expect("ok"); + assert_eq!(out.len(), 1); + assert_eq!(out[0].uuid, "free"); + assert_eq!(out[0].tier, "free"); + } +} diff --git a/crates/socket-patch-cli/tests/cli_parse_get.rs b/crates/socket-patch-cli/tests/cli_parse_get.rs new file mode 100644 index 0000000..902dfb8 --- /dev/null +++ b/crates/socket-patch-cli/tests/cli_parse_get.rs @@ -0,0 +1,232 @@ +//! Clap parser snapshot tests for the `get` subcommand. +//! +//! These tests pin the public CLI contract for `socket-patch get`: every +//! flag, every alias (including the hidden `--no-apply` and the visible +//! `download` alias), and every default. Changing any assertion here is a +//! breaking change to the CLI surface — see +//! `crates/socket-patch-cli/CLI_CONTRACT.md`. + +use clap::Parser; +use socket_patch_cli::commands::get::GetArgs; +use socket_patch_cli::{Cli, Commands}; +use std::path::PathBuf; + +/// Parse `socket-patch get ` and return the `GetArgs`. +fn parse_get(extra: &[&str]) -> GetArgs { + let mut argv = vec!["socket-patch", "get"]; + argv.extend_from_slice(extra); + let cli = Cli::try_parse_from(&argv).expect("parse"); + match cli.command { + Commands::Get(a) => a, + _ => panic!("expected Get"), + } +} + +// --- Defaults ---------------------------------------------------------------- + +#[test] +fn defaults_with_only_required_identifier() { + let a = parse_get(&["some-id"]); + assert_eq!(a.identifier, "some-id"); + assert_eq!(a.org, None); + assert_eq!(a.cwd, PathBuf::from(".")); + assert!(!a.id); + assert!(!a.cve); + assert!(!a.ghsa); + assert!(!a.package); + assert!(!a.yes); + assert_eq!(a.api_url, None); + assert_eq!(a.api_token, None); + assert!(!a.save_only); + assert!(!a.global); + assert_eq!(a.global_prefix, None); + assert!(!a.one_off); + assert!(!a.json); + assert_eq!(a.download_mode, "diff"); +} + +#[test] +fn default_download_mode_is_diff() { + let a = parse_get(&["some-id"]); + assert_eq!(a.download_mode, "diff"); +} + +// --- Positional -------------------------------------------------------------- + +#[test] +fn positional_identifier_stored() { + let a = parse_get(&["pkg:npm/foo@1.0"]); + assert_eq!(a.identifier, "pkg:npm/foo@1.0"); +} + +// --- Short flags ------------------------------------------------------------- + +#[test] +fn short_p_sets_package() { + let a = parse_get(&["some-id", "-p"]); + assert!(a.package); +} + +#[test] +fn long_package_sets_package() { + let a = parse_get(&["some-id", "--package"]); + assert!(a.package); +} + +#[test] +fn short_y_sets_yes() { + let a = parse_get(&["some-id", "-y"]); + assert!(a.yes); +} + +#[test] +fn long_yes_sets_yes() { + let a = parse_get(&["some-id", "--yes"]); + assert!(a.yes); +} + +#[test] +fn short_g_sets_global() { + let a = parse_get(&["some-id", "-g"]); + assert!(a.global); +} + +#[test] +fn long_global_sets_global() { + let a = parse_get(&["some-id", "--global"]); + assert!(a.global); +} + +// --- Long-only flags --------------------------------------------------------- + +#[test] +fn cwd_flag_sets_cwd() { + let a = parse_get(&["some-id", "--cwd", "/tmp/project"]); + assert_eq!(a.cwd, PathBuf::from("/tmp/project")); +} + +#[test] +fn org_flag_sets_org() { + let a = parse_get(&["some-id", "--org", "acme"]); + assert_eq!(a.org.as_deref(), Some("acme")); +} + +#[test] +fn id_flag_sets_id() { + let a = parse_get(&["some-id", "--id"]); + assert!(a.id); +} + +#[test] +fn cve_flag_sets_cve() { + let a = parse_get(&["some-id", "--cve"]); + assert!(a.cve); +} + +#[test] +fn ghsa_flag_sets_ghsa() { + let a = parse_get(&["some-id", "--ghsa"]); + assert!(a.ghsa); +} + +#[test] +fn api_url_flag_sets_api_url() { + let a = parse_get(&["some-id", "--api-url", "https://api.example.com"]); + assert_eq!(a.api_url.as_deref(), Some("https://api.example.com")); +} + +#[test] +fn api_token_flag_sets_api_token() { + let a = parse_get(&["some-id", "--api-token", "sktsec_abc"]); + assert_eq!(a.api_token.as_deref(), Some("sktsec_abc")); +} + +#[test] +fn global_prefix_flag_sets_global_prefix() { + let a = parse_get(&["some-id", "--global-prefix", "/usr/local/lib"]); + assert_eq!(a.global_prefix, Some(PathBuf::from("/usr/local/lib"))); +} + +#[test] +fn one_off_flag_sets_one_off() { + let a = parse_get(&["some-id", "--one-off"]); + assert!(a.one_off); +} + +#[test] +fn json_flag_sets_json() { + let a = parse_get(&["some-id", "--json"]); + assert!(a.json); +} + +// --- save-only / --no-apply alias ------------------------------------------- + +#[test] +fn save_only_flag_sets_save_only() { + let a = parse_get(&["some-id", "--save-only"]); + assert!(a.save_only); +} + +#[test] +fn no_apply_hidden_alias_sets_save_only() { + // `--no-apply` is a hidden alias for `--save-only`. It does not appear in + // `--help` but is widely used in existing scripts — this is part of the + // CLI contract. + let a = parse_get(&["some-id", "--no-apply"]); + assert!(a.save_only); +} + +// --- download-mode ----------------------------------------------------------- + +#[test] +fn download_mode_package() { + let a = parse_get(&["some-id", "--download-mode", "package"]); + assert_eq!(a.download_mode, "package"); +} + +#[test] +fn download_mode_diff() { + let a = parse_get(&["some-id", "--download-mode", "diff"]); + assert_eq!(a.download_mode, "diff"); +} + +#[test] +fn download_mode_file() { + let a = parse_get(&["some-id", "--download-mode", "file"]); + assert_eq!(a.download_mode, "file"); +} + +// --- `download` visible alias for `get` ------------------------------------- + +#[test] +fn download_visible_alias_routes_to_get() { + let cli = + Cli::try_parse_from(["socket-patch", "download", "some-id"]).expect("parse"); + match cli.command { + Commands::Get(a) => { + assert_eq!(a.identifier, "some-id"); + } + _ => panic!("expected Get from `download` alias"), + } +} + +// --- Error paths ------------------------------------------------------------- + +#[test] +fn missing_required_identifier_errors() { + let err = match Cli::try_parse_from(["socket-patch", "get"]) { + Err(e) => e, + Ok(_) => panic!("expected parse error for missing required positional"), + }; + assert_eq!(err.kind(), clap::error::ErrorKind::MissingRequiredArgument); +} + +#[test] +fn unknown_flag_errors() { + let err = match Cli::try_parse_from(["socket-patch", "get", "some-id", "--bogus"]) + { + Err(e) => e, + Ok(_) => panic!("expected parse error for unknown flag"), + }; + assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); +}