diff --git a/src/compile/common.rs b/src/compile/common.rs index 5693c212..4c420d2e 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -678,6 +678,17 @@ const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = &[ "update-work-item", "create-wiki-page", "update-wiki-page", + "add-pr-comment", + "link-work-items", + "queue-build", + "create-git-tag", + "add-build-tag", + "create-branch", + "update-pr", + "upload-attachment", + "submit-pr-review", + "reply-to-pr-review-comment", + "resolve-pr-review-thread", ]; /// Validate that write-requiring safe-outputs have a write service connection configured. @@ -759,6 +770,118 @@ pub fn validate_update_work_item_target(front_matter: &FrontMatter) -> Result<() Ok(()) } +/// Validate that submit-pr-review has a required `allowed-events` field when configured. +/// +/// An empty or missing `allowed-events` list would allow agents to cast any review vote, +/// including auto-approvals. Operators must explicitly opt in to each allowed event. +pub fn validate_submit_pr_review_events(front_matter: &FrontMatter) -> Result<()> { + if let Some(config_value) = front_matter.safe_outputs.get("submit-pr-review") { + if let Some(obj) = config_value.as_object() { + let allowed_events = obj.get("allowed-events"); + let is_empty = match allowed_events { + None => true, + Some(v) => v.as_array().map_or(true, |a| a.is_empty()), + }; + if is_empty { + anyhow::bail!( + "safe-outputs.submit-pr-review requires a non-empty 'allowed-events' list \ + to prevent agents from casting unrestricted review votes. Example:\n\n \ + safe-outputs:\n submit-pr-review:\n allowed-events:\n \ + - comment\n - approve-with-suggestions\n\n\ + Valid events: approve, approve-with-suggestions, request-changes, comment\n" + ); + } + } else { + anyhow::bail!( + "safe-outputs.submit-pr-review must be a configuration object with an \ + 'allowed-events' list. Example:\n\n \ + safe-outputs:\n submit-pr-review:\n allowed-events:\n - comment\n" + ); + } + } + Ok(()) +} + +/// Validate that update-pr has a required `allowed-votes` field when the `vote` operation +/// is enabled (i.e., `allowed-operations` is empty — meaning all ops — or explicitly contains +/// "vote"). +/// +/// An empty `allowed-votes` list when vote is enabled would always fail at Stage 2 with a +/// runtime error. Catching this at compile time is consistent with how +/// `validate_submit_pr_review_events` handles the analogous case. +pub fn validate_update_pr_votes(front_matter: &FrontMatter) -> Result<()> { + if let Some(config_value) = front_matter.safe_outputs.get("update-pr") { + if let Some(obj) = config_value.as_object() { + // Determine whether the vote operation is reachable: + // - allowed-operations absent or empty → all operations allowed (includes vote) + // - allowed-operations non-empty → vote is allowed only if explicitly listed + let vote_reachable = match obj.get("allowed-operations") { + None => true, + Some(v) => v + .as_array() + .map_or(true, |a| a.is_empty() || a.iter().any(|x| x == "vote")), + }; + + if vote_reachable { + let allowed_votes_empty = match obj.get("allowed-votes") { + None => true, + Some(v) => v.as_array().map_or(true, |a| a.is_empty()), + }; + if allowed_votes_empty { + anyhow::bail!( + "safe-outputs.update-pr enables the 'vote' operation but has no \ + 'allowed-votes' list. This would reject all votes at Stage 2. \ + Either restrict 'allowed-operations' to exclude 'vote', or add an \ + explicit 'allowed-votes' list:\n\n \ + safe-outputs:\n update-pr:\n allowed-votes:\n \ + - approve-with-suggestions\n - wait-for-author\n\n\ + Valid votes: approve, approve-with-suggestions, reject, \ + wait-for-author, reset\n" + ); + } + } + } + // If the value is a scalar (e.g. `update-pr: true`) we don't error here — + // the config will default to empty allowed-votes, which is safe (vote always rejected). + } + Ok(()) +} + +/// Validate that resolve-pr-review-thread has a required `allowed-statuses` field when configured. +/// +/// An empty or missing `allowed-statuses` list would let agents set any thread status, +/// including "fixed" or "wontFix" on security-critical review threads. Operators must +/// explicitly opt in to each allowed status transition. +pub fn validate_resolve_pr_thread_statuses(front_matter: &FrontMatter) -> Result<()> { + if let Some(config_value) = front_matter.safe_outputs.get("resolve-pr-review-thread") { + if let Some(obj) = config_value.as_object() { + let allowed_statuses = obj.get("allowed-statuses"); + let is_empty = match allowed_statuses { + None => true, + Some(v) => v.as_array().map_or(true, |a| a.is_empty()), + }; + if is_empty { + anyhow::bail!( + "safe-outputs.resolve-pr-review-thread requires a non-empty \ + 'allowed-statuses' list to prevent agents from manipulating thread \ + statuses without explicit operator consent. Example:\n\n \ + safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n\ + \x20 - fixed\n\n\ + Valid statuses: active, fixed, wont-fix, closed, by-design\n" + ); + } + } else { + anyhow::bail!( + "safe-outputs.resolve-pr-review-thread must be a configuration object \ + with an 'allowed-statuses' list. Example:\n\n \ + safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n\ + \x20 - fixed\n" + ); + } + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -1350,4 +1473,167 @@ mod tests { let result = generate_pipeline_path(&abs_path); assert_eq!(result, "{{ workspace }}/agents/ctf.yml"); } + + // ─── validate_submit_pr_review_events ──────────────────────────────────── + + #[test] + fn test_submit_pr_review_events_passes_when_not_configured() { + let fm = minimal_front_matter(); + assert!(validate_submit_pr_review_events(&fm).is_ok()); + } + + #[test] + fn test_submit_pr_review_events_fails_when_allowed_events_missing() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n submit-pr-review:\n allowed-repositories:\n - self\n---\n" + ).unwrap(); + let result = validate_submit_pr_review_events(&fm); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("allowed-events"), "message: {msg}"); + } + + #[test] + fn test_submit_pr_review_events_fails_when_allowed_events_empty() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n submit-pr-review:\n allowed-events: []\n---\n" + ).unwrap(); + let result = validate_submit_pr_review_events(&fm); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("allowed-events"), "message: {msg}"); + } + + #[test] + fn test_submit_pr_review_events_fails_when_value_is_scalar() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n submit-pr-review: true\n---\n" + ).unwrap(); + let result = validate_submit_pr_review_events(&fm); + assert!(result.is_err()); + } + + #[test] + fn test_submit_pr_review_events_passes_when_events_provided() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n submit-pr-review:\n allowed-events:\n - comment\n - approve\n---\n" + ).unwrap(); + assert!(validate_submit_pr_review_events(&fm).is_ok()); + } + + // ─── validate_update_pr_votes ───────────────────────────────────────────── + + #[test] + fn test_update_pr_votes_passes_when_not_configured() { + let fm = minimal_front_matter(); + assert!(validate_update_pr_votes(&fm).is_ok()); + } + + #[test] + fn test_update_pr_votes_fails_when_vote_reachable_and_no_allowed_votes() { + // allowed-operations absent → vote is reachable; no allowed-votes → should fail + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n update-pr:\n allowed-repositories:\n - self\n---\n" + ).unwrap(); + let result = validate_update_pr_votes(&fm); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("allowed-votes"), "message: {msg}"); + } + + #[test] + fn test_update_pr_votes_fails_when_vote_explicit_and_no_allowed_votes() { + // allowed-operations contains "vote"; no allowed-votes → should fail + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n update-pr:\n allowed-operations:\n - vote\n---\n" + ).unwrap(); + let result = validate_update_pr_votes(&fm); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("allowed-votes"), "message: {msg}"); + } + + #[test] + fn test_update_pr_votes_fails_when_allowed_votes_empty() { + // allowed-operations absent; allowed-votes is empty list → should fail + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n update-pr:\n allowed-votes: []\n---\n" + ).unwrap(); + let result = validate_update_pr_votes(&fm); + assert!(result.is_err()); + } + + #[test] + fn test_update_pr_votes_passes_when_vote_excluded_from_allowed_operations() { + // allowed-operations is non-empty and does not contain "vote" → safe, no error + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n update-pr:\n allowed-operations:\n - add-reviewers\n - set-auto-complete\n---\n" + ).unwrap(); + assert!(validate_update_pr_votes(&fm).is_ok()); + } + + #[test] + fn test_update_pr_votes_passes_when_vote_reachable_and_allowed_votes_set() { + // allowed-operations absent; allowed-votes non-empty → OK + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n update-pr:\n allowed-votes:\n - approve-with-suggestions\n---\n" + ).unwrap(); + assert!(validate_update_pr_votes(&fm).is_ok()); + } + + #[test] + fn test_update_pr_votes_passes_when_vote_explicit_and_allowed_votes_set() { + // allowed-operations contains "vote"; allowed-votes non-empty → OK + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n update-pr:\n allowed-operations:\n - vote\n allowed-votes:\n - wait-for-author\n---\n" + ).unwrap(); + assert!(validate_update_pr_votes(&fm).is_ok()); + } + + // ─── validate_resolve_pr_thread_statuses ────────────────────────────────── + + #[test] + fn test_resolve_pr_thread_passes_when_not_configured() { + let fm = minimal_front_matter(); + assert!(validate_resolve_pr_thread_statuses(&fm).is_ok()); + } + + #[test] + fn test_resolve_pr_thread_fails_when_allowed_statuses_missing() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-repositories:\n - self\n---\n" + ).unwrap(); + let result = validate_resolve_pr_thread_statuses(&fm); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("allowed-statuses"), "message: {msg}"); + } + + #[test] + fn test_resolve_pr_thread_fails_when_allowed_statuses_empty() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-statuses: []\n---\n" + ).unwrap(); + let result = validate_resolve_pr_thread_statuses(&fm); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("allowed-statuses"), "message: {msg}"); + } + + #[test] + fn test_resolve_pr_thread_fails_when_value_is_scalar() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread: true\n---\n" + ).unwrap(); + let result = validate_resolve_pr_thread_statuses(&fm); + assert!(result.is_err()); + } + + #[test] + fn test_resolve_pr_thread_passes_when_statuses_provided() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n - fixed\n - wont-fix\n---\n" + ).unwrap(); + assert!(validate_resolve_pr_thread_statuses(&fm).is_ok()); + } } diff --git a/src/compile/onees.rs b/src/compile/onees.rs index 470eea74..55caf74d 100644 --- a/src/compile/onees.rs +++ b/src/compile/onees.rs @@ -24,7 +24,8 @@ use super::common::{ generate_pipeline_resources, generate_pr_trigger, generate_repositories, generate_schedule, generate_source_path, generate_working_directory, replace_with_indent, validate_comment_target, validate_update_work_item_target, - validate_write_permissions, + validate_write_permissions, validate_submit_pr_review_events, + validate_update_pr_votes, validate_resolve_pr_thread_statuses, }; use super::types::{FrontMatter, McpConfig}; @@ -139,6 +140,12 @@ displayName: "Finalize""#, validate_comment_target(front_matter)?; // Validate update-work-item has required target field validate_update_work_item_target(front_matter)?; + // Validate submit-pr-review has required allowed-events field + validate_submit_pr_review_events(front_matter)?; + // Validate update-pr vote operation has required allowed-votes field + validate_update_pr_votes(front_matter)?; + // Validate resolve-pr-review-thread has required allowed-statuses field + validate_resolve_pr_thread_statuses(front_matter)?; // Replace all template markers let compiler_version = env!("CARGO_PKG_VERSION"); diff --git a/src/compile/standalone.rs b/src/compile/standalone.rs index 804b7c21..a0dd20c1 100644 --- a/src/compile/standalone.rs +++ b/src/compile/standalone.rs @@ -21,7 +21,8 @@ use super::common::{ generate_pipeline_path, generate_pipeline_resources, generate_pr_trigger, generate_repositories, generate_schedule, generate_source_path, generate_working_directory, replace_with_indent, sanitize_filename, - validate_write_permissions, validate_comment_target, validate_update_work_item_target, + validate_write_permissions, validate_comment_target, validate_update_work_item_target, validate_submit_pr_review_events, + validate_update_pr_votes, validate_resolve_pr_thread_statuses, }; use super::types::{FrontMatter, McpConfig}; use crate::allowed_hosts::{CORE_ALLOWED_HOSTS, mcp_required_hosts}; @@ -130,6 +131,12 @@ impl Compiler for StandaloneCompiler { validate_comment_target(front_matter)?; // Validate update-work-item has required target field validate_update_work_item_target(front_matter)?; + // Validate submit-pr-review has required allowed-events field + validate_submit_pr_review_events(front_matter)?; + // Validate update-pr vote operation has required allowed-votes field + validate_update_pr_votes(front_matter)?; + // Validate resolve-pr-review-thread has required allowed-statuses field + validate_resolve_pr_thread_statuses(front_matter)?; // Load threat analysis prompt template let threat_analysis_prompt = include_str!("../../templates/threat-analysis.md"); diff --git a/src/execute.rs b/src/execute.rs index b59c5301..9a7bea8c 100644 --- a/src/execute.rs +++ b/src/execute.rs @@ -10,10 +10,14 @@ use std::collections::HashMap; use std::path::Path; use crate::ndjson::{self, SAFE_OUTPUT_FILENAME}; +use crate::sanitize::Sanitize; use crate::tools::{ + AddBuildTagResult, AddPrCommentResult, CreateBranchResult, CreateGitTagResult, CreatePrResult, CreateWikiPageResult, CreateWorkItemResult, CommentOnWorkItemResult, - ExecutionContext, ExecutionResult, Executor, ToolResult, - UpdateWikiPageResult, UpdateWorkItemResult, + ExecutionContext, ExecutionResult, Executor, LinkWorkItemsResult, QueueBuildResult, + ReplyToPrCommentResult, ReportIncompleteResult, ResolvePrThreadResult, SubmitPrReviewResult, + ToolResult, UpdatePrResult, UpdateWikiPageResult, UpdateWorkItemResult, + UploadAttachmentResult, }; // Re-export memory types for use by main.rs @@ -113,6 +117,17 @@ pub async fn execute_safe_outputs( CommentOnWorkItemResult, CreateWikiPageResult, UpdateWikiPageResult, + AddPrCommentResult, + LinkWorkItemsResult, + QueueBuildResult, + CreateGitTagResult, + AddBuildTagResult, + CreateBranchResult, + UpdatePrResult, + UploadAttachmentResult, + SubmitPrReviewResult, + ReplyToPrCommentResult, + ResolvePrThreadResult, ); let mut results = Vec::new(); @@ -264,10 +279,116 @@ pub async fn execute_safe_output( ); output.execute_sanitized(ctx).await? } + "add-pr-comment" => { + debug!("Parsing add-pr-comment payload"); + let mut output: AddPrCommentResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse add-pr-comment: {}", e))?; + debug!( + "add-pr-comment: pr_id={}, content length={}", + output.pull_request_id, + output.content.len() + ); + output.execute_sanitized(ctx).await? + } + "link-work-items" => { + debug!("Parsing link-work-items payload"); + let mut output: LinkWorkItemsResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse link-work-items: {}", e))?; + debug!( + "link-work-items: source={}, target={}, type={}", + output.source_id, output.target_id, output.link_type + ); + output.execute_sanitized(ctx).await? + } + "queue-build" => { + debug!("Parsing queue-build payload"); + let mut output: QueueBuildResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse queue-build: {}", e))?; + debug!("queue-build: pipeline_id={}", output.pipeline_id); + output.execute_sanitized(ctx).await? + } + "create-git-tag" => { + debug!("Parsing create-git-tag payload"); + let mut output: CreateGitTagResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse create-git-tag: {}", e))?; + debug!("create-git-tag: tag_name='{}'", output.tag_name); + output.execute_sanitized(ctx).await? + } + "add-build-tag" => { + debug!("Parsing add-build-tag payload"); + let mut output: AddBuildTagResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse add-build-tag: {}", e))?; + debug!("add-build-tag: build_id={}, tag='{}'", output.build_id, output.tag); + output.execute_sanitized(ctx).await? + } + "create-branch" => { + debug!("Parsing create-branch payload"); + let mut output: CreateBranchResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse create-branch: {}", e))?; + debug!("create-branch: branch_name='{}'", output.branch_name); + output.execute_sanitized(ctx).await? + } + "update-pr" => { + debug!("Parsing update-pr payload"); + let mut output: UpdatePrResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse update-pr: {}", e))?; + debug!( + "update-pr: pr_id={}, operation='{}'", + output.pull_request_id, output.operation + ); + output.execute_sanitized(ctx).await? + } + "upload-attachment" => { + debug!("Parsing upload-attachment payload"); + let mut output: UploadAttachmentResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse upload-attachment: {}", e))?; + debug!( + "upload-attachment: work_item_id={}, file_path='{}'", + output.work_item_id, output.file_path + ); + output.execute_sanitized(ctx).await? + } + "submit-pr-review" => { + debug!("Parsing submit-pr-review payload"); + let mut output: SubmitPrReviewResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse submit-pr-review: {}", e))?; + debug!( + "submit-pr-review: pr_id={}, event='{}'", + output.pull_request_id, output.event + ); + output.execute_sanitized(ctx).await? + } + "reply-to-pr-review-comment" => { + debug!("Parsing reply-to-pr-review-comment payload"); + let mut output: ReplyToPrCommentResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse reply-to-pr-review-comment: {}", e))?; + debug!( + "reply-to-pr-review-comment: pr_id={}, thread_id={}", + output.pull_request_id, output.thread_id + ); + output.execute_sanitized(ctx).await? + } + "resolve-pr-review-thread" => { + debug!("Parsing resolve-pr-review-thread payload"); + let mut output: ResolvePrThreadResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse resolve-pr-review-thread: {}", e))?; + debug!( + "resolve-pr-review-thread: pr_id={}, thread_id={}, status='{}'", + output.pull_request_id, output.thread_id, output.status + ); + output.execute_sanitized(ctx).await? + } "noop" => { debug!("Skipping noop entry"); ExecutionResult::success("Skipped informational output: noop") } + "report-incomplete" => { + let mut output: ReportIncompleteResult = serde_json::from_value(entry.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse report-incomplete: {}", e))?; + output.sanitize_fields(); + debug!("report-incomplete: {}", output.reason); + ExecutionResult::failure(format!("Agent reported task incomplete: {}", output.reason)) + } "missing-tool" => { debug!("Skipping missing-tool entry"); ExecutionResult::success("Skipped informational output: missing-tool") diff --git a/src/mcp.rs b/src/mcp.rs index 0d1c2b37..0abca5a9 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -11,12 +11,23 @@ use std::path::PathBuf; use crate::ndjson::{self, SAFE_OUTPUT_FILENAME}; use crate::sanitize::{Sanitize, sanitize as sanitize_text}; use crate::tools::{ + AddBuildTagParams, AddBuildTagResult, + AddPrCommentParams, AddPrCommentResult, CommentOnWorkItemParams, CommentOnWorkItemResult, + CreateBranchParams, CreateBranchResult, + CreateGitTagParams, CreateGitTagResult, CreatePrParams, CreatePrResult, CreateWikiPageParams, CreateWikiPageResult, CreateWorkItemParams, CreateWorkItemResult, + LinkWorkItemsParams, LinkWorkItemsResult, + ReplyToPrCommentParams, ReplyToPrCommentResult, + ReportIncompleteParams, ReportIncompleteResult, + ResolvePrThreadParams, ResolvePrThreadResult, UpdateWikiPageParams, UpdateWikiPageResult, MissingDataParams, MissingDataResult, - MissingToolParams, MissingToolResult, NoopParams, NoopResult, ToolResult, + MissingToolParams, MissingToolResult, NoopParams, NoopResult, QueueBuildParams, + QueueBuildResult, SubmitPrReviewParams, SubmitPrReviewResult, ToolResult, + UpdatePrParams, UpdatePrResult, UpdateWorkItemParams, UpdateWorkItemResult, + UploadAttachmentParams, UploadAttachmentResult, anyhow_to_mcp_error, }; @@ -533,6 +544,290 @@ structured output that should be visible in the project wiki." result.path ))])) } + + #[tool( + name = "add-pr-comment", + description = "Add a comment thread to an Azure DevOps pull request. Supports both \ +general comments and file-specific inline comments with optional line positioning. \ +The comment will be posted during safe output processing." + )] + async fn add_pr_comment( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: add-pr-comment - PR #{}", + params.0.pull_request_id + ); + debug!("Content length: {} chars", params.0.content.len()); + let mut sanitized = params.0; + sanitized.content = sanitize_text(&sanitized.content); + let result: AddPrCommentResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + info!("PR comment queued for PR #{}", result.pull_request_id); + Ok(CallToolResult::success(vec![Content::text(format!( + "Comment queued for PR #{}. The comment will be posted during safe output processing.", + result.pull_request_id + ))])) + } + + #[tool( + name = "link-work-items", + description = "Create a relationship link between two Azure DevOps work items. \ +Supported link types: parent, child, related, predecessor, successor, duplicate, duplicate-of. \ +The link will be created during safe output processing." + )] + async fn link_work_items( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: link-work-items - {} -> {} ({})", + params.0.source_id, params.0.target_id, params.0.link_type + ); + let mut sanitized = params.0; + sanitized.comment = sanitized.comment.map(|c| sanitize_text(&c)); + let result: LinkWorkItemsResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Link queued: work item #{} → #{} ({}). The link will be created during safe output processing.", + result.source_id, result.target_id, result.link_type + ))])) + } + + #[tool( + name = "queue-build", + description = "Trigger an Azure DevOps pipeline/build run. The pipeline must be in the \ +allowed-pipelines list configured in the pipeline definition. Optionally specify a branch \ +and template parameters." + )] + async fn queue_build( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: queue-build - pipeline {}", + params.0.pipeline_id + ); + let mut sanitized = params.0; + sanitized.reason = sanitized.reason.map(|r| sanitize_text(&r)); + let result: QueueBuildResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Build queued for pipeline {}. The build will be triggered during safe output processing.", + result.pipeline_id + ))])) + } + + #[tool( + name = "create-git-tag", + description = "Create an annotated git tag on a commit in an Azure DevOps repository. \ +The tag will be created during safe output processing." + )] + async fn create_git_tag( + &self, + params: Parameters, + ) -> Result { + info!("Tool called: create-git-tag - '{}'", params.0.tag_name); + let mut sanitized = params.0; + sanitized.message = sanitized.message.map(|m| sanitize_text(&m)); + let result: CreateGitTagResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Git tag '{}' queued. The tag will be created during safe output processing.", + result.tag_name + ))])) + } + + #[tool( + name = "add-build-tag", + description = "Add a tag to an Azure DevOps build for classification and filtering. \ +The tag will be added during safe output processing." + )] + async fn add_build_tag( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: add-build-tag - build {} tag '{}'", + params.0.build_id, params.0.tag + ); + let result: AddBuildTagResult = params.0.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Build tag '{}' queued for build #{}. The tag will be added during safe output processing.", + result.tag, result.build_id + ))])) + } + + #[tool( + name = "create-branch", + description = "Create a new branch in an Azure DevOps repository without creating a \ +pull request. The branch will be created during safe output processing." + )] + async fn create_branch( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: create-branch - '{}'", + params.0.branch_name + ); + let result: CreateBranchResult = params.0.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Branch '{}' queued for creation. The branch will be created during safe output processing.", + result.branch_name + ))])) + } + + #[tool( + name = "update-pr", + description = "Update pull request metadata in Azure DevOps. Supports operations: \ +add-reviewers, add-labels, set-auto-complete, vote, update-description. \ +Changes will be applied during safe output processing." + )] + async fn update_pr( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: update-pr - PR #{} operation '{}'", + params.0.pull_request_id, params.0.operation + ); + let mut sanitized = params.0; + sanitized.description = sanitized.description.map(|d| sanitize_text(&d)); + let result: UpdatePrResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "PR #{} '{}' operation queued. Changes will be applied during safe output processing.", + result.pull_request_id, result.operation + ))])) + } + + #[tool( + name = "upload-attachment", + description = "Upload a file attachment to an Azure DevOps work item. The file will be \ +uploaded and linked during safe output processing. File size and type restrictions may apply." + )] + async fn upload_attachment( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: upload-attachment - work item #{} file '{}'", + params.0.work_item_id, params.0.file_path + ); + let mut sanitized = params.0; + sanitized.comment = sanitized.comment.map(|c| sanitize_text(&c)); + let result: UploadAttachmentResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Attachment '{}' queued for work item #{}. The file will be uploaded during safe output processing.", + result.file_path, result.work_item_id + ))])) + } + + #[tool( + name = "submit-pr-review", + description = "Submit a pull request review with a decision (approve, request-changes, \ +or comment-only) and an optional body explaining the rationale. The review will be \ +submitted during safe output processing. Requires 'allowed-events' to be configured." + )] + async fn submit_pr_review( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: submit-pr-review - PR #{} event '{}'", + params.0.pull_request_id, params.0.event + ); + let mut sanitized = params.0; + sanitized.body = sanitized.body.map(|b| sanitize_text(&b)); + let result: SubmitPrReviewResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "PR review '{}' queued for PR #{}. The review will be submitted during safe output processing.", + result.event, result.pull_request_id + ))])) + } + + #[tool( + name = "reply-to-pr-review-comment", + description = "Reply to an existing review comment thread on an Azure DevOps pull request. \ +Provide the PR ID, thread ID, and reply content. The reply will be posted during safe output processing." + )] + async fn reply_to_pr_review_comment( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: reply-to-pr-review-comment - PR #{} thread #{}", + params.0.pull_request_id, params.0.thread_id + ); + let mut sanitized = params.0; + sanitized.content = sanitize_text(&sanitized.content); + let result: ReplyToPrCommentResult = sanitized.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Reply queued for thread #{} on PR #{}. The reply will be posted during safe output processing.", + result.thread_id, result.pull_request_id + ))])) + } + + #[tool( + name = "resolve-pr-review-thread", + description = "Resolve or change the status of a review thread on an Azure DevOps pull request. \ +Valid statuses: fixed, wont-fix, closed, by-design, active. \ +The status change will be applied during safe output processing." + )] + async fn resolve_pr_review_thread( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: resolve-pr-review-thread - PR #{} thread #{} → '{}'", + params.0.pull_request_id, params.0.thread_id, params.0.status + ); + let result: ResolvePrThreadResult = params.0.try_into()?; + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + Ok(CallToolResult::success(vec![Content::text(format!( + "Thread #{} status change to '{}' queued for PR #{}. The change will be applied during safe output processing.", + result.thread_id, result.status, result.pull_request_id + ))])) + } + + #[tool( + name = "report-incomplete", + description = "Signal that the task could not be completed due to infrastructure failure, \ +tool errors, or other environmental issues beyond the agent's control. Use this when the \ +agent attempted work but couldn't finish (e.g., API timeouts, build failures, resource limits)." + )] + async fn report_incomplete( + &self, + params: Parameters, + ) -> Result { + warn!("Tool called: report-incomplete - '{}'", params.0.reason); + let mut sanitized = params.0; + sanitized.reason = sanitize_text(&sanitized.reason); + sanitized.context = sanitized.context.map(|c| sanitize_text(&c)); + let result: ReportIncompleteResult = sanitized.try_into()?; + if let Err(e) = self.write_safe_output_file(&result).await { + warn!("Failed to write report-incomplete safe output: {}", e); + } + Ok(CallToolResult::success(vec![])) + } } // Implement the server handler diff --git a/src/ndjson.rs b/src/ndjson.rs index e75d13d8..ceb63113 100644 --- a/src/ndjson.rs +++ b/src/ndjson.rs @@ -51,6 +51,9 @@ pub async fn append_to_ndjson_file(path: &Path, value: &T) -> Res file.write_all(line.as_bytes()) .await .with_context(|| format!("Failed to write to NDJSON file: {}", path.display()))?; + file.flush() + .await + .with_context(|| format!("Failed to flush NDJSON file: {}", path.display()))?; debug!("Appended {} bytes to NDJSON", line.len()); Ok(()) } diff --git a/src/sanitize.rs b/src/sanitize.rs index 598b9bfe..136ebd87 100644 --- a/src/sanitize.rs +++ b/src/sanitize.rs @@ -34,6 +34,7 @@ const MAX_LINE_COUNT: usize = 65_536; /// 7. Apply content size limits (IS-08) pub fn sanitize(input: &str) -> String { let mut s = remove_control_characters(input); + s = neutralize_pipeline_commands(&s); s = neutralize_mentions(&s); s = neutralize_bot_triggers(&s); s = remove_xml_comments(&s); @@ -85,6 +86,37 @@ fn remove_control_characters(input: &str) -> String { result } +// ── Azure DevOps pipeline command neutralization ─────────────────────────── + +/// Neutralize `##vso[` logging command sequences that Azure DevOps interprets +/// when they appear in pipeline stdout/stderr. Wraps them in backticks so they +/// render as code instead of being executed. +/// +/// Also handles `##[` (the shorthand form used for `##[section]`, `##[error]`, +/// etc.) which ADO pipelines also interpret. +fn neutralize_pipeline_commands(input: &str) -> String { + let mut result = String::with_capacity(input.len() + 32); + let mut rest = input; + + while let Some(pos) = rest.find("##") { + result.push_str(&rest[..pos]); + let after = &rest[pos + 2..]; + if after.starts_with("vso[") { + result.push_str("`##vso[`"); + rest = &after[4..]; + } else if after.starts_with('[') { + result.push_str("`##[`"); + rest = &after[1..]; + } else { + // Harmless "##" (e.g. markdown heading) + result.push_str("##"); + rest = after; + } + } + result.push_str(rest); + result +} + // ── IS-04: @mention neutralization ───────────────────────────────────────── /// Wrap @mentions in backticks to prevent unintended notifications. @@ -482,4 +514,32 @@ mod tests { let input = "This is a normal description of a work item."; assert_eq!(sanitize(input), input); } + + // ── Pipeline command neutralization tests ────────────────────────────── + + #[test] + fn test_neutralize_vso_command() { + let input = "##vso[task.setvariable variable=secret]hack"; + let result = sanitize(input); + // The raw ##vso[ should be wrapped in backticks so ADO won't interpret it + assert!(result.contains("`##vso[`")); + // Verify it's not present in its original unescaped form (i.e. without backtick prefix) + assert!(!result.contains("##vso[task.")); + } + + #[test] + fn test_neutralize_vso_shorthand() { + let input = "##[error]Something bad happened"; + let result = sanitize(input); + assert!(result.contains("`##[`")); + } + + #[test] + fn test_neutralize_vso_preserves_single_hash() { + let input = "# Heading\n## Sub-heading\nIssue #123"; + let result = sanitize(input); + assert!(result.contains("# Heading")); + assert!(result.contains("## Sub-heading")); + assert!(result.contains("#123")); + } } diff --git a/src/tools/add_build_tag.rs b/src/tools/add_build_tag.rs new file mode 100644 index 00000000..536ce236 --- /dev/null +++ b/src/tools/add_build_tag.rs @@ -0,0 +1,316 @@ +//! Add build tag safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::PATH_SEGMENT; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +// ── Stage 1: Params (agent-provided) ────────────────────────────────────── + +/// Parameters for adding a tag to an Azure DevOps build +#[derive(Deserialize, JsonSchema)] +pub struct AddBuildTagParams { + /// Build ID to tag (must be positive) + pub build_id: i32, + /// Tag string (1-100 chars, alphanumeric + dashes only) + pub tag: String, +} + +impl Validate for AddBuildTagParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!(self.build_id > 0, "build_id must be positive"); + ensure!(!self.tag.is_empty(), "tag must not be empty"); + ensure!( + self.tag.len() <= 100, + "tag must be at most 100 characters" + ); + ensure!( + self.tag + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'-'), + "tag must contain only alphanumeric characters and dashes" + ); + Ok(()) + } +} + +// ── Stage 1: Result (generated by macro) ────────────────────────────────── + +tool_result! { + name = "add-build-tag", + params = AddBuildTagParams, + /// Result of adding a tag to a build + pub struct AddBuildTagResult { + build_id: i32, + tag: String, + } +} + +// ── Stage 2: Sanitization ───────────────────────────────────────────────── + +impl Sanitize for AddBuildTagResult { + fn sanitize_fields(&mut self) { + self.tag = sanitize_text(&self.tag); + } +} + +// ── Stage 2: Configuration (from front matter) ──────────────────────────── + +/// Configuration for the add-build-tag tool +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// add-build-tag: +/// allowed-tags: +/// - verified +/// - release-candidate +/// tag-prefix: "agent-" +/// allow-any-build: false +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct AddBuildTagConfig { + /// Restrict which tags can be applied. Empty means any tag is allowed. + /// Supports simple wildcard patterns: entries ending with `*` match by prefix. + #[serde(default, rename = "allowed-tags")] + pub allowed_tags: Vec, + + /// Prefix prepended to all tags before applying + #[serde(default, rename = "tag-prefix")] + pub tag_prefix: Option, + + /// Whether the agent may tag any build. When false (default), only the + /// current pipeline build (BUILD_BUILDID) may be tagged. + #[serde(default, rename = "allow-any-build")] + pub allow_any_build: bool, +} + +impl Default for AddBuildTagConfig { + fn default() -> Self { + Self { + allowed_tags: Vec::new(), + tag_prefix: None, + allow_any_build: false, + } + } +} + +// ── Stage 2: Execution ──────────────────────────────────────────────────── + +#[async_trait::async_trait] +impl Executor for AddBuildTagResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!("Adding tag '{}' to build #{}", self.tag, self.build_id); + + // 1. Extract required context + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available")?; + debug!("ADO org: {}, project: {}", org_url, project); + + // 2. Get tool-specific configuration + let config: AddBuildTagConfig = ctx.get_tool_config("add-build-tag"); + debug!("Config: {:?}", config); + + // 2b. Scope check: by default only the current build can be tagged + if !config.allow_any_build { + let current_build_id: Option = std::env::var("BUILD_BUILDID") + .ok() + .and_then(|s| s.parse().ok()); + if let Some(current_id) = current_build_id { + if self.build_id != current_id { + return Ok(ExecutionResult::failure(format!( + "Build #{} cannot be tagged: only the current build (#{}) is \ + allowed unless 'allow-any-build: true' is configured", + self.build_id, current_id + ))); + } + } + // If BUILD_BUILDID is not set (e.g. local execution), allow any build + } + + // 3. Apply tag prefix if configured + let final_tag = match &config.tag_prefix { + Some(prefix) => format!("{}{}", prefix, self.tag), + None => self.tag.clone(), + }; + debug!("Final tag (after prefix): {}", final_tag); + + // 4. Validate against allowed tags (if non-empty) + if !config.allowed_tags.is_empty() { + let allowed = config.allowed_tags.iter().any(|pattern| { + if let Some(prefix) = pattern.strip_suffix('*') { + final_tag.starts_with(prefix) + } else { + *pattern == final_tag + } + }); + if !allowed { + return Ok(ExecutionResult::failure(format!( + "Tag '{}' is not in the allowed tags list", + final_tag + ))); + } + } + + // 5. Build the API URL + let url = format!( + "{}/{}/_apis/build/builds/{}/tags/{}?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + self.build_id, + utf8_percent_encode(&final_tag, PATH_SEGMENT), + ); + debug!("API URL: {}", url); + + // 6. PUT request (empty body) + let client = reqwest::Client::new(); + info!("Sending PUT to add tag '{}' to build #{}", final_tag, self.build_id); + let response = client + .put(&url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to send request")?; + + if response.status().is_success() { + info!( + "Tag '{}' added to build #{}", + final_tag, self.build_id + ); + + Ok(ExecutionResult::success_with_data( + format!( + "Added tag '{}' to build #{}", + final_tag, self.build_id + ), + serde_json::json!({ + "build_id": self.build_id, + "tag": final_tag, + "project": project, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to add tag (HTTP {}): {}", + status, error_body + ))) + } + } +} + +// ── Tests ───────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(AddBuildTagResult::NAME, "add-build-tag"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{"build_id": 42, "tag": "verified"}"#; + let params: AddBuildTagParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.build_id, 42); + assert_eq!(params.tag, "verified"); + } + + #[test] + fn test_params_converts_to_result() { + let params = AddBuildTagParams { + build_id: 42, + tag: "release-candidate".to_string(), + }; + let result: AddBuildTagResult = params.try_into().unwrap(); + assert_eq!(result.name, "add-build-tag"); + assert_eq!(result.build_id, 42); + assert_eq!(result.tag, "release-candidate"); + } + + #[test] + fn test_validation_rejects_zero_build_id() { + let params = AddBuildTagParams { + build_id: 0, + tag: "verified".to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_empty_tag() { + let params = AddBuildTagParams { + build_id: 42, + tag: "".to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_tag_chars() { + let params = AddBuildTagParams { + build_id: 42, + tag: "invalid tag!".to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = AddBuildTagParams { + build_id: 42, + tag: "verified".to_string(), + }; + let result: AddBuildTagResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + assert!(json.contains(r#""name":"add-build-tag""#)); + assert!(json.contains(r#""build_id":42"#)); + assert!(json.contains(r#""tag":"verified""#)); + } + + #[test] + fn test_config_defaults() { + let config = AddBuildTagConfig::default(); + assert!(config.allowed_tags.is_empty()); + assert!(config.tag_prefix.is_none()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +allowed-tags: + - verified + - release-candidate +tag-prefix: "agent-" +"#; + let config: AddBuildTagConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.allowed_tags, vec!["verified", "release-candidate"]); + assert_eq!(config.tag_prefix, Some("agent-".to_string())); + } +} diff --git a/src/tools/add_pr_comment.rs b/src/tools/add_pr_comment.rs new file mode 100644 index 00000000..b6672a5c --- /dev/null +++ b/src/tools/add_pr_comment.rs @@ -0,0 +1,610 @@ +//! Add PR comment safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::PATH_SEGMENT; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Parameters for adding a comment thread on a pull request +#[derive(Deserialize, JsonSchema)] +pub struct AddPrCommentParams { + /// The pull request ID to comment on + pub pull_request_id: i32, + + /// Comment text in markdown format. Ensure adequate content > 10 characters. + pub content: String, + + /// Repository alias: "self" for pipeline repo, or an alias from the checkout list. + /// Defaults to "self" if omitted. + #[serde(default = "default_repository")] + pub repository: String, + + /// File path for an inline comment. When set, the comment is anchored to this file. + #[serde(default)] + pub file_path: Option, + + /// Starting line number for a multi-line inline comment. Requires `file_path` and `line`. + /// When set, the comment spans from `start_line` to `line`. Must be strictly less than + /// `line` (use `line` alone for single-line comments — do not pass `start_line == line`). + #[serde(default)] + pub start_line: Option, + + /// Line number for an inline comment. Requires `file_path` to be set. + #[serde(default)] + pub line: Option, + + /// Thread status: "active" (default), "fixed", "wont-fix", "closed", or "by-design". + /// CamelCase forms ("Active", "WontFix", etc.) are also accepted for backwards compatibility. + #[serde(default = "default_status")] + pub status: String, +} + +fn default_repository() -> String { + "self".to_string() +} + +fn default_status() -> String { + "active".to_string() +} + +impl Validate for AddPrCommentParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!(self.pull_request_id > 0, "pull_request_id must be positive"); + ensure!( + self.content.len() >= 10, + "content must be at least 10 characters" + ); + ensure!( + status_to_int(&self.status).is_some(), + "status must be one of: {}", + VALID_STATUSES.join(", ") + ); + if self.line.is_some() { + ensure!( + self.file_path.is_some(), + "line requires file_path to be set" + ); + } + if self.start_line.is_some() { + ensure!( + self.line.is_some(), + "start_line requires line to be set" + ); + if let (Some(start), Some(end)) = (self.start_line, self.line) { + ensure!( + start < end, + "start_line ({}) must be less than line ({})", + start, + end + ); + } + } + if let Some(fp) = &self.file_path { + validate_file_path(fp)?; + } + Ok(()) + } +} + +tool_result! { + name = "add-pr-comment", + params = AddPrCommentParams, + /// Result of adding a comment thread on a pull request + pub struct AddPrCommentResult { + pull_request_id: i32, + content: String, + repository: String, + file_path: Option, + start_line: Option, + line: Option, + status: String, + } +} + +impl Sanitize for AddPrCommentResult { + fn sanitize_fields(&mut self) { + self.content = sanitize_text(&self.content); + // Strip control characters from structural fields for defense-in-depth + self.repository = self.repository.chars().filter(|c| !c.is_control()).collect(); + self.status = self.status.chars().filter(|c| !c.is_control()).collect(); + self.file_path = self.file_path.as_ref().map(|fp| { + fp.chars().filter(|c| !c.is_control()).collect() + }); + } +} + +/// Configuration for the add-pr-comment tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// add-pr-comment: +/// comment-prefix: "[Agent Review] " +/// allowed-repositories: +/// - self +/// - other-repo +/// allowed-statuses: +/// - Active +/// - Closed +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct AddPrCommentConfig { + /// Prefix prepended to all comments (e.g., "[Agent Review] ") + #[serde(default, rename = "comment-prefix")] + pub comment_prefix: Option, + + /// Restrict which repositories the agent can comment on. + /// If empty, all repositories in the checkout list (plus "self") are allowed. + #[serde(default, rename = "allowed-repositories")] + pub allowed_repositories: Vec, + + /// Restrict which thread statuses can be set. + /// If empty, all valid statuses are allowed. + #[serde(default, rename = "allowed-statuses")] + pub allowed_statuses: Vec, +} + +impl Default for AddPrCommentConfig { + fn default() -> Self { + Self { + comment_prefix: None, + allowed_repositories: Vec::new(), + allowed_statuses: Vec::new(), + } + } +} + +/// Map a thread status string to the ADO API integer value. +/// Accepts both kebab-case (preferred) and CamelCase for backwards compatibility. +fn status_to_int(status: &str) -> Option { + match status { + "active" | "Active" => Some(1), + "fixed" | "Fixed" => Some(2), + "wont-fix" | "WontFix" => Some(3), + "closed" | "Closed" => Some(4), + "by-design" | "ByDesign" => Some(5), + _ => None, + } +} + +/// All valid thread status strings (kebab-case canonical form) +const VALID_STATUSES: &[&str] = &["active", "fixed", "wont-fix", "closed", "by-design"]; + +/// Validate a file path for inline comments: no `..` path traversal, not absolute +fn validate_file_path(path: &str) -> anyhow::Result<()> { + ensure!(!path.is_empty(), "file_path must not be empty"); + ensure!( + !path.split(['/', '\\']).any(|component| component == ".."), + "file_path must not contain a '..' path component" + ); + ensure!( + !path.starts_with('/') && !path.starts_with('\\'), + "file_path must not be absolute" + ); + Ok(()) +} + +#[async_trait::async_trait] +impl Executor for AddPrCommentResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!( + "Adding comment to PR #{}: {} chars", + self.pull_request_id, + self.content.len() + ); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: AddPrCommentConfig = ctx.get_tool_config("add-pr-comment"); + debug!("Config: {:?}", config); + + // Validate repository against allowed-repositories config + if !config.allowed_repositories.is_empty() + && !config.allowed_repositories.contains(&self.repository) + { + return Ok(ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed-repositories list", + self.repository + ))); + } + + // Validate status against allowed-statuses config (case-insensitive) + if !config.allowed_statuses.is_empty() + && !config + .allowed_statuses + .iter() + .any(|s| s.eq_ignore_ascii_case(&self.status)) + { + return Ok(ExecutionResult::failure(format!( + "Status '{}' is not in the allowed-statuses list", + self.status + ))); + } + + // Validate status is a known value + let status_int = match status_to_int(&self.status) { + Some(v) => v, + None => { + return Ok(ExecutionResult::failure(format!( + "Invalid status '{}'. Valid statuses: {}", + self.status, + VALID_STATUSES.join(", ") + ))); + } + }; + + // Validate file_path if present + if let Some(ref fp) = self.file_path { + if let Err(e) = validate_file_path(fp) { + return Ok(ExecutionResult::failure(format!( + "Invalid file_path: {}", + e + ))); + } + } + + // Determine the repository name for the API call + let repo_name = if self.repository == "self" || self.repository.is_empty() { + ctx.repository_name + .as_ref() + .context("BUILD_REPOSITORY_NAME not set and repository is 'self'")? + .clone() + } else { + match ctx.allowed_repositories.get(&self.repository) { + Some(name) => name.clone(), + None => { + return Ok(ExecutionResult::failure(format!( + "Repository alias '{}' not found in allowed repositories", + self.repository + ))); + } + } + }; + + // Build comment content with optional prefix + let comment_body = match &config.comment_prefix { + Some(prefix) => format!("{}{}", prefix, self.content), + None => self.content.clone(), + }; + + // Build the API URL + let url = format!( + "{}/{}/_apis/git/repositories/{}/pullRequests/{}/threads?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(&repo_name, PATH_SEGMENT), + self.pull_request_id, + ); + debug!("API URL: {}", url); + + // Build the request body + let comment_obj = serde_json::json!({ + "parentCommentId": 0, + "content": comment_body, + "commentType": 1 + }); + + let mut thread_body = serde_json::json!({ + "comments": [comment_obj], + "status": status_int + }); + + // Add thread context for inline comments + if let Some(ref fp) = self.file_path { + let end_line = self.line.unwrap_or(1); + let start_line = self.start_line.unwrap_or(end_line); + thread_body["threadContext"] = serde_json::json!({ + "filePath": format!("/{}", fp), + "rightFileStart": { "line": start_line, "offset": 1 }, + "rightFileEnd": { "line": end_line, "offset": 1 } + }); + } + + let client = reqwest::Client::new(); + + info!("Sending comment thread to PR #{}", self.pull_request_id); + let response = client + .post(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&thread_body) + .send() + .await + .context("Failed to send request to Azure DevOps")?; + + if response.status().is_success() { + let body: serde_json::Value = response + .json() + .await + .context("Failed to parse response JSON")?; + + let thread_id = body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + + info!( + "Comment thread added to PR #{}: thread #{}", + self.pull_request_id, thread_id + ); + + Ok(ExecutionResult::success_with_data( + format!( + "Added comment thread #{} to PR #{}", + thread_id, self.pull_request_id + ), + serde_json::json!({ + "thread_id": thread_id, + "pull_request_id": self.pull_request_id, + "repository": repo_name, + "project": project, + "status": self.status, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to add comment to PR #{} (HTTP {}): {}", + self.pull_request_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(AddPrCommentResult::NAME, "add-pr-comment"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{"pull_request_id": 42, "content": "This is a review comment on the PR."}"#; + let params: AddPrCommentParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.pull_request_id, 42); + assert!(params.content.contains("review comment")); + assert_eq!(params.repository, "self"); + assert!(params.file_path.is_none()); + assert!(params.line.is_none()); + assert_eq!(params.status, "active"); + } + + #[test] + fn test_params_converts_to_result() { + let params = AddPrCommentParams { + pull_request_id: 42, + content: "This is a test comment with enough characters.".to_string(), + repository: "self".to_string(), + file_path: None, + start_line: None, + line: None, + status: "active".to_string(), + }; + let result: AddPrCommentResult = params.try_into().unwrap(); + assert_eq!(result.name, "add-pr-comment"); + assert_eq!(result.pull_request_id, 42); + assert!(result.content.contains("test comment")); + } + + #[test] + fn test_validation_rejects_zero_pr_id() { + let params = AddPrCommentParams { + pull_request_id: 0, + content: "This is a valid comment body text.".to_string(), + repository: "self".to_string(), + file_path: None, + start_line: None, + line: None, + status: "active".to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_short_content() { + let params = AddPrCommentParams { + pull_request_id: 42, + content: "Too short".to_string(), + repository: "self".to_string(), + file_path: None, + start_line: None, + line: None, + status: "active".to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_line_without_file_path() { + let params = AddPrCommentParams { + pull_request_id: 42, + content: "This is a valid comment body text.".to_string(), + repository: "self".to_string(), + file_path: None, + start_line: None, + line: Some(10), + status: "active".to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = AddPrCommentParams { + pull_request_id: 42, + content: "A comment body that is definitely longer than ten characters.".to_string(), + repository: "self".to_string(), + file_path: Some("src/main.rs".to_string()), + start_line: None, + line: Some(10), + status: "active".to_string(), + }; + let result: AddPrCommentResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"add-pr-comment""#)); + assert!(json.contains(r#""pull_request_id":42"#)); + } + + #[test] + fn test_config_defaults() { + let config = AddPrCommentConfig::default(); + assert!(config.comment_prefix.is_none()); + assert!(config.allowed_repositories.is_empty()); + assert!(config.allowed_statuses.is_empty()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +comment-prefix: "[Agent Review] " +allowed-repositories: + - self + - other-repo +allowed-statuses: + - Active + - Closed +"#; + let config: AddPrCommentConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.comment_prefix, Some("[Agent Review] ".to_string())); + assert_eq!(config.allowed_repositories, vec!["self", "other-repo"]); + assert_eq!(config.allowed_statuses, vec!["Active", "Closed"]); + } + + #[test] + fn test_status_to_int_mapping() { + // Kebab-case (canonical) + assert_eq!(status_to_int("active"), Some(1)); + assert_eq!(status_to_int("fixed"), Some(2)); + assert_eq!(status_to_int("wont-fix"), Some(3)); + assert_eq!(status_to_int("closed"), Some(4)); + assert_eq!(status_to_int("by-design"), Some(5)); + // CamelCase (backwards compat) + assert_eq!(status_to_int("Active"), Some(1)); + assert_eq!(status_to_int("WontFix"), Some(3)); + assert_eq!(status_to_int("ByDesign"), Some(5)); + // Invalid + assert_eq!(status_to_int("Invalid"), None); + } + + #[test] + fn test_validate_file_path_rejects_traversal() { + assert!(validate_file_path("../etc/passwd").is_err()); + assert!(validate_file_path("src/../secret").is_err()); + } + + #[test] + fn test_validate_file_path_rejects_absolute() { + assert!(validate_file_path("/etc/passwd").is_err()); + assert!(validate_file_path("\\windows\\system32").is_err()); + } + + #[test] + fn test_validate_file_path_accepts_valid() { + assert!(validate_file_path("src/main.rs").is_ok()); + assert!(validate_file_path("path/to/file.txt").is_ok()); + // ".." within a component name is not a traversal — must be accepted + assert!(validate_file_path("releases..notes/v1.md").is_ok()); + assert!(validate_file_path("v2..beta/file.txt").is_ok()); + } + + #[test] + fn test_validation_rejects_invalid_status() { + let params = AddPrCommentParams { + pull_request_id: 42, + content: "This is a valid comment body text.".to_string(), + repository: "self".to_string(), + file_path: None, + start_line: None, + line: None, + status: "unknown".to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!(err.contains("status must be one of")); + } + + #[test] + fn test_validation_accepts_valid_statuses() { + for s in &["active", "fixed", "wont-fix", "closed", "by-design", "Active", "WontFix"] { + let params = AddPrCommentParams { + pull_request_id: 42, + content: "This is a valid comment body text.".to_string(), + repository: "self".to_string(), + file_path: None, + start_line: None, + line: None, + status: s.to_string(), + }; + let result: Result = params.try_into(); + assert!(result.is_ok(), "Expected status '{}' to be valid", s); + } + } + + #[test] + fn test_allowed_statuses_case_insensitive_match() { + // Config has "Active" but agent sends "active" (canonical lowercase) — should be allowed + let config = AddPrCommentConfig { + comment_prefix: None, + allowed_repositories: Vec::new(), + allowed_statuses: vec!["Active".to_string(), "Closed".to_string()], + }; + // Test the exact comparison logic extracted from execute_impl + let status = "active"; + let matched = config + .allowed_statuses + .iter() + .any(|s| s.eq_ignore_ascii_case(status)); + assert!( + matched, + "lowercase 'active' should match config value 'Active'" + ); + } + + #[test] + fn test_allowed_statuses_case_insensitive_reverse() { + // Config has "active" but agent sends "Active" — should be allowed + let config = AddPrCommentConfig { + comment_prefix: None, + allowed_repositories: Vec::new(), + allowed_statuses: vec!["active".to_string()], + }; + let status = "Active"; + let matched = config + .allowed_statuses + .iter() + .any(|s| s.eq_ignore_ascii_case(status)); + assert!( + matched, + "uppercase 'Active' should match config value 'active'" + ); + } +} diff --git a/src/tools/create_branch.rs b/src/tools/create_branch.rs new file mode 100644 index 00000000..6df71669 --- /dev/null +++ b/src/tools/create_branch.rs @@ -0,0 +1,495 @@ +//! Create branch safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::{PATH_SEGMENT, validate_git_ref_name}; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Parameters for creating a branch +#[derive(Deserialize, JsonSchema)] +pub struct CreateBranchParams { + /// Branch name to create (e.g., "feature/my-analysis"). 1-200 characters. + pub branch_name: String, + + /// Branch to create from (default: "main") + pub source_branch: Option, + + /// Specific commit SHA to branch from (overrides source_branch). Must be a valid 40-character hex string. + pub source_commit: Option, + + /// Repository alias: "self" for pipeline repo, or an alias from the checkout list (default: "self") + pub repository: Option, +} + +impl Validate for CreateBranchParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!(!self.branch_name.is_empty(), "branch_name must not be empty"); + ensure!( + self.branch_name.len() <= 200, + "branch_name must be at most 200 characters" + ); + ensure!( + !self.branch_name.contains(".."), + "branch_name must not contain '..'" + ); + ensure!( + !self.branch_name.contains('\0'), + "branch_name must not contain null bytes" + ); + ensure!( + !self.branch_name.starts_with('-'), + "branch_name must not start with '-'" + ); + ensure!( + !self.branch_name.contains(' '), + "branch_name must not contain spaces" + ); + validate_git_ref_name(&self.branch_name, "branch_name")?; + + if let Some(ref commit) = self.source_commit { + ensure!( + commit.len() == 40 && commit.chars().all(|c| c.is_ascii_hexdigit()), + "source_commit must be a valid 40-character hex SHA" + ); + } + + if let Some(ref branch) = self.source_branch { + ensure!( + !branch.contains(".."), + "source_branch must not contain '..'" + ); + ensure!( + !branch.contains('\0'), + "source_branch must not contain null bytes" + ); + validate_git_ref_name(branch, "source_branch")?; + } + + Ok(()) + } +} + +tool_result! { + name = "create-branch", + params = CreateBranchParams, + /// Result of creating a branch + pub struct CreateBranchResult { + branch_name: String, + source_branch: Option, + source_commit: Option, + repository: Option, + } +} + +impl Sanitize for CreateBranchResult { + fn sanitize_fields(&mut self) { + self.branch_name = sanitize_text(&self.branch_name); + } +} + +/// Configuration for the create-branch tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// create-branch: +/// branch-pattern: "^agent/.*" +/// allowed-repositories: +/// - self +/// - other-repo +/// allowed-source-branches: +/// - main +/// - develop +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CreateBranchConfig { + /// Regex pattern that branch names must match + #[serde(default, rename = "branch-pattern")] + pub branch_pattern: Option, + + /// Repositories the agent is allowed to create branches in + #[serde(default, rename = "allowed-repositories")] + pub allowed_repositories: Vec, + + /// Source branches the agent is allowed to branch from + #[serde(default, rename = "allowed-source-branches")] + pub allowed_source_branches: Vec, +} + +impl Default for CreateBranchConfig { + fn default() -> Self { + Self { + branch_pattern: None, + allowed_repositories: Vec::new(), + allowed_source_branches: Vec::new(), + } + } +} + +/// Resolve a branch name to its latest commit SHA via the Azure DevOps refs API +async fn resolve_branch_to_commit( + client: &reqwest::Client, + org_url: &str, + project: &str, + token: &str, + repo_name: &str, + branch: &str, +) -> anyhow::Result { + let url = format!( + "{}/{}/_apis/git/repositories/{}/refs", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(repo_name, PATH_SEGMENT), + ); + debug!("Resolving branch '{}' via: {}", branch, url); + + let response = client + .get(&url) + .query(&[ + ("filter", format!("heads/{}", branch).as_str()), + ("api-version", "7.1"), + ]) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to query refs API")?; + + if !response.status().is_success() { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + anyhow::bail!( + "Failed to resolve branch '{}' (HTTP {}): {}", + branch, + status, + error_body + ); + } + + let body: serde_json::Value = response + .json() + .await + .context("Failed to parse refs response")?; + + body.get("value") + .and_then(|v| v.as_array()) + .and_then(|arr| arr.first()) + .and_then(|r| r.get("objectId")) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + .context(format!( + "Branch '{}' not found in repository '{}'", + branch, repo_name + )) +} + +#[async_trait::async_trait] +impl Executor for CreateBranchResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!("Creating branch: '{}'", self.branch_name); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: CreateBranchConfig = ctx.get_tool_config("create-branch"); + debug!("Branch pattern: {:?}", config.branch_pattern); + debug!("Allowed repositories: {:?}", config.allowed_repositories); + debug!( + "Allowed source branches: {:?}", + config.allowed_source_branches + ); + + // Validate branch name against branch-pattern regex (if configured) + if let Some(ref pattern) = config.branch_pattern { + let re = regex_lite::Regex::new(pattern).context(format!( + "Invalid branch-pattern regex: '{}'", + pattern + ))?; + if !re.is_match(&self.branch_name) { + return Ok(ExecutionResult::failure(format!( + "Branch name '{}' does not match required pattern '{}'", + self.branch_name, pattern + ))); + } + debug!("Branch name matches pattern '{}'", pattern); + } + + // Determine repository alias + let repo_alias = self + .repository + .as_deref() + .unwrap_or("self"); + + // Validate repository against config policy BEFORE resolving the name, + // so operators see a policy error rather than a confusing "not in checkout list" error. + if !config.allowed_repositories.is_empty() + && !config.allowed_repositories.contains(&repo_alias.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed-repositories list: [{}]", + repo_alias, + config.allowed_repositories.join(", ") + ))); + } + + // Resolve the alias to the actual ADO repo name + let repo_name = if repo_alias == "self" { + ctx.repository_name + .as_deref() + .context("BUILD_REPOSITORY_NAME not set")? + .to_string() + } else { + ctx.allowed_repositories + .get(repo_alias) + .cloned() + .context(format!( + "Repository alias '{}' is not in the allowed checkout list", + repo_alias + ))? + }; + debug!("Resolved repository: {}", repo_name); + + // Validate source_branch against allowed-source-branches (if configured) + let source_branch = self.source_branch.as_deref().unwrap_or("main"); + if !config.allowed_source_branches.is_empty() + && !config.allowed_source_branches.contains(&source_branch.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Source branch '{}' is not in the allowed-source-branches list", + source_branch + ))); + } + + let client = reqwest::Client::new(); + + // Resolve the source commit SHA + let source_sha = if let Some(ref commit) = self.source_commit { + debug!("Using explicit source commit: {}", commit); + commit.clone() + } else { + debug!("Resolving source branch '{}' to commit", source_branch); + resolve_branch_to_commit(&client, org_url, project, token, &repo_name, source_branch) + .await? + }; + debug!("Source commit SHA: {}", source_sha); + + // Build the refs update URL + let url = format!( + "{}/{}/_apis/git/repositories/{}/refs?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(&repo_name, PATH_SEGMENT), + ); + debug!("API URL: {}", url); + + // Build the ref update request body + let ref_name = if self.branch_name.starts_with("refs/heads/") { + self.branch_name.clone() + } else { + format!("refs/heads/{}", self.branch_name) + }; + + let ref_updates = serde_json::json!([{ + "name": ref_name, + "oldObjectId": "0000000000000000000000000000000000000000", + "newObjectId": source_sha, + }]); + + info!("Creating branch '{}' from commit {}", ref_name, source_sha); + let response = client + .post(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&ref_updates) + .send() + .await + .context("Failed to send request to Azure DevOps")?; + + if response.status().is_success() { + let body: serde_json::Value = response + .json() + .await + .context("Failed to parse response JSON")?; + + // Check for per-ref errors in the response + let success = body + .get("value") + .and_then(|v| v.as_array()) + .and_then(|arr| arr.first()) + .and_then(|r| r.get("success")) + .and_then(|v| v.as_bool()) + .unwrap_or(false); + + if !success { + let custom_message = body + .get("value") + .and_then(|v| v.as_array()) + .and_then(|arr| arr.first()) + .and_then(|r| r.get("customMessage")) + .and_then(|v| v.as_str()) + .unwrap_or("Unknown error"); + + return Ok(ExecutionResult::failure(format!( + "Failed to create branch '{}': {}", + self.branch_name, custom_message + ))); + } + + info!("Branch '{}' created successfully", self.branch_name); + + Ok(ExecutionResult::success_with_data( + format!( + "Created branch '{}' in repository '{}' from commit {}", + self.branch_name, repo_name, &source_sha[..8] + ), + serde_json::json!({ + "branch": self.branch_name, + "ref": ref_name, + "repository": repo_name, + "source_commit": source_sha, + "project": project, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to create branch '{}' (HTTP {}): {}", + self.branch_name, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(CreateBranchResult::NAME, "create-branch"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{"branch_name": "feature/my-analysis", "source_branch": "main", "repository": "self"}"#; + let params: CreateBranchParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.branch_name, "feature/my-analysis"); + assert_eq!(params.source_branch, Some("main".to_string())); + assert_eq!(params.repository, Some("self".to_string())); + } + + #[test] + fn test_params_converts_to_result() { + let params = CreateBranchParams { + branch_name: "feature/new-work".to_string(), + source_branch: Some("develop".to_string()), + source_commit: None, + repository: None, + }; + let result: CreateBranchResult = params.try_into().unwrap(); + assert_eq!(result.name, "create-branch"); + assert_eq!(result.branch_name, "feature/new-work"); + assert_eq!(result.source_branch, Some("develop".to_string())); + } + + #[test] + fn test_validation_rejects_empty_branch() { + let params = CreateBranchParams { + branch_name: "".to_string(), + source_branch: None, + source_commit: None, + repository: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_path_traversal() { + let params = CreateBranchParams { + branch_name: "feature/../main".to_string(), + source_branch: None, + source_commit: None, + repository: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_commit() { + let params = CreateBranchParams { + branch_name: "feature/valid".to_string(), + source_branch: None, + source_commit: Some("not-a-valid-sha".to_string()), + repository: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = CreateBranchParams { + branch_name: "feature/test-branch".to_string(), + source_branch: Some("main".to_string()), + source_commit: None, + repository: None, + }; + let result: CreateBranchResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"create-branch""#)); + assert!(json.contains(r#""branch_name":"feature/test-branch""#)); + } + + #[test] + fn test_config_defaults() { + let config = CreateBranchConfig::default(); + assert!(config.branch_pattern.is_none()); + assert!(config.allowed_repositories.is_empty()); + assert!(config.allowed_source_branches.is_empty()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +branch-pattern: "^agent/.*" +allowed-repositories: + - self + - other-repo +allowed-source-branches: + - main + - develop +"#; + let config: CreateBranchConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.branch_pattern, Some("^agent/.*".to_string())); + assert_eq!(config.allowed_repositories, vec!["self", "other-repo"]); + assert_eq!(config.allowed_source_branches, vec!["main", "develop"]); + } +} diff --git a/src/tools/create_git_tag.rs b/src/tools/create_git_tag.rs new file mode 100644 index 00000000..f7986f9d --- /dev/null +++ b/src/tools/create_git_tag.rs @@ -0,0 +1,514 @@ +//! Create git tag safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::{PATH_SEGMENT, validate_git_ref_name}; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Parameters for creating a git tag (agent-provided) +#[derive(Deserialize, JsonSchema)] +pub struct CreateGitTagParams { + /// Tag name (e.g., "v1.2.3"). Must be 3-100 characters, alphanumeric + /// plus dots, dashes, underscores, and slashes only. + pub tag_name: String, + + /// Commit SHA to tag. If omitted, the executor resolves HEAD of the + /// default branch. Must be a valid 40-character hex string if present. + pub commit: Option, + + /// Tag annotation message. Must be at least 5 characters if present. + pub message: Option, + + /// Repository alias: "self" for the pipeline repo, or an alias from + /// the `checkout:` list. Defaults to "self". + pub repository: Option, +} + +/// Regex pattern for valid tag names: alphanumeric, dots, dashes, underscores, slashes. +static TAG_NAME_PATTERN: std::sync::LazyLock = + std::sync::LazyLock::new(|| regex_lite::Regex::new(r"^[a-zA-Z0-9._/\-]+$").unwrap()); + +impl Validate for CreateGitTagParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!( + !self.tag_name.starts_with('-'), + "tag_name must not start with '-'" + ); + ensure!( + self.tag_name.len() >= 3, + "tag_name must be at least 3 characters" + ); + ensure!( + self.tag_name.len() <= 100, + "tag_name must be at most 100 characters" + ); + ensure!( + TAG_NAME_PATTERN.is_match(&self.tag_name), + "tag_name contains invalid characters (only alphanumeric, dots, dashes, underscores, and slashes are allowed): {}", + self.tag_name + ); + validate_git_ref_name(&self.tag_name, "tag_name")?; + + if let Some(commit) = &self.commit { + ensure!( + commit.len() == 40, + "commit must be exactly 40 hex characters, got {} characters", + commit.len() + ); + ensure!( + commit.chars().all(|c| c.is_ascii_hexdigit()), + "commit must be a valid hex string: {}", + commit + ); + } + + if let Some(message) = &self.message { + ensure!( + message.len() >= 5, + "message must be at least 5 characters" + ); + } + + Ok(()) + } +} + +tool_result! { + name = "create-git-tag", + params = CreateGitTagParams, + /// Result of creating a git tag + pub struct CreateGitTagResult { + tag_name: String, + commit: Option, + message: Option, + repository: Option, + } +} + +impl Sanitize for CreateGitTagResult { + fn sanitize_fields(&mut self) { + // tag_name is a structural identifier — only strip control characters + self.tag_name = self + .tag_name + .chars() + .filter(|c| !c.is_control()) + .collect(); + self.message = self.message.as_ref().map(|m| sanitize_text(m)); + // commit and repository are structural identifiers; strip control chars only + self.commit = self.commit.as_ref().map(|c| { + c.chars().filter(|ch| !ch.is_control()).collect() + }); + self.repository = self.repository.as_ref().map(|r| { + r.chars().filter(|ch| !ch.is_control()).collect() + }); + } +} + +/// Configuration for the create-git-tag tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// create-git-tag: +/// tag-pattern: "^v\\d+\\.\\d+\\.\\d+$" +/// allowed-repositories: +/// - self +/// - my-lib +/// message-prefix: "[release] " +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CreateGitTagConfig { + /// Regex pattern that tag names must match (if configured) + #[serde(default, rename = "tag-pattern")] + pub tag_pattern: Option, + + /// Repositories the agent is allowed to tag (if empty, all allowed) + #[serde(default, rename = "allowed-repositories")] + pub allowed_repositories: Vec, + + /// Prefix prepended to the tag message + #[serde(default, rename = "message-prefix")] + pub message_prefix: Option, +} + +impl Default for CreateGitTagConfig { + fn default() -> Self { + Self { + tag_pattern: None, + allowed_repositories: Vec::new(), + message_prefix: None, + } + } +} + +/// Resolve HEAD commit for a repository by querying the repository's default branch. +async fn resolve_head_commit( + client: &reqwest::Client, + org_url: &str, + project: &str, + token: &str, + repo_name: &str, +) -> anyhow::Result { + // First, discover the default branch from the repository metadata + let repo_url = format!( + "{}/{}/_apis/git/repositories/{}?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(repo_name, PATH_SEGMENT), + ); + debug!("Fetching repository metadata: {}", repo_url); + + let repo_response = client + .get(&repo_url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to query repository metadata")?; + + ensure!( + repo_response.status().is_success(), + "Failed to fetch repository metadata (HTTP {})", + repo_response.status() + ); + + let repo_body: serde_json::Value = repo_response + .json() + .await + .context("Failed to parse repository metadata")?; + + let default_branch = repo_body + .get("defaultBranch") + .and_then(|v| v.as_str()) + .unwrap_or("refs/heads/main"); + + // Strip "refs/heads/" prefix for the filter parameter + let branch_filter = default_branch + .strip_prefix("refs/") + .unwrap_or(default_branch); + debug!("Default branch: {} (filter: {})", default_branch, branch_filter); + + // Now resolve the HEAD commit of the default branch + let url = format!( + "{}/{}/_apis/git/repositories/{}/refs?filter={}&api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(repo_name, PATH_SEGMENT), + branch_filter, + ); + + debug!("Resolving HEAD commit via: {}", url); + + let response = client + .get(&url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to query refs for HEAD resolution")?; + + ensure!( + response.status().is_success(), + "Failed to resolve HEAD (HTTP {})", + response.status() + ); + + let body: serde_json::Value = response + .json() + .await + .context("Failed to parse refs response")?; + + body.get("value") + .and_then(|v| v.as_array()) + .and_then(|arr| arr.first()) + .and_then(|r| r.get("objectId")) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + .context(format!( + "No refs found for {} — cannot resolve HEAD commit", + default_branch + )) +} + +#[async_trait::async_trait] +impl Executor for CreateGitTagResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!("Creating git tag: '{}'", self.tag_name); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: CreateGitTagConfig = ctx.get_tool_config("create-git-tag"); + debug!("Tag pattern: {:?}", config.tag_pattern); + debug!("Allowed repositories: {:?}", config.allowed_repositories); + + // Validate tag against configured pattern + if let Some(pattern) = &config.tag_pattern { + let re = regex_lite::Regex::new(pattern).context(format!( + "Invalid tag-pattern regex in config: {}", + pattern + ))?; + if !re.is_match(&self.tag_name) { + return Ok(ExecutionResult::failure(format!( + "Tag name '{}' does not match required pattern '{}'", + self.tag_name, pattern + ))); + } + } + + // Resolve repository + let repo_alias = self.repository.as_deref().unwrap_or("self"); + + // Validate repository against config policy BEFORE resolving the name, + // so operators see a policy error rather than a confusing resolution error. + if !config.allowed_repositories.is_empty() + && !config.allowed_repositories.contains(&repo_alias.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed-repositories list: [{}]", + repo_alias, + config.allowed_repositories.join(", ") + ))); + } + + let repo_name = if repo_alias == "self" { + ctx.repository_name + .as_deref() + .context("BUILD_REPOSITORY_NAME not set and repository is 'self'")? + .to_string() + } else { + ctx.allowed_repositories + .get(repo_alias) + .cloned() + .context(format!( + "Repository alias '{}' not found in allowed repositories", + repo_alias + ))? + }; + + let client = reqwest::Client::new(); + + // Resolve commit SHA — use provided value or look up HEAD + let commit_sha = match &self.commit { + Some(sha) => sha.clone(), + None => { + info!("No commit specified, resolving HEAD of default branch"); + resolve_head_commit(&client, org_url, project, token, &repo_name).await? + } + }; + debug!("Tagging commit: {}", commit_sha); + + // Build tag message with optional prefix + let tag_message = match (&config.message_prefix, &self.message) { + (Some(prefix), Some(msg)) => format!("{}{}", prefix, msg), + (Some(prefix), None) => format!("{}Tag {}", prefix, self.tag_name), + (None, Some(msg)) => msg.clone(), + (None, None) => format!("Tag {}", self.tag_name), + }; + + // POST annotated tag + let url = format!( + "{}/{}/_apis/git/repositories/{}/annotatedtags?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(&repo_name, PATH_SEGMENT), + ); + debug!("API URL: {}", url); + + let body = serde_json::json!({ + "name": self.tag_name, + "taggedObject": { + "objectId": commit_sha + }, + "message": tag_message + }); + + info!("Sending annotated tag creation request to ADO"); + let response = client + .post(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&body) + .send() + .await + .context("Failed to send request to Azure DevOps")?; + + if response.status().is_success() { + let resp_body: serde_json::Value = response + .json() + .await + .context("Failed to parse response JSON")?; + + let tag_url = resp_body + .get("url") + .and_then(|v| v.as_str()) + .unwrap_or(""); + + info!("Tag created: {} -> {}", self.tag_name, commit_sha); + + Ok(ExecutionResult::success_with_data( + format!( + "Created tag '{}' on commit {} in repository '{}'", + self.tag_name, commit_sha, repo_name + ), + serde_json::json!({ + "tag": self.tag_name, + "commit": commit_sha, + "repository": repo_name, + "url": tag_url, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to create tag (HTTP {}): {}", + status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(CreateGitTagResult::NAME, "create-git-tag"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{"tag_name": "v1.2.3", "commit": "abcdef1234567890abcdef1234567890abcdef12", "message": "Release v1.2.3"}"#; + let params: CreateGitTagParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.tag_name, "v1.2.3"); + assert_eq!( + params.commit.as_deref(), + Some("abcdef1234567890abcdef1234567890abcdef12") + ); + assert_eq!(params.message.as_deref(), Some("Release v1.2.3")); + } + + #[test] + fn test_params_converts_to_result() { + let params = CreateGitTagParams { + tag_name: "v1.0.0".to_string(), + commit: Some("abcdef1234567890abcdef1234567890abcdef12".to_string()), + message: Some("Initial release".to_string()), + repository: None, + }; + let result: CreateGitTagResult = params.try_into().unwrap(); + assert_eq!(result.name, "create-git-tag"); + assert_eq!(result.tag_name, "v1.0.0"); + } + + #[test] + fn test_validation_rejects_empty_tag() { + let params = CreateGitTagParams { + tag_name: "ab".to_string(), + commit: None, + message: None, + repository: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_tag_chars() { + let params = CreateGitTagParams { + tag_name: "v1.0 invalid!".to_string(), + commit: None, + message: None, + repository: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_commit_sha() { + let params = CreateGitTagParams { + tag_name: "v1.0.0".to_string(), + commit: Some("not-a-valid-sha".to_string()), + message: None, + repository: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_short_message() { + let params = CreateGitTagParams { + tag_name: "v1.0.0".to_string(), + commit: None, + message: Some("Hi".to_string()), + repository: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = CreateGitTagParams { + tag_name: "v2.0.0".to_string(), + commit: Some("abcdef1234567890abcdef1234567890abcdef12".to_string()), + message: Some("Major release".to_string()), + repository: None, + }; + let result: CreateGitTagResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"create-git-tag""#)); + assert!(json.contains(r#""tag_name":"v2.0.0""#)); + } + + #[test] + fn test_config_defaults() { + let config = CreateGitTagConfig::default(); + assert!(config.tag_pattern.is_none()); + assert!(config.allowed_repositories.is_empty()); + assert!(config.message_prefix.is_none()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +tag-pattern: "^v\\d+\\.\\d+\\.\\d+$" +allowed-repositories: + - self + - my-lib +message-prefix: "[release] " +"#; + let config: CreateGitTagConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!( + config.tag_pattern.as_deref(), + Some("^v\\d+\\.\\d+\\.\\d+$") + ); + assert_eq!(config.allowed_repositories, vec!["self", "my-lib"]); + assert_eq!(config.message_prefix.as_deref(), Some("[release] ")); + } +} diff --git a/src/tools/link_work_items.rs b/src/tools/link_work_items.rs new file mode 100644 index 00000000..71609cd7 --- /dev/null +++ b/src/tools/link_work_items.rs @@ -0,0 +1,460 @@ +//! Link work items safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::PATH_SEGMENT; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::tools::comment_on_work_item::CommentTarget; +use anyhow::{Context, ensure}; + +/// Resolve a human-friendly link type name to the ADO relation type string. +fn resolve_link_type(link_type: &str) -> Option<&'static str> { + match link_type { + "parent" => Some("System.LinkTypes.Hierarchy-Reverse"), + "child" => Some("System.LinkTypes.Hierarchy-Forward"), + "related" => Some("System.LinkTypes.Related"), + "predecessor" => Some("System.LinkTypes.Dependency-Reverse"), + "successor" => Some("System.LinkTypes.Dependency-Forward"), + "duplicate" => Some("System.LinkTypes.Duplicate-Forward"), + "duplicate-of" => Some("System.LinkTypes.Duplicate-Reverse"), + _ => None, + } +} + +/// All valid link type names accepted by this tool. +const VALID_LINK_TYPES: &[&str] = &[ + "parent", + "child", + "related", + "predecessor", + "successor", + "duplicate", + "duplicate-of", +]; + +/// Parameters for linking two work items +#[derive(Deserialize, JsonSchema)] +pub struct LinkWorkItemsParams { + /// The source work item ID (the item the link is added to) + pub source_id: i64, + + /// The target work item ID (the item being linked to) + pub target_id: i64, + + /// Link type: parent, child, related, predecessor, successor, duplicate, duplicate-of + pub link_type: String, + + /// Optional comment describing the relationship + pub comment: Option, +} + +impl Validate for LinkWorkItemsParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!(self.source_id > 0, "source_id must be positive"); + ensure!(self.target_id > 0, "target_id must be positive"); + ensure!( + self.source_id != self.target_id, + "source_id and target_id must be different" + ); + ensure!( + resolve_link_type(&self.link_type).is_some(), + "invalid link_type '{}'; must be one of: {}", + self.link_type, + VALID_LINK_TYPES.join(", ") + ); + if let Some(ref comment) = self.comment { + ensure!( + comment.len() >= 5, + "comment must be at least 5 characters" + ); + } + Ok(()) + } +} + +tool_result! { + name = "link-work-items", + params = LinkWorkItemsParams, + default_max = 5, + /// Result of linking two work items + pub struct LinkWorkItemsResult { + source_id: i64, + target_id: i64, + link_type: String, + comment: Option, + } +} + +impl Sanitize for LinkWorkItemsResult { + fn sanitize_fields(&mut self) { + self.link_type = sanitize_text(&self.link_type); + self.comment = self.comment.as_deref().map(sanitize_text); + } +} + +/// Configuration for the link-work-items tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// link-work-items: +/// target: "*" +/// allowed-link-types: +/// - parent +/// - child +/// - related +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LinkWorkItemsConfig { + /// Restrict which link types the agent may use. + /// An empty list (the default) means all link types are allowed. + #[serde(default, rename = "allowed-link-types")] + pub allowed_link_types: Vec, + + /// Target scope — which work items can be linked. + /// `None` means no target was configured; execution must reject this. + /// Accepts the same values as comment-on-work-item: "*", a single ID, + /// a list of IDs, or an area path string. + pub target: Option, +} + +impl Default for LinkWorkItemsConfig { + fn default() -> Self { + Self { + allowed_link_types: Vec::new(), + target: None, + } + } +} + +#[async_trait::async_trait] +impl Executor for LinkWorkItemsResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!( + "Linking work item #{} -> #{} ({})", + self.source_id, self.target_id, self.link_type + ); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: LinkWorkItemsConfig = ctx.get_tool_config("link-work-items"); + debug!("Allowed link types: {:?}", config.allowed_link_types); + + // Validate work item IDs against target scope + match &config.target { + None => { + return Ok(ExecutionResult::failure( + "link-work-items requires a 'target' field in safe-outputs configuration \ + to scope which work items can be linked. Example:\n safe-outputs:\n \ + link-work-items:\n target: \"*\"" + .to_string(), + )); + } + Some(target) => { + // Check source_id + if let Some(false) = target.allows_id(self.source_id) { + return Ok(ExecutionResult::failure(format!( + "Source work item #{} is not allowed by the configured target scope", + self.source_id + ))); + } + // Check target_id + if let Some(false) = target.allows_id(self.target_id) { + return Ok(ExecutionResult::failure(format!( + "Target work item #{} is not allowed by the configured target scope", + self.target_id + ))); + } + // Area path validation is deferred — would need API calls for both IDs. + // For now, ID-based and wildcard scoping is enforced. + } + } + + // Validate link type against configured allow-list + if !config.allowed_link_types.is_empty() + && !config.allowed_link_types.contains(&self.link_type) + { + return Ok(ExecutionResult::failure(format!( + "Link type '{}' is not in the allowed set: {}", + self.link_type, + config.allowed_link_types.join(", ") + ))); + } + + let relation_type = match resolve_link_type(&self.link_type) { + Some(rt) => rt, + None => { + return Ok(ExecutionResult::failure(format!( + "Unknown link type '{}'; must be one of: {}", + self.link_type, + VALID_LINK_TYPES.join(", ") + ))); + } + }; + + // Build the target work item URL for the relation + let target_url = format!( + "{}/{}/_apis/wit/workitems/{}", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + self.target_id, + ); + + // Build the JSON Patch body + let mut relation_value = serde_json::json!({ + "rel": relation_type, + "url": target_url, + }); + + if let Some(ref comment) = self.comment { + relation_value["attributes"] = serde_json::json!({ + "comment": comment, + }); + } + + let patch_doc = vec![serde_json::json!({ + "op": "add", + "path": "/relations/-", + "value": relation_value, + })]; + + // PATCH https://dev.azure.com/{org}/{project}/_apis/wit/workitems/{id}?api-version=7.1 + let url = format!( + "{}/{}/_apis/wit/workitems/{}?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + self.source_id, + ); + debug!("API URL: {}", url); + + let client = reqwest::Client::new(); + + info!( + "Sending link request: #{} -[{}]-> #{}", + self.source_id, self.link_type, self.target_id + ); + let response = client + .patch(&url) + .header("Content-Type", "application/json-patch+json") + .basic_auth("", Some(token)) + .json(&patch_doc) + .send() + .await + .context("Failed to send request to Azure DevOps")?; + + if response.status().is_success() { + let body: serde_json::Value = response + .json() + .await + .context("Failed to parse response JSON")?; + + let work_item_id = body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + + info!( + "Linked work item #{} -> #{} ({})", + self.source_id, self.target_id, self.link_type + ); + + Ok(ExecutionResult::success_with_data( + format!( + "Linked work item #{} -> #{} ({})", + self.source_id, self.target_id, self.link_type + ), + serde_json::json!({ + "source_id": self.source_id, + "target_id": self.target_id, + "link_type": self.link_type, + "relation_type": relation_type, + "work_item_id": work_item_id, + "project": project, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to link work item #{} -> #{} (HTTP {}): {}", + self.source_id, self.target_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(LinkWorkItemsResult::NAME, "link-work-items"); + } + + #[test] + fn test_params_deserializes() { + let json = + r#"{"source_id": 100, "target_id": 200, "link_type": "parent", "comment": "test linking"}"#; + let params: LinkWorkItemsParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.source_id, 100); + assert_eq!(params.target_id, 200); + assert_eq!(params.link_type, "parent"); + assert_eq!(params.comment.as_deref(), Some("test linking")); + } + + #[test] + fn test_params_converts_to_result() { + let params = LinkWorkItemsParams { + source_id: 100, + target_id: 200, + link_type: "child".to_string(), + comment: Some("Links parent to child".to_string()), + }; + let result: LinkWorkItemsResult = params.try_into().unwrap(); + assert_eq!(result.name, "link-work-items"); + assert_eq!(result.source_id, 100); + assert_eq!(result.target_id, 200); + assert_eq!(result.link_type, "child"); + assert_eq!(result.comment.as_deref(), Some("Links parent to child")); + } + + #[test] + fn test_validation_rejects_zero_source_id() { + let params = LinkWorkItemsParams { + source_id: 0, + target_id: 200, + link_type: "related".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_zero_target_id() { + let params = LinkWorkItemsParams { + source_id: 100, + target_id: 0, + link_type: "related".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_same_ids() { + let params = LinkWorkItemsParams { + source_id: 100, + target_id: 100, + link_type: "related".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_link_type() { + let params = LinkWorkItemsParams { + source_id: 100, + target_id: 200, + link_type: "unknown".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_resolve_link_type() { + assert_eq!( + resolve_link_type("parent"), + Some("System.LinkTypes.Hierarchy-Reverse") + ); + assert_eq!( + resolve_link_type("child"), + Some("System.LinkTypes.Hierarchy-Forward") + ); + assert_eq!( + resolve_link_type("related"), + Some("System.LinkTypes.Related") + ); + assert_eq!( + resolve_link_type("predecessor"), + Some("System.LinkTypes.Dependency-Reverse") + ); + assert_eq!( + resolve_link_type("successor"), + Some("System.LinkTypes.Dependency-Forward") + ); + assert_eq!( + resolve_link_type("duplicate"), + Some("System.LinkTypes.Duplicate-Forward") + ); + assert_eq!( + resolve_link_type("duplicate-of"), + Some("System.LinkTypes.Duplicate-Reverse") + ); + assert_eq!(resolve_link_type("invalid"), None); + assert_eq!(resolve_link_type(""), None); + } + + #[test] + fn test_config_defaults() { + let config = LinkWorkItemsConfig::default(); + assert!(config.allowed_link_types.is_empty()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +allowed-link-types: + - parent + - child + - related +"#; + let config: LinkWorkItemsConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.allowed_link_types.len(), 3); + assert!(config.allowed_link_types.contains(&"parent".to_string())); + assert!(config.allowed_link_types.contains(&"child".to_string())); + assert!(config.allowed_link_types.contains(&"related".to_string())); + } + + #[test] + fn test_result_serializes_correctly() { + let params = LinkWorkItemsParams { + source_id: 100, + target_id: 200, + link_type: "related".to_string(), + comment: None, + }; + let result: LinkWorkItemsResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"link-work-items""#)); + assert!(json.contains(r#""source_id":100"#)); + assert!(json.contains(r#""target_id":200"#)); + assert!(json.contains(r#""link_type":"related""#)); + } +} diff --git a/src/tools/mod.rs b/src/tools/mod.rs index ecd4dca9..38972265 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -104,27 +104,112 @@ pub(crate) async fn resolve_wiki_branch( } } +/// Resolve a repository alias to its ADO repo name. +/// +/// "self" (or None) → `ctx.repository_name`, otherwise look up in `ctx.allowed_repositories`. +pub(crate) fn resolve_repo_name( + repo_alias: Option<&str>, + ctx: &ExecutionContext, +) -> Result { + let alias = repo_alias.unwrap_or("self"); + if alias == "self" { + ctx.repository_name + .clone() + .ok_or_else(|| ExecutionResult::failure("BUILD_REPOSITORY_NAME not set")) + } else { + ctx.allowed_repositories + .get(alias) + .cloned() + .ok_or_else(|| { + ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed repository list", + alias + )) + }) + } +} + +/// Validate a string against `git check-ref-format` rules. +/// +/// Returns `Ok(())` if the name is valid, or an `Err` describing the violation. +/// This covers the structural rules that Azure DevOps also enforces — catching +/// them early gives clearer error messages than letting the API fail. +pub(crate) fn validate_git_ref_name(name: &str, label: &str) -> anyhow::Result<()> { + use anyhow::ensure; + + ensure!(!name.is_empty(), "{label} must not be empty"); + ensure!(!name.contains(".."), "{label} must not contain '..'"); + ensure!(!name.contains("@{{"), "{label} must not contain '@{{'"); + ensure!(!name.ends_with('.'), "{label} must not end with '.'"); + ensure!(!name.ends_with(".lock"), "{label} must not end with '.lock'"); + ensure!( + !name.contains('\\'), + "{label} must not contain backslash" + ); + ensure!( + !name.contains("//"), + "{label} must not contain consecutive slashes" + ); + for ch in ['~', '^', ':', '?', '*', '['] { + ensure!( + !name.contains(ch), + "{label} must not contain '{ch}'" + ); + } + for component in name.split('/') { + ensure!( + !component.starts_with('.'), + "{label} path component must not start with '.'" + ); + } + Ok(()) +} + +mod add_build_tag; +mod add_pr_comment; mod comment_on_work_item; +mod create_branch; +mod create_git_tag; mod create_pr; mod create_wiki_page; mod create_work_item; -mod update_wiki_page; +mod link_work_items; pub mod memory; mod missing_data; mod missing_tool; mod noop; +mod queue_build; +mod reply_to_pr_comment; +mod report_incomplete; +mod resolve_pr_thread; mod result; +mod submit_pr_review; +mod update_pr; +mod update_wiki_page; mod update_work_item; +mod upload_attachment; +pub use add_build_tag::*; +pub use add_pr_comment::*; pub use comment_on_work_item::*; +pub use create_branch::*; +pub use create_git_tag::*; pub use create_pr::*; pub use create_wiki_page::*; pub use create_work_item::*; -pub use update_wiki_page::*; +pub use link_work_items::*; pub use missing_data::*; pub use missing_tool::*; pub use noop::*; +pub use queue_build::*; +pub use reply_to_pr_comment::*; +pub use report_incomplete::*; +pub use resolve_pr_thread::*; pub use result::{ ExecutionContext, ExecutionResult, Executor, ToolResult, Validate, anyhow_to_mcp_error, }; +pub use submit_pr_review::*; +pub use update_pr::*; +pub use update_wiki_page::*; pub use update_work_item::*; +pub use upload_attachment::*; diff --git a/src/tools/queue_build.rs b/src/tools/queue_build.rs new file mode 100644 index 00000000..853c3f57 --- /dev/null +++ b/src/tools/queue_build.rs @@ -0,0 +1,500 @@ +//! Queue build safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::PATH_SEGMENT; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Parameters for queuing a build +#[derive(Deserialize, JsonSchema)] +pub struct QueueBuildParams { + /// Pipeline definition ID to trigger (must be positive) + pub pipeline_id: i32, + + /// Branch to build (optional, defaults to configured default or "main") + pub branch: Option, + + /// Template parameter key-value pairs (optional) + pub parameters: Option>, + + /// Human-readable reason for triggering the build; at least 5 characters if provided + pub reason: Option, +} + +impl Validate for QueueBuildParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!(self.pipeline_id > 0, "pipeline_id must be positive"); + if let Some(reason) = &self.reason { + ensure!( + reason.len() >= 5, + "reason must be at least 5 characters" + ); + } + if let Some(branch) = &self.branch { + ensure!( + !branch.contains(".."), + "branch name must not contain '..'" + ); + ensure!( + !branch.contains('\0'), + "branch name must not contain null bytes" + ); + } + Ok(()) + } +} + +tool_result! { + name = "queue-build", + params = QueueBuildParams, + default_max = 3, + /// Result of queuing a build + pub struct QueueBuildResult { + pipeline_id: i32, + branch: Option, + parameters: Option>, + reason: Option, + } +} + +impl Sanitize for QueueBuildResult { + fn sanitize_fields(&mut self) { + if let Some(reason) = &self.reason { + self.reason = Some(sanitize_text(reason)); + } + if let Some(branch) = &self.branch { + self.branch = Some(sanitize_text(branch)); + } + if let Some(params) = &self.parameters { + self.parameters = Some( + params + .iter() + .map(|(k, v)| (sanitize_text(k), sanitize_text(v))) + .collect(), + ); + } + } +} + +/// Configuration for the queue-build tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// queue-build: +/// allowed-pipelines: +/// - 123 +/// - 456 +/// allowed-branches: +/// - main +/// - release/* +/// allowed-parameters: +/// - environment +/// - version +/// default-branch: main +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct QueueBuildConfig { + /// Pipeline definition IDs that are allowed to be triggered (REQUIRED — empty rejects all) + #[serde(default, rename = "allowed-pipelines")] + pub allowed_pipelines: Vec, + + /// Branches that are allowed to be built (if empty, any branch is allowed) + #[serde(default, rename = "allowed-branches")] + pub allowed_branches: Vec, + + /// Parameter keys that are allowed to be passed (if empty, any parameters are allowed) + #[serde(default, rename = "allowed-parameters")] + pub allowed_parameters: Vec, + + /// Default branch to use when the agent does not specify one + #[serde(default, rename = "default-branch")] + pub default_branch: Option, +} + +impl Default for QueueBuildConfig { + fn default() -> Self { + Self { + allowed_pipelines: Vec::new(), + allowed_branches: Vec::new(), + allowed_parameters: Vec::new(), + default_branch: None, + } + } +} + +#[async_trait::async_trait] +impl Executor for QueueBuildResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!("Queuing build for pipeline definition {}", self.pipeline_id); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + // Get tool-specific configuration + let config: QueueBuildConfig = ctx.get_tool_config("queue-build"); + debug!("Allowed pipelines: {:?}", config.allowed_pipelines); + debug!("Allowed branches: {:?}", config.allowed_branches); + debug!("Allowed parameters: {:?}", config.allowed_parameters); + + // Validate pipeline_id against allowed-pipelines (REQUIRED) + if config.allowed_pipelines.is_empty() { + return Ok(ExecutionResult::failure( + "queue-build allowed-pipelines is not configured. \ + This is required to scope which pipelines the agent can trigger." + .to_string(), + )); + } + if !config.allowed_pipelines.contains(&self.pipeline_id) { + return Ok(ExecutionResult::failure(format!( + "Pipeline definition {} is not in the allowed-pipelines list", + self.pipeline_id + ))); + } + + // Resolve the effective branch + let effective_branch = self + .branch + .as_deref() + .or(config.default_branch.as_deref()) + .unwrap_or("main"); + debug!("Effective branch: {}", effective_branch); + + // Validate branch against allowed-branches (if configured) + if !config.allowed_branches.is_empty() { + let branch_allowed = config.allowed_branches.iter().any(|pattern| { + if pattern.ends_with("/*") { + let prefix = &pattern[..pattern.len() - 2]; + effective_branch.starts_with(prefix) + && effective_branch[prefix.len()..].starts_with('/') + } else { + pattern == effective_branch + } + }); + if !branch_allowed { + return Ok(ExecutionResult::failure(format!( + "Branch '{}' is not in the allowed-branches list", + effective_branch + ))); + } + } + + // Validate parameter keys against allowed-parameters (if configured) + if let Some(params) = &self.parameters { + if !config.allowed_parameters.is_empty() { + for key in params.keys() { + if !config.allowed_parameters.contains(key) { + return Ok(ExecutionResult::failure(format!( + "Parameter '{}' is not in the allowed-parameters list", + key + ))); + } + } + } + // Reject parameter values containing ADO variable/expression syntax + for (key, value) in params { + if value.contains("$(") || value.contains("${{") || value.contains("$[") { + return Ok(ExecutionResult::failure(format!( + "Parameter '{}' value contains ADO variable/expression syntax \ + which is not allowed", + key + ))); + } + } + } + + // Build the source branch ref + let source_branch = if effective_branch.starts_with("refs/") { + effective_branch.to_string() + } else { + format!("refs/heads/{}", effective_branch) + }; + debug!("Source branch ref: {}", source_branch); + + // Build the request body + let mut body = serde_json::json!({ + "definition": { "id": self.pipeline_id }, + "sourceBranch": source_branch, + "reason": "userCreated", + }); + + // Add template parameters as a JSON string if provided + if let Some(params) = &self.parameters { + if !params.is_empty() { + let params_json = serde_json::to_string(params) + .context("Failed to serialize template parameters")?; + body["parameters"] = serde_json::Value::String(params_json); + } + } + + // Build the API URL + let url = format!( + "{}/{}/_apis/build/builds?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + ); + debug!("API URL: {}", url); + + // Make the API call + let client = reqwest::Client::new(); + info!( + "Sending queue build request for pipeline {} on branch {}", + self.pipeline_id, source_branch + ); + let response = client + .post(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&body) + .send() + .await + .context("Failed to send request to Azure DevOps")?; + + if response.status().is_success() { + let resp_body: serde_json::Value = response + .json() + .await + .context("Failed to parse response JSON")?; + + let build_id = resp_body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + let build_url = resp_body + .get("_links") + .and_then(|l| l.get("web")) + .and_then(|h| h.get("href")) + .and_then(|h| h.as_str()) + .unwrap_or(""); + + info!("Build queued: #{} - {}", build_id, build_url); + + Ok(ExecutionResult::success_with_data( + format!( + "Queued build #{} for pipeline {} on branch {}", + build_id, self.pipeline_id, effective_branch + ), + serde_json::json!({ + "build_id": build_id, + "pipeline_id": self.pipeline_id, + "branch": effective_branch, + "url": build_url, + "project": project, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to queue build for pipeline {} (HTTP {}): {}", + self.pipeline_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(QueueBuildResult::NAME, "queue-build"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{"pipeline_id": 123, "branch": "main", "reason": "Nightly rebuild"}"#; + let params: QueueBuildParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.pipeline_id, 123); + assert_eq!(params.branch, Some("main".to_string())); + assert_eq!(params.reason, Some("Nightly rebuild".to_string())); + assert!(params.parameters.is_none()); + } + + #[test] + fn test_params_converts_to_result() { + let params = QueueBuildParams { + pipeline_id: 42, + branch: Some("develop".to_string()), + parameters: None, + reason: Some("Trigger nightly build".to_string()), + }; + let result: QueueBuildResult = params.try_into().unwrap(); + assert_eq!(result.name, "queue-build"); + assert_eq!(result.pipeline_id, 42); + assert_eq!(result.branch, Some("develop".to_string())); + assert_eq!(result.reason, Some("Trigger nightly build".to_string())); + } + + #[test] + fn test_validation_rejects_zero_pipeline_id() { + let params = QueueBuildParams { + pipeline_id: 0, + branch: None, + parameters: None, + reason: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_negative_pipeline_id() { + let params = QueueBuildParams { + pipeline_id: -1, + branch: None, + parameters: None, + reason: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_short_reason() { + let params = QueueBuildParams { + pipeline_id: 1, + branch: None, + parameters: None, + reason: Some("Hi".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_branch_with_dotdot() { + let params = QueueBuildParams { + pipeline_id: 1, + branch: Some("../../etc/passwd".to_string()), + parameters: None, + reason: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_branch_with_null_byte() { + let params = QueueBuildParams { + pipeline_id: 1, + branch: Some("main\0evil".to_string()), + parameters: None, + reason: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_accepts_valid_params() { + let params = QueueBuildParams { + pipeline_id: 123, + branch: Some("main".to_string()), + parameters: None, + reason: Some("Scheduled nightly build".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_ok()); + } + + #[test] + fn test_validation_accepts_minimal_params() { + let params = QueueBuildParams { + pipeline_id: 1, + branch: None, + parameters: None, + reason: None, + }; + let result: Result = params.try_into(); + assert!(result.is_ok()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = QueueBuildParams { + pipeline_id: 99, + branch: Some("release/v1".to_string()), + parameters: None, + reason: Some("Release build trigger".to_string()), + }; + let result: QueueBuildResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"queue-build""#)); + assert!(json.contains(r#""pipeline_id":99"#)); + } + + #[test] + fn test_config_defaults() { + let config = QueueBuildConfig::default(); + assert!(config.allowed_pipelines.is_empty()); + assert!(config.allowed_branches.is_empty()); + assert!(config.allowed_parameters.is_empty()); + assert!(config.default_branch.is_none()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +allowed-pipelines: + - 123 + - 456 +allowed-branches: + - main + - release/* +allowed-parameters: + - environment + - version +default-branch: main +"#; + let config: QueueBuildConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.allowed_pipelines, vec![123, 456]); + assert_eq!(config.allowed_branches, vec!["main", "release/*"]); + assert_eq!(config.allowed_parameters, vec!["environment", "version"]); + assert_eq!(config.default_branch, Some("main".to_string())); + } + + #[test] + fn test_config_partial_deserialize_uses_defaults() { + let yaml = r#" +allowed-pipelines: + - 42 +"#; + let config: QueueBuildConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.allowed_pipelines, vec![42]); + assert!(config.allowed_branches.is_empty()); + assert!(config.allowed_parameters.is_empty()); + assert!(config.default_branch.is_none()); + } + + #[test] + fn test_params_deserializes_with_parameters() { + let json = r#"{"pipeline_id": 10, "parameters": {"env": "prod", "version": "2.0"}}"#; + let params: QueueBuildParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.pipeline_id, 10); + let map = params.parameters.unwrap(); + assert_eq!(map.get("env"), Some(&"prod".to_string())); + assert_eq!(map.get("version"), Some(&"2.0".to_string())); + } +} diff --git a/src/tools/reply_to_pr_comment.rs b/src/tools/reply_to_pr_comment.rs new file mode 100644 index 00000000..2839aea8 --- /dev/null +++ b/src/tools/reply_to_pr_comment.rs @@ -0,0 +1,346 @@ +//! Reply to PR review comment safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::PATH_SEGMENT; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Parameters for replying to an existing review comment thread on a pull request +#[derive(Deserialize, JsonSchema)] +pub struct ReplyToPrCommentParams { + /// The pull request ID containing the thread + pub pull_request_id: i32, + + /// The thread ID to reply to + pub thread_id: i32, + + /// Reply text in markdown format. Ensure adequate content > 10 characters. + pub content: String, + + /// Repository alias: "self" for pipeline repo, or an alias from the checkout list. + /// Defaults to "self" if omitted. + #[serde(default = "default_repository")] + pub repository: Option, +} + +fn default_repository() -> Option { + Some("self".to_string()) +} + +impl Validate for ReplyToPrCommentParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!(self.pull_request_id > 0, "pull_request_id must be positive"); + ensure!(self.thread_id > 0, "thread_id must be positive"); + ensure!( + self.content.len() >= 10, + "content must be at least 10 characters" + ); + Ok(()) + } +} + +tool_result! { + name = "reply-to-pr-review-comment", + params = ReplyToPrCommentParams, + /// Result of replying to a review comment thread on a pull request + pub struct ReplyToPrCommentResult { + pull_request_id: i32, + thread_id: i32, + content: String, + repository: Option, + } +} + +impl Sanitize for ReplyToPrCommentResult { + fn sanitize_fields(&mut self) { + self.content = sanitize_text(&self.content); + } +} + +/// Configuration for the reply-to-pr-review-comment tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// reply-to-pr-review-comment: +/// comment-prefix: "[Agent] " +/// allowed-repositories: +/// - self +/// - other-repo +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ReplyToPrCommentConfig { + /// Prefix prepended to all replies (e.g., "[Agent] ") + #[serde(default, rename = "comment-prefix")] + pub comment_prefix: Option, + + /// Restrict which repositories the agent can reply on. + /// If empty, all repositories in the checkout list (plus "self") are allowed. + #[serde(default, rename = "allowed-repositories")] + pub allowed_repositories: Vec, +} + +impl Default for ReplyToPrCommentConfig { + fn default() -> Self { + Self { + comment_prefix: None, + allowed_repositories: Vec::new(), + } + } +} + +#[async_trait::async_trait] +impl Executor for ReplyToPrCommentResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!( + "Replying to PR #{} thread #{}: {} chars", + self.pull_request_id, + self.thread_id, + self.content.len() + ); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: ReplyToPrCommentConfig = ctx.get_tool_config("reply-to-pr-review-comment"); + debug!("Config: {:?}", config); + + let repository = self + .repository + .as_deref() + .unwrap_or("self"); + + // Validate repository against allowed-repositories config + if !config.allowed_repositories.is_empty() + && !config.allowed_repositories.contains(&repository.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed-repositories list", + repository + ))); + } + + // Determine the repository name for the API call + let repo_name = if repository == "self" || repository.is_empty() { + ctx.repository_name + .as_ref() + .context("BUILD_REPOSITORY_NAME not set and repository is 'self'")? + .clone() + } else { + match ctx.allowed_repositories.get(repository) { + Some(name) => name.clone(), + None => { + return Ok(ExecutionResult::failure(format!( + "Repository alias '{}' not found in allowed repositories", + repository + ))); + } + } + }; + + // Build comment content with optional prefix + let comment_body = match &config.comment_prefix { + Some(prefix) => format!("{}{}", prefix, self.content), + None => self.content.clone(), + }; + + // Build the API URL for adding a comment to an existing thread + let url = format!( + "{}/{}/_apis/git/repositories/{}/pullRequests/{}/threads/{}/comments?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(&repo_name, PATH_SEGMENT), + self.pull_request_id, + self.thread_id, + ); + debug!("API URL: {}", url); + + // parentCommentId=1 targets the root comment in the thread. In ADO, + // the first comment in a thread is always ID 1 (IDs are thread-scoped). + let request_body = serde_json::json!({ + "parentCommentId": 1, + "content": comment_body, + "commentType": 1 + }); + + let client = reqwest::Client::new(); + + info!( + "Sending reply to PR #{} thread #{}", + self.pull_request_id, self.thread_id + ); + let response = client + .post(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&request_body) + .send() + .await + .context("Failed to send request to Azure DevOps")?; + + if response.status().is_success() { + let body: serde_json::Value = response + .json() + .await + .context("Failed to parse response JSON")?; + + let comment_id = body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + + info!( + "Reply added to PR #{} thread #{}: comment #{}", + self.pull_request_id, self.thread_id, comment_id + ); + + Ok(ExecutionResult::success_with_data( + format!( + "Added reply #{} to PR #{} thread #{}", + comment_id, self.pull_request_id, self.thread_id + ), + serde_json::json!({ + "comment_id": comment_id, + "pull_request_id": self.pull_request_id, + "thread_id": self.thread_id, + "repository": repo_name, + "project": project, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to reply to PR #{} thread #{} (HTTP {}): {}", + self.pull_request_id, self.thread_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(ReplyToPrCommentResult::NAME, "reply-to-pr-review-comment"); + } + + #[test] + fn test_params_deserializes() { + let json = + r#"{"pull_request_id": 42, "thread_id": 7, "content": "This is a reply to the review comment."}"#; + let params: ReplyToPrCommentParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.pull_request_id, 42); + assert_eq!(params.thread_id, 7); + assert!(params.content.contains("reply")); + assert_eq!(params.repository, Some("self".to_string())); + } + + #[test] + fn test_params_converts_to_result() { + let params = ReplyToPrCommentParams { + pull_request_id: 42, + thread_id: 7, + content: "This is a test reply with enough characters.".to_string(), + repository: Some("self".to_string()), + }; + let result: ReplyToPrCommentResult = params.try_into().unwrap(); + assert_eq!(result.name, "reply-to-pr-review-comment"); + assert_eq!(result.pull_request_id, 42); + assert_eq!(result.thread_id, 7); + assert!(result.content.contains("test reply")); + } + + #[test] + fn test_validation_rejects_zero_pr_id() { + let params = ReplyToPrCommentParams { + pull_request_id: 0, + thread_id: 7, + content: "This is a valid reply body text.".to_string(), + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_zero_thread_id() { + let params = ReplyToPrCommentParams { + pull_request_id: 42, + thread_id: 0, + content: "This is a valid reply body text.".to_string(), + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_short_content() { + let params = ReplyToPrCommentParams { + pull_request_id: 42, + thread_id: 7, + content: "Too short".to_string(), + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = ReplyToPrCommentParams { + pull_request_id: 42, + thread_id: 7, + content: "A reply body that is definitely longer than ten characters.".to_string(), + repository: Some("self".to_string()), + }; + let result: ReplyToPrCommentResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"reply-to-pr-review-comment""#)); + assert!(json.contains(r#""pull_request_id":42"#)); + assert!(json.contains(r#""thread_id":7"#)); + } + + #[test] + fn test_config_defaults() { + let config = ReplyToPrCommentConfig::default(); + assert!(config.comment_prefix.is_none()); + assert!(config.allowed_repositories.is_empty()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +comment-prefix: "[Agent] " +allowed-repositories: + - self + - other-repo +"#; + let config: ReplyToPrCommentConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.comment_prefix, Some("[Agent] ".to_string())); + assert_eq!(config.allowed_repositories, vec!["self", "other-repo"]); + } +} diff --git a/src/tools/report_incomplete.rs b/src/tools/report_incomplete.rs new file mode 100644 index 00000000..98248c63 --- /dev/null +++ b/src/tools/report_incomplete.rs @@ -0,0 +1,105 @@ +//! Report incomplete task safe output tool + +use schemars::JsonSchema; +use serde::Deserialize; + +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::Validate; +use anyhow::ensure; + +/// Parameters for reporting that a task could not be completed +#[derive(Deserialize, JsonSchema)] +pub struct ReportIncompleteParams { + /// Why the task could not be completed + pub reason: String, + + /// Additional context about what was attempted + #[serde(default)] + pub context: Option, +} + +impl Validate for ReportIncompleteParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!( + self.reason.len() >= 10, + "reason must be at least 10 characters" + ); + Ok(()) + } +} + +tool_result! { + name = "report-incomplete", + params = ReportIncompleteParams, + /// Result of reporting an incomplete task + pub struct ReportIncompleteResult { + reason: String, + #[serde(default)] + context: Option, + } +} + +impl Sanitize for ReportIncompleteResult { + fn sanitize_fields(&mut self) { + self.reason = sanitize_text(&self.reason); + if let Some(ref ctx) = self.context { + self.context = Some(sanitize_text(ctx)); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(ReportIncompleteResult::NAME, "report-incomplete"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{"reason": "API timed out after 30s", "context": "tried 3 retries"}"#; + let params: ReportIncompleteParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.reason, "API timed out after 30s"); + assert_eq!(params.context, Some("tried 3 retries".to_string())); + } + + #[test] + fn test_params_converts_to_result() { + let params = ReportIncompleteParams { + reason: "Build failed with exit code 1".to_string(), + context: Some("ran cargo build".to_string()), + }; + let result: ReportIncompleteResult = params.try_into().unwrap(); + assert_eq!(result.name, "report-incomplete"); + assert_eq!(result.reason, "Build failed with exit code 1"); + assert_eq!(result.context, Some("ran cargo build".to_string())); + } + + #[test] + fn test_validation_rejects_short_reason() { + let params = ReportIncompleteParams { + reason: "short".to_string(), + context: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let result: ReportIncompleteResult = ReportIncompleteParams { + reason: "API timed out after 30s".to_string(), + context: None, + } + .try_into() + .unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"report-incomplete""#)); + assert!(json.contains(r#""reason":"API timed out after 30s""#)); + } +} diff --git a/src/tools/resolve_pr_thread.rs b/src/tools/resolve_pr_thread.rs new file mode 100644 index 00000000..7d54cbd1 --- /dev/null +++ b/src/tools/resolve_pr_thread.rs @@ -0,0 +1,404 @@ +//! Resolve PR review thread safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::resolve_repo_name; +use super::PATH_SEGMENT; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// All valid thread status strings (lowercase, agent-facing) +const VALID_STATUSES: &[&str] = &["active", "fixed", "wont-fix", "closed", "by-design"]; + +/// Map a thread status string to the ADO API integer value. +/// +/// ADO thread status values: +/// - 1 = Active +/// - 2 = Fixed (resolved) +/// - 3 = WontFix +/// - 4 = Closed +/// - 5 = ByDesign +fn status_to_int(status: &str) -> Option { + match status { + "active" => Some(1), + "fixed" => Some(2), + "wont-fix" => Some(3), + "closed" => Some(4), + "by-design" => Some(5), + _ => None, + } +} + +fn default_repository() -> Option { + Some("self".to_string()) +} + +/// Parameters for resolving or reactivating a PR review thread +#[derive(Deserialize, JsonSchema)] +pub struct ResolvePrThreadParams { + /// The pull request ID containing the thread + pub pull_request_id: i32, + + /// The thread ID to resolve or reactivate + pub thread_id: i32, + + /// Target status: "fixed", "wont-fix", "closed", "by-design", or "active" (to reactivate) + pub status: String, + + /// Repository alias: "self" for pipeline repo, or an alias from the checkout list. + /// Defaults to "self" if omitted. + #[serde(default = "default_repository")] + pub repository: Option, +} + +impl Validate for ResolvePrThreadParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!( + self.pull_request_id > 0, + "pull_request_id must be positive" + ); + ensure!(self.thread_id > 0, "thread_id must be positive"); + ensure!( + VALID_STATUSES.contains(&self.status.as_str()), + "Invalid status '{}'. Valid statuses: {}", + self.status, + VALID_STATUSES.join(", ") + ); + Ok(()) + } +} + +tool_result! { + name = "resolve-pr-review-thread", + params = ResolvePrThreadParams, + /// Result of resolving or reactivating a PR review thread + pub struct ResolvePrThreadResult { + pull_request_id: i32, + thread_id: i32, + status: String, + repository: Option, + } +} + +impl Sanitize for ResolvePrThreadResult { + fn sanitize_fields(&mut self) { + self.status = sanitize_text(&self.status); + if let Some(ref repo) = self.repository { + self.repository = Some(sanitize_text(repo)); + } + } +} + +/// Configuration for the resolve-pr-review-thread tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// resolve-pr-review-thread: +/// allowed-repositories: +/// - self +/// - other-repo +/// allowed-statuses: +/// - fixed +/// - wont-fix +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ResolvePrThreadConfig { + /// Restrict which repositories the agent can operate on. + /// If empty, all repositories in the checkout list (plus "self") are allowed. + #[serde(default, rename = "allowed-repositories")] + pub allowed_repositories: Vec, + + /// Restrict which thread statuses can be set. + /// REQUIRED — empty list rejects all status transitions. + #[serde(default, rename = "allowed-statuses")] + pub allowed_statuses: Vec, +} + +impl Default for ResolvePrThreadConfig { + fn default() -> Self { + Self { + allowed_repositories: Vec::new(), + allowed_statuses: Vec::new(), + } + } +} + +#[async_trait::async_trait] +impl Executor for ResolvePrThreadResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!( + "Resolving thread #{} on PR #{} with status '{}'", + self.thread_id, self.pull_request_id, self.status + ); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: ResolvePrThreadConfig = ctx.get_tool_config("resolve-pr-review-thread"); + debug!("Config: {:?}", config); + + // Validate status against allowed-statuses — REQUIRED. + // An empty allowed-statuses list means the operator hasn't opted in, so reject. + // This prevents agents from resolving review threads (e.g. marking security + // concerns as "fixed") without explicit operator consent. + if config.allowed_statuses.is_empty() { + return Ok(ExecutionResult::failure( + "resolve-pr-review-thread requires 'allowed-statuses' to be configured in \ + safe-outputs.resolve-pr-review-thread. This prevents agents from \ + manipulating thread statuses without explicit operator consent. Example:\n \ + safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n \ + - fixed\n\nValid statuses: active, fixed, wont-fix, closed, by-design" + .to_string(), + )); + } + if !config.allowed_statuses.contains(&self.status) { + return Ok(ExecutionResult::failure(format!( + "Status '{}' is not in the allowed-statuses list", + self.status + ))); + } + + let effective_repo = self + .repository + .as_deref() + .unwrap_or("self"); + + // Validate repository against allowed-repositories config + if !config.allowed_repositories.is_empty() + && !config.allowed_repositories.contains(&effective_repo.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed-repositories list", + effective_repo + ))); + } + + // Map status string to ADO integer + let status_int = match status_to_int(&self.status) { + Some(v) => v, + None => { + return Ok(ExecutionResult::failure(format!( + "Invalid status '{}'. Valid statuses: {}", + self.status, + VALID_STATUSES.join(", ") + ))); + } + }; + + // Resolve repository alias to actual repo name via the shared helper. + // Treat an empty string the same as "self" (pipeline repository). + let alias = self.repository.as_deref().filter(|s| !s.is_empty()); + let repo_name = match resolve_repo_name(alias, ctx) { + Ok(name) => name, + Err(result) => return Ok(result), + }; + + // Build the Azure DevOps REST API URL for updating a thread + // PATCH https://dev.azure.com/{org}/{project}/_apis/git/repositories/{repo}/pullRequests/{prId}/threads/{threadId}?api-version=7.1 + let url = format!( + "{}/{}/_apis/git/repositories/{}/pullRequests/{}/threads/{}?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(&repo_name, PATH_SEGMENT), + self.pull_request_id, + self.thread_id, + ); + debug!("API URL: {}", url); + + let body = serde_json::json!({ + "status": status_int + }); + + let client = reqwest::Client::new(); + + info!( + "Updating thread #{} on PR #{} to status '{}'", + self.thread_id, self.pull_request_id, self.status + ); + let response = client + .patch(&url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&body) + .send() + .await + .context("Failed to send request to Azure DevOps")?; + + if response.status().is_success() { + let resp_body: serde_json::Value = response + .json() + .await + .context("Failed to parse response JSON")?; + + let returned_id = resp_body.get("id").and_then(|v| v.as_i64()).unwrap_or(0); + + info!( + "Thread #{} on PR #{} updated to status '{}'", + self.thread_id, self.pull_request_id, self.status + ); + + Ok(ExecutionResult::success_with_data( + format!( + "Updated thread #{} on PR #{} to status '{}'", + self.thread_id, self.pull_request_id, self.status + ), + serde_json::json!({ + "thread_id": returned_id, + "pull_request_id": self.pull_request_id, + "repository": repo_name, + "project": project, + "status": self.status, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + Ok(ExecutionResult::failure(format!( + "Failed to update thread #{} on PR #{} (HTTP {}): {}", + self.thread_id, self.pull_request_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(ResolvePrThreadResult::NAME, "resolve-pr-review-thread"); + } + + #[test] + fn test_params_deserializes() { + let json = + r#"{"pull_request_id": 42, "thread_id": 7, "status": "fixed"}"#; + let params: ResolvePrThreadParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.pull_request_id, 42); + assert_eq!(params.thread_id, 7); + assert_eq!(params.status, "fixed"); + assert_eq!(params.repository, Some("self".to_string())); + } + + #[test] + fn test_params_converts_to_result() { + let params = ResolvePrThreadParams { + pull_request_id: 42, + thread_id: 7, + status: "fixed".to_string(), + repository: Some("self".to_string()), + }; + let result: ResolvePrThreadResult = params.try_into().unwrap(); + assert_eq!(result.name, "resolve-pr-review-thread"); + assert_eq!(result.pull_request_id, 42); + assert_eq!(result.thread_id, 7); + assert_eq!(result.status, "fixed"); + } + + #[test] + fn test_validation_rejects_zero_pr_id() { + let params = ResolvePrThreadParams { + pull_request_id: 0, + thread_id: 7, + status: "fixed".to_string(), + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_zero_thread_id() { + let params = ResolvePrThreadParams { + pull_request_id: 42, + thread_id: 0, + status: "fixed".to_string(), + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_status() { + let params = ResolvePrThreadParams { + pull_request_id: 42, + thread_id: 7, + status: "invalid-status".to_string(), + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = ResolvePrThreadParams { + pull_request_id: 42, + thread_id: 7, + status: "fixed".to_string(), + repository: Some("self".to_string()), + }; + let result: ResolvePrThreadResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"resolve-pr-review-thread""#)); + assert!(json.contains(r#""pull_request_id":42"#)); + assert!(json.contains(r#""thread_id":7"#)); + } + + #[test] + fn test_config_defaults() { + let config = ResolvePrThreadConfig::default(); + assert!(config.allowed_repositories.is_empty()); + assert!(config.allowed_statuses.is_empty()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +allowed-repositories: + - self + - other-repo +allowed-statuses: + - fixed + - wont-fix +"#; + let config: ResolvePrThreadConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.allowed_repositories, vec!["self", "other-repo"]); + assert_eq!(config.allowed_statuses, vec!["fixed", "wont-fix"]); + } + + #[test] + fn test_status_mapping() { + assert_eq!(status_to_int("active"), Some(1)); + assert_eq!(status_to_int("fixed"), Some(2)); + assert_eq!(status_to_int("wont-fix"), Some(3)); + assert_eq!(status_to_int("closed"), Some(4)); + assert_eq!(status_to_int("by-design"), Some(5)); + assert_eq!(status_to_int("invalid"), None); + } +} diff --git a/src/tools/submit_pr_review.rs b/src/tools/submit_pr_review.rs new file mode 100644 index 00000000..5cf1d1cb --- /dev/null +++ b/src/tools/submit_pr_review.rs @@ -0,0 +1,545 @@ +//! Submit PR review safe output tool + +use log::{debug, info}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::{PATH_SEGMENT, resolve_repo_name}; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Valid event values for submit-pr-review +const VALID_EVENTS: &[&str] = &[ + "approve", + "approve-with-suggestions", + "request-changes", + "comment", +]; + +/// Map a review event string to its ADO vote numeric value +fn event_to_vote(event: &str) -> Option { + match event { + "approve" => Some(10), + "approve-with-suggestions" => Some(5), + "request-changes" => Some(-5), + "comment" => Some(0), + _ => None, + } +} + +fn default_repository() -> String { + "self".to_string() +} + +/// Parameters for submitting a pull request review +#[derive(Deserialize, JsonSchema)] +pub struct SubmitPrReviewParams { + /// The pull request ID to review (must be positive) + pub pull_request_id: i32, + + /// Review decision: "approve", "approve-with-suggestions", "request-changes", or "comment" + pub event: String, + + /// Review rationale in markdown. Required for "request-changes", optional otherwise. + /// Must be at least 10 characters when provided. + #[serde(default)] + pub body: Option, + + /// Repository alias: "self" for pipeline repo, or an alias from the checkout list. + /// Defaults to "self" if omitted. + #[serde(default)] + pub repository: Option, +} + +impl Validate for SubmitPrReviewParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!( + self.pull_request_id > 0, + "pull_request_id must be a positive integer" + ); + ensure!( + VALID_EVENTS.contains(&self.event.as_str()), + "event must be one of: {}", + VALID_EVENTS.join(", ") + ); + if self.event == "request-changes" { + ensure!( + self.body.is_some(), + "body is required when event is 'request-changes'" + ); + } + if let Some(ref body) = self.body { + ensure!( + body.len() >= 10, + "body must be at least 10 characters" + ); + } + Ok(()) + } +} + +tool_result! { + name = "submit-pr-review", + params = SubmitPrReviewParams, + /// Result of submitting a pull request review + pub struct SubmitPrReviewResult { + pull_request_id: i32, + event: String, + body: Option, + repository: Option, + } +} + +impl Sanitize for SubmitPrReviewResult { + fn sanitize_fields(&mut self) { + self.event = sanitize_text(&self.event); + self.body = self.body.as_deref().map(sanitize_text); + self.repository = self.repository.as_deref().map(sanitize_text); + } +} + +/// Configuration for the submit-pr-review tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// submit-pr-review: +/// allowed-events: +/// - approve +/// - comment +/// allowed-repositories: +/// - self +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SubmitPrReviewConfig { + /// Which events are permitted. REQUIRED — empty list rejects all. + #[serde(default, rename = "allowed-events")] + pub allowed_events: Vec, + + /// Which repositories the agent may target. Empty list means all allowed repos. + #[serde(default, rename = "allowed-repositories")] + pub allowed_repositories: Vec, +} + +impl Default for SubmitPrReviewConfig { + fn default() -> Self { + Self { + allowed_events: Vec::new(), + allowed_repositories: Vec::new(), + } + } +} + +#[async_trait::async_trait] +impl Executor for SubmitPrReviewResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!( + "Submitting review on PR #{} — event: {}", + self.pull_request_id, self.event + ); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: SubmitPrReviewConfig = ctx.get_tool_config("submit-pr-review"); + debug!("Config: {:?}", config); + + // Validate event against allowed-events — REQUIRED. + // An empty allowed-events list means the operator hasn't opted in, so reject. + if config.allowed_events.is_empty() { + return Ok(ExecutionResult::failure( + "submit-pr-review requires 'allowed-events' to be configured in \ + safe-outputs.submit-pr-review. This prevents agents from casting \ + unrestricted review votes. Example:\n safe-outputs:\n submit-pr-review:\n \ + allowed-events:\n - comment\n - approve-with-suggestions" + .to_string(), + )); + } + if !config.allowed_events.contains(&self.event) { + return Ok(ExecutionResult::failure(format!( + "Event '{}' is not in the allowed-events list: [{}]", + self.event, + config.allowed_events.join(", ") + ))); + } + + // Validate repository against allowed-repositories config + let repo_alias = self.repository.as_deref().unwrap_or("self"); + if !config.allowed_repositories.is_empty() + && !config.allowed_repositories.contains(&repo_alias.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed-repositories list: [{}]", + repo_alias, + config.allowed_repositories.join(", ") + ))); + } + + // Resolve repo name + let repo_name = match resolve_repo_name(self.repository.as_deref(), ctx) { + Ok(name) => name, + Err(failure) => return Ok(failure), + }; + debug!("Resolved repository: {}", repo_name); + + // Map event to vote value + let vote_value = event_to_vote(&self.event).context(format!( + "Invalid event: '{}'. Must be one of: {}", + self.event, + VALID_EVENTS.join(", ") + ))?; + + let client = reqwest::Client::new(); + let encoded_project = utf8_percent_encode(project, PATH_SEGMENT).to_string(); + let encoded_repo = utf8_percent_encode(&repo_name, PATH_SEGMENT).to_string(); + let base_url = format!( + "{}/{}/_apis/git/repositories", + org_url.trim_end_matches('/'), + encoded_project, + ); + + // Resolve the current user identity via connection data. + // Use the org URL — supports vanity domains and national clouds. + let connection_url = format!( + "{}/_apis/connectiondata", + org_url.trim_end_matches('/') + ); + debug!("Connection data URL: {}", connection_url); + + let conn_response = client + .get(&connection_url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to fetch connection data")?; + + if !conn_response.status().is_success() { + let status = conn_response.status(); + let error_body = conn_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to fetch connection data (HTTP {}): {}", + status, error_body + ))); + } + + let conn_body: serde_json::Value = conn_response + .json() + .await + .context("Failed to parse connection data response")?; + + let user_id = conn_body + .get("authenticatedUser") + .and_then(|au| au.get("id")) + .and_then(|id| id.as_str()) + .context("Connection data response missing authenticatedUser.id")?; + debug!("Authenticated user ID: {}", user_id); + + // Self-approval guard: prevent the agent from approving PRs it created. + // Positive votes (approve=10, approve-with-suggestions=5) are blocked when + // the authenticated user is also the PR author. + if vote_value > 0 { + let pr_url = format!( + "{}/{}/pullRequests/{}?api-version=7.1", + base_url, encoded_repo, self.pull_request_id + ); + let pr_response = client + .get(&pr_url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to fetch PR for self-approval check")?; + + if pr_response.status().is_success() { + let pr_body: serde_json::Value = pr_response + .json() + .await + .context("Failed to parse PR response")?; + + let creator_id = pr_body + .get("createdBy") + .and_then(|cb| cb.get("id")) + .and_then(|id| id.as_str()); + + if creator_id == Some(user_id) { + return Ok(ExecutionResult::failure(format!( + "Self-approval blocked: the authenticated identity created PR #{} \ + and cannot cast a positive vote ('{}') on it", + self.pull_request_id, self.event + ))); + } + } else { + let status = pr_response.status(); + let error_body = pr_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to fetch PR #{} for self-approval check (HTTP {}): {}", + self.pull_request_id, status, error_body + ))); + } + } + + // PUT vote to reviewers endpoint + let encoded_user_id = utf8_percent_encode(user_id, PATH_SEGMENT).to_string(); + let vote_url = format!( + "{}/{}/pullRequests/{}/reviewers/{}?api-version=7.1", + base_url, encoded_repo, self.pull_request_id, encoded_user_id + ); + let vote_body = serde_json::json!({ + "vote": vote_value + }); + + info!( + "Voting '{}' ({}) on PR #{}", + self.event, vote_value, self.pull_request_id + ); + let response = client + .put(&vote_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&vote_body) + .send() + .await + .context("Failed to submit vote")?; + + if !response.status().is_success() { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to submit vote on PR #{} (HTTP {}): {}", + self.pull_request_id, status, error_body + ))); + } + + info!( + "Vote '{}' submitted on PR #{}", + self.event, self.pull_request_id + ); + + // If body is provided, also POST a comment thread with the review rationale + if let Some(ref body) = self.body { + let thread_url = format!( + "{}/{}/pullRequests/{}/threads?api-version=7.1", + base_url, encoded_repo, self.pull_request_id + ); + let thread_body = serde_json::json!({ + "comments": [{ + "parentCommentId": 0, + "content": body, + "commentType": 1 + }], + "status": 1 + }); + + info!( + "Posting review comment on PR #{} ({} chars)", + self.pull_request_id, + body.len() + ); + let thread_response = client + .post(&thread_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&thread_body) + .send() + .await + .context("Failed to post review comment thread")?; + + if !thread_response.status().is_success() { + let status = thread_response.status(); + let error_body = thread_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Vote submitted but failed to post review comment on PR #{} (HTTP {}): {}", + self.pull_request_id, status, error_body + ))); + } + + let thread_resp: serde_json::Value = thread_response + .json() + .await + .context("Failed to parse comment thread response")?; + + let thread_id = thread_resp + .get("id") + .and_then(|v| v.as_i64()) + .unwrap_or(0); + + info!( + "Review comment thread #{} posted on PR #{}", + thread_id, self.pull_request_id + ); + + return Ok(ExecutionResult::success_with_data( + format!( + "Review '{}' submitted on PR #{} with comment thread #{}", + self.event, self.pull_request_id, thread_id + ), + serde_json::json!({ + "pull_request_id": self.pull_request_id, + "event": self.event, + "vote_value": vote_value, + "thread_id": thread_id, + "repository": repo_name, + }), + )); + } + + Ok(ExecutionResult::success_with_data( + format!( + "Review '{}' submitted on PR #{}", + self.event, self.pull_request_id + ), + serde_json::json!({ + "pull_request_id": self.pull_request_id, + "event": self.event, + "vote_value": vote_value, + "repository": repo_name, + }), + )) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(SubmitPrReviewResult::NAME, "submit-pr-review"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{"pull_request_id": 42, "event": "approve"}"#; + let params: SubmitPrReviewParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.pull_request_id, 42); + assert_eq!(params.event, "approve"); + assert!(params.body.is_none()); + assert!(params.repository.is_none()); + } + + #[test] + fn test_params_converts_to_result() { + let params = SubmitPrReviewParams { + pull_request_id: 42, + event: "approve".to_string(), + body: None, + repository: Some("self".to_string()), + }; + let result: SubmitPrReviewResult = params.try_into().unwrap(); + assert_eq!(result.name, "submit-pr-review"); + assert_eq!(result.pull_request_id, 42); + assert_eq!(result.event, "approve"); + } + + #[test] + fn test_validation_rejects_zero_pr_id() { + let params = SubmitPrReviewParams { + pull_request_id: 0, + event: "approve".to_string(), + body: None, + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_event() { + let params = SubmitPrReviewParams { + pull_request_id: 1, + event: "merge".to_string(), + body: None, + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_request_changes_without_body() { + let params = SubmitPrReviewParams { + pull_request_id: 1, + event: "request-changes".to_string(), + body: None, + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_accepts_approve_without_body() { + let params = SubmitPrReviewParams { + pull_request_id: 1, + event: "approve".to_string(), + body: None, + repository: Some("self".to_string()), + }; + let result: Result = params.try_into(); + assert!(result.is_ok()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = SubmitPrReviewParams { + pull_request_id: 99, + event: "request-changes".to_string(), + body: Some("This needs significant rework before merging.".to_string()), + repository: Some("self".to_string()), + }; + let result: SubmitPrReviewResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"submit-pr-review""#)); + assert!(json.contains(r#""pull_request_id":99"#)); + assert!(json.contains(r#""event":"request-changes""#)); + } + + #[test] + fn test_config_defaults() { + let config = SubmitPrReviewConfig::default(); + assert!(config.allowed_events.is_empty()); + assert!(config.allowed_repositories.is_empty()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +allowed-events: + - approve + - comment +allowed-repositories: + - self + - other-repo +"#; + let config: SubmitPrReviewConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.allowed_events, vec!["approve", "comment"]); + assert_eq!(config.allowed_repositories, vec!["self", "other-repo"]); + } +} diff --git a/src/tools/update_pr.rs b/src/tools/update_pr.rs new file mode 100644 index 00000000..aa670cbd --- /dev/null +++ b/src/tools/update_pr.rs @@ -0,0 +1,1131 @@ +//! Update pull request safe output tool + +use log::{debug, info, warn}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::{PATH_SEGMENT, resolve_repo_name}; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Valid operation names for update-pr +const VALID_OPERATIONS: &[&str] = &[ + "add-reviewers", + "add-labels", + "set-auto-complete", + "vote", + "update-description", +]; + +/// Valid vote values +const VALID_VOTES: &[&str] = &[ + "approve", + "approve-with-suggestions", + "wait-for-author", + "reject", + "reset", +]; + +/// Valid merge strategy values accepted by ADO's completionOptions.mergeStrategy +const VALID_MERGE_STRATEGIES: &[&str] = &["squash", "noFastForward", "rebase", "rebaseMerge"]; + +/// Map a vote string to its ADO numeric value +fn vote_to_ado_value(vote: &str) -> Option { + match vote { + "approve" => Some(10), + "approve-with-suggestions" => Some(5), + "wait-for-author" => Some(-5), + "reject" => Some(-10), + "reset" => Some(0), + _ => None, + } +} + +/// Parameters for updating a pull request +#[derive(Deserialize, JsonSchema)] +pub struct UpdatePrParams { + /// Pull request ID (must be positive) + pub pull_request_id: i32, + + /// Repository alias: "self" for the pipeline repo, or an alias from the checkout list + #[serde(default)] + pub repository: Option, + + /// Operation to perform: "add-reviewers", "add-labels", "set-auto-complete", "vote", or "update-description" + pub operation: String, + + /// Reviewer emails (required for add-reviewers operation) + pub reviewers: Option>, + + /// Label names (required for add-labels operation) + pub labels: Option>, + + /// Vote value: "approve", "approve-with-suggestions", "wait-for-author", "reject", or "reset" + pub vote: Option, + + /// New PR description in markdown (required for update-description, must be >= 10 chars) + pub description: Option, +} + +impl Validate for UpdatePrParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!( + self.pull_request_id > 0, + "pull_request_id must be a positive integer" + ); + ensure!( + VALID_OPERATIONS.contains(&self.operation.as_str()), + "operation must be one of: {}", + VALID_OPERATIONS.join(", ") + ); + + match self.operation.as_str() { + "add-reviewers" => { + let reviewers = self + .reviewers + .as_ref() + .context("reviewers must be provided for add-reviewers operation")?; + ensure!( + !reviewers.is_empty(), + "reviewers list must not be empty for add-reviewers operation" + ); + } + "add-labels" => { + let labels = self + .labels + .as_ref() + .context("labels must be provided for add-labels operation")?; + ensure!( + !labels.is_empty(), + "labels list must not be empty for add-labels operation" + ); + } + "vote" => { + let vote = self + .vote + .as_ref() + .context("vote must be provided for vote operation")?; + ensure!( + VALID_VOTES.contains(&vote.as_str()), + "vote must be one of: {}", + VALID_VOTES.join(", ") + ); + } + "update-description" => { + let desc = self + .description + .as_ref() + .context("description must be provided for update-description operation")?; + ensure!( + desc.len() >= 10, + "description must be at least 10 characters" + ); + } + _ => {} // set-auto-complete has no extra required fields + } + + Ok(()) + } +} + +tool_result! { + name = "update-pr", + params = UpdatePrParams, + /// Result of updating a pull request + pub struct UpdatePrResult { + pull_request_id: i32, + repository: Option, + operation: String, + reviewers: Option>, + labels: Option>, + vote: Option, + description: Option, + } +} + +impl Sanitize for UpdatePrResult { + fn sanitize_fields(&mut self) { + self.repository = self.repository.as_deref().map(sanitize_text); + self.operation = sanitize_text(&self.operation); + self.reviewers = self + .reviewers + .as_ref() + .map(|rs| rs.iter().map(|r| sanitize_text(r)).collect()); + self.labels = self + .labels + .as_ref() + .map(|ls| ls.iter().map(|l| sanitize_text(l)).collect()); + self.vote = self.vote.as_deref().map(sanitize_text); + self.description = self.description.as_deref().map(sanitize_text); + } +} + +/// Configuration for the update-pr tool (specified in front matter) +/// +/// **Allow-list semantics note:** `allowed-operations` and `allowed-repositories` use +/// permissive defaults (empty = all allowed), while `allowed-votes` uses a secure default +/// (empty = all rejected). This asymmetry is intentional — vote operations can auto-approve +/// PRs, so they require explicit opt-in to prevent accidental privilege escalation. +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// update-pr: +/// allowed-operations: +/// - add-reviewers +/// - set-auto-complete +/// allowed-repositories: +/// - self +/// allowed-votes: +/// - approve +/// - reject +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct UpdatePrConfig { + /// Which operations are permitted. Empty list means all operations are allowed. + #[serde(default, rename = "allowed-operations")] + pub allowed_operations: Vec, + + /// Which repositories the agent may target. Empty list means all allowed repos. + #[serde(default, rename = "allowed-repositories")] + pub allowed_repositories: Vec, + + /// Which vote values are permitted. REQUIRED for vote operation — + /// empty list rejects all votes to prevent accidental auto-approve. + #[serde(default, rename = "allowed-votes")] + pub allowed_votes: Vec, + + /// Whether to delete the source branch after merge (for set-auto-complete, default: true) + #[serde(default = "default_true", rename = "delete-source-branch")] + pub delete_source_branch: bool, + + /// Merge strategy for auto-complete: "squash", "noFastForward", "rebase", "rebaseMerge" (default: "squash") + #[serde(default = "default_merge_strategy", rename = "merge-strategy")] + pub merge_strategy: String, +} + +fn default_true() -> bool { + true +} + +fn default_merge_strategy() -> String { + "squash".to_string() +} + +impl Default for UpdatePrConfig { + fn default() -> Self { + Self { + allowed_operations: Vec::new(), + allowed_repositories: Vec::new(), + allowed_votes: Vec::new(), + delete_source_branch: true, + merge_strategy: "squash".to_string(), + } + } +} + +#[async_trait::async_trait] +impl Executor for UpdatePrResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!( + "Updating PR #{} — operation: {}", + self.pull_request_id, self.operation + ); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: UpdatePrConfig = ctx.get_tool_config("update-pr"); + debug!("Config: {:?}", config); + + // Validate operation against allowed-operations + if !config.allowed_operations.is_empty() + && !config.allowed_operations.contains(&self.operation) + { + return Ok(ExecutionResult::failure(format!( + "Operation '{}' is not in the allowed-operations list: [{}]", + self.operation, + config.allowed_operations.join(", ") + ))); + } + + // Validate repository against allowed-repositories + let repo_alias = self.repository.as_deref().unwrap_or("self"); + if !config.allowed_repositories.is_empty() + && !config.allowed_repositories.contains(&repo_alias.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Repository '{}' is not in the allowed-repositories list: [{}]", + repo_alias, + config.allowed_repositories.join(", ") + ))); + } + + // Resolve repo name + let repo_name = match resolve_repo_name(self.repository.as_deref(), ctx) { + Ok(name) => name, + Err(failure) => return Ok(failure), + }; + debug!("Resolved repository: {}", repo_name); + + let client = reqwest::Client::new(); + let encoded_project = utf8_percent_encode(project, PATH_SEGMENT).to_string(); + let base_url = format!( + "{}/{}/_apis/git/repositories", + org_url.trim_end_matches('/'), + encoded_project, + ); + + match self.operation.as_str() { + "set-auto-complete" => { + self.execute_set_auto_complete(&client, &base_url, &repo_name, token, org_url, &config) + .await + } + "vote" => { + self.execute_vote( + &client, + &base_url, + &repo_name, + token, + org_url, + &config, + ) + .await + } + "add-reviewers" => { + self.execute_add_reviewers( + &client, + &base_url, + &repo_name, + token, + org_url, + ) + .await + } + "add-labels" => { + self.execute_add_labels(&client, &base_url, &repo_name, token) + .await + } + "update-description" => { + self.execute_update_description(&client, &base_url, &repo_name, token) + .await + } + _ => Ok(ExecutionResult::failure(format!( + "Unknown operation: {}", + self.operation + ))), + } + } +} + +impl UpdatePrResult { + /// Set auto-complete on a pull request. + /// + /// Resolves the authenticated user identity via `_apis/connectiondata`, then + /// patches the PR with `autoCompleteSetBy` and default completion options. + /// Uses the agent's own identity (not the PR creator) for proper audit trail. + async fn execute_set_auto_complete( + &self, + client: &reqwest::Client, + base_url: &str, + repo_name: &str, + token: &str, + org_url: &str, + config: &UpdatePrConfig, + ) -> anyhow::Result { + // Validate merge_strategy before any network I/O + if !VALID_MERGE_STRATEGIES.contains(&config.merge_strategy.as_str()) { + return Ok(ExecutionResult::failure(format!( + "Invalid merge-strategy '{}'. Must be one of: {}", + config.merge_strategy, + VALID_MERGE_STRATEGIES.join(", ") + ))); + } + + let encoded_repo = utf8_percent_encode(repo_name, PATH_SEGMENT).to_string(); + + // Resolve the agent's identity via connection data + let connection_url = format!( + "{}/_apis/connectiondata", + org_url.trim_end_matches('/') + ); + let conn_response = client + .get(&connection_url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to fetch connection data for auto-complete identity")?; + + if !conn_response.status().is_success() { + let status = conn_response.status(); + let error_body = conn_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to fetch connection data (HTTP {}): {}", + status, error_body + ))); + } + + let conn_body: serde_json::Value = conn_response + .json() + .await + .context("Failed to parse connection data response")?; + + let agent_user_id = conn_body + .get("authenticatedUser") + .and_then(|au| au.get("id")) + .and_then(|id| id.as_str()) + .context("Connection data response missing authenticatedUser.id")?; + debug!("Agent user ID for auto-complete: {}", agent_user_id); + + // PATCH to set auto-complete using the agent's identity + let patch_url = format!( + "{}/{}/pullRequests/{}?api-version=7.1", + base_url, encoded_repo, self.pull_request_id + ); + let patch_body = serde_json::json!({ + "autoCompleteSetBy": { + "id": agent_user_id + }, + "completionOptions": { + "deleteSourceBranch": config.delete_source_branch, + "mergeStrategy": config.merge_strategy + } + }); + + info!("Setting auto-complete on PR #{}", self.pull_request_id); + let response = client + .patch(&patch_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&patch_body) + .send() + .await + .context("Failed to set auto-complete on PR")?; + + if response.status().is_success() { + info!( + "Auto-complete set on PR #{}", + self.pull_request_id + ); + Ok(ExecutionResult::success_with_data( + format!("Auto-complete set on PR #{}", self.pull_request_id), + serde_json::json!({ + "pull_request_id": self.pull_request_id, + "operation": "set-auto-complete", + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + Ok(ExecutionResult::failure(format!( + "Failed to set auto-complete on PR #{} (HTTP {}): {}", + self.pull_request_id, status, error_body + ))) + } + } + + /// Submit a vote on a pull request. + /// + /// Resolves the current user identity via `_apis/connectiondata`, then + /// PUTs the vote to the reviewers endpoint. + async fn execute_vote( + &self, + client: &reqwest::Client, + base_url: &str, + repo_name: &str, + token: &str, + org_url: &str, + config: &UpdatePrConfig, + ) -> anyhow::Result { + let vote_str = self + .vote + .as_deref() + .context("vote value is required for vote operation")?; + + // Validate against allowed-votes — REQUIRED for vote operation. + // An empty allowed-votes list means the operator hasn't opted in, so reject. + if config.allowed_votes.is_empty() { + return Ok(ExecutionResult::failure( + "vote operation requires 'allowed-votes' to be configured in safe-outputs.update-pr. \ + This prevents agents from casting unrestricted votes (including approve). \ + Example:\n safe-outputs:\n update-pr:\n allowed-votes:\n - approve-with-suggestions\n - wait-for-author" + .to_string(), + )); + } + if !config.allowed_votes.contains(&vote_str.to_string()) + { + return Ok(ExecutionResult::failure(format!( + "Vote '{}' is not in the allowed-votes list: [{}]", + vote_str, + config.allowed_votes.join(", ") + ))); + } + + let vote_value = vote_to_ado_value(vote_str).context(format!( + "Invalid vote value: '{}'. Must be one of: {}", + vote_str, + VALID_VOTES.join(", ") + ))?; + + // Resolve the current user identity. + // Use the org URL for connection data — supports vanity domains and national clouds. + let connection_url = format!( + "{}/_apis/connectiondata", + org_url.trim_end_matches('/') + ); + debug!("Connection data URL: {}", connection_url); + + let conn_response = client + .get(&connection_url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to fetch connection data")?; + + if !conn_response.status().is_success() { + let status = conn_response.status(); + let error_body = conn_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to fetch connection data (HTTP {}): {}", + status, error_body + ))); + } + + let conn_body: serde_json::Value = conn_response + .json() + .await + .context("Failed to parse connection data response")?; + + let user_id = conn_body + .get("authenticatedUser") + .and_then(|au| au.get("id")) + .and_then(|id| id.as_str()) + .context("Connection data response missing authenticatedUser.id")?; + debug!("Authenticated user ID: {}", user_id); + + // Self-approval guard: prevent the agent from approving PRs it created. + // Positive votes (approve=10, approve-with-suggestions=5) are blocked when + // the authenticated user is also the PR author. + if vote_value > 0 { + let encoded_repo_check = + utf8_percent_encode(repo_name, PATH_SEGMENT).to_string(); + let pr_url = format!( + "{}/{}/pullRequests/{}?api-version=7.1", + base_url, encoded_repo_check, self.pull_request_id + ); + let pr_response = client + .get(&pr_url) + .basic_auth("", Some(token)) + .send() + .await + .context("Failed to fetch PR for self-approval check")?; + + if pr_response.status().is_success() { + let pr_body: serde_json::Value = pr_response + .json() + .await + .context("Failed to parse PR response")?; + + let creator_id = pr_body + .get("createdBy") + .and_then(|cb| cb.get("id")) + .and_then(|id| id.as_str()); + + if creator_id == Some(user_id) { + return Ok(ExecutionResult::failure(format!( + "Self-approval blocked: the authenticated identity created PR #{} \ + and cannot cast a positive vote ('{}') on it", + self.pull_request_id, vote_str + ))); + } + } else { + let status = pr_response.status(); + let error_body = pr_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to fetch PR #{} for self-approval check (HTTP {}): {}", + self.pull_request_id, status, error_body + ))); + } + } + + // PUT vote to reviewers endpoint + let encoded_repo = utf8_percent_encode(repo_name, PATH_SEGMENT).to_string(); + let encoded_user_id = utf8_percent_encode(user_id, PATH_SEGMENT).to_string(); + let vote_url = format!( + "{}/{}/pullRequests/{}/reviewers/{}?api-version=7.1", + base_url, encoded_repo, self.pull_request_id, encoded_user_id + ); + let vote_body = serde_json::json!({ + "vote": vote_value + }); + + info!( + "Voting '{}' ({}) on PR #{}", + vote_str, vote_value, self.pull_request_id + ); + let response = client + .put(&vote_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&vote_body) + .send() + .await + .context("Failed to submit vote")?; + + if response.status().is_success() { + info!( + "Vote '{}' submitted on PR #{}", + vote_str, self.pull_request_id + ); + Ok(ExecutionResult::success_with_data( + format!( + "Vote '{}' submitted on PR #{}", + vote_str, self.pull_request_id + ), + serde_json::json!({ + "pull_request_id": self.pull_request_id, + "operation": "vote", + "vote": vote_str, + "vote_value": vote_value, + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + Ok(ExecutionResult::failure(format!( + "Failed to submit vote on PR #{} (HTTP {}): {}", + self.pull_request_id, status, error_body + ))) + } + } + + /// Add reviewers to a pull request. + /// + /// For each reviewer email, resolves the identity via VSSPS, then PUTs to + /// the reviewers endpoint with vote 0. + async fn execute_add_reviewers( + &self, + client: &reqwest::Client, + base_url: &str, + repo_name: &str, + token: &str, + org_url: &str, + ) -> anyhow::Result { + let reviewers = self + .reviewers + .as_ref() + .context("reviewers list is required for add-reviewers operation")?; + + let encoded_repo = utf8_percent_encode(repo_name, PATH_SEGMENT).to_string(); + let mut added = Vec::new(); + let mut failed = Vec::new(); + + // Derive VSSPS base URL once, before the loop. + let trimmed_org = org_url.trim_end_matches('/'); + let vssps_base = trimmed_org + .replace("://dev.azure.com/", "://vssps.dev.azure.com/"); + if vssps_base == trimmed_org { + return Ok(ExecutionResult::failure(format!( + "Cannot derive VSSPS identity endpoint from org URL '{}'. \ + The add-reviewers operation requires dev.azure.com-style URLs \ + to resolve reviewer identities. Legacy *.visualstudio.com \ + organizations are not currently supported for this operation.", + trimmed_org + ))); + } + + for reviewer in reviewers { + let identity_url = format!( + "{}/_apis/identities?searchFilter=General&filterValue={}&api-version=7.1", + vssps_base, + utf8_percent_encode(reviewer, PATH_SEGMENT), + ); + debug!("Resolving identity for '{}': {}", reviewer, identity_url); + + let identity_response = client + .get(&identity_url) + .basic_auth("", Some(token)) + .send() + .await; + + let reviewer_id = match identity_response { + Ok(resp) if resp.status().is_success() => { + let body: serde_json::Value = resp.json().await.unwrap_or_default(); + body.get("value") + .and_then(|v| v.as_array()) + .and_then(|arr| arr.first()) + .and_then(|entry| entry.get("id")) + .and_then(|id| id.as_str()) + .map(|s| s.to_string()) + } + Ok(resp) => { + warn!( + "Identity lookup for '{}' returned HTTP {}", + reviewer, + resp.status() + ); + None + } + Err(e) => { + warn!("Identity lookup for '{}' failed: {}", reviewer, e); + None + } + }; + + let reviewer_id = match reviewer_id { + Some(id) => id, + None => { + warn!( + "Could not resolve identity for '{}', skipping", + reviewer + ); + failed.push(format!("{} (identity not found)", reviewer)); + continue; + } + }; + + let reviewer_url = format!( + "{}/{}/pullRequests/{}/reviewers/{}?api-version=7.1", + base_url, + encoded_repo, + self.pull_request_id, + reviewer_id, + ); + let reviewer_body = serde_json::json!({ + "vote": 0, + "isRequired": false + }); + + debug!("Adding reviewer '{}' to PR #{}", reviewer, self.pull_request_id); + let response = client + .put(&reviewer_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&reviewer_body) + .send() + .await; + + match response { + Ok(resp) if resp.status().is_success() => { + info!("Added reviewer '{}' to PR #{}", reviewer, self.pull_request_id); + added.push(reviewer.clone()); + } + Ok(resp) => { + let status = resp.status(); + let error_body = resp + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + warn!( + "Failed to add reviewer '{}' to PR #{} (HTTP {}): {}", + reviewer, self.pull_request_id, status, error_body + ); + failed.push(format!("{} (HTTP {})", reviewer, status)); + } + Err(e) => { + warn!( + "Request failed for reviewer '{}' on PR #{}: {}", + reviewer, self.pull_request_id, e + ); + failed.push(format!("{} (request error)", reviewer)); + } + } + } + + if added.is_empty() && !failed.is_empty() { + Ok(ExecutionResult::failure(format!( + "Failed to add any reviewers to PR #{}: {}", + self.pull_request_id, + failed.join(", ") + ))) + } else { + let mut message = format!( + "Added {} reviewer(s) to PR #{}", + added.len(), + self.pull_request_id + ); + if !failed.is_empty() { + message.push_str(&format!( + " ({} failed: {})", + failed.len(), + failed.join(", ") + )); + } + Ok(ExecutionResult::success_with_data( + message, + serde_json::json!({ + "pull_request_id": self.pull_request_id, + "operation": "add-reviewers", + "added": added, + "failed": failed, + }), + )) + } + } + + /// Add labels to a pull request. + /// + /// For each label, POSTs to the labels endpoint. + async fn execute_add_labels( + &self, + client: &reqwest::Client, + base_url: &str, + repo_name: &str, + token: &str, + ) -> anyhow::Result { + let labels = self + .labels + .as_ref() + .context("labels list is required for add-labels operation")?; + + let encoded_repo = utf8_percent_encode(repo_name, PATH_SEGMENT).to_string(); + let labels_url = format!( + "{}/{}/pullRequests/{}/labels?api-version=7.1", + base_url, encoded_repo, self.pull_request_id + ); + + let mut added = Vec::new(); + let mut failed = Vec::new(); + + for label in labels { + let label_body = serde_json::json!({ + "name": label + }); + + debug!( + "Adding label '{}' to PR #{}", + label, self.pull_request_id + ); + let response = client + .post(&labels_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&label_body) + .send() + .await; + + match response { + Ok(resp) if resp.status().is_success() => { + info!("Added label '{}' to PR #{}", label, self.pull_request_id); + added.push(label.clone()); + } + Ok(resp) => { + let status = resp.status(); + let error_body = resp + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + warn!( + "Failed to add label '{}' to PR #{} (HTTP {}): {}", + label, self.pull_request_id, status, error_body + ); + failed.push(format!("{} (HTTP {})", label, status)); + } + Err(e) => { + warn!( + "Request failed for label '{}' on PR #{}: {}", + label, self.pull_request_id, e + ); + failed.push(format!("{} (request error)", label)); + } + } + } + + if added.is_empty() && !failed.is_empty() { + Ok(ExecutionResult::failure(format!( + "Failed to add any labels to PR #{}: {}", + self.pull_request_id, + failed.join(", ") + ))) + } else { + let mut message = format!( + "Added {} label(s) to PR #{}", + added.len(), + self.pull_request_id + ); + if !failed.is_empty() { + message.push_str(&format!( + " ({} failed: {})", + failed.len(), + failed.join(", ") + )); + } + Ok(ExecutionResult::success_with_data( + message, + serde_json::json!({ + "pull_request_id": self.pull_request_id, + "operation": "add-labels", + "added": added, + "failed": failed, + }), + )) + } + } + + /// Update the description of a pull request. + async fn execute_update_description( + &self, + client: &reqwest::Client, + base_url: &str, + repo_name: &str, + token: &str, + ) -> anyhow::Result { + let description = self + .description + .as_ref() + .context("description is required for update-description operation")?; + + let encoded_repo = utf8_percent_encode(repo_name, PATH_SEGMENT).to_string(); + let patch_url = format!( + "{}/{}/pullRequests/{}?api-version=7.1", + base_url, encoded_repo, self.pull_request_id + ); + let patch_body = serde_json::json!({ + "description": description + }); + + info!( + "Updating description on PR #{} ({} chars)", + self.pull_request_id, + description.len() + ); + let response = client + .patch(&patch_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&patch_body) + .send() + .await + .context("Failed to update PR description")?; + + if response.status().is_success() { + info!("Description updated on PR #{}", self.pull_request_id); + Ok(ExecutionResult::success_with_data( + format!( + "Description updated on PR #{}", + self.pull_request_id + ), + serde_json::json!({ + "pull_request_id": self.pull_request_id, + "operation": "update-description", + }), + )) + } else { + let status = response.status(); + let error_body = response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + Ok(ExecutionResult::failure(format!( + "Failed to update description on PR #{} (HTTP {}): {}", + self.pull_request_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(UpdatePrResult::NAME, "update-pr"); + } + + #[test] + fn test_params_deserializes() { + let json = r#"{ + "pull_request_id": 42, + "operation": "set-auto-complete" + }"#; + let params: UpdatePrParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.pull_request_id, 42); + assert_eq!(params.operation, "set-auto-complete"); + assert!(params.repository.is_none()); + } + + #[test] + fn test_params_converts_to_result() { + let params = UpdatePrParams { + pull_request_id: 42, + repository: Some("self".to_string()), + operation: "set-auto-complete".to_string(), + reviewers: None, + labels: None, + vote: None, + description: None, + }; + let result: UpdatePrResult = params.try_into().unwrap(); + assert_eq!(result.name, "update-pr"); + assert_eq!(result.pull_request_id, 42); + assert_eq!(result.operation, "set-auto-complete"); + } + + #[test] + fn test_validation_rejects_zero_pr_id() { + let params = UpdatePrParams { + pull_request_id: 0, + repository: None, + operation: "set-auto-complete".to_string(), + reviewers: None, + labels: None, + vote: None, + description: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_invalid_operation() { + let params = UpdatePrParams { + pull_request_id: 1, + repository: None, + operation: "delete-pr".to_string(), + reviewers: None, + labels: None, + vote: None, + description: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_vote_without_value() { + let params = UpdatePrParams { + pull_request_id: 1, + repository: None, + operation: "vote".to_string(), + reviewers: None, + labels: None, + vote: None, + description: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_reviewers_without_list() { + let params = UpdatePrParams { + pull_request_id: 1, + repository: None, + operation: "add-reviewers".to_string(), + reviewers: None, + labels: None, + vote: None, + description: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = UpdatePrParams { + pull_request_id: 99, + repository: Some("self".to_string()), + operation: "vote".to_string(), + reviewers: None, + labels: None, + vote: Some("approve".to_string()), + description: None, + }; + let result: UpdatePrResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"update-pr""#)); + assert!(json.contains(r#""pull_request_id":99"#)); + assert!(json.contains(r#""operation":"vote""#)); + } + + #[test] + fn test_config_defaults() { + let config = UpdatePrConfig::default(); + assert!(config.allowed_operations.is_empty()); + assert!(config.allowed_repositories.is_empty()); + assert!(config.allowed_votes.is_empty()); + assert_eq!(config.merge_strategy, "squash"); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +allowed-operations: + - add-reviewers + - set-auto-complete +allowed-repositories: + - self +allowed-votes: + - approve + - reject +"#; + let config: UpdatePrConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.allowed_operations.len(), 2); + assert!(config.allowed_operations.contains(&"add-reviewers".to_string())); + assert!(config.allowed_operations.contains(&"set-auto-complete".to_string())); + assert_eq!(config.allowed_repositories.len(), 1); + assert_eq!(config.allowed_votes.len(), 2); + } + + #[test] + fn test_valid_merge_strategies_are_expected_values() { + assert_eq!( + VALID_MERGE_STRATEGIES, + &["squash", "noFastForward", "rebase", "rebaseMerge"] + ); + } + + #[test] + fn test_config_deserializes_merge_strategy() { + let yaml = r#"merge-strategy: rebase"#; + let config: UpdatePrConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.merge_strategy, "rebase"); + } + + #[test] + fn test_valid_merge_strategies_are_recognized() { + for strategy in VALID_MERGE_STRATEGIES { + assert!( + VALID_MERGE_STRATEGIES.contains(strategy), + "'{}' should be a valid merge strategy", + strategy + ); + } + // Ensure invalid strategy is NOT in the list + assert!(!VALID_MERGE_STRATEGIES.contains(&"invalid")); + assert!(!VALID_MERGE_STRATEGIES.contains(&"Squash")); + } +} diff --git a/src/tools/upload_attachment.rs b/src/tools/upload_attachment.rs new file mode 100644 index 00000000..b258dfaf --- /dev/null +++ b/src/tools/upload_attachment.rs @@ -0,0 +1,528 @@ +//! Upload attachment safe output tool + +use log::{debug, info, warn}; +use percent_encoding::utf8_percent_encode; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use super::PATH_SEGMENT; +use crate::sanitize::{Sanitize, sanitize as sanitize_text}; +use crate::tool_result; +use crate::tools::{ExecutionContext, ExecutionResult, Executor, Validate}; +use anyhow::{Context, ensure}; + +/// Parameters for uploading an attachment to a work item +#[derive(Deserialize, JsonSchema)] +pub struct UploadAttachmentParams { + /// The work item ID to attach the file to + pub work_item_id: i64, + + /// Path to the file in the workspace to upload. Must be a relative path with no directory traversal. + pub file_path: String, + + /// Optional description of the attachment. Must be at least 3 characters if provided. + pub comment: Option, +} + +impl Validate for UploadAttachmentParams { + fn validate(&self) -> anyhow::Result<()> { + ensure!(self.work_item_id > 0, "work_item_id must be positive"); + ensure!(!self.file_path.is_empty(), "file_path must not be empty"); + ensure!( + !self.file_path.split(['/', '\\']).any(|component| component == ".."), + "file_path must not contain '..' path components" + ); + ensure!( + !self.file_path.starts_with('/') && !self.file_path.starts_with('\\'), + "file_path must not be an absolute path" + ); + ensure!( + !self.file_path.contains(':'), + "file_path must not contain ':'" + ); + ensure!( + !self.file_path.contains('\0'), + "file_path must not contain null bytes" + ); + ensure!( + !self + .file_path + .split(['/', '\\']) + .any(|component| component == ".git"), + "file_path must not contain '.git' components" + ); + if let Some(comment) = &self.comment { + ensure!( + comment.len() >= 3, + "comment must be at least 3 characters" + ); + } + Ok(()) + } +} + +tool_result! { + name = "upload-attachment", + params = UploadAttachmentParams, + /// Result of uploading an attachment to a work item + pub struct UploadAttachmentResult { + work_item_id: i64, + file_path: String, + comment: Option, + } +} + +impl Sanitize for UploadAttachmentResult { + fn sanitize_fields(&mut self) { + if let Some(comment) = &self.comment { + self.comment = Some(sanitize_text(comment)); + } + } +} + +const DEFAULT_MAX_FILE_SIZE: u64 = 5 * 1024 * 1024; // 5 MB + +/// Configuration for the upload-attachment tool (specified in front matter) +/// +/// Example front matter: +/// ```yaml +/// safe-outputs: +/// upload-attachment: +/// max-file-size: 5242880 +/// allowed-extensions: +/// - .png +/// - .pdf +/// - .log +/// comment-prefix: "[Agent] " +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct UploadAttachmentConfig { + /// Maximum file size in bytes (default: 5 MB) + #[serde(default = "default_max_file_size", rename = "max-file-size")] + pub max_file_size: u64, + + /// Allowed file extensions (e.g., [".png", ".pdf"]). Empty means all extensions allowed. + #[serde(default, rename = "allowed-extensions")] + pub allowed_extensions: Vec, + + /// Prefix to prepend to the comment + #[serde(default, rename = "comment-prefix")] + pub comment_prefix: Option, +} + +fn default_max_file_size() -> u64 { + DEFAULT_MAX_FILE_SIZE +} + +impl Default for UploadAttachmentConfig { + fn default() -> Self { + Self { + max_file_size: DEFAULT_MAX_FILE_SIZE, + allowed_extensions: Vec::new(), + comment_prefix: None, + } + } +} + +#[async_trait::async_trait] +impl Executor for UploadAttachmentResult { + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + info!( + "Uploading attachment '{}' to work item #{}", + self.file_path, self.work_item_id + ); + + let org_url = ctx + .ado_org_url + .as_ref() + .context("AZURE_DEVOPS_ORG_URL not set")?; + let project = ctx + .ado_project + .as_ref() + .context("SYSTEM_TEAMPROJECT not set")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + debug!("ADO org: {}, project: {}", org_url, project); + + let config: UploadAttachmentConfig = ctx.get_tool_config("upload-attachment"); + debug!("Max file size: {} bytes", config.max_file_size); + debug!("Allowed extensions: {:?}", config.allowed_extensions); + + // Validate file extension against allowed-extensions (if configured) + if !config.allowed_extensions.is_empty() { + let has_valid_ext = config.allowed_extensions.iter().any(|ext| { + self.file_path + .to_lowercase() + .ends_with(&ext.to_lowercase()) + }); + if !has_valid_ext { + return Ok(ExecutionResult::failure(format!( + "File '{}' has an extension not in the allowed list: {:?}", + self.file_path, config.allowed_extensions + ))); + } + } + + // Resolve file path relative to source_directory + let resolved_path = ctx.source_directory.join(&self.file_path); + debug!("Resolved file path: {}", resolved_path.display()); + + // Canonicalize to resolve symlinks, then verify the path stays within source_directory. + let canonical = resolved_path + .canonicalize() + .context("Failed to canonicalize file path — file may not exist or contains broken symlinks")?; + let canonical_base = ctx + .source_directory + .canonicalize() + .context("Failed to canonicalize source directory")?; + if !canonical.starts_with(&canonical_base) { + return Ok(ExecutionResult::failure(format!( + "File path '{}' resolves outside the workspace (symlink escape)", + self.file_path + ))); + } + + // Check file size + let metadata = std::fs::metadata(&canonical) + .context("Failed to read file metadata")?; + let file_size = metadata.len(); + debug!("File size: {} bytes", file_size); + if file_size > config.max_file_size { + return Ok(ExecutionResult::failure(format!( + "File size ({} bytes) exceeds maximum allowed size ({} bytes)", + file_size, config.max_file_size + ))); + } + + // Read file bytes + let file_bytes = + std::fs::read(&canonical).context("Failed to read file contents")?; + + // Check if file is text (valid UTF-8) — if text, scan for ##vso[ command injection. + // Binary files (where from_utf8 fails) skip this check intentionally: ADO's attachment + // viewer won't execute ##vso[ sequences from binary content. Note that a binary file + // with a valid UTF-8 preamble but malformed tail will also skip the scan, but this is + // acceptable because the injection risk from binary attachments is negligible. + if let Ok(text) = std::str::from_utf8(&file_bytes) { + if text.contains("##vso[") { + return Ok(ExecutionResult::failure(format!( + "File '{}' contains '##vso[' command injection sequence", + self.file_path + ))); + } + } + + // Extract filename for upload + let filename = std::path::Path::new(&self.file_path) + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("attachment"); + debug!("Upload filename: {}", filename); + + // Apply comment-prefix to comment if configured + let effective_comment = match (&self.comment, &config.comment_prefix) { + (Some(c), Some(prefix)) => format!("{}{}", prefix, c), + (Some(c), None) => c.clone(), + (None, _) => "Uploaded by agent".to_string(), + }; + debug!("Effective comment: {}", effective_comment); + + let client = reqwest::Client::new(); + + // Step 1: Upload file + // POST {org_url}/{project}/_apis/wit/attachments?fileName={filename}&api-version=7.1 + let upload_url = format!( + "{}/{}/_apis/wit/attachments?fileName={}&api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + utf8_percent_encode(filename, PATH_SEGMENT), + ); + debug!("Upload URL: {}", upload_url); + + info!("Uploading file '{}' ({} bytes)", filename, file_size); + let upload_response = client + .post(&upload_url) + .header("Content-Type", "application/octet-stream") + .basic_auth("", Some(token)) + .body(file_bytes) + .send() + .await + .context("Failed to upload attachment to Azure DevOps")?; + + if !upload_response.status().is_success() { + let status = upload_response.status(); + let error_body = upload_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to upload attachment (HTTP {}): {}", + status, error_body + ))); + } + + let upload_body: serde_json::Value = upload_response + .json() + .await + .context("Failed to parse upload response JSON")?; + + let attachment_url = upload_body + .get("url") + .and_then(|v| v.as_str()) + .context("Upload response missing 'url' field")? + .to_string(); + debug!("Attachment URL: {}", attachment_url); + + // Step 2: Link attachment to work item + // PATCH {org_url}/{project}/_apis/wit/workitems/{work_item_id}?api-version=7.1 + let link_url = format!( + "{}/{}/_apis/wit/workitems/{}?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + self.work_item_id, + ); + debug!("Link URL: {}", link_url); + + let patch_doc = serde_json::json!([{ + "op": "add", + "path": "/relations/-", + "value": { + "rel": "AttachedFile", + "url": attachment_url, + "attributes": { + "comment": effective_comment, + } + } + }]); + + info!( + "Linking attachment to work item #{}", + self.work_item_id + ); + let link_response = client + .patch(&link_url) + .header("Content-Type", "application/json-patch+json") + .basic_auth("", Some(token)) + .json(&patch_doc) + .send() + .await + .context("Failed to link attachment to work item")?; + + if link_response.status().is_success() { + info!( + "Attachment '{}' linked to work item #{}", + filename, self.work_item_id + ); + + Ok(ExecutionResult::success_with_data( + format!( + "Uploaded '{}' and linked to work item #{}", + filename, self.work_item_id + ), + serde_json::json!({ + "work_item_id": self.work_item_id, + "file_path": self.file_path, + "attachment_url": attachment_url, + "project": project, + }), + )) + } else { + let status = link_response.status(); + let error_body = link_response + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + + warn!( + "Attachment uploaded but linking failed — the attachment at {} is orphaned", + attachment_url + ); + + Ok(ExecutionResult::failure(format!( + "Attachment uploaded but failed to link to work item #{} (HTTP {}): {}", + self.work_item_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tools::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(UploadAttachmentResult::NAME, "upload-attachment"); + } + + #[test] + fn test_params_deserializes() { + let json = + r#"{"work_item_id": 42, "file_path": "output/report.pdf", "comment": "Weekly report"}"#; + let params: UploadAttachmentParams = serde_json::from_str(json).unwrap(); + assert_eq!(params.work_item_id, 42); + assert_eq!(params.file_path, "output/report.pdf"); + assert_eq!(params.comment, Some("Weekly report".to_string())); + } + + #[test] + fn test_params_converts_to_result() { + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "output/report.pdf".to_string(), + comment: Some("Weekly report".to_string()), + }; + let result: UploadAttachmentResult = params.try_into().unwrap(); + assert_eq!(result.name, "upload-attachment"); + assert_eq!(result.work_item_id, 42); + assert_eq!(result.file_path, "output/report.pdf"); + assert_eq!(result.comment, Some("Weekly report".to_string())); + } + + #[test] + fn test_validation_rejects_zero_work_item_id() { + let params = UploadAttachmentParams { + work_item_id: 0, + file_path: "output/report.pdf".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_empty_file_path() { + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_path_traversal() { + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "../etc/passwd".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_embedded_traversal() { + // "src/../secret" has ".." as a standalone component + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "src/../secret".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_backslash_traversal() { + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "src\\..\\secret.txt".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_rejects_backslash_absolute_path() { + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "\\etc\\passwd".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_validation_accepts_filename_with_dots_in_name() { + // "report..v2.pdf" has ".." inside a filename, not as a standalone component + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "report..v2.pdf".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_ok(), "report..v2.pdf should be a valid filename"); + } + + #[test] + fn test_validation_accepts_directory_with_dots_in_name() { + // "v2..3/notes.md" — ".." inside a directory name, not a standalone component + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "v2..3/notes.md".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_ok(), "v2..3/notes.md should be valid"); + } + + #[test] + fn test_validation_rejects_absolute_path() { + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "/etc/passwd".to_string(), + comment: None, + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_result_serializes_correctly() { + let params = UploadAttachmentParams { + work_item_id: 42, + file_path: "output/report.pdf".to_string(), + comment: Some("Test attachment".to_string()), + }; + let result: UploadAttachmentResult = params.try_into().unwrap(); + let json = serde_json::to_string(&result).unwrap(); + + assert!(json.contains(r#""name":"upload-attachment""#)); + assert!(json.contains(r#""work_item_id":42"#)); + assert!(json.contains(r#""file_path":"output/report.pdf""#)); + } + + #[test] + fn test_config_defaults() { + let config = UploadAttachmentConfig::default(); + assert_eq!(config.max_file_size, 5 * 1024 * 1024); + assert!(config.allowed_extensions.is_empty()); + assert!(config.comment_prefix.is_none()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +max-file-size: 1048576 +allowed-extensions: + - .png + - .pdf + - .log +comment-prefix: "[Agent] " +"#; + let config: UploadAttachmentConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.max_file_size, 1_048_576); + assert_eq!( + config.allowed_extensions, + vec![".png", ".pdf", ".log"] + ); + assert_eq!(config.comment_prefix, Some("[Agent] ".to_string())); + } +} diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index 8908288d..cbc76b0f 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -1660,3 +1660,309 @@ fn test_compile_auto_discover_skips_missing_source() { let _ = fs::remove_dir_all(&temp_dir); } + +/// Test that submit-pr-review fails compilation when allowed-events is missing +#[test] +fn test_submit_pr_review_requires_allowed_events() { + let temp_dir = std::env::temp_dir().join(format!( + "agentic-pipeline-spr-events-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + + let test_input = temp_dir.join("spr-agent.md"); + let test_content = r#"--- +name: "PR Review Agent" +description: "Agent that submits PR reviews but has no allowed-events" +permissions: + write: my-write-sc +safe-outputs: + submit-pr-review: + allowed-repositories: + - self +--- + +## PR Review Agent + +Submit PR reviews. +"#; + fs::write(&test_input, test_content).expect("Failed to write test input"); + + let output_path = temp_dir.join("spr-agent.yml"); + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + test_input.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + !output.status.success(), + "Compiler should fail when submit-pr-review lacks allowed-events" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("allowed-events"), + "Error message should mention allowed-events: {stderr}" + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + +/// Test that submit-pr-review fails compilation when allowed-events is an empty list +#[test] +fn test_submit_pr_review_requires_nonempty_allowed_events() { + let temp_dir = std::env::temp_dir().join(format!( + "agentic-pipeline-spr-empty-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + + let test_input = temp_dir.join("spr-agent.md"); + let test_content = r#"--- +name: "PR Review Agent" +description: "Agent that submits PR reviews but has empty allowed-events" +permissions: + write: my-write-sc +safe-outputs: + submit-pr-review: + allowed-events: [] +--- + +## PR Review Agent + +Submit PR reviews. +"#; + fs::write(&test_input, test_content).expect("Failed to write test input"); + + let output_path = temp_dir.join("spr-agent.yml"); + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + test_input.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + !output.status.success(), + "Compiler should fail when submit-pr-review has empty allowed-events" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("allowed-events"), + "Error message should mention allowed-events: {stderr}" + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + +/// Test that submit-pr-review compiles successfully with proper config +#[test] +fn test_submit_pr_review_compiles_with_allowed_events() { + let temp_dir = std::env::temp_dir().join(format!( + "agentic-pipeline-spr-pass-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + + let test_input = temp_dir.join("spr-agent.md"); + let test_content = r#"--- +name: "PR Review Agent" +description: "Agent that submits PR reviews with proper config" +permissions: + write: my-write-sc +safe-outputs: + submit-pr-review: + allowed-events: + - comment + - approve-with-suggestions +--- + +## PR Review Agent + +Submit PR reviews. +"#; + fs::write(&test_input, test_content).expect("Failed to write test input"); + + let output_path = temp_dir.join("spr-agent.yml"); + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + test_input.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + output.status.success(), + "Compiler should succeed with proper submit-pr-review config: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + +/// Test that update-pr fails compilation when vote is reachable but allowed-votes is missing +#[test] +fn test_update_pr_requires_allowed_votes_when_vote_reachable() { + let temp_dir = std::env::temp_dir().join(format!( + "agentic-pipeline-uprvote-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + + let test_input = temp_dir.join("upr-agent.md"); + // No allowed-operations → vote is reachable; no allowed-votes → should fail + let test_content = r#"--- +name: "Update PR Agent" +description: "Agent that votes on PRs but forgot to set allowed-votes" +permissions: + write: my-write-sc +safe-outputs: + update-pr: + allowed-repositories: + - self +--- + +## Update PR Agent + +Vote on pull requests. +"#; + fs::write(&test_input, test_content).expect("Failed to write test input"); + + let output_path = temp_dir.join("upr-agent.yml"); + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + test_input.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + !output.status.success(), + "Compiler should fail when update-pr lacks allowed-votes with vote reachable" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("allowed-votes"), + "Error message should mention allowed-votes: {stderr}" + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + +/// Test that update-pr compiles successfully when vote is restricted via allowed-operations +#[test] +fn test_update_pr_compiles_when_vote_excluded_from_allowed_operations() { + let temp_dir = std::env::temp_dir().join(format!( + "agentic-pipeline-uprnotvote-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + + let test_input = temp_dir.join("upr-agent.md"); + let test_content = r#"--- +name: "Update PR Agent" +description: "Agent that sets reviewers but cannot vote" +permissions: + write: my-write-sc +safe-outputs: + update-pr: + allowed-operations: + - add-reviewers + - set-auto-complete +--- + +## Update PR Agent + +Manage pull requests. +"#; + fs::write(&test_input, test_content).expect("Failed to write test input"); + + let output_path = temp_dir.join("upr-agent.yml"); + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + test_input.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + output.status.success(), + "Compiler should succeed when vote is excluded from allowed-operations: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + +/// Test that update-pr compiles successfully when allowed-votes is set +#[test] +fn test_update_pr_compiles_when_allowed_votes_set() { + let temp_dir = std::env::temp_dir().join(format!( + "agentic-pipeline-uprvoteset-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + + let test_input = temp_dir.join("upr-agent.md"); + let test_content = r#"--- +name: "Update PR Agent" +description: "Agent that can vote on PRs with proper config" +permissions: + write: my-write-sc +safe-outputs: + update-pr: + allowed-votes: + - approve-with-suggestions + - wait-for-author +--- + +## Update PR Agent + +Vote on pull requests. +"#; + fs::write(&test_input, test_content).expect("Failed to write test input"); + + let output_path = temp_dir.join("upr-agent.yml"); + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + test_input.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + output.status.success(), + "Compiler should succeed with proper update-pr vote config: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let _ = fs::remove_dir_all(&temp_dir); +}