diff --git a/CHANGELOG.md b/CHANGELOG.md index cb96f62..3eb5554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,3 +31,11 @@ so the two surfaces expose identical operations with identical parameters. - `page`/`count` values below 1 are clamped to 1 (`count` is capped at 100). - MCP calls using the legacy arg shapes fail with a migration hint naming this change. +- **Issues can be ignored and unignored** — the first issue write surface, + as new top-level verbs on both surfaces. CLI: `ignore issue + --category [--notes ] [--reason ]` and `unignore issue + --category `; MCP: new `ignore` and `unignore` tools. Ignoring an + already-ignored issue is refused with a prompt to unignore first (see ADR + 0002), and `--reason` is accepted on vulnerability issues only (FOSSA + never displays reasons elsewhere). Requires a full API token (push-only + tokens cannot write issues). diff --git a/CLAUDE.md b/CLAUDE.md index c2c49cb..688fa3d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,6 +123,7 @@ Project (top-level container) | Dependencies | `GET /v2/revisions/{locator}/dependencies` | For a revision | | Issues | `GET /v2/issues` | `category` **required**; `count` clamps to a minimum of 5 | | Issue | `GET /v2/issues/{id}` | Single issue with full details | +| Issue actions | `PUT /v2/issues/` | Ignore/unignore. Targets in the query (`category` required, `ids[]`), action in the body (`{type, notes?, reason?}`). By ID the server is an **unguarded upsert**: the `status` query param is silently ignored, and re-ignoring overwrites notes/reason and resets the ignored-at timestamp — which is why we guard client-side (see ADR 0002). Responds `{count, issueId?}` (`issueId` only when count==1); `count: 0` = target not visible to the token. Full token only — push-only tokens can't write issues; missing resolve permission is a **400**, not 403 | | Snippets | `GET /revisions/{locator}/snippets` | Paginated; `pageSize` capped at 50 (`list_all` overrides) | | Snippet paths | `GET /revisions/{locator}/snippets/paths` | File/dir tree, drill in via `path` | | Snippet details | `GET /revisions/{locator}/snippets/{id}` | Single snippet + its per-file matches | @@ -161,7 +162,7 @@ advertise them is issue #41. - **Project** - Top-level container, implements Get/List/Update - **Revision** - Snapshot at point in time, implements Get/List - **Dependency** - Package dependency, implements List only (via revision) -- **Issue** - Vulnerability/licensing/quality issue, implements Get/List. `Issue` has no `deny_unknown_fields`, so any API key not declared on the struct is silently dropped — when the API grows a field, add it here or callers never see it. `cpes` is deliberately unmodeled (empty on all 80 sampled issues); `patchedVersionRanges` is modeled but rarely populated (1/80) — prefer `remediation` for upgrade targets. `IssueProject` entries carry the revision the issue was found in (`revision_id`, `latest`, `first_found_at`), not just the project. +- **Issue** - Vulnerability/licensing/quality issue, implements Get/List/Update. Update = ignore/unignore via `IssueUpdateParams` (`IssueAction::Ignore { notes, reason }` / `Unignore`), surfaced as the top-level `ignore`/`unignore` verbs on both surfaces; it pre-fetches the issue and refuses actions whose target state already fully holds ("unignore it first" — see ADR 0002; partial org-wide states accept both actions, like the UI's global view), then re-fetches and returns the refreshed issue. `reason` is refused outside the vulnerability category (the server stores it for any category, but nothing ever displays it for licensing/quality — write-only noise). `IssueIgnoreReason` is an enum because the API resolves the reason string against its `ResolutionReasons` table and silently stores NULL on a mismatch. `Issue` has no `deny_unknown_fields`, so any API key not declared on the struct is silently dropped — when the API grows a field, add it here or callers never see it. `cpes` is deliberately unmodeled (empty on all 80 sampled issues); `patchedVersionRanges` is modeled but rarely populated (1/80) — prefer `remediation` for upgrade targets. `IssueProject` entries carry the revision the issue was found in (`revision_id`, `latest`, `first_found_at`), not just the project. - **Snippet** - Third-party (OSS) code matched into first-party files, implements List only (via revision). Read-only; reached through the `get_snippet_*` convenience functions. Quirks: `id` is a string, `matchDetails.matchPercentage` is 0-100 (other percentages are 0-1), and whole-file matches highlight a trailing blank EOF line that is excluded from the reported range. - **LicenseInfo** - Can be simple string ("MIT") or full object @@ -182,6 +183,9 @@ All three categories also carry `url` (deep link into the FOSSA UI). - **IssueScan** - Issue scans tied to revisions (not yet implemented) - **Snippet reject/unreject** - Mutating a snippet's rejection status (out of scope for v1) - **Cross-revision snippet compare** - Diffing snippet matches across revisions (out of scope for v1) +- **Issue exceptions** - `PUT /v2/issues/` also accepts `type: issueException` (org/policy-wide ignores, expirations, package labels; premium-gated) and there are `PUT`/`DELETE /v2/issues/exceptions` endpoints — the single-issue ignore/unignore we model is the common case; exceptions are unmodeled +- **Bulk issue actions** - the API natively batches (`ids[]` array, or filter-wide when `ids` is omitted — the filter-wide form mass-ignores everything matching, so exposing it needs care); we deliberately send one ID per call and guard each with a pre-flight fetch +- **Editing an ignore's notes** - deliberately requires unignore-then-re-ignore (matching the UI); the server's raw re-ignore overwrite is reachable only by hand-rolled API calls ## Nudge diff --git a/CONTEXT.md b/CONTEXT.md index dd4e795..6cf8749 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -11,10 +11,10 @@ and exposed identically by both surfaces. _Avoid_: endpoint, command **Verb**: -One of `get`, `list`, `update` — the top-level grouping of operations. Each -verb is one shared enum and one MCP tool. (In code the enums are spelled -`GetCommand`/`ListCommand`/`UpdateCommand`; "command" in a type name means -verb, not operation.) +One of `get`, `list`, `update`, `ignore`, `unignore` — the top-level grouping +of operations. Each verb is one shared enum and one MCP tool. (In code the +enums are spelled `GetCommand`/`ListCommand`/etc.; "command" in a type name +means verb, not operation.) _Avoid_: action, method **Entity**: @@ -40,3 +40,54 @@ same parameters. The defaults and global bounds applied to `page`/`count` before an operation runs. One policy for all operations; individual FOSSA endpoints may impose their own tighter bounds. + +### Issues + +**Issue**: +A detected problem in a dependency — a vulnerability, licensing conflict, or +quality concern. Always scoped to exactly one Category and identified by a +numeric ID, not a locator. +_Avoid_: alert, finding + +**Category**: +Which of the three issue kinds an issue belongs to: vulnerability, licensing, +or quality. Every issue read and write requires one. +_Avoid_: type, kind + +**Ignore**: +The only issue status transition: an active issue becomes ignored, optionally +carrying Notes and a Reason. There is no "resolved" status — when people say +"resolve an issue" they mean ignore it. +_Avoid_: resolve, suppress, dismiss, mute + +**Unignore**: +Reverting an Ignore, returning the issue to active. +_Avoid_: reopen, reactivate + +**Notes**: +Free text attached to an Ignore explaining it (e.g. "false positive patch"). +Not a Comment. +_Avoid_: comment, message + +**Reason**: +One of a closed set of structured explanations attached to a vulnerability +Ignore (Fixed, Vulnerable code not in execute path, Other, …). Vulnerability +issues only — nothing in FOSSA ever displays a reason for licensing or +quality ignores. +_Avoid_: justification, cause + +**Issue exception**: +An org- or policy-wide ignore that can expire, distinct from ignoring one +issue. Exists in FOSSA but is not modeled in fossapi. + +### Adjacent FOSSA concepts (not issue ignores) + +**Comment**: +A separate FOSSA feature: discussion threads attached to a package, org-wide +across versions. Unrelated to an Ignore's Notes. Not modeled in fossapi. + +**Package ignore**: +A separate FOSSA feature: hiding a dependency from the inventory entirely +("Ignore package" in the UI). Not an issue status change. Not modeled in +fossapi. +_Avoid_: conflating with Ignore diff --git a/README.md b/README.md index 235e5de..36b957f 100644 --- a/README.md +++ b/README.md @@ -80,8 +80,29 @@ fossapi get issue 12345 # Skip the search when you know the category fossapi get issue 12345 --category licensing + +# Ignore an issue with a comment +fossapi ignore issue 12345 --category licensing --notes "false positive patch" + +# Revert the ignore +fossapi unignore issue 12345 --category licensing ``` +Ignoring supports an optional `--notes` free-text comment and, for +**vulnerability issues only**, a structured `--reason` (one of `fixed`, +`under-investigation`, `incorrect-data-found`, `component-not-present`, +`vulnerable-code-not-present`, `vulnerable-code-not-in-execute-path`, +`vulnerable-code-cannot-be-controlled-by-adversary`, +`inline-mitigations-already-exist`, `other`) — FOSSA never displays reasons +for licensing or quality ignores, so fossapi rejects them there. Issue writes +require a full API token; push-only tokens can only read. + +Ignoring an issue that is already fully ignored fails with a prompt to +unignore it first — changing an existing ignore's notes is a deliberate +two-step (`unignore`, then `ignore --notes ...`), matching the web UI. An +issue ignored in some projects but active in others accepts both actions, +which then apply org-wide. + ### Snippets Snippet scanning finds third-party (open-source) code copied into your @@ -163,9 +184,13 @@ input schemas are generated from the same declarations the CLI parses into). | `get` | `project`, `revision`, `issue` (category optional — omitted probes all three), `snippet`, `snippet_match` | | `list` | `projects`, `issues` (category required), `dependencies`, `revisions`, `snippets`, `snippet_locations`, `snippet_paths` | | `update` | `project` (title, description, url, public, policy_id, default_branch) | +| `ignore` | `issue` (category required; optional notes, and reason on vulnerabilities) | +| `unignore` | `issue` (category required) | For example, `fossapi get issue 12345 --category licensing` is -`get {"entity": "issue", "id": 12345, "category": "licensing"}` over MCP. +`get {"entity": "issue", "id": 12345, "category": "licensing"}` over MCP, and +`fossapi ignore issue 12345 --category licensing --notes "false positive patch"` +is `ignore {"entity": "issue", "id": 12345, "category": "licensing", "notes": "false positive patch"}`. > **Snippets over MCP:** use `list` with `entity: snippet_locations` and > `revision: ` (optional `path` and `with_lines`) to map diff --git a/docs/adr/0002-client-side-issue-write-guard.md b/docs/adr/0002-client-side-issue-write-guard.md new file mode 100644 index 0000000..b38ad56 --- /dev/null +++ b/docs/adr/0002-client-side-issue-write-guard.md @@ -0,0 +1,31 @@ +# Issue writes add a client-side state guard the server doesn't have + +FOSSA's `PUT /v2/issues/` is an unguarded upsert when targeting by ID: it +silently ignores the `status` query filter, and re-ignoring an already-ignored +issue overwrites its notes/reason and resets its ignored-at timestamp (the +server's `ON CONFLICT DO UPDATE` on `IssueResolutions`). We decided fossapi's +`ignore issue` pre-fetches the issue and **refuses** an action whose target +state already fully holds ("already ignored; unignore it first"), mirroring +the web UI, which only ever offers Ignore on active issues and Unignore on +ignored ones. Partially ignored issues (org-wide rollup: ignored in some +projects, active in others) accept both actions, like the UI's global issue +view. + +## Considered Options + +Mirroring the server (letting re-ignore silently overwrite) would have given +one-step "edit the notes" — the raw API is in fact the only way to do that in +one step — but an agent retrying or mis-aiming an ignore would clobber a +human's hand-written justification without any signal. We chose the guard and +made editing notes a deliberate two-step: unignore, then re-ignore with the +new notes. + +## Consequences + +- Every issue write costs a pre-flight GET (and a post-write refresh GET); + there is a benign race window between fetch and write. +- `count: 0` from the server, after the pre-flight has ruled out a wrong ID or + category, means only "not visible to this token". +- The same UI-mirroring stance also rejects a `reason` on non-vulnerability + ignores: the server stores one for any category, but only vulnerability + ignores ever display it (UI, SBOM/VEX) — elsewhere it is write-only. diff --git a/src/bin/fossapi.rs b/src/bin/fossapi.rs index 0cf2a6f..a8335b8 100644 --- a/src/bin/fossapi.rs +++ b/src/bin/fossapi.rs @@ -7,7 +7,7 @@ use clap::Parser; use fossapi::cli::{Cli, Command}; -use fossapi::ops::{run_get, run_list, run_update, ListOutput}; +use fossapi::ops::{run_get, run_ignore, run_list, run_unignore, run_update, ListOutput}; use fossapi::{FossaClient, Page, PrettyPrint, Project, Snippet, SnippetLocation, SnippetPath}; use serde::Serialize; use std::process::ExitCode; @@ -49,6 +49,14 @@ async fn run(client: &FossaClient, cli: Cli) -> fossapi::Result<()> { let output = run_update(client, command).await?; output_single(&output, cli.json) } + Command::Ignore { command } => { + let output = run_ignore(client, command).await?; + output_single(&output, cli.json) + } + Command::Unignore { command } => { + let output = run_unignore(client, command).await?; + output_single(&output, cli.json) + } Command::Mcp { verbose } => handle_mcp(client, verbose).await, } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 20a72cd..6b3e683 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -7,7 +7,7 @@ use clap::{Parser, Subcommand}; -pub use crate::ops::{GetCommand, ListCommand, UpdateCommand}; +pub use crate::ops::{GetCommand, IgnoreCommand, ListCommand, UnignoreCommand, UpdateCommand}; /// FOSSA API command-line interface. #[derive(Parser, Debug)] @@ -42,6 +42,18 @@ pub enum Command { command: UpdateCommand, }, + /// Ignore an issue (with an optional comment and reason). + Ignore { + #[command(subcommand)] + command: IgnoreCommand, + }, + + /// Revert a previous ignore, returning the issue to active. + Unignore { + #[command(subcommand)] + command: UnignoreCommand, + }, + /// Run the MCP server on stdio. Mcp { /// Enable verbose (debug) logging. diff --git a/src/lib.rs b/src/lib.rs index 93faae9..9bee59b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -85,9 +85,12 @@ pub use models::{ DependencyQuery, // Issue types Issue, + IssueAction, + IssueActionResponse, IssueCategory, IssueDepths, IssueEpss, + IssueIgnoreReason, IssueListQuery, IssueMetric, IssueProject, @@ -96,6 +99,7 @@ pub use models::{ IssueStatus, IssueStatuses, IssueType, + IssueUpdateParams, // Project types LatestRevision, LicenseInfo, diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 2ec036b..f162209 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -15,7 +15,10 @@ use serde::Serialize; use std::sync::Arc; use crate::{ - ops::{run_get, run_list, run_update, GetCommand, ListCommand, UpdateCommand}, + ops::{ + run_get, run_ignore, run_list, run_unignore, run_update, GetCommand, IgnoreCommand, + ListCommand, UnignoreCommand, UpdateCommand, + }, FossaClient, FossaError, }; @@ -26,8 +29,8 @@ use crate::{ /// /// # Tools /// -/// The server exposes one tool per CLI verb — `get`, `list`, and `update` — -/// and each tool's input schema is generated from the same +/// The server exposes one tool per CLI verb — `get`, `list`, `update`, +/// `ignore`, and `unignore` — and each tool's input schema is generated from the same /// [`crate::ops`] enum that the CLI parses into, so the two surfaces /// always expose the same operations with the same parameters. /// @@ -100,6 +103,23 @@ impl FossaServer { url, public, policy_id, default_branch).", Self::schema::(), ), + Tool::new( + "ignore", + "Ignore a FOSSA issue, optionally recording notes (a free-text \ + comment) and, for vulnerability issues only, a structured reason. \ + entity values: issue (numeric id; category required). Refused if \ + the issue is already fully ignored — unignore it first to change \ + its notes or reason. Requires a full API token (push-only tokens \ + cannot write issues).", + Self::schema::(), + ), + Tool::new( + "unignore", + "Revert a previous ignore, returning a FOSSA issue to active. \ + entity values: issue (numeric id; category required). Refused if \ + nothing on the issue is ignored. Requires a full API token.", + Self::schema::(), + ), ] } @@ -175,6 +195,25 @@ impl FossaServer { .map_err(Self::to_mcp_error)?; Self::to_result(&output) } + + /// Handle the `ignore` tool. + pub async fn handle_ignore(&self, command: IgnoreCommand) -> Result { + let output = run_ignore(&self.client, command) + .await + .map_err(Self::to_mcp_error)?; + Self::to_result(&output) + } + + /// Handle the `unignore` tool. + pub async fn handle_unignore( + &self, + command: UnignoreCommand, + ) -> Result { + let output = run_unignore(&self.client, command) + .await + .map_err(Self::to_mcp_error)?; + Self::to_result(&output) + } } impl ServerHandler for FossaServer { @@ -241,6 +280,16 @@ impl ServerHandler for FossaServer { .map_err(|e| Self::describe_args_error(e, &args))?; self.handle_update(command).await } + "ignore" => { + let command: IgnoreCommand = serde_json::from_value(args.clone()) + .map_err(|e| Self::describe_args_error(e, &args))?; + self.handle_ignore(command).await + } + "unignore" => { + let command: UnignoreCommand = serde_json::from_value(args.clone()) + .map_err(|e| Self::describe_args_error(e, &args))?; + self.handle_unignore(command).await + } other => Err(McpError::invalid_params( format!("Unknown tool: {other}"), None, @@ -254,8 +303,9 @@ mod tests { use super::*; use crate::ops::{ GetIssueParams, GetProjectParams, GetRevisionParams, GetSnippetMatchParams, - GetSnippetParams, ListDependenciesParams, ListProjectsParams, ListRevisionsParams, - ListSnippetLocationsParams, ListSnippetsParams, PageArgs, UpdateProjectParams, + GetSnippetParams, IgnoreIssueParams, ListDependenciesParams, ListProjectsParams, + ListRevisionsParams, ListSnippetLocationsParams, ListSnippetsParams, PageArgs, + UnignoreIssueParams, UpdateProjectParams, }; use crate::IssueCategory; use wiremock::matchers::{method, path, path_regex, query_param}; @@ -269,12 +319,12 @@ mod tests { } #[test] - fn tools_are_get_list_update() { + fn tools_are_the_cli_verbs() { let names: Vec<_> = FossaServer::tools() .iter() .map(|t| t.name.to_string()) .collect(); - assert_eq!(names, ["get", "list", "update"]); + assert_eq!(names, ["get", "list", "update", "ignore", "unignore"]); } #[test] @@ -282,6 +332,8 @@ mod tests { assert!(!FossaServer::schema::().is_empty()); assert!(!FossaServer::schema::().is_empty()); assert!(!FossaServer::schema::().is_empty()); + assert!(!FossaServer::schema::().is_empty()); + assert!(!FossaServer::schema::().is_empty()); } #[test] @@ -927,4 +979,212 @@ mod tests { assert!(text.contains("detectedCode") || text.contains("detected_code")); assert!(text.contains("42")); } + + // ========================================================================= + // ignore / unignore handler tests + // ========================================================================= + + fn issue_body(id: u64, active: u32, ignored: u32) -> serde_json::Value { + serde_json::json!({ + "id": id, + "type": "licensing", + "source": {"id": "npm+leftpad$1.0.0"}, + "statuses": {"active": active, "ignored": ignored}, + "license": "GPL-3.0" + }) + } + + fn ignore_licensing_issue() -> IgnoreCommand { + IgnoreCommand::Issue(IgnoreIssueParams { + id: 987654, + category: IssueCategory::Licensing, + notes: Some("false positive patch".to_string()), + reason: None, + }) + } + + /// Test: ignore fetches the issue, sends the action, and returns the + /// refreshed issue. + #[tokio::test] + async fn handle_ignore_issue_succeeds() { + use wiremock::matchers::body_json; + + let mock_server = MockServer::start().await; + + // Pre-flight guard fetch (active) and post-write refresh (ignored). + Mock::given(method("GET")) + .and(path("/v2/issues/987654")) + .and(query_param("category", "licensing")) + .respond_with(ResponseTemplate::new(200).set_body_json(issue_body(987654, 1, 0))) + .up_to_n_times(1) + .expect(1) + .mount(&mock_server) + .await; + + Mock::given(method("PUT")) + .and(path("/v2/issues/")) + .and(query_param("category", "licensing")) + .and(query_param("ids[]", "987654")) + .and(body_json(serde_json::json!({ + "type": "ignore", + "notes": "false positive patch" + }))) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"count": 1, "issueId": 987654})), + ) + .expect(1) + .mount(&mock_server) + .await; + + Mock::given(method("GET")) + .and(path("/v2/issues/987654")) + .and(query_param("category", "licensing")) + .respond_with(ResponseTemplate::new(200).set_body_json(issue_body(987654, 0, 1))) + .expect(1) + .mount(&mock_server) + .await; + + let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); + let server = FossaServer::new(client); + + let result = server + .handle_ignore(ignore_licensing_issue()) + .await + .unwrap(); + assert!(!result.is_error.unwrap_or(false)); + let text = response_text(&result); + assert!(text.contains("987654")); + assert!(text.contains("\"ignored\": 1")); + } + + /// Test: ignoring an already-ignored issue is refused before any PUT. + #[tokio::test] + async fn handle_ignore_already_ignored_is_rejected_without_put() { + let mock_server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/v2/issues/987654")) + .respond_with(ResponseTemplate::new(200).set_body_json(issue_body(987654, 0, 1))) + .expect(1) + .mount(&mock_server) + .await; + + Mock::given(method("PUT")) + .and(path("/v2/issues/")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&mock_server) + .await; + + let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); + let server = FossaServer::new(client); + + let err = server + .handle_ignore(ignore_licensing_issue()) + .await + .unwrap_err(); + assert!(err.message.contains("unignore it first"), "{err:?}"); + } + + /// Test: unignoring an issue with nothing ignored is refused before any PUT. + #[tokio::test] + async fn handle_unignore_active_is_rejected_without_put() { + let mock_server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/v2/issues/987654")) + .respond_with(ResponseTemplate::new(200).set_body_json(issue_body(987654, 1, 0))) + .expect(1) + .mount(&mock_server) + .await; + + Mock::given(method("PUT")) + .and(path("/v2/issues/")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&mock_server) + .await; + + let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); + let server = FossaServer::new(client); + + let err = server + .handle_unignore(UnignoreCommand::Issue(UnignoreIssueParams { + id: 987654, + category: IssueCategory::Licensing, + })) + .await + .unwrap_err(); + assert!(err.message.contains("nothing to unignore"), "{err:?}"); + } + + /// Test: a partially ignored issue (org-wide rollup) accepts both actions, + /// like the UI's global issue view. + #[tokio::test] + async fn handle_ignore_partial_state_is_allowed() { + use wiremock::matchers::body_json; + + let mock_server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/v2/issues/987654")) + .respond_with(ResponseTemplate::new(200).set_body_json(issue_body(987654, 2, 1))) + .up_to_n_times(1) + .expect(1) + .mount(&mock_server) + .await; + + Mock::given(method("PUT")) + .and(path("/v2/issues/")) + .and(body_json(serde_json::json!({ + "type": "ignore", + "notes": "false positive patch" + }))) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"count": 2, "issueId": null})), + ) + .expect(1) + .mount(&mock_server) + .await; + + Mock::given(method("GET")) + .and(path("/v2/issues/987654")) + .respond_with(ResponseTemplate::new(200).set_body_json(issue_body(987654, 0, 3))) + .expect(1) + .mount(&mock_server) + .await; + + let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); + let server = FossaServer::new(client); + + let result = server + .handle_ignore(ignore_licensing_issue()) + .await + .unwrap(); + assert!(!result.is_error.unwrap_or(false)); + } + + /// Test: a reason on a non-vulnerability ignore is refused before any + /// HTTP call — the server would store it, but nothing ever displays it. + #[tokio::test] + async fn handle_ignore_reason_on_licensing_is_rejected() { + let client = FossaClient::new("test-token", "http://localhost:9999").unwrap(); + let server = FossaServer::new(client); + + let err = server + .handle_ignore(IgnoreCommand::Issue(IgnoreIssueParams { + id: 987654, + category: IssueCategory::Licensing, + notes: None, + reason: Some(crate::IssueIgnoreReason::Other), + })) + .await + .unwrap_err(); + assert!( + err.message.contains("only apply to vulnerability ignores"), + "{err:?}" + ); + } } diff --git a/src/mock_server/CLAUDE.md b/src/mock_server/CLAUDE.md index 494c5bd..9967f4c 100644 --- a/src/mock_server/CLAUDE.md +++ b/src/mock_server/CLAUDE.md @@ -19,7 +19,7 @@ mock_server/ ├── projects.rs # GET/PUT /projects/:locator, GET /v2/projects ├── revisions.rs # GET /revisions/:locator, GET /projects/:locator/revisions ├── dependencies.rs # GET /v2/revisions/:locator/dependencies - └── issues.rs # GET /v2/issues/:id, GET /v2/issues + └── issues.rs # GET /v2/issues/:id, GET /v2/issues, PUT /v2/issues/ ``` ## Usage @@ -57,6 +57,7 @@ async fn test_workflow() { | `/v2/revisions/:locator/dependencies` | GET | list_dependencies | | `/v2/issues/:id` | GET | get_issue | | `/v2/issues` | GET | list_issues | +| `/v2/issues/` | PUT | update_issues (ignore/unignore; trailing slash matches the real API) | | `/health` | GET | health_check | ## Running Tests diff --git a/src/mock_server/handlers/issues.rs b/src/mock_server/handlers/issues.rs index 28330d5..ca9c605 100644 --- a/src/mock_server/handlers/issues.rs +++ b/src/mock_server/handlers/issues.rs @@ -121,3 +121,77 @@ pub async fn list_issues( (StatusCode::OK, Json(ListIssuesResponse { issues })).into_response() } + +/// PUT /v2/issues/ +/// +/// Mirrors the real API's by-ID semantics: targets come from the query string +/// (`category` required, `ids[]`), the action from the JSON body +/// (`{"type": "ignore", "notes": ..., "reason": ...}` or +/// `{"type": "unignore"}`). Like core, the by-ID path is an unguarded upsert: +/// the `status` query param is ignored, re-ignoring an ignored issue succeeds +/// (server-side it overwrites notes/reason), and unignoring an active issue +/// succeeds as a rewrite. Responds `{count, issueId?}` — `issueId` only when +/// exactly one issue matched; `count: 0` only when no target was found. +pub async fn update_issues( + State(state): State>>, + Query(params): Query>, + Json(body): Json, +) -> impl IntoResponse { + let category = params + .iter() + .find(|(k, _)| k == "category") + .map(|(_, v)| v.clone()); + let Some(category) = category else { + return missing_category(); + }; + let ids: Vec = params + .iter() + .filter(|(k, _)| k == "ids[]" || k == "ids") + .filter_map(|(_, v)| v.parse().ok()) + .collect(); + + let ignore = match body.get("type").and_then(|t| t.as_str()) { + Some("ignore") => true, + Some("unignore") => false, + _ => { + return ( + StatusCode::BAD_REQUEST, + Json(serde_json::json!({ + "error": "Validation error", + "message": "Invalid issue action" + })), + ) + .into_response() + } + }; + + let mut state = state.write().await; + + let mut count: u64 = 0; + let mut last_issue_id = None; + for id in ids { + let Some(issue) = state.issues.get_mut(&id) else { + continue; + }; + if issue.issue_type != category { + continue; + } + let total = issue.statuses.active + issue.statuses.ignored; + if ignore { + issue.statuses.active = 0; + issue.statuses.ignored = total; + } else { + issue.statuses.active = total; + issue.statuses.ignored = 0; + } + count += 1; + last_issue_id = Some(id); + } + + let response = if count == 1 { + serde_json::json!({"count": 1, "issueId": last_issue_id}) + } else { + serde_json::json!({"count": count}) + }; + (StatusCode::OK, Json(response)).into_response() +} diff --git a/src/mock_server/server.rs b/src/mock_server/server.rs index 305e4a3..927ee97 100644 --- a/src/mock_server/server.rs +++ b/src/mock_server/server.rs @@ -139,6 +139,8 @@ impl MockServer { // Issue routes .route("/v2/issues/:id", get(handlers::get_issue)) .route("/v2/issues", get(handlers::list_issues)) + // The real endpoint's path has a trailing slash: PUT /api/v2/issues/ + .route("/v2/issues/", put(handlers::update_issues)) // Health check .route("/health", get(health_check)) .with_state(state) diff --git a/src/models/issue.rs b/src/models/issue.rs index 679bf3f..a8a29e1 100644 --- a/src/models/issue.rs +++ b/src/models/issue.rs @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize}; use crate::client::FossaClient; use crate::error::{FossaError, Result}; use crate::pagination::Page; -use crate::traits::{Get, List}; +use crate::traits::{Get, List, Update}; // ============================================================================= // TESTS FIRST (TDD Red Phase) @@ -535,6 +535,103 @@ mod tests { IssueCategory::Quality )); } + + #[test] + fn test_issue_action_ignore_serializes_full() { + let action = IssueAction::Ignore { + notes: Some("false positive patch".to_string()), + reason: Some(IssueIgnoreReason::VulnerableCodeNotInExecutePath), + }; + let json = serde_json::to_value(&action).unwrap(); + assert_eq!( + json, + serde_json::json!({ + "type": "ignore", + "notes": "false positive patch", + "reason": "Vulnerable_code_not_in_execute_path" + }) + ); + } + + #[test] + fn test_issue_action_ignore_omits_empty_fields() { + let action = IssueAction::Ignore { + notes: None, + reason: None, + }; + let json = serde_json::to_value(&action).unwrap(); + assert_eq!(json, serde_json::json!({"type": "ignore"})); + } + + #[test] + fn test_issue_action_unignore_serializes() { + let json = serde_json::to_value(&IssueAction::Unignore).unwrap(); + assert_eq!(json, serde_json::json!({"type": "unignore"})); + } + + #[test] + fn test_issue_ignore_reason_api_strings() { + // The API matches these strings against its ResolutionReasons table; + // a mismatch silently records no reason, so pin every variant. + let cases = [ + (IssueIgnoreReason::Fixed, "Fixed"), + (IssueIgnoreReason::UnderInvestigation, "Under_investigation"), + ( + IssueIgnoreReason::IncorrectDataFound, + "incorrect_data_found", + ), + ( + IssueIgnoreReason::ComponentNotPresent, + "Component_not_present", + ), + ( + IssueIgnoreReason::VulnerableCodeNotPresent, + "Vulnerable_code_not_present", + ), + ( + IssueIgnoreReason::VulnerableCodeNotInExecutePath, + "Vulnerable_code_not_in_execute_path", + ), + ( + IssueIgnoreReason::VulnerableCodeCannotBeControlledByAdversary, + "Vulnerable_code_cannot_be_controlled_by_adversary", + ), + ( + IssueIgnoreReason::InlineMitigationsAlreadyExist, + "Inline_mitigations_already_exist", + ), + (IssueIgnoreReason::Other, "other"), + ]; + for (reason, expected) in cases { + assert_eq!( + serde_json::to_value(reason).unwrap(), + serde_json::json!(expected) + ); + } + } + + #[test] + fn test_issue_action_response_single() { + let json = r#"{"count": 1, "issueId": 987654}"#; + let resp: IssueActionResponse = serde_json::from_str(json).unwrap(); + assert_eq!(resp.count, 1); + assert_eq!(resp.issue_id, Some(987654)); + } + + #[test] + fn test_issue_action_response_batch_has_no_issue_id() { + let json = r#"{"count": 42}"#; + let resp: IssueActionResponse = serde_json::from_str(json).unwrap(); + assert_eq!(resp.count, 42); + assert_eq!(resp.issue_id, None); + } + + #[test] + fn test_issue_category_as_str() { + assert_eq!(IssueCategory::Vulnerability.as_str(), "vulnerability"); + assert_eq!(IssueCategory::Licensing.as_str(), "licensing"); + assert_eq!(IssueCategory::Quality.as_str(), "quality"); + } } // ============================================================================= @@ -926,6 +1023,15 @@ impl IssueCategory { IssueCategory::Licensing, IssueCategory::Quality, ]; + + /// The lowercase string the API uses for this category. + pub fn as_str(&self) -> &'static str { + match self { + IssueCategory::Vulnerability => "vulnerability", + IssueCategory::Licensing => "licensing", + IssueCategory::Quality => "quality", + } + } } /// Query parameters for listing issues. @@ -1104,3 +1210,172 @@ pub async fn get_project_issues( }; Issue::list_all(client, &query).await } + +/// Reason recorded when ignoring an issue. +/// +/// Serialized as the exact strings the API stores; anything else is silently +/// dropped server-side (the reason lookup returns NULL), which is why this is +/// an enum rather than a free-form string. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, ValueEnum)] +pub enum IssueIgnoreReason { + /// The vulnerability has been fixed. + #[serde(rename = "Fixed")] + Fixed, + /// Still being investigated. + #[serde(rename = "Under_investigation")] + UnderInvestigation, + /// The advisory data is incorrect. + #[serde(rename = "incorrect_data_found")] + IncorrectDataFound, + /// The affected component is not present. + #[serde(rename = "Component_not_present")] + ComponentNotPresent, + /// The vulnerable code is not present. + #[serde(rename = "Vulnerable_code_not_present")] + VulnerableCodeNotPresent, + /// The vulnerable code is never executed. + #[serde(rename = "Vulnerable_code_not_in_execute_path")] + VulnerableCodeNotInExecutePath, + /// The vulnerable code cannot be controlled by an adversary. + #[serde(rename = "Vulnerable_code_cannot_be_controlled_by_adversary")] + VulnerableCodeCannotBeControlledByAdversary, + /// Inline mitigations already exist. + #[serde(rename = "Inline_mitigations_already_exist")] + InlineMitigationsAlreadyExist, + /// Some other reason (use notes to explain). + #[serde(rename = "other")] + Other, +} + +/// The status-changing action sent in the body of `PUT /v2/issues/`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(tag = "type", rename_all = "lowercase")] +pub enum IssueAction { + /// Ignore the issue, optionally with a comment and a reason. + Ignore { + /// Free-text comment shown alongside the ignore in the FOSSA UI. + #[serde(skip_serializing_if = "Option::is_none")] + notes: Option, + /// Structured reason for the ignore. + #[serde(skip_serializing_if = "Option::is_none")] + reason: Option, + }, + /// Revert a previous ignore, returning the issue to active. + Unignore, +} + +/// Parameters for [`Issue::update`]. +#[derive(Debug, Clone)] +pub struct IssueUpdateParams { + /// The issue's category. The API requires it on every issue write. + pub category: IssueCategory, + /// The action to perform. + pub action: IssueAction, +} + +/// Response body of `PUT /v2/issues/`. +/// +/// `count` is the number of issue-project rows changed; `issue_id` is only +/// present when exactly one row changed. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueActionResponse { + /// Number of issue-project rows the action changed. + pub count: u64, + /// The affected issue ID, present only when `count == 1`. + #[serde(default)] + pub issue_id: Option, +} + +#[async_trait] +impl Update for Issue { + type Id = u64; + type Params = IssueUpdateParams; + + /// Ignore or un-ignore an issue, then return its refreshed state. + /// + /// The server's `PUT /v2/issues/` is an unguarded upsert when targeting by + /// ID: re-ignoring an already-ignored issue silently overwrites its notes + /// and reason and resets its ignored-at timestamp. To keep that + /// intentional, this first fetches the issue and refuses actions whose + /// target state already holds everywhere — mirroring the web UI, which + /// only offers Ignore on active issues and Unignore on ignored ones. To + /// change an existing ignore's notes or reason, unignore first, then + /// re-ignore. + /// + /// Statuses are an org-wide rollup, so a partially ignored issue (ignored + /// in one project, active in another) accepts both actions, like the UI's + /// global issue view; the server applies the action to every project. + /// + /// A `count` of 0 from the server means the issue wasn't visible to the + /// token (the pre-flight fetch already rules out a wrong ID or category). + /// + /// Requires a full API token: push-only tokens cannot write issues. + #[tracing::instrument(skip(client))] + async fn update(client: &FossaClient, id: u64, params: IssueUpdateParams) -> Result { + // The server stores a reason for any category, but only vulnerability + // ignores ever surface it (UI, SBOM/VEX); elsewhere it is write-only + // noise, so mirror the UI and refuse it. + if let IssueAction::Ignore { + reason: Some(_), .. + } = ¶ms.action + { + if params.category != IssueCategory::Vulnerability { + return Err(FossaError::InvalidParams( + "reasons only apply to vulnerability ignores; use notes for \ + licensing and quality issues" + .to_string(), + )); + } + } + + let current = Issue::get_with_category(client, id, params.category).await?; + match params.action { + IssueAction::Ignore { .. } if current.statuses.active == 0 => { + return Err(FossaError::InvalidParams(format!( + "issue {id} is already ignored; unignore it first to change \ + its notes or reason" + ))); + } + IssueAction::Unignore if current.statuses.ignored == 0 => { + return Err(FossaError::InvalidParams(format!( + "issue {id} is not ignored; there is nothing to unignore" + ))); + } + _ => {} + } + + let path = format!( + "v2/issues/?category={}&ids[]={id}", + params.category.as_str() + ); + + let response = client.put(&path, ¶ms.action).await?; + let result: IssueActionResponse = response.json().await.map_err(FossaError::HttpError)?; + + if result.count == 0 { + return Err(FossaError::ApiError { + message: format!( + "issue {id} was not modified; it may not be visible to this token" + ), + status_code: None, + }); + } + + // The write has already applied; a refresh failure here must not read + // as a failed write, or callers will retry an action that succeeded. + let verb = match params.action { + IssueAction::Ignore { .. } => "ignore", + IssueAction::Unignore => "unignore", + }; + Issue::get_with_category(client, id, params.category) + .await + .map_err(|e| FossaError::ApiError { + message: format!( + "the {verb} of issue {id} succeeded, but re-fetching the \ + issue failed: {e}" + ), + status_code: None, + }) + } +} diff --git a/src/ops/ignore.rs b/src/ops/ignore.rs new file mode 100644 index 0000000..c5fb648 --- /dev/null +++ b/src/ops/ignore.rs @@ -0,0 +1,129 @@ +//! The `ignore` and `unignore` verbs: issue status transitions. +//! +//! Ignore/unignore are first-class verbs (not `update` flags) because they +//! are the domain's own names for the actions, and separate MCP tools let +//! agent harnesses permission-gate writes independently of metadata updates. + +use clap::{Args, Subcommand}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::{ + FossaClient, Issue, IssueAction, IssueCategory, IssueIgnoreReason, IssueUpdateParams, + PrettyPrint, Result, Update, +}; + +/// Parameters for `ignore issue`. +#[derive(Args, Debug, Clone, PartialEq, Eq, Deserialize, JsonSchema)] +pub struct IgnoreIssueParams { + /// The issue ID. + pub id: u64, + + /// Issue category (required for writes; the API scopes them to one category). + #[arg(long, value_enum)] + pub category: IssueCategory, + + /// Free-text comment recorded with the ignore. + #[arg(long)] + pub notes: Option, + + /// Structured reason recorded with the ignore (vulnerability issues only). + #[arg(long, value_enum)] + pub reason: Option, +} + +/// Parameters for `unignore issue`. +#[derive(Args, Debug, Clone, PartialEq, Eq, Deserialize, JsonSchema)] +pub struct UnignoreIssueParams { + /// The issue ID. + pub id: u64, + + /// Issue category (required for writes; the API scopes them to one category). + #[arg(long, value_enum)] + pub category: IssueCategory, +} + +/// The `ignore` operation, declared once for both the CLI and the MCP server. +/// +/// Ignoring an issue that is already fully ignored is refused with a prompt +/// to unignore first (see ADR 0002); requires a full API token. +#[derive(Subcommand, Debug, Clone, PartialEq, Eq, Deserialize, JsonSchema)] +#[serde(tag = "entity", rename_all = "snake_case")] +pub enum IgnoreCommand { + /// Ignore an issue, optionally with notes and a reason. + #[command(alias = "issues")] + Issue(IgnoreIssueParams), +} + +/// The `unignore` operation, declared once for both the CLI and the MCP server. +#[derive(Subcommand, Debug, Clone, PartialEq, Eq, Deserialize, JsonSchema)] +#[serde(tag = "entity", rename_all = "snake_case")] +pub enum UnignoreCommand { + /// Revert a previous ignore, returning the issue to active. + #[command(alias = "issues")] + Issue(UnignoreIssueParams), +} + +/// The result of an [`IgnoreCommand`], serialized as the inner entity. +#[derive(Debug, Serialize)] +#[serde(untagged)] +pub enum IgnoreOutput { + /// The ignored issue, refreshed after the write. + Issue(Box), +} + +/// The result of an [`UnignoreCommand`], serialized as the inner entity. +#[derive(Debug, Serialize)] +#[serde(untagged)] +pub enum UnignoreOutput { + /// The unignored issue, refreshed after the write. + Issue(Box), +} + +impl PrettyPrint for IgnoreOutput { + fn pretty_print(&self) -> String { + match self { + IgnoreOutput::Issue(i) => i.pretty_print(), + } + } +} + +impl PrettyPrint for UnignoreOutput { + fn pretty_print(&self) -> String { + match self { + UnignoreOutput::Issue(i) => i.pretty_print(), + } + } +} + +/// Execute an `ignore` operation. +pub async fn run_ignore(client: &FossaClient, command: IgnoreCommand) -> Result { + Ok(match command { + IgnoreCommand::Issue(p) => { + let params = IssueUpdateParams { + category: p.category, + action: IssueAction::Ignore { + notes: p.notes, + reason: p.reason, + }, + }; + IgnoreOutput::Issue(Box::new(Issue::update(client, p.id, params).await?)) + } + }) +} + +/// Execute an `unignore` operation. +pub async fn run_unignore( + client: &FossaClient, + command: UnignoreCommand, +) -> Result { + Ok(match command { + UnignoreCommand::Issue(p) => { + let params = IssueUpdateParams { + category: p.category, + action: IssueAction::Unignore, + }; + UnignoreOutput::Issue(Box::new(Issue::update(client, p.id, params).await?)) + } + }) +} diff --git a/src/ops/mod.rs b/src/ops/mod.rs index 880a962..e03e473 100644 --- a/src/ops/mod.rs +++ b/src/ops/mod.rs @@ -1,8 +1,8 @@ //! Shared operation definitions consumed by both the CLI and the MCP server. //! -//! Each verb (`get`, `list`, `update`) is declared once as an enum whose -//! variants wrap per-entity parameter structs. The same declaration drives -//! both frontends: +//! Each verb (`get`, `list`, `update`, `ignore`, `unignore`) is declared once +//! as an enum whose variants wrap per-entity parameter structs. The same +//! declaration drives both frontends: //! //! - **CLI**: the enums derive [`clap::Subcommand`] and the param structs //! derive [`clap::Args`], so `fossapi get issue 123 --category licensing` @@ -18,6 +18,7 @@ //! between the surfaces is checked by `tests/parity.rs`. mod get; +mod ignore; mod list; mod update; @@ -25,6 +26,10 @@ pub use get::{ run_get, GetCommand, GetIssueParams, GetOutput, GetProjectParams, GetRevisionParams, GetSnippetMatchParams, GetSnippetParams, }; +pub use ignore::{ + run_ignore, run_unignore, IgnoreCommand, IgnoreIssueParams, IgnoreOutput, UnignoreCommand, + UnignoreIssueParams, UnignoreOutput, +}; pub use list::{ run_list, ListCommand, ListDependenciesParams, ListIssuesParams, ListOutput, ListProjectsParams, ListRevisionsParams, ListSnippetLocationsParams, ListSnippetPathsParams, diff --git a/tests/cli_args.rs b/tests/cli_args.rs index f8b7ba5..f28486b 100644 --- a/tests/cli_args.rs +++ b/tests/cli_args.rs @@ -5,12 +5,15 @@ //! consumes. use clap::Parser; -use fossapi::cli::{Cli, Command, GetCommand, ListCommand, UpdateCommand}; +use fossapi::cli::{ + Cli, Command, GetCommand, IgnoreCommand, ListCommand, UnignoreCommand, UpdateCommand, +}; use fossapi::ops::{ - GetIssueParams, GetProjectParams, GetRevisionParams, ListDependenciesParams, ListIssuesParams, - ListProjectsParams, ListRevisionsParams, PageArgs, UpdateProjectParams, + GetIssueParams, GetProjectParams, GetRevisionParams, IgnoreIssueParams, ListDependenciesParams, + ListIssuesParams, ListProjectsParams, ListRevisionsParams, PageArgs, UnignoreIssueParams, + UpdateProjectParams, }; -use fossapi::IssueCategory; +use fossapi::{IssueCategory, IssueIgnoreReason}; #[test] fn test_cli_parses_get_subcommand() { @@ -482,3 +485,117 @@ fn test_cli_parses_mcp_with_verbose_flag() { _ => panic!("Expected Mcp command"), } } + +// ============================================================================= +// ignore / unignore verbs +// ============================================================================= + +#[test] +fn test_ignore_issue_with_notes_and_reason() { + let cli = Cli::parse_from([ + "fossapi", + "ignore", + "issue", + "987654", + "--category", + "vulnerability", + "--notes", + "false positive patch", + "--reason", + "other", + ]); + match cli.command { + Command::Ignore { + command: + IgnoreCommand::Issue(IgnoreIssueParams { + id, + category, + notes, + reason, + }), + } => { + assert_eq!(id, 987654); + assert_eq!(category, IssueCategory::Vulnerability); + assert_eq!(notes, Some("false positive patch".to_string())); + assert_eq!(reason, Some(IssueIgnoreReason::Other)); + } + _ => panic!("Expected Ignore command"), + } +} + +// Multi-word reasons are where the CLI and API surfaces diverge: clap derives +// kebab-case value names, while the API stores `Vulnerable_code_not_in_execute_path`. +#[test] +fn test_ignore_issue_reason_uses_kebab_case_value_names() { + let cli = Cli::parse_from([ + "fossapi", + "ignore", + "issue", + "987654", + "--category", + "vulnerability", + "--reason", + "vulnerable-code-not-in-execute-path", + ]); + match cli.command { + Command::Ignore { + command: IgnoreCommand::Issue(IgnoreIssueParams { reason, .. }), + } => { + assert_eq!( + reason, + Some(IssueIgnoreReason::VulnerableCodeNotInExecutePath) + ); + } + _ => panic!("Expected Ignore command"), + } +} + +#[test] +fn test_unignore_issue() { + let cli = Cli::parse_from([ + "fossapi", + "unignore", + "issue", + "987654", + "--category", + "licensing", + ]); + match cli.command { + Command::Unignore { + command: UnignoreCommand::Issue(UnignoreIssueParams { id, category }), + } => { + assert_eq!(id, 987654); + assert_eq!(category, IssueCategory::Licensing); + } + _ => panic!("Expected Unignore command"), + } +} + +#[test] +fn test_ignore_issue_requires_category() { + let result = Cli::try_parse_from(["fossapi", "ignore", "issue", "987654"]); + assert!(result.is_err(), "--category must be required"); +} + +#[test] +fn test_unignore_issue_requires_category() { + let result = Cli::try_parse_from(["fossapi", "unignore", "issue", "987654"]); + assert!(result.is_err(), "--category must be required"); +} + +#[test] +fn test_unignore_issue_has_no_notes_flag() { + // notes belong to ignore only; the unignore declaration has no such + // field, so clap rejects the flag outright. + let result = Cli::try_parse_from([ + "fossapi", + "unignore", + "issue", + "987654", + "--category", + "licensing", + "--notes", + "orphan comment", + ]); + assert!(result.is_err(), "--notes is not a flag of unignore"); +} diff --git a/tests/e2e_mock_server.rs b/tests/e2e_mock_server.rs index 2553b3f..8998f01 100644 --- a/tests/e2e_mock_server.rs +++ b/tests/e2e_mock_server.rs @@ -7,8 +7,8 @@ use fossapi::mock_server::{Fixtures, MockServer, MockState}; use fossapi::{ - get_dependencies, FossaClient, Get, Issue, IssueCategory, IssueListQuery, List, Project, - Revision, Update, + get_dependencies, FossaClient, Get, Issue, IssueAction, IssueCategory, IssueIgnoreReason, + IssueListQuery, IssueUpdateParams, List, Project, Revision, Update, }; /// A list query scoped to one category, which the API requires. @@ -246,6 +246,110 @@ async fn test_issues_have_correct_types() { server.shutdown().await; } +#[tokio::test] +async fn test_ignore_and_unignore_issue_workflow() { + let state = MockState::new().with_issue(Fixtures::licensing_issue( + 987654, + "GPL-3.0", + "npm+leftpad$1.0.0", + )); + let server = MockServer::with_state(state).await; + let client = FossaClient::new("test-token", server.url()).unwrap(); + + // A reason on a licensing ignore is refused up front: only vulnerability + // ignores surface reasons anywhere in FOSSA. + let with_reason = Issue::update( + &client, + 987654, + IssueUpdateParams { + category: IssueCategory::Licensing, + action: IssueAction::Ignore { + notes: None, + reason: Some(IssueIgnoreReason::Other), + }, + }, + ) + .await; + assert!( + with_reason + .unwrap_err() + .to_string() + .contains("only apply to vulnerability ignores"), + "reason on a licensing ignore should be rejected" + ); + + // Ignore with a comment, as in "ignore this issue with comment 'false positive patch'". + let ignored = Issue::update( + &client, + 987654, + IssueUpdateParams { + category: IssueCategory::Licensing, + action: IssueAction::Ignore { + notes: Some("false positive patch".to_string()), + reason: None, + }, + }, + ) + .await + .expect("Failed to ignore issue"); + + assert_eq!(ignored.id, 987654); + assert_eq!(ignored.statuses.active, 0); + assert_eq!(ignored.statuses.ignored, 1); + + // The client-side guard refuses to re-ignore (server-side it would + // silently overwrite the notes) and prompts to unignore first. + let again = Issue::update( + &client, + 987654, + IssueUpdateParams { + category: IssueCategory::Licensing, + action: IssueAction::Ignore { + notes: None, + reason: None, + }, + }, + ) + .await; + assert!(again.is_err(), "Re-ignoring an ignored issue should error"); + assert!( + again.unwrap_err().to_string().contains("unignore it first"), + "guard error should prompt to unignore first" + ); + + // Unignore restores the active status. + let restored = Issue::update( + &client, + 987654, + IssueUpdateParams { + category: IssueCategory::Licensing, + action: IssueAction::Unignore, + }, + ) + .await + .expect("Failed to unignore issue"); + + assert_eq!(restored.statuses.active, 1); + assert_eq!(restored.statuses.ignored, 0); + + // Wrong category: the pre-flight fetch 404s before any write is sent. + let wrong_category = Issue::update( + &client, + 987654, + IssueUpdateParams { + category: IssueCategory::Vulnerability, + action: IssueAction::Ignore { + notes: None, + reason: None, + }, + }, + ) + .await; + assert!(wrong_category.is_err(), "Wrong category should error"); + + server.shutdown().await; +} + // ============================================================================= // Full Workflow Tests // ============================================================================= diff --git a/tests/parity.rs b/tests/parity.rs index 4f40f12..78417c6 100644 --- a/tests/parity.rs +++ b/tests/parity.rs @@ -9,7 +9,7 @@ use clap::CommandFactory; use fossapi::cli::Cli; use fossapi::mcp::FossaServer; -use fossapi::ops::{GetCommand, ListCommand, UpdateCommand}; +use fossapi::ops::{GetCommand, IgnoreCommand, ListCommand, UnignoreCommand, UpdateCommand}; use schemars::schema_for; use std::collections::BTreeSet; @@ -28,6 +28,14 @@ fn verbs() -> Vec<(&'static str, serde_json::Value)> { "update", serde_json::to_value(schema_for!(UpdateCommand)).unwrap(), ), + ( + "ignore", + serde_json::to_value(schema_for!(IgnoreCommand)).unwrap(), + ), + ( + "unignore", + serde_json::to_value(schema_for!(UnignoreCommand)).unwrap(), + ), ] } @@ -202,4 +210,28 @@ fn tagged_payloads_round_trip() { assert_eq!(p.locator, "custom+org/repo"); assert_eq!(p.title.as_deref(), Some("New Title")); assert_eq!(p.description, None); + + let ignore: IgnoreCommand = serde_json::from_value(serde_json::json!({ + "entity": "issue", + "id": 987654, + "category": "vulnerability", + "notes": "false positive patch", + "reason": "other" + })) + .expect("ignore payload deserializes"); + let IgnoreCommand::Issue(p) = ignore; + assert_eq!(p.id, 987654); + assert_eq!(p.category, fossapi::IssueCategory::Vulnerability); + assert_eq!(p.notes.as_deref(), Some("false positive patch")); + assert_eq!(p.reason, Some(fossapi::IssueIgnoreReason::Other)); + + let unignore: UnignoreCommand = serde_json::from_value(serde_json::json!({ + "entity": "issue", + "id": 987654, + "category": "licensing" + })) + .expect("unignore payload deserializes"); + let UnignoreCommand::Issue(p) = unignore; + assert_eq!(p.id, 987654); + assert_eq!(p.category, fossapi::IssueCategory::Licensing); }