From 7500b14395f014b202249e50f951df86e9f92c31 Mon Sep 17 00:00:00 2001 From: Sara Date: Tue, 18 Aug 2026 09:19:13 -0400 Subject: [PATCH 1/3] Support ignoring and un-ignoring issues with a comment Adds the first issue write surface: Issue now implements Update, sending PUT /v2/issues/ with the target in the query string (category, status filter, ids[]) and the action in the body ({type, notes?, reason?}). On success the issue is re-fetched so callers see the refreshed statuses; a count of 0 (nothing matched) is surfaced as an error instead of silently succeeding. - CLI: fossapi update issue --category --ignore [--notes ] [--reason ] / --unignore. Update flags moved into a flattened UpdateArgs struct. - MCP: the update tool accepts entity=issue with category, action (ignore/unignore), notes, and reason parameters. - IssueIgnoreReason is a closed enum because the API resolves reason strings against its ResolutionReasons table and silently stores NULL on any mismatch. - Mock server gains PUT /v2/issues/ (trailing slash, like the real API); e2e covers ignore -> re-ignore error -> unignore -> wrong category error. Issue writes need a full API token; push-only tokens are not allowlisted for this endpoint. Issue exceptions (org/policy-wide ignores) and native batch actions are documented as future work. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NExycfgM4tR8NvMjAnnhbJ --- CLAUDE.md | 5 +- README.md | 17 +- src/bin/fossapi.rs | 56 ++++-- src/cli/mod.rs | 58 ++++-- src/lib.rs | 4 + src/mcp/params.rs | 64 ++++++- src/mcp/server.rs | 296 +++++++++++++++++++++++++++-- src/mock_server/CLAUDE.md | 3 +- src/mock_server/handlers/issues.rs | 84 ++++++++ src/mock_server/server.rs | 2 + src/models/issue.rs | 223 +++++++++++++++++++++- tests/cli_args.rs | 94 +++++++-- tests/e2e_mock_server.rs | 81 +++++++- 13 files changed, 925 insertions(+), 62 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 398a01d..29f6cfe 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[]`, `status` filter), action in the body (`{type, notes?, reason?}`). Responds `{count, issueId?}`; `count: 0` = nothing matched (we surface it as an error). 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 | @@ -141,7 +142,7 @@ Project (top-level container) - **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`); on success it re-fetches and returns the refreshed issue. `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 @@ -162,6 +163,8 @@ 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); we deliberately send one ID per call so `count: 0` stays an unambiguous failure signal ## Nudge diff --git a/README.md b/README.md index 4034db1..15c2563 100644 --- a/README.md +++ b/README.md @@ -80,8 +80,23 @@ 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 update issue 12345 --category licensing --ignore \ + --notes "false positive patch" --reason other + +# Revert the ignore +fossapi update issue 12345 --category licensing --unignore ``` +Ignoring supports an optional `--notes` free-text comment and a `--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`). Issue writes require a full API +token; push-only tokens can only read. + ### Snippets Snippet scanning finds third-party (open-source) code copied into your @@ -157,7 +172,7 @@ Add to your MCP config: |------|-------------| | `get` | Fetch a single project, revision, or issue by ID | | `list` | List projects, revisions, dependencies, issues, or snippet match locations | -| `update` | Update project metadata (title, description, url, public) | +| `update` | Update project metadata (title, description, url, public), or ignore/unignore an issue (with optional notes and reason) | | `snippet_match` | Drill into one snippet match: the matched first-party and reference code | > **Snippets over MCP:** use `list` with `entity: snippet` and `parent: fossapi::Result<()> { Command::Update { entity, locator, - title, - description, - public, - } => handle_update(client, entity, &locator, title, description, public, cli.json).await, + args, + } => handle_update(client, entity, &locator, args, cli.json).await, Command::Mcp { verbose } => handle_mcp(client, verbose).await, } } @@ -196,26 +195,53 @@ async fn handle_update( client: &FossaClient, entity: Entity, locator: &str, - title: Option, - description: Option, - public: Option, + args: UpdateArgs, json: bool, ) -> fossapi::Result<()> { match entity { Entity::Project => { let params = ProjectUpdateParams { - title, - description, - public, + title: args.title, + description: args.description, + public: args.public, ..Default::default() }; let project = Project::update(client, locator.to_string(), params).await?; output_single(&project, json)?; } + Entity::Issue => { + let id: u64 = locator.parse().map_err(|_| { + fossapi::FossaError::InvalidLocator(format!( + "issue updates take the numeric issue ID, got: {locator}" + )) + })?; + let Some(category) = args.category else { + eprintln!("Error: --category is required when updating an issue"); + return Err(fossapi::FossaError::InvalidLocator( + "--category is required for issue updates".to_string(), + )); + }; + let action = if args.ignore { + IssueAction::Ignore { + notes: args.notes, + reason: args.reason, + } + } else if args.unignore { + IssueAction::Unignore + } else { + eprintln!("Error: pass --ignore or --unignore to update an issue"); + return Err(fossapi::FossaError::InvalidLocator( + "issue updates require --ignore or --unignore".to_string(), + )); + }; + let issue = + Issue::update(client, id, IssueUpdateParams { category, action }).await?; + output_single(&issue, json)?; + } _ => { - eprintln!("Error: Only projects can be updated via CLI"); + eprintln!("Error: Only projects and issues can be updated via CLI"); return Err(fossapi::FossaError::InvalidLocator( - "only projects support update".to_string(), + "only projects and issues support update".to_string(), )); } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 6a1c881..01400e2 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -2,9 +2,9 @@ //! //! This module provides the command-line interface structure for the fossapi binary. -use clap::{Parser, Subcommand, ValueEnum}; +use clap::{Args, Parser, Subcommand, ValueEnum}; -use crate::IssueCategory; +use crate::{IssueCategory, IssueIgnoreReason}; /// FOSSA API command-line interface. #[derive(Parser, Debug)] @@ -38,20 +38,11 @@ pub enum Command { /// The type of entity to update. entity: Entity, - /// The locator of the entity to update. + /// The locator of the entity to update (numeric issue ID for issues). locator: String, - /// New title for the entity. - #[arg(long)] - title: Option, - - /// New description for the entity. - #[arg(long)] - description: Option, - - /// Set project visibility (true = public, false = private). - #[arg(long)] - public: Option, + #[command(flatten)] + args: UpdateArgs, }, /// Run the MCP server on stdio. @@ -62,6 +53,45 @@ pub enum Command { }, } +/// Flags for the `update` command. +/// +/// Projects and issues take disjoint flag sets; the handler rejects +/// mismatched combinations. +#[derive(Args, Debug, Clone, PartialEq)] +pub struct UpdateArgs { + /// New title (project only). + #[arg(long)] + pub title: Option, + + /// New description (project only). + #[arg(long)] + pub description: Option, + + /// Set project visibility (true = public, false = private; project only). + #[arg(long)] + pub public: Option, + + /// Issue category (issue only; required for issue updates). + #[arg(long, value_enum)] + pub category: Option, + + /// Ignore the issue (issue only). + #[arg(long, conflicts_with = "unignore")] + pub ignore: bool, + + /// Revert a previous ignore, returning the issue to active (issue only). + #[arg(long)] + pub unignore: bool, + + /// Free-text comment recorded with --ignore. + #[arg(long, requires = "ignore")] + pub notes: Option, + + /// Structured reason recorded with --ignore. + #[arg(long, value_enum, requires = "ignore")] + pub reason: Option, +} + /// Subcommands for the `get` command with type-safe ID parsing. #[derive(Subcommand, Debug, Clone, PartialEq, Eq)] pub enum GetCommand { diff --git a/src/lib.rs b/src/lib.rs index 9fdf6e3..080e7e0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -96,15 +96,19 @@ pub use models::{ LicenseInfo, // Issue types Issue, + IssueAction, + IssueActionResponse, IssueCategory, IssueDepths, IssueEpss, + IssueIgnoreReason, IssueListQuery, IssueMetric, IssueProject, IssueRemediation, IssueSource, IssueStatuses, + IssueUpdateParams, // Snippet types CodeLine, Snippet, diff --git a/src/mcp/params.rs b/src/mcp/params.rs index 7b722b8..fa0045c 100644 --- a/src/mcp/params.rs +++ b/src/mcp/params.rs @@ -3,7 +3,7 @@ use schemars::JsonSchema; use serde::Deserialize; -use crate::IssueCategory; +use crate::{IssueCategory, IssueIgnoreReason}; /// Entity types supported by MCP tools. #[derive(Debug, Clone, Deserialize, JsonSchema)] @@ -58,12 +58,22 @@ pub struct ListParams { pub with_lines: Option, } +/// Status-changing actions for the `update` MCP tool on issues. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum UpdateAction { + /// Ignore the issue (optionally with notes and a reason). + Ignore, + /// Revert a previous ignore, returning the issue to active. + Unignore, +} + /// Parameters for the `update` MCP tool. #[derive(Debug, Clone, Deserialize, JsonSchema)] pub struct UpdateParams { /// The type of entity to update. pub entity: EntityType, - /// The entity locator. + /// The entity locator (Project) or numeric issue ID (Issue). pub locator: String, /// New title (Project only). #[serde(default)] @@ -77,6 +87,18 @@ pub struct UpdateParams { /// Whether the project is public (Project only). #[serde(default)] pub public: Option, + /// Issue category (required for Issue entity: vulnerability, licensing, quality). + #[serde(default)] + pub category: Option, + /// Action to perform (required for Issue entity: ignore, unignore). + #[serde(default)] + pub action: Option, + /// Free-text comment recorded with an ignore (Issue only). + #[serde(default)] + pub notes: Option, + /// Structured reason recorded with an ignore (Issue only). + #[serde(default)] + pub reason: Option, } /// Parameters for the `snippet_match` MCP tool (the snippet drill-in). @@ -213,6 +235,44 @@ mod tests { assert_eq!(params.with_lines, Some(true)); } + #[test] + fn update_params_deserializes_issue_ignore() { + let json = r#"{ + "entity": "issue", + "locator": "987654", + "category": "licensing", + "action": "ignore", + "notes": "false positive patch", + "reason": "other" + }"#; + let params: UpdateParams = serde_json::from_str(json).unwrap(); + assert!(matches!(params.entity, EntityType::Issue)); + assert_eq!(params.locator, "987654"); + assert!(matches!(params.category, Some(IssueCategory::Licensing))); + assert_eq!(params.action, Some(UpdateAction::Ignore)); + assert_eq!(params.notes.as_deref(), Some("false positive patch")); + assert_eq!(params.reason, Some(IssueIgnoreReason::Other)); + } + + #[test] + fn update_params_deserializes_issue_unignore() { + let json = r#"{"entity": "issue", "locator": "987654", "category": "vulnerability", "action": "unignore"}"#; + let params: UpdateParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.action, Some(UpdateAction::Unignore)); + assert!(params.notes.is_none()); + assert!(params.reason.is_none()); + } + + #[test] + fn update_params_schema_includes_issue_fields() { + let schema = schemars::schema_for!(UpdateParams); + let json = serde_json::to_string(&schema).unwrap(); + assert!(json.contains("action")); + assert!(json.contains("notes")); + assert!(json.contains("reason")); + assert!(json.contains("category")); + } + #[test] fn snippet_match_params_deserializes() { let json = r#"{"revision": "custom+org/repo$main", "snippet": "1295019", "path": "/src/a.rs"}"#; diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 5211b69..d9b8201 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -14,10 +14,10 @@ use schemars::JsonSchema; use std::sync::Arc; use crate::{ - mcp::{EntityType, GetParams, ListParams, SnippetMatchParams, UpdateParams}, - DependencyListQuery, FossaClient, FossaError, Get, Issue, IssueListQuery, List, - Project, ProjectListQuery, ProjectUpdateParams, Revision, RevisionListQuery, SnippetListQuery, - Update, + mcp::{EntityType, GetParams, ListParams, SnippetMatchParams, UpdateAction, UpdateParams}, + DependencyListQuery, FossaClient, FossaError, Get, Issue, IssueAction, IssueListQuery, + IssueUpdateParams, List, Project, ProjectListQuery, ProjectUpdateParams, Revision, + RevisionListQuery, SnippetListQuery, Update, }; /// FOSSA MCP Server. @@ -284,10 +284,44 @@ impl FossaServer { "Update not supported for Revision", None, )), - EntityType::Issue => Err(McpError::invalid_params( - "Update not supported for Issue", - None, - )), + EntityType::Issue => { + let id: u64 = params.locator.parse().map_err(|_| { + McpError::invalid_params( + "Issue updates take the numeric issue ID as the locator", + None, + ) + })?; + let category = params.category.ok_or_else(|| { + McpError::invalid_params( + "category is required when updating an issue \ + (vulnerability, licensing, quality)", + None, + ) + })?; + let action = match params.action { + Some(UpdateAction::Ignore) => IssueAction::Ignore { + notes: params.notes, + reason: params.reason, + }, + Some(UpdateAction::Unignore) => IssueAction::Unignore, + None => { + return Err(McpError::invalid_params( + "action is required when updating an issue (ignore, unignore)", + None, + )) + } + }; + let issue = Issue::update( + &self.client, + id, + IssueUpdateParams { category, action }, + ) + .await + .map_err(Self::to_mcp_error)?; + let result = serde_json::to_string_pretty(&issue) + .map_err(|e| McpError::internal_error(e.to_string(), None))?; + Ok(CallToolResult::success(vec![Content::text(result)])) + } EntityType::Dependency => Err(McpError::invalid_params( "Update not supported for Dependency", None, @@ -350,8 +384,11 @@ impl ServerHandler for FossaServer { ), Tool::new( "update", - "Update a FOSSA entity. Currently only Project is supported. \ - Can update: title, description, url, public.", + "Update a FOSSA entity. \ + Project: locator = project locator; can update title, description, url, public. \ + Issue: locator = numeric issue ID; category required; action = ignore or \ + unignore, with optional notes (free-text comment) and reason on ignore. \ + Requires a full API token (push-only tokens cannot write issues).", Self::schema::(), ), Tool::new( @@ -925,6 +962,10 @@ mod tests { description: None, url: None, public: None, + category: None, + action: None, + notes: None, + reason: None, }; let result = server.handle_update(params).await; @@ -935,24 +976,241 @@ mod tests { } #[tokio::test] - async fn handle_update_issue_returns_error() { + async fn handle_update_issue_without_category_returns_error() { let client = FossaClient::new("test-token", "http://localhost:9999").unwrap(); let server = FossaServer::new(client); let params = UpdateParams { entity: EntityType::Issue, locator: "12345".to_string(), - title: Some("New Title".to_string()), + title: None, description: None, url: None, public: None, + category: None, + action: Some(crate::mcp::UpdateAction::Ignore), + notes: None, + reason: None, }; let result = server.handle_update(params).await; assert!(result.is_err()); let err = result.unwrap_err(); - assert!(err.message.contains("not supported")); + assert!(err.message.contains("category is required")); + } + + #[tokio::test] + async fn handle_update_issue_without_action_returns_error() { + let client = FossaClient::new("test-token", "http://localhost:9999").unwrap(); + let server = FossaServer::new(client); + + let params = UpdateParams { + entity: EntityType::Issue, + locator: "12345".to_string(), + title: None, + description: None, + url: None, + public: None, + category: Some(crate::IssueCategory::Licensing), + action: None, + notes: None, + reason: None, + }; + + let result = server.handle_update(params).await; + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.message.contains("action is required")); + } + + #[tokio::test] + async fn handle_update_issue_with_non_numeric_locator_returns_error() { + let client = FossaClient::new("test-token", "http://localhost:9999").unwrap(); + let server = FossaServer::new(client); + + let params = UpdateParams { + entity: EntityType::Issue, + locator: "custom+org/repo".to_string(), + title: None, + description: None, + url: None, + public: None, + category: Some(crate::IssueCategory::Licensing), + action: Some(crate::mcp::UpdateAction::Ignore), + notes: None, + reason: None, + }; + + let result = server.handle_update(params).await; + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.message.contains("numeric issue ID")); + } + + #[tokio::test] + async fn handle_update_issue_ignore_sends_action_and_returns_refreshed_issue() { + use wiremock::matchers::{body_json, query_param}; + + let mock_server = MockServer::start().await; + + let expected_body = serde_json::json!({ + "type": "ignore", + "notes": "false positive patch", + "reason": "other" + }); + + // The write: PUT /v2/issues/ with the target in the query string. + Mock::given(method("PUT")) + .and(path("/v2/issues/")) + .and(query_param("category", "licensing")) + .and(query_param("status", "active")) + .and(query_param("ids[]", "987654")) + .and(body_json(&expected_body)) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"count": 1, "issueId": 987654})), + ) + .expect(1) + .mount(&mock_server) + .await; + + // The refresh read that follows a successful write. + let refreshed = serde_json::json!({ + "id": 987654, + "type": "licensing", + "source": {"id": "npm+leftpad$1.0.0"}, + "statuses": {"active": 0, "ignored": 1}, + "license": "GPL-3.0" + }); + Mock::given(method("GET")) + .and(path("/v2/issues/987654")) + .and(query_param("category", "licensing")) + .respond_with(ResponseTemplate::new(200).set_body_json(&refreshed)) + .expect(1) + .mount(&mock_server) + .await; + + let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); + let server = FossaServer::new(client); + + let params = UpdateParams { + entity: EntityType::Issue, + locator: "987654".to_string(), + title: None, + description: None, + url: None, + public: None, + category: Some(crate::IssueCategory::Licensing), + action: Some(crate::mcp::UpdateAction::Ignore), + notes: Some("false positive patch".to_string()), + reason: Some(crate::IssueIgnoreReason::Other), + }; + + let result = server.handle_update(params).await; + + assert!(result.is_ok(), "expected success, got {result:?}"); + let call_result = result.unwrap(); + let content = &call_result.content[0]; + if let rmcp::model::RawContent::Text(text) = &content.raw { + assert!(text.text.contains("987654")); + assert!(text.text.contains("\"ignored\": 1")); + } else { + panic!("Expected text content"); + } + } + + #[tokio::test] + async fn handle_update_issue_unignore_targets_ignored_rows() { + use wiremock::matchers::{body_json, query_param}; + + let mock_server = MockServer::start().await; + + Mock::given(method("PUT")) + .and(path("/v2/issues/")) + .and(query_param("category", "vulnerability")) + .and(query_param("status", "ignored")) + .and(query_param("ids[]", "555")) + .and(body_json(&serde_json::json!({"type": "unignore"}))) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"count": 1, "issueId": 555})), + ) + .expect(1) + .mount(&mock_server) + .await; + + let refreshed = serde_json::json!({ + "id": 555, + "type": "vulnerability", + "source": {"id": "npm+lodash$4.17.20"}, + "statuses": {"active": 1, "ignored": 0} + }); + Mock::given(method("GET")) + .and(path("/v2/issues/555")) + .respond_with(ResponseTemplate::new(200).set_body_json(&refreshed)) + .expect(1) + .mount(&mock_server) + .await; + + let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); + let server = FossaServer::new(client); + + let params = UpdateParams { + entity: EntityType::Issue, + locator: "555".to_string(), + title: None, + description: None, + url: None, + public: None, + category: Some(crate::IssueCategory::Vulnerability), + action: Some(crate::mcp::UpdateAction::Unignore), + notes: None, + reason: None, + }; + + let result = server.handle_update(params).await; + assert!(result.is_ok(), "expected success, got {result:?}"); + } + + #[tokio::test] + async fn handle_update_issue_count_zero_is_an_error() { + use wiremock::matchers::query_param; + + let mock_server = MockServer::start().await; + + Mock::given(method("PUT")) + .and(path("/v2/issues/")) + .and(query_param("ids[]", "987654")) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({"count": 0})), + ) + .expect(1) + .mount(&mock_server) + .await; + + let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); + let server = FossaServer::new(client); + + let params = UpdateParams { + entity: EntityType::Issue, + locator: "987654".to_string(), + title: None, + description: None, + url: None, + public: None, + category: Some(crate::IssueCategory::Licensing), + action: Some(crate::mcp::UpdateAction::Ignore), + notes: None, + reason: None, + }; + + let result = server.handle_update(params).await; + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.message.contains("no active licensing issue matched")); } #[tokio::test] @@ -967,6 +1225,10 @@ mod tests { description: None, url: None, public: None, + category: None, + action: None, + notes: None, + reason: None, }; let result = server.handle_update(params).await; @@ -1012,6 +1274,10 @@ mod tests { description: None, url: None, public: None, + category: None, + action: None, + notes: None, + reason: None, }; let result = server.handle_update(params).await; @@ -1066,6 +1332,10 @@ mod tests { description: Some("New project description".to_string()), url: None, public: None, + category: None, + action: None, + notes: None, + reason: None, }; let result = server.handle_update(params).await; 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 8027b66..a326dae 100644 --- a/src/mock_server/handlers/issues.rs +++ b/src/mock_server/handlers/issues.rs @@ -118,3 +118,87 @@ pub async fn list_issues( (StatusCode::OK, Json(ListIssuesResponse { issues })).into_response() } + +/// PUT /v2/issues/ +/// +/// Mirrors the real API's shape: targets come from the query string +/// (`category`, `status` filter, `ids[]`), the action from the JSON body +/// (`{"type": "ignore", "notes": ..., "reason": ...}` or +/// `{"type": "unignore"}`). Responds `{count, issueId?}`, where `issueId` is +/// only present when exactly one issue changed and `count: 0` signals that +/// nothing matched. +pub async fn update_issues( + State(state): State>>, + Query(params): Query>, + Json(body): Json, +) -> impl IntoResponse { + let find = |key: &str| { + params + .iter() + .find(|(k, _)| k == key) + .map(|(_, v)| v.as_str()) + }; + + let Some(category) = find("category") else { + return missing_category(); + }; + let status_filter = find("status").unwrap_or("active"); + 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 matches_filter = match status_filter { + "active" => issue.statuses.active > 0, + "ignored" => issue.statuses.ignored > 0, + _ => true, + }; + if !matches_filter { + 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 88253e6..3d55e58 100644 --- a/src/mock_server/server.rs +++ b/src/mock_server/server.rs @@ -141,6 +141,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 6fb2c48..9488198 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) @@ -528,6 +528,97 @@ 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"); + } } // ============================================================================= @@ -922,6 +1013,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. @@ -1100,3 +1200,124 @@ 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. + /// + /// Sends `PUT /v2/issues/` targeting this single issue. The API applies + /// actions only to rows matching a status filter, so ignore targets + /// `active` rows and unignore targets `ignored` rows; a `count` of 0 means + /// nothing matched (wrong category, no access, or the issue is already in + /// the requested state) and is surfaced as an error rather than silently + /// succeeding. + /// + /// 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 { + let status = match params.action { + IssueAction::Ignore { .. } => "active", + IssueAction::Unignore => "ignored", + }; + let path = format!( + "v2/issues/?category={}&status={status}&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!( + "no {status} {} issue matched ID {id}; it may not exist, be outside \ + your token's access, or already be in the requested state", + params.category.as_str() + ), + status_code: None, + }); + } + + Issue::get_with_category(client, id, params.category).await + } +} diff --git a/tests/cli_args.rs b/tests/cli_args.rs index fdeb97a..5676eda 100644 --- a/tests/cli_args.rs +++ b/tests/cli_args.rs @@ -46,12 +46,11 @@ fn test_cli_parses_update_subcommand() { Command::Update { entity, locator, - title, - .. + args, } => { assert!(matches!(entity, Entity::Project)); assert_eq!(locator, "custom+acme/myapp"); - assert_eq!(title, Some("New Title".to_string())); + assert_eq!(args.title, Some("New Title".to_string())); } _ => panic!("Expected Update command"), } @@ -297,8 +296,8 @@ fn test_update_project_title_flag() { "New Title", ]); match cli.command { - Command::Update { title, .. } => { - assert_eq!(title, Some("New Title".to_string())); + Command::Update { args, .. } => { + assert_eq!(args.title, Some("New Title".to_string())); } _ => panic!("Expected Update command"), } @@ -315,8 +314,8 @@ fn test_update_project_public_flag() { "true", ]); match cli.command { - Command::Update { public, .. } => { - assert_eq!(public, Some(true)); + Command::Update { args, .. } => { + assert_eq!(args.public, Some(true)); } _ => panic!("Expected Update command"), } @@ -338,14 +337,12 @@ fn test_update_project_multiple_flags() { Command::Update { entity, locator, - title, - public, - .. + args, } => { assert!(matches!(entity, Entity::Project)); assert_eq!(locator, "custom+acme/myapp"); - assert_eq!(title, Some("New Title".to_string())); - assert_eq!(public, Some(false)); + assert_eq!(args.title, Some("New Title".to_string())); + assert_eq!(args.public, Some(false)); } _ => panic!("Expected Update command"), } @@ -376,3 +373,76 @@ fn test_cli_parses_mcp_with_verbose_flag() { _ => panic!("Expected Mcp command"), } } + +// ============================================================================= +// Issue update flags (ignore / unignore with comment) +// ============================================================================= + +#[test] +fn test_update_issue_ignore_with_notes_and_reason() { + let cli = Cli::parse_from([ + "fossapi", + "update", + "issue", + "987654", + "--category", + "licensing", + "--ignore", + "--notes", + "false positive patch", + "--reason", + "other", + ]); + match cli.command { + Command::Update { + entity, + locator, + args, + } => { + assert!(matches!(entity, Entity::Issue)); + assert_eq!(locator, "987654"); + assert_eq!(args.category, Some(IssueCategory::Licensing)); + assert!(args.ignore); + assert!(!args.unignore); + assert_eq!(args.notes, Some("false positive patch".to_string())); + assert_eq!(args.reason, Some(fossapi::IssueIgnoreReason::Other)); + } + _ => panic!("Expected Update command"), + } +} + +#[test] +fn test_update_issue_unignore() { + let cli = Cli::parse_from([ + "fossapi", + "update", + "issue", + "987654", + "--category", + "vulnerability", + "--unignore", + ]); + match cli.command { + Command::Update { args, .. } => { + assert!(args.unignore); + assert!(!args.ignore); + } + _ => panic!("Expected Update command"), + } +} + +#[test] +fn test_update_issue_ignore_conflicts_with_unignore() { + let result = Cli::try_parse_from([ + "fossapi", "update", "issue", "987654", "--ignore", "--unignore", + ]); + assert!(result.is_err()); +} + +#[test] +fn test_update_issue_notes_requires_ignore() { + let result = Cli::try_parse_from([ + "fossapi", "update", "issue", "987654", "--notes", "orphan comment", + ]); + assert!(result.is_err()); +} diff --git a/tests/e2e_mock_server.rs b/tests/e2e_mock_server.rs index 8035835..b606433 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. @@ -244,6 +244,83 @@ 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(); + + // 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: Some(IssueIgnoreReason::Other), + }, + }, + ) + .await + .expect("Failed to ignore issue"); + + assert_eq!(ignored.id, 987654); + assert_eq!(ignored.statuses.active, 0); + assert_eq!(ignored.statuses.ignored, 1); + + // Re-ignoring matches nothing (the status filter targets active rows) and errors. + 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"); + + // 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 matches nothing. + 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 // ============================================================================= From 9bb752834b7a2a2b691c91230d94dcf7de6a12ec Mon Sep 17 00:00:00 2001 From: Sara Date: Tue, 18 Aug 2026 11:53:06 -0400 Subject: [PATCH 2/3] Promote ignore/unignore to top-level verbs; restrict reasons to vulnerabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignore and unignore are now first-class verbs on both surfaces (new src/ops/ignore.rs declarations, IgnoreCommand/UnignoreCommand, dedicated MCP tools) instead of --ignore/--unignore flags on update issue. The bool-pair wart disappears: unignore has no notes/reason fields at all, and agent harnesses can permission-gate the write tools independently of metadata updates. update returns to project-only, matching main. Issue::update also now rejects a reason outside the vulnerability category: core stores one for any category, but only vulnerability ignores ever display it (UI, SBOM/VEX) — for licensing/quality it is write-only noise. Documented in ADR 0002, README, and the glossary. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NExycfgM4tR8NvMjAnnhbJ --- CHANGELOG.md | 14 +- CLAUDE.md | 2 +- CONTEXT.md | 14 +- README.md | 28 ++-- .../adr/0002-client-side-issue-write-guard.md | 5 +- src/bin/fossapi.rs | 10 +- src/cli/mod.rs | 14 +- src/mcp/server.rs | 134 ++++++++++++------ src/models/issue.rs | 16 +++ src/ops/ignore.rs | 129 +++++++++++++++++ src/ops/mod.rs | 13 +- src/ops/update.rs | 81 +---------- tests/cli_args.rs | 105 +++++--------- tests/e2e_mock_server.rs | 24 +++- tests/parity.rs | 39 +++-- 15 files changed, 383 insertions(+), 245 deletions(-) create mode 100644 src/ops/ignore.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b0138..3eb5554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +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. - CLI: `update issue --category --ignore [--notes ] - [--reason ]` / `--unignore`; MCP: `update` with `entity: "issue"`. - Ignoring an already-ignored issue is refused with a prompt to unignore - first (see ADR 0002); requires a full API token (push-only tokens cannot - write issues). +- **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 b363e18..688fa3d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -162,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/Update. Update = ignore/unignore via `IssueUpdateParams` (`IssueAction::Ignore { notes, reason }` / `Unignore`); 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. `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. +- **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 diff --git a/CONTEXT.md b/CONTEXT.md index a7d9f00..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**: @@ -70,8 +70,10 @@ Not a Comment. _Avoid_: comment, message **Reason**: -One of a closed set of structured explanations attached to an Ignore (Fixed, -Vulnerable code not in execute path, Other, …). +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**: diff --git a/README.md b/README.md index 6dee16c..36b957f 100644 --- a/README.md +++ b/README.md @@ -82,24 +82,24 @@ fossapi get issue 12345 fossapi get issue 12345 --category licensing # Ignore an issue with a comment -fossapi update issue 12345 --category licensing --ignore \ - --notes "false positive patch" --reason other +fossapi ignore issue 12345 --category licensing --notes "false positive patch" # Revert the ignore -fossapi update issue 12345 --category licensing --unignore +fossapi unignore issue 12345 --category licensing ``` -Ignoring supports an optional `--notes` free-text comment and a `--reason` -(one of `fixed`, `under-investigation`, `incorrect-data-found`, -`component-not-present`, `vulnerable-code-not-present`, -`vulnerable-code-not-in-execute-path`, +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`). Issue writes require a full API -token; push-only tokens can only read. +`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 +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. @@ -183,12 +183,14 @@ 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), `issue` (ignore/unignore with optional notes and reason; category required) | +| `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, and -`fossapi update issue 12345 --category licensing --ignore --notes "false positive patch"` -is `update {"entity": "issue", "id": 12345, "category": "licensing", "ignore": true, "notes": "false positive patch"}`. +`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 index 5aac9bb..b38ad56 100644 --- a/docs/adr/0002-client-side-issue-write-guard.md +++ b/docs/adr/0002-client-side-issue-write-guard.md @@ -4,7 +4,7 @@ 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 -`update issue` pre-fetches the issue and **refuses** an action whose target +`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 @@ -26,3 +26,6 @@ new notes. 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/mcp/server.rs b/src/mcp/server.rs index 0329df0..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,9 +303,9 @@ mod tests { use super::*; use crate::ops::{ GetIssueParams, GetProjectParams, GetRevisionParams, GetSnippetMatchParams, - GetSnippetParams, ListDependenciesParams, ListProjectsParams, ListRevisionsParams, - ListSnippetLocationsParams, ListSnippetsParams, PageArgs, UpdateIssueParams, - UpdateProjectParams, + GetSnippetParams, IgnoreIssueParams, ListDependenciesParams, ListProjectsParams, + ListRevisionsParams, ListSnippetLocationsParams, ListSnippetsParams, PageArgs, + UnignoreIssueParams, UpdateProjectParams, }; use crate::IssueCategory; use wiremock::matchers::{method, path, path_regex, query_param}; @@ -270,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] @@ -283,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] @@ -930,7 +981,7 @@ mod tests { } // ========================================================================= - // update issue handler tests + // ignore / unignore handler tests // ========================================================================= fn issue_body(id: u64, active: u32, ignored: u32) -> serde_json::Value { @@ -943,21 +994,19 @@ mod tests { }) } - fn issue_params(ignore: bool, unignore: bool) -> UpdateIssueParams { - UpdateIssueParams { + fn ignore_licensing_issue() -> IgnoreCommand { + IgnoreCommand::Issue(IgnoreIssueParams { id: 987654, category: IssueCategory::Licensing, - ignore, - unignore, notes: Some("false positive patch".to_string()), - reason: Some(crate::IssueIgnoreReason::Other), - } + reason: None, + }) } /// Test: ignore fetches the issue, sends the action, and returns the /// refreshed issue. #[tokio::test] - async fn handle_update_issue_ignore_succeeds() { + async fn handle_ignore_issue_succeeds() { use wiremock::matchers::body_json; let mock_server = MockServer::start().await; @@ -978,8 +1027,7 @@ mod tests { .and(query_param("ids[]", "987654")) .and(body_json(serde_json::json!({ "type": "ignore", - "notes": "false positive patch", - "reason": "other" + "notes": "false positive patch" }))) .respond_with( ResponseTemplate::new(200) @@ -1001,7 +1049,7 @@ mod tests { let server = FossaServer::new(client); let result = server - .handle_update(UpdateCommand::Issue(issue_params(true, false))) + .handle_ignore(ignore_licensing_issue()) .await .unwrap(); assert!(!result.is_error.unwrap_or(false)); @@ -1012,7 +1060,7 @@ mod tests { /// Test: ignoring an already-ignored issue is refused before any PUT. #[tokio::test] - async fn handle_update_issue_already_ignored_is_rejected_without_put() { + async fn handle_ignore_already_ignored_is_rejected_without_put() { let mock_server = MockServer::start().await; Mock::given(method("GET")) @@ -1033,7 +1081,7 @@ mod tests { let server = FossaServer::new(client); let err = server - .handle_update(UpdateCommand::Issue(issue_params(true, false))) + .handle_ignore(ignore_licensing_issue()) .await .unwrap_err(); assert!(err.message.contains("unignore it first"), "{err:?}"); @@ -1041,7 +1089,7 @@ mod tests { /// Test: unignoring an issue with nothing ignored is refused before any PUT. #[tokio::test] - async fn handle_update_issue_unignore_active_is_rejected_without_put() { + async fn handle_unignore_active_is_rejected_without_put() { let mock_server = MockServer::start().await; Mock::given(method("GET")) @@ -1061,11 +1109,11 @@ mod tests { let client = FossaClient::new("test-token", &mock_server.uri()).unwrap(); let server = FossaServer::new(client); - let mut params = issue_params(false, true); - params.notes = None; - params.reason = None; let err = server - .handle_update(UpdateCommand::Issue(params)) + .handle_unignore(UnignoreCommand::Issue(UnignoreIssueParams { + id: 987654, + category: IssueCategory::Licensing, + })) .await .unwrap_err(); assert!(err.message.contains("nothing to unignore"), "{err:?}"); @@ -1074,7 +1122,7 @@ mod tests { /// Test: a partially ignored issue (org-wide rollup) accepts both actions, /// like the UI's global issue view. #[tokio::test] - async fn handle_update_issue_partial_state_allows_ignore() { + async fn handle_ignore_partial_state_is_allowed() { use wiremock::matchers::body_json; let mock_server = MockServer::start().await; @@ -1091,8 +1139,7 @@ mod tests { .and(path("/v2/issues/")) .and(body_json(serde_json::json!({ "type": "ignore", - "notes": "false positive patch", - "reason": "other" + "notes": "false positive patch" }))) .respond_with( ResponseTemplate::new(200) @@ -1113,38 +1160,31 @@ mod tests { let server = FossaServer::new(client); let result = server - .handle_update(UpdateCommand::Issue(issue_params(true, false))) + .handle_ignore(ignore_licensing_issue()) .await .unwrap(); assert!(!result.is_error.unwrap_or(false)); } - /// Test: MCP arguments bypass clap, so exactly-one-action is re-checked. + /// 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_update_issue_without_action_is_rejected() { + 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_update(UpdateCommand::Issue(issue_params(false, false))) + .handle_ignore(IgnoreCommand::Issue(IgnoreIssueParams { + id: 987654, + category: IssueCategory::Licensing, + notes: None, + reason: Some(crate::IssueIgnoreReason::Other), + })) .await .unwrap_err(); assert!( - err.message.contains("exactly one of ignore or unignore"), + err.message.contains("only apply to vulnerability ignores"), "{err:?}" ); } - - /// Test: notes/reason with unignore is rejected before any HTTP call. - #[tokio::test] - async fn handle_update_issue_unignore_with_notes_is_rejected() { - let client = FossaClient::new("test-token", "http://localhost:9999").unwrap(); - let server = FossaServer::new(client); - - let err = server - .handle_update(UpdateCommand::Issue(issue_params(false, true))) - .await - .unwrap_err(); - assert!(err.message.contains("only apply when ignoring"), "{err:?}"); - } } diff --git a/src/models/issue.rs b/src/models/issue.rs index 4cab27b..1c42520 100644 --- a/src/models/issue.rs +++ b/src/models/issue.rs @@ -1313,6 +1313,22 @@ impl Update for Issue { /// 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 => { 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 02bf309..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,12 +26,16 @@ 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, ListSnippetsParams, }; -pub use update::{run_update, UpdateCommand, UpdateIssueParams, UpdateOutput, UpdateProjectParams}; +pub use update::{run_update, UpdateCommand, UpdateOutput, UpdateProjectParams}; use schemars::JsonSchema; use serde::Deserialize; diff --git a/src/ops/update.rs b/src/ops/update.rs index 1857b30..9cac03d 100644 --- a/src/ops/update.rs +++ b/src/ops/update.rs @@ -1,14 +1,10 @@ -//! The `update` verb: modify an entity. Projects (metadata) and issues -//! (ignore/unignore) are updatable. +//! The `update` verb: modify an entity. Currently only projects are updatable. use clap::{Args, Subcommand}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::{ - FossaClient, FossaError, Issue, IssueAction, IssueCategory, IssueIgnoreReason, - IssueUpdateParams, PrettyPrint, Project, ProjectUpdateParams, Result, Update, -}; +use crate::{FossaClient, FossaError, PrettyPrint, Project, ProjectUpdateParams, Result, Update}; /// Parameters for `update project`. #[derive(Args, Debug, Clone, PartialEq, Eq, Deserialize, JsonSchema)] @@ -41,39 +37,6 @@ pub struct UpdateProjectParams { pub default_branch: Option, } -/// Parameters for `update issue` (ignore/unignore). -#[derive(Args, Debug, Clone, PartialEq, Eq, Deserialize, JsonSchema)] -#[command(group = clap::ArgGroup::new("issue_action") - .required(true) - .args(["ignore", "unignore"]))] -pub struct UpdateIssueParams { - /// 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, - - /// Ignore the issue. Fails if it is already fully ignored — unignore - /// first to change its notes or reason. - #[arg(long)] - #[serde(default)] - pub ignore: bool, - - /// Revert a previous ignore, returning the issue to active. - #[arg(long)] - #[serde(default)] - pub unignore: bool, - - /// Free-text comment recorded with --ignore. - #[arg(long)] - pub notes: Option, - - /// Structured reason recorded with --ignore. - #[arg(long, value_enum)] - pub reason: Option, -} - /// The `update` 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")] @@ -81,9 +44,6 @@ pub enum UpdateCommand { /// Update a project. #[command(alias = "projects")] Project(UpdateProjectParams), - /// Ignore or unignore an issue. - #[command(alias = "issues")] - Issue(UpdateIssueParams), } /// The result of an [`UpdateCommand`], serialized as the inner entity. @@ -91,16 +51,13 @@ pub enum UpdateCommand { #[serde(untagged)] pub enum UpdateOutput { /// The updated project. - Project(Box), - /// The updated issue, refreshed after the write. - Issue(Box), + Project(Project), } impl PrettyPrint for UpdateOutput { fn pretty_print(&self) -> String { match self { UpdateOutput::Project(p) => p.pretty_print(), - UpdateOutput::Issue(i) => i.pretty_print(), } } } @@ -130,37 +87,7 @@ pub async fn run_update(client: &FossaClient, command: UpdateCommand) -> Result< policy_id: p.policy_id, default_branch: p.default_branch, }; - UpdateOutput::Project(Box::new(Project::update(client, p.locator, params).await?)) - } - UpdateCommand::Issue(p) => { - // clap's ArgGroup enforces exactly-one on the CLI; MCP arguments - // bypass clap, so re-check here. - let action = match (p.ignore, p.unignore) { - (true, false) => IssueAction::Ignore { - notes: p.notes, - reason: p.reason, - }, - (false, true) => { - if p.notes.is_some() || p.reason.is_some() { - return Err(FossaError::InvalidParams( - "notes and reason only apply when ignoring; unignore removes \ - the existing resolution" - .to_string(), - )); - } - IssueAction::Unignore - } - _ => { - return Err(FossaError::InvalidParams( - "update issue requires exactly one of ignore or unignore".to_string(), - )) - } - }; - let params = IssueUpdateParams { - category: p.category, - action, - }; - UpdateOutput::Issue(Box::new(Issue::update(client, p.id, params).await?)) + UpdateOutput::Project(Project::update(client, p.locator, params).await?) } }) } diff --git a/tests/cli_args.rs b/tests/cli_args.rs index dc409b7..0cc07bd 100644 --- a/tests/cli_args.rs +++ b/tests/cli_args.rs @@ -5,10 +5,13 @@ //! 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, UpdateIssueParams, UpdateProjectParams, + GetIssueParams, GetProjectParams, GetRevisionParams, IgnoreIssueParams, ListDependenciesParams, + ListIssuesParams, ListProjectsParams, ListRevisionsParams, PageArgs, UnignoreIssueParams, + UpdateProjectParams, }; use fossapi::{IssueCategory, IssueIgnoreReason}; @@ -484,132 +487,88 @@ fn test_cli_parses_mcp_with_verbose_flag() { } // ============================================================================= -// Issue update flags (ignore / unignore with notes) +// ignore / unignore verbs // ============================================================================= #[test] -fn test_update_issue_ignore_with_notes_and_reason() { +fn test_ignore_issue_with_notes_and_reason() { let cli = Cli::parse_from([ "fossapi", - "update", + "ignore", "issue", "987654", "--category", - "licensing", - "--ignore", + "vulnerability", "--notes", "false positive patch", "--reason", "other", ]); match cli.command { - Command::Update { + Command::Ignore { command: - UpdateCommand::Issue(UpdateIssueParams { + IgnoreCommand::Issue(IgnoreIssueParams { id, category, - ignore, - unignore, notes, reason, }), } => { assert_eq!(id, 987654); - assert_eq!(category, IssueCategory::Licensing); - assert!(ignore); - assert!(!unignore); + assert_eq!(category, IssueCategory::Vulnerability); assert_eq!(notes, Some("false positive patch".to_string())); assert_eq!(reason, Some(IssueIgnoreReason::Other)); } - _ => panic!("Expected Update command"), + _ => panic!("Expected Ignore command"), } } #[test] -fn test_update_issue_unignore() { +fn test_unignore_issue() { let cli = Cli::parse_from([ "fossapi", - "update", + "unignore", "issue", "987654", "--category", - "vulnerability", - "--unignore", + "licensing", ]); match cli.command { - Command::Update { - command: - UpdateCommand::Issue(UpdateIssueParams { - ignore, unignore, .. - }), + Command::Unignore { + command: UnignoreCommand::Issue(UnignoreIssueParams { id, category }), } => { - assert!(!ignore); - assert!(unignore); + assert_eq!(id, 987654); + assert_eq!(category, IssueCategory::Licensing); } - _ => panic!("Expected Update command"), + _ => panic!("Expected Unignore command"), } } #[test] -fn test_update_issue_requires_category() { - let result = Cli::try_parse_from(["fossapi", "update", "issue", "987654", "--ignore"]); +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_update_issue_requires_an_action() { - let result = Cli::try_parse_from([ - "fossapi", - "update", - "issue", - "987654", - "--category", - "licensing", - ]); - assert!(result.is_err(), "one of --ignore/--unignore is required"); +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_update_issue_ignore_conflicts_with_unignore() { +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", - "update", + "unignore", "issue", "987654", "--category", "licensing", - "--ignore", - "--unignore", - ]); - assert!(result.is_err(), "--ignore and --unignore are exclusive"); -} - -#[test] -fn test_update_issue_notes_with_unignore_parses_but_is_rejected_later() { - // clap can't tie --notes to --ignore (SetTrue flags defeat `requires`, - // and MCP bypasses clap entirely), so the parse succeeds and run_update - // rejects the combination at runtime for both surfaces. - let cli = Cli::parse_from([ - "fossapi", - "update", - "issue", - "987654", - "--category", - "licensing", - "--unignore", "--notes", "orphan comment", ]); - match cli.command { - Command::Update { - command: - UpdateCommand::Issue(UpdateIssueParams { - unignore, notes, .. - }), - } => { - assert!(unignore); - assert_eq!(notes, Some("orphan comment".to_string())); - } - _ => panic!("Expected Update command"), - } + 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 c086d63..8998f01 100644 --- a/tests/e2e_mock_server.rs +++ b/tests/e2e_mock_server.rs @@ -256,6 +256,28 @@ async fn test_ignore_and_unignore_issue_workflow() { 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, @@ -264,7 +286,7 @@ async fn test_ignore_and_unignore_issue_workflow() { category: IssueCategory::Licensing, action: IssueAction::Ignore { notes: Some("false positive patch".to_string()), - reason: Some(IssueIgnoreReason::Other), + reason: None, }, }, ) diff --git a/tests/parity.rs b/tests/parity.rs index 5af61f4..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(), + ), ] } @@ -198,29 +206,32 @@ fn tagged_payloads_round_trip() { "title": "New Title" })) .expect("update payload deserializes"); - let UpdateCommand::Project(p) = update else { - panic!("expected Project variant"); - }; + let UpdateCommand::Project(p) = update; assert_eq!(p.locator, "custom+org/repo"); assert_eq!(p.title.as_deref(), Some("New Title")); assert_eq!(p.description, None); - let update: UpdateCommand = serde_json::from_value(serde_json::json!({ + let ignore: IgnoreCommand = serde_json::from_value(serde_json::json!({ "entity": "issue", "id": 987654, - "category": "licensing", - "ignore": true, + "category": "vulnerability", "notes": "false positive patch", "reason": "other" })) - .expect("update issue payload deserializes"); - let UpdateCommand::Issue(p) = update else { - panic!("expected Issue variant"); - }; + .expect("ignore payload deserializes"); + let IgnoreCommand::Issue(p) = ignore; assert_eq!(p.id, 987654); - assert_eq!(p.category, fossapi::IssueCategory::Licensing); - assert!(p.ignore); - assert!(!p.unignore); + 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); } From 60b75b1144ef2d56399d97cbd6966b8ef57900d6 Mon Sep 17 00:00:00 2001 From: Sara Date: Tue, 18 Aug 2026 12:47:41 -0400 Subject: [PATCH 3/3] Address CodeRabbit review: wrap post-write refresh errors, pin kebab-case reasons The post-write refresh in Issue::update returned a bare read error, making a successful ignore/unignore look like a failed write. Wrap it so callers know the write applied. Also add a CLI parse test for a multi-word --reason value, pinning clap's kebab-case value names where they diverge from the API strings. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01C5iUcFvos1Kf7kibnRKdBX --- src/models/issue.rs | 16 +++++++++++++++- tests/cli_args.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/models/issue.rs b/src/models/issue.rs index 1c42520..a8a29e1 100644 --- a/src/models/issue.rs +++ b/src/models/issue.rs @@ -1362,6 +1362,20 @@ impl Update for Issue { }); } - Issue::get_with_category(client, id, params.category).await + // 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/tests/cli_args.rs b/tests/cli_args.rs index 0cc07bd..f28486b 100644 --- a/tests/cli_args.rs +++ b/tests/cli_args.rs @@ -523,6 +523,33 @@ fn test_ignore_issue_with_notes_and_reason() { } } +// 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([