Skip to content

Commit b96a13f

Browse files
test(cli): foundation for CLI-contract unit-test campaign (#68)
* test(cli): comprehensive unit-test campaign for the CLI contract Add unit-test coverage for every subcommand, flag, default, alias, and helper in the socket-patch CLI, plus a new CLI_CONTRACT.md that documents the surface as semver-significant. Before this change the CLI crate had zero unit tests under src/ — only network-dependent tests/e2e_*.rs suites gated on --ignored. A flag rename, a default change, or a JSON key drift could land green and break every shipped wrapper (npm @socketsecurity/socket-patch, pypi socket-patch, cargo, prebuilt binaries). ## What's covered Library surface (new src/lib.rs): - Cli, Commands, looks_like_uuid, parse_with_uuid_fallback extracted from main.rs so integration tests can verify the parser without spawning the binary. - main.rs becomes a thin wrapper that delegates to the lib. - Cargo.toml gains [lib] alongside [[bin]]. Core helper extraction: - socket_patch_core::manifest::operations::resolve_manifest_path replaces the 5-line absolute-vs-relative join block previously copy-pasted into apply/rollback/list/remove/repair (5 callers). CLI_CONTRACT.md (new): - Documents every subcommand, flag, default value, visible alias (download, gc), hidden alias (--no-apply), JSON output shape, and exit code as semver-significant. - Pins the divergent defaults: --download-mode=diff for apply/get/scan and --download-mode=file for repair, --batch-size=100 for scan. - Spells out the bump policy and how to invoke scripts/version-sync.sh. Helper unit tests (#[cfg(test)] mod tests in each file): - src/lib.rs — looks_like_uuid (valid/invalid shapes, case-insensitive), parse_with_uuid_fallback (success, fallback, fallback-fails preserves original error, no double-rewrite). - src/output.rs — format_severity, color, confirm (skip_prompt and is_json short-circuits; interactive path intentionally not tested). - src/ecosystem_dispatch.rs — partition_purls (filter, dedup, unknown ecosystem dropped). - commands/apply.rs — verify_status_str (all 4 VerifyStatus variants), result_to_json (top-level + filesVerified key sets). - commands/rollback.rs — find_patches_to_rollback (None=all, PURL match, UUID match, no-match=empty). - commands/get.rs — detect_identifier_type (UUID/CVE/GHSA/PURL/None, case-insensitive for CVE+GHSA), select_patches (paid auto, free single auto, free multi+is_json -> Err(1)). Clap parser snapshot tests (new tests/cli_parse_*.rs): - One file per command: apply, get, list, main, remove, repair, rollback, scan, setup. - Every flag (long + short) has at least one parser assertion. - Every #[arg(default_value)] is asserted on the no-flag parse. - The download visible alias on get is exercised. - The gc visible alias on repair is exercised. - The hidden --no-apply alias on get --save-only is exercised. - --ecosystems CSV splitting is verified on apply/rollback/scan. - The bare-UUID rewrite is exercised end-to-end via Cli::try_parse_from. - Failure paths assert clap::error::ErrorKind variants. Async run() integration tests: - tests/cli_parse_list.rs covers missing manifest -> 1, empty -> 0, populated -> 0, absolute-path override, and a subprocess JSON-shape assertion against the compiled binary. - tests/cli_parse_remove.rs covers missing-manifest -> 1. - tests/cli_parse_setup.rs covers no-package-json -> 0 with the JSON status:"no_files" shape pinned via subprocess. ## Verification cargo build --workspace --all-features cargo clippy --workspace --all-features -- -D warnings cargo test --workspace --all-features All clean. 79 new lib tests + 156 new integration tests added on top of the existing 415 unit tests; cumulative 650 tests pass. ## Why squashed Originally landed as a foundation PR (#68) plus 10 sibling test PRs (#69-#78), one per command/file, dispatched in parallel. Squashing the sibling PRs back into #68 so the contract + tests land as one self-contained unit — reviewers see the full picture, and a future revert touches one commit instead of eleven. Assisted-by: Claude Code:claude-opus-4-7 * ci: tighten CI matrix — add macOS, release-mode tests, explicit build step The CI workflow's unit-test job ran on ubuntu-latest + windows-latest; extend the matrix with macos-latest so the new CLI parser tests are exercised on every platform the binary ships for. socket-patch ships prebuilt binaries for x86_64-apple-darwin and aarch64-apple-darwin (see release.yml), so silent macOS-specific regressions in path handling, TTY detection, or terminal escapes are real risks today. Three changes: - Add `macos-latest` to the test matrix. - Add `fail-fast: false` so a failure on one OS doesn't mask failures on the others. - Add an explicit `cargo build --workspace --all-features` step before `cargo test`. `cargo test` already builds, but a dedicated build step gives a cleaner red signal when a build-only failure happens (e.g. a feature-gated compile error) without the noise of test-discovery output. - New `test-release` job: `cargo test --workspace --all-features --release` on ubuntu-latest. Catches optimization-level regressions that debug mode hides (e.g. release-mode-only inlining changes that affect assertion behavior). One OS keeps total CI time reasonable while still locking in release-mode correctness. Assisted-by: Claude Code:claude-opus-4-7 * fix(patch): reject POSIX-style absolute paths in archive entries on Windows `read_archive_to_map` rejects entries whose path is absolute or contains a `..` component, but the check used `Path::is_absolute()` alone. On Windows that function requires a drive letter or UNC prefix, so a tar entry like `/etc/passwd` is NOT considered absolute and would slip through the guard — when later joined to the target directory, Windows would treat it as relative to the current drive's root. Add an explicit check for a leading `/` or `\` byte alongside `Path::is_absolute()` so the guard rejects POSIX-style absolute paths on every platform. The new test_read_archive_rejects_backslash_absolute_paths case locks the symmetric backslash form in. This was uncovered when the CI matrix was extended to actually run on Windows. The existing test_read_archive_rejects_absolute_paths failed on windows-latest because it constructed the archive with a POSIX-style path that the platform-specific `is_absolute()` did not catch. Assisted-by: Claude Code:claude-opus-4-7
1 parent 6288a37 commit b96a13f

24 files changed

Lines changed: 3150 additions & 108 deletions

.github/workflows/ci.yml

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ jobs:
3838

3939
test:
4040
strategy:
41+
fail-fast: false
4142
matrix:
42-
os: [ubuntu-latest, windows-latest]
43+
os: [ubuntu-latest, macos-latest, windows-latest]
4344
runs-on: ${{ matrix.os }}
4445
steps:
4546
- name: Checkout
@@ -62,9 +63,38 @@ jobs:
6263
key: ${{ matrix.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
6364
restore-keys: ${{ matrix.os }}-cargo-
6465

66+
- name: Build
67+
run: cargo build --workspace --all-features
68+
6569
- name: Run tests
6670
run: cargo test --workspace --all-features
6771

72+
test-release:
73+
runs-on: ubuntu-latest
74+
steps:
75+
- name: Checkout
76+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
77+
with:
78+
persist-credentials: false
79+
80+
- name: Install Rust
81+
uses: dtolnay/rust-toolchain@efa25f7f19611383d5b0ccf2d1c8914531636bf9 # stable
82+
with:
83+
toolchain: stable
84+
85+
- name: Cache cargo
86+
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
87+
with:
88+
path: |
89+
~/.cargo/registry
90+
~/.cargo/git
91+
target
92+
key: ubuntu-latest-cargo-release-${{ hashFiles('**/Cargo.lock') }}
93+
restore-keys: ubuntu-latest-cargo-release-
94+
95+
- name: Run tests (release)
96+
run: cargo test --workspace --all-features --release
97+
6898
dispatch-tests:
6999
runs-on: ubuntu-latest
70100
steps:
Lines changed: 266 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,266 @@
1+
# socket-patch CLI contract
2+
3+
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.
4+
5+
> **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.
6+
7+
## Subcommands
8+
9+
| Name | Visible alias(es) | Notes |
10+
|---|---|---|
11+
| `apply` || Apply patches from the local manifest |
12+
| `rollback` || Restore original files; takes optional positional `identifier` |
13+
| `get` | `download` | Fetch + apply patch; requires positional `identifier` |
14+
| `scan` || Crawl installed packages for available patches |
15+
| `list` || Print patches in the local manifest |
16+
| `remove` || Remove patch from manifest (rolls back first); requires positional `identifier` |
17+
| `setup` || Configure package.json postinstall scripts |
18+
| `repair` | `gc` | Download missing blobs + clean up unused ones |
19+
20+
**Bare-UUID fallback.** `socket-patch <UUID>` is rewritten to `socket-patch get <UUID>`. 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).
21+
22+
## Flags — long and short forms
23+
24+
Every flag below is part of the contract. The default values are pinned by parser tests.
25+
26+
### `apply`
27+
28+
| Long | Short | Default | Type |
29+
|---|---|---|---|
30+
| `--cwd` || `.` | path |
31+
| `--dry-run` | `-d` | `false` | bool |
32+
| `--silent` | `-s` | `false` | bool |
33+
| `--manifest-path` | `-m` | `.socket/manifest.json` | string |
34+
| `--offline` || `false` | bool |
35+
| `--global` | `-g` | `false` | bool |
36+
| `--global-prefix` || (none) | path |
37+
| `--ecosystems` || (none) | CSV → `Vec<String>` |
38+
| `--force` | `-f` | `false` | bool |
39+
| `--json` || `false` | bool |
40+
| `--verbose` | `-v` | `false` | bool |
41+
| `--download-mode` || **`diff`** | string |
42+
43+
### `rollback`
44+
45+
Same as `apply` plus: `--one-off` (bool), `--org` (string), `--api-url` (string), `--api-token` (string). Positional `identifier` is **optional** (omit to rollback everything).
46+
47+
### `get`
48+
49+
Required positional `identifier`. Flags:
50+
51+
| Long | Short | Alias | Default | Type |
52+
|---|---|---|---|---|
53+
| `--org` ||| (none) | string |
54+
| `--cwd` ||| `.` | path |
55+
| `--id` ||| `false` | bool |
56+
| `--cve` ||| `false` | bool |
57+
| `--ghsa` ||| `false` | bool |
58+
| `--package` | `-p` || `false` | bool |
59+
| `--yes` | `-y` || `false` | bool |
60+
| `--api-url` ||| (none) | string |
61+
| `--api-token` ||| (none) | string |
62+
| `--save-only` || **`--no-apply`** | `false` | bool |
63+
| `--global` | `-g` || `false` | bool |
64+
| `--global-prefix` ||| (none) | path |
65+
| `--one-off` ||| `false` | bool |
66+
| `--json` ||| `false` | bool |
67+
| `--download-mode` ||| **`diff`** | string |
68+
69+
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.
70+
71+
### `scan`
72+
73+
| Long | Short | Default | Type |
74+
|---|---|---|---|
75+
| `--cwd` || `.` | path |
76+
| `--org` || (none) | string |
77+
| `--json` || `false` | bool |
78+
| `--yes` | `-y` | `false` | bool |
79+
| `--global` | `-g` | `false` | bool |
80+
| `--global-prefix` || (none) | path |
81+
| `--batch-size` || **`100`** | usize |
82+
| `--api-url` || (none) | string |
83+
| `--api-token` || (none) | string |
84+
| `--ecosystems` || (none) | CSV → `Vec<String>` |
85+
| `--download-mode` || **`diff`** | string |
86+
87+
### `list`
88+
89+
| Long | Short | Default | Type |
90+
|---|---|---|---|
91+
| `--cwd` || `.` | path |
92+
| `--manifest-path` | `-m` | `.socket/manifest.json` | string |
93+
| `--json` || `false` | bool |
94+
95+
### `remove`
96+
97+
Required positional `identifier`. Flags:
98+
99+
| Long | Short | Default | Type |
100+
|---|---|---|---|
101+
| `--cwd` || `.` | path |
102+
| `--manifest-path` | `-m` | `.socket/manifest.json` | string |
103+
| `--skip-rollback` || `false` | bool |
104+
| `--yes` | `-y` | `false` | bool |
105+
| `--global` | `-g` | `false` | bool |
106+
| `--global-prefix` || (none) | path |
107+
| `--json` || `false` | bool |
108+
109+
### `setup`
110+
111+
| Long | Short | Default | Type |
112+
|---|---|---|---|
113+
| `--cwd` || `.` | path |
114+
| `--dry-run` | `-d` | `false` | bool |
115+
| `--yes` | `-y` | `false` | bool |
116+
| `--json` || `false` | bool |
117+
118+
### `repair`
119+
120+
| Long | Short | Default | Type |
121+
|---|---|---|---|
122+
| `--cwd` || `.` | path |
123+
| `--manifest-path` | `-m` | `.socket/manifest.json` | string |
124+
| `--dry-run` | `-d` | `false` | bool |
125+
| `--offline` || `false` | bool |
126+
| `--download-only` || `false` | bool |
127+
| `--json` || `false` | bool |
128+
| `--download-mode` || **`file`** | string |
129+
130+
**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.
131+
132+
## CSV value parsing
133+
134+
`--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.
135+
136+
## JSON output shapes
137+
138+
When `--json` is set, commands print a single JSON object to stdout. The schemas below are stable.
139+
140+
### Missing-manifest error (`apply`/`list`/`remove`/`repair`/`rollback`)
141+
142+
```json
143+
{
144+
"status": "error",
145+
"error": "Manifest not found",
146+
"path": "<absolute path that was looked up>"
147+
}
148+
```
149+
150+
### Invalid-manifest error
151+
152+
```json
153+
{ "status": "error", "error": "Invalid manifest" }
154+
```
155+
156+
### Generic error
157+
158+
```json
159+
{ "status": "error", "error": "<message>" }
160+
```
161+
162+
### `list` success — empty manifest
163+
164+
```json
165+
{ "status": "success", "patches": [] }
166+
```
167+
168+
### `list` success — populated
169+
170+
```json
171+
{
172+
"status": "success",
173+
"patches": [
174+
{
175+
"purl": "pkg:npm/foo@1.2.3",
176+
"uuid": "",
177+
"exportedAt": "",
178+
"tier": "free|paid",
179+
"license": "",
180+
"description": "",
181+
"files": [""],
182+
"vulnerabilities": [
183+
{ "id": "", "cves": [""], "summary": "", "severity": "", "description": "" }
184+
]
185+
}
186+
]
187+
}
188+
```
189+
190+
### `setup` — no package.json files found
191+
192+
```json
193+
{
194+
"status": "no_files",
195+
"updated": 0,
196+
"alreadyConfigured": 0,
197+
"errors": 0,
198+
"files": []
199+
}
200+
```
201+
202+
### `get` — multiple-patch selection required (JSON mode)
203+
204+
```json
205+
{
206+
"status": "selection_required",
207+
"error": "Multiple patches available for <purl>. Specify --id <UUID> to select one.",
208+
"purl": "<purl>",
209+
"options": [
210+
{ "uuid": "", "tier": "", "published_at": "", "description": "", "vulnerabilities": [ ] }
211+
]
212+
}
213+
```
214+
215+
## Exit codes
216+
217+
| Code | Meaning |
218+
|---|---|
219+
| `0` | Success |
220+
| `1` | Error (missing/invalid manifest, fetch failed, apply failed, selection cancelled in non-JSON mode, etc.) |
221+
222+
`list` returns **`0`** for an empty manifest and **`1`** for a missing manifest — these are distinct and load-bearing.
223+
224+
## Semver policy
225+
226+
Versioning lives in **`Cargo.toml`** at the workspace root (`version = "..."`) and is propagated to npm, pypi, and cargo wrappers by **`scripts/version-sync.sh <new-version>`**.
227+
228+
| Change | Bump |
229+
|---|---|
230+
| Rename or remove a subcommand | **MAJOR** |
231+
| Rename or remove a visible alias (`download`, `gc`) | **MAJOR** |
232+
| Rename or remove a hidden alias (`--no-apply`) | **MAJOR** |
233+
| Rename, remove, or change short form of a flag (`-d`, `-m`, etc.) | **MAJOR** |
234+
| Change a default value (`--download-mode`, `--batch-size`, `--manifest-path`, …) | **MAJOR** |
235+
| Change an exit code's meaning or add a new non-zero code with different semantics | **MAJOR** |
236+
| Rename a JSON output key or change a `status` string | **MAJOR** |
237+
| Remove a JSON output key | **MAJOR** |
238+
| Drop the bare-UUID fallback | **MAJOR** |
239+
| Add a *required* new flag | **MAJOR** |
240+
| Add a new subcommand | **MINOR** |
241+
| Add a new optional flag | **MINOR** |
242+
| Add a new optional JSON output key (additive) | **MINOR** |
243+
| Add a new visible alias to an existing subcommand | **MINOR** |
244+
| Fix a bug without changing any of the above | **PATCH** |
245+
246+
After bumping `Cargo.toml`, run:
247+
248+
```bash
249+
scripts/version-sync.sh <new-version>
250+
```
251+
252+
This syncs the workspace package version into:
253+
254+
- `npm/socket-patch/package.json` (and its `optionalDependencies`)
255+
- every per-platform `npm/socket-patch-*/package.json`
256+
- `pypi/socket-patch/pyproject.toml`
257+
258+
## How the contract is enforced
259+
260+
Every item in this document is locked in by at least one of:
261+
262+
- **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(...)`.
263+
- **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.
264+
- **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::<Value>` + per-key assertions.
265+
266+
If you add a new flag/subcommand/JSON key, add a test here that locks the new surface in the same PR.

crates/socket-patch-cli/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ license.workspace = true
77
repository.workspace = true
88
readme = "README.md"
99

10+
[lib]
11+
name = "socket_patch_cli"
12+
path = "src/lib.rs"
13+
1014
[[bin]]
1115
name = "socket-patch"
1216
path = "src/main.rs"

0 commit comments

Comments
 (0)