diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index 18274054..bf4ce835 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -64,13 +64,15 @@ Creates an Azure DevOps work item. **Agent parameters:** - `title` - A concise title for the work item (required, must be more than 5 characters) - `description` - Work item description in markdown format (required, must be more than 30 characters) +- `tags` - Tags to apply to the work item (optional list; each tag must not contain a semicolon). May be subject to the `allowed-tags` allowlist. Merged with any static `tags` configured in front matter. **Configuration options (front matter):** - `work-item-type` - Work item type (default: "Task") - `area-path` - Area path for the work item - `iteration-path` - Iteration path for the work item - `assignee` - User to assign (email or display name) -- `tags` - List of tags to apply +- `tags` - Static list of tags always applied to the work item (regardless of agent input) +- `allowed-tags` - Allowlist of tags the agent is permitted to use via the `tags` parameter. If empty, any agent-provided tags are accepted. Supports prefix wildcards: entries ending with `*` match by prefix (e.g., `"agent-*"` matches `"agent-created"`, `"agent-review"`, etc.). - `custom-fields` - Map of custom field reference names to values (e.g., `Custom.MyField: "value"`) - `max` - Maximum number of create-work-item outputs allowed per run (default: 1) - `include-stats` - Whether to append agent execution stats to the work item description (default: true) @@ -110,6 +112,7 @@ safe-outputs: iteration-path: true # enable iteration path updates (default: false) assignee: true # enable assignee updates (default: false) tags: true # enable tag updates (default: false) + allowed-tags: [] # Optional — restrict which tags the agent can set (empty = any; supports prefix wildcards like "agent-*") ``` **Security note:** Every field that can be modified requires explicit opt-in (`true`) in the front matter configuration. If the `max` limit is exceeded, additional entries are skipped rather than aborting the entire batch. diff --git a/src/safeoutputs/create_work_item.rs b/src/safeoutputs/create_work_item.rs index 6cb7ab53..86a2effb 100644 --- a/src/safeoutputs/create_work_item.rs +++ b/src/safeoutputs/create_work_item.rs @@ -20,12 +20,26 @@ pub struct CreateWorkItemParams { /// Work item description in markdown format. Use headings, lists, code blocks, and other markdown formatting. Ensure adequate content > 30 characters. pub description: String, + + /// Tags to apply to the work item. May be subject to an operator-configured + /// allowlist (`allowed-tags` in front matter). Each tag must not contain a + /// semicolon (ADO uses semicolons as tag separators). + #[serde(default)] + pub tags: Vec, } impl Validate for CreateWorkItemParams { fn validate(&self) -> anyhow::Result<()> { ensure!(self.title.len() > 5); ensure!(self.description.len() > 30); + for tag in &self.tags { + ensure!( + !tag.contains(';'), + "Tag '{}' contains a semicolon, which is not allowed \ + (ADO uses semicolons as tag separators)", + tag + ); + } Ok(()) } } @@ -38,6 +52,9 @@ tool_result! { pub struct CreateWorkItemResult { title: String, description: String, + /// Agent-provided tags (validated against allowed-tags at execution time) + #[serde(default)] + tags: Vec, } } @@ -45,6 +62,9 @@ impl SanitizeContent for CreateWorkItemResult { fn sanitize_content_fields(&mut self) { self.title = sanitize_text(&self.title); self.description = sanitize_text(&self.description); + for tag in &mut self.tags { + *tag = sanitize_text(tag); + } } } @@ -61,6 +81,9 @@ impl SanitizeContent for CreateWorkItemResult { /// tags: /// - agent-created /// - automated +/// allowed-tags: +/// - "agent-*" +/// - automated /// artifact_link: /// enabled: true /// repository: "my-repo-name" # optional, defaults to current repo @@ -84,10 +107,17 @@ pub struct CreateWorkItemConfig { #[serde(default)] pub assignee: Option, - /// Tags to apply to the work item + /// Static tags always applied to the work item (regardless of agent input) #[serde(default)] pub tags: Vec, + /// Allowlist of tags the agent is permitted to use via the `tags` parameter. + /// If empty, any agent-provided tags are accepted. + /// Supports prefix wildcards: entries ending with `*` match by prefix + /// (e.g., `"agent-*"` matches `"agent-created"`, `"agent-review"`, etc.). + #[serde(default, rename = "allowed-tags")] + pub allowed_tags: Vec, + /// Additional custom fields as key-value pairs /// Keys should be the full field reference name (e.g., "Custom.MyField") #[serde(default, rename = "custom-fields")] @@ -143,6 +173,7 @@ impl Default for CreateWorkItemConfig { iteration_path: None, assignee: None, tags: Vec::new(), + allowed_tags: Vec::new(), custom_fields: std::collections::HashMap::new(), artifact_link: ArtifactLinkConfig::default(), include_stats: true, @@ -271,6 +302,26 @@ impl Executor for CreateWorkItemResult { debug!("Iteration path: {:?}", config.iteration_path); debug!("Assignee: {:?}", config.assignee); + // Validate agent-provided tags against allowed-tags (if configured) + if !self.tags.is_empty() && !config.allowed_tags.is_empty() { + let disallowed: Vec<_> = self + .tags + .iter() + .filter(|tag| { + !config + .allowed_tags + .iter() + .any(|pattern| super::tag_matches_pattern(tag, pattern)) + }) + .collect(); + if !disallowed.is_empty() { + return Ok(ExecutionResult::failure(format!( + "Agent-provided tags not in allowed-tags: {}", + disallowed.iter().map(|t| t.as_str()).collect::>().join(", ") + ))); + } + } + // Build the Azure DevOps REST API URL for creating work items // POST https://dev.azure.com/{organization}/{project}/_apis/wit/workitems/${type}?api-version=7.0 debug!("Building work item creation request"); @@ -309,8 +360,15 @@ impl Executor for CreateWorkItemResult { if let Some(assignee) = &config.assignee { patch_doc.push(field_op("System.AssignedTo", assignee)); } - if !config.tags.is_empty() { - patch_doc.push(field_op("System.Tags", config.tags.join("; "))); + // Merge static config tags with validated agent-provided tags (dedup, case-insensitive) + let mut all_tags = config.tags.clone(); + for tag in &self.tags { + if !all_tags.iter().any(|t| t.eq_ignore_ascii_case(tag)) { + all_tags.push(tag.clone()); + } + } + if !all_tags.is_empty() { + patch_doc.push(field_op("System.Tags", all_tags.join("; "))); } // Add any custom fields @@ -456,6 +514,7 @@ mod tests { let params = CreateWorkItemParams { title: "Implement feature".to_string(), description: "This is a sufficiently long description for the work item.".to_string(), + tags: vec![], }; let result: CreateWorkItemResult = params.try_into().unwrap(); assert_eq!(result.name, "create-work-item"); @@ -468,6 +527,7 @@ mod tests { let params = CreateWorkItemParams { title: "Hi".to_string(), description: "This is a sufficiently long description for the work item.".to_string(), + tags: vec![], }; let result: Result = params.try_into(); assert!(result.is_err()); @@ -478,17 +538,41 @@ mod tests { let params = CreateWorkItemParams { title: "Valid title here".to_string(), description: "Too short".to_string(), + tags: vec![], }; let result: Result = params.try_into(); assert!(result.is_err()); } + #[test] + fn test_validation_rejects_tag_with_semicolon() { + let params = CreateWorkItemParams { + title: "Valid title here".to_string(), + description: "This is a sufficiently long description for the work item.".to_string(), + tags: vec!["tag-one; tag-two".to_string()], + }; + let result: Result = params.try_into(); + assert!(result.is_err()); + } + + #[test] + fn test_params_with_tags_converts_to_result() { + let params = CreateWorkItemParams { + title: "Implement feature".to_string(), + description: "This is a sufficiently long description for the work item.".to_string(), + tags: vec!["agent-created".to_string(), "automated".to_string()], + }; + let result: CreateWorkItemResult = params.try_into().unwrap(); + assert_eq!(result.tags, vec!["agent-created", "automated"]); + } + #[test] fn test_result_serializes_correctly() { let params = CreateWorkItemParams { title: "Test work item".to_string(), description: "A description that is definitely longer than thirty characters." .to_string(), + tags: vec![], }; let result: CreateWorkItemResult = params.try_into().unwrap(); let json = serde_json::to_string(&result).unwrap(); @@ -505,6 +589,7 @@ mod tests { assert!(config.iteration_path.is_none()); assert!(config.assignee.is_none()); assert!(config.tags.is_empty()); + assert!(config.allowed_tags.is_empty()); assert!(config.custom_fields.is_empty()); } @@ -517,6 +602,9 @@ assignee: "user@example.com" tags: - agent-created - automated +allowed-tags: + - "agent-*" + - review custom-fields: Custom.Priority: "High" "#; @@ -525,6 +613,7 @@ custom-fields: assert_eq!(config.area_path, Some("MyProject\\MyTeam".to_string())); assert_eq!(config.assignee, Some("user@example.com".to_string())); assert_eq!(config.tags, vec!["agent-created", "automated"]); + assert_eq!(config.allowed_tags, vec!["agent-*", "review"]); assert_eq!( config.custom_fields.get("Custom.Priority"), Some(&"High".to_string()) @@ -541,5 +630,6 @@ tags: assert_eq!(config.work_item_type, "Task"); // default assert!(config.area_path.is_none()); // default assert_eq!(config.tags, vec!["my-tag"]); + assert!(config.allowed_tags.is_empty()); // default } } diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs index deb37ad7..9dfb55b0 100644 --- a/src/safeoutputs/mod.rs +++ b/src/safeoutputs/mod.rs @@ -209,6 +209,24 @@ pub(crate) fn resolve_repo_name( } } +/// Return `true` if `tag` is matched by `pattern`. +/// +/// Pattern matching rules (consistent with `add-build-tag` and `allowed-labels` in gh-aw): +/// - Patterns ending with `*` are prefix wildcards: `"agent-*"` matches any tag whose +/// prefix (before the `*`) case-insensitively equals the start of `tag`. +/// - All other patterns are compared with case-insensitive exact equality. +/// +/// Both comparisons are **case-insensitive** so that an operator who writes +/// `allowed-tags: ["Agent-*"]` correctly matches an agent-provided tag `"agent-created"`. +pub(crate) fn tag_matches_pattern(tag: &str, pattern: &str) -> bool { + if let Some(prefix) = pattern.strip_suffix('*') { + tag.to_ascii_lowercase() + .starts_with(&prefix.to_ascii_lowercase()) + } else { + pattern.eq_ignore_ascii_case(tag) + } +} + /// Validate a string against `git check-ref-format` rules. /// /// Returns `Ok(())` if the name is valid, or an `Err` describing the violation. @@ -462,4 +480,39 @@ mod tests { assert!(validate_git_ref_name("v1.2.3", "b").is_ok()); assert!(validate_git_ref_name("release/2026-04-17", "b").is_ok()); } + + // ─── tag_matches_pattern ─────────────────────────────────────────────── + + #[test] + fn test_tag_matches_pattern_exact_case_insensitive() { + assert!(tag_matches_pattern("Review", "review")); + assert!(tag_matches_pattern("AUTOMATED", "Automated")); + assert!(tag_matches_pattern("automated", "automated")); + } + + #[test] + fn test_tag_matches_pattern_exact_mismatch() { + assert!(!tag_matches_pattern("other", "review")); + } + + #[test] + fn test_tag_matches_pattern_prefix_wildcard_case_insensitive() { + // Uppercase pattern prefix must match lowercase tag + assert!(tag_matches_pattern("agent-created", "Agent-*")); + // Lowercase pattern prefix must match mixed-case tag + assert!(tag_matches_pattern("Agent-Review", "agent-*")); + // Exact prefix boundary + assert!(tag_matches_pattern("agent-", "agent-*")); + } + + #[test] + fn test_tag_matches_pattern_prefix_wildcard_mismatch() { + assert!(!tag_matches_pattern("bot-created", "agent-*")); + } + + #[test] + fn test_tag_matches_pattern_star_only_matches_everything() { + assert!(tag_matches_pattern("anything", "*")); + assert!(tag_matches_pattern("", "*")); + } } diff --git a/src/safeoutputs/update_work_item.rs b/src/safeoutputs/update_work_item.rs index 0a58ec48..504f100f 100644 --- a/src/safeoutputs/update_work_item.rs +++ b/src/safeoutputs/update_work_item.rs @@ -187,6 +187,13 @@ pub struct UpdateWorkItemConfig { /// Enable tag updates (default: false) #[serde(default)] pub tags: bool, + + /// Allowlist of tags the agent is permitted to set. Only checked when `tags: true`. + /// If empty, any agent-provided tags are accepted. + /// Supports prefix wildcards: entries ending with `*` match by prefix + /// (e.g., `"agent-*"` matches `"agent-created"`, `"agent-review"`, etc.). + #[serde(default, rename = "allowed-tags")] + pub allowed_tags: Vec, } /// Check that `config.target` permits updating the given work item ID. @@ -458,6 +465,27 @@ impl Executor for UpdateWorkItemResult { return Ok(result); } + // Validate agent-provided tags against allowed-tags (if configured) + if let Some(tags) = &self.tags { + if !config.allowed_tags.is_empty() { + let disallowed: Vec<_> = tags + .iter() + .filter(|tag| { + !config + .allowed_tags + .iter() + .any(|pattern| super::tag_matches_pattern(tag, pattern)) + }) + .collect(); + if !disallowed.is_empty() { + return Ok(ExecutionResult::failure(format!( + "Agent-provided tags not in allowed-tags: {}", + disallowed.iter().map(|t| t.as_str()).collect::>().join(", ") + ))); + } + } + } + let client = reqwest::Client::new(); // Enforce title-prefix / tag-prefix guards (requires a GET if either is set) @@ -1029,4 +1057,130 @@ tag-prefix: "agent-" let result: Result = params.try_into(); assert!(result.is_ok()); } + + #[test] + fn test_config_allowed_tags_defaults_to_empty() { + let config = UpdateWorkItemConfig::default(); + assert!(config.allowed_tags.is_empty()); + } + + #[test] + fn test_config_allowed_tags_deserializes() { + let yaml = r#" +tags: true +target: "*" +allowed-tags: + - "agent-*" + - automated +"#; + let config: UpdateWorkItemConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(config.tags); + assert_eq!(config.allowed_tags, vec!["agent-*", "automated"]); + } + + #[tokio::test] + async fn test_execute_rejects_disallowed_tags() { + use std::collections::HashMap; + use std::path::PathBuf; + + let params = UpdateWorkItemParams { + id: 42, + title: None, + body: None, + state: None, + area_path: None, + iteration_path: None, + assignee: None, + tags: Some(vec!["disallowed-tag".to_string()]), + }; + let mut result: UpdateWorkItemResult = params.try_into().unwrap(); + + let mut tool_configs: HashMap = HashMap::new(); + tool_configs.insert( + "update-work-item".to_string(), + serde_json::json!({ + "tags": true, + "target": "*", + "allowed-tags": ["agent-*", "automated"] + }), + ); + + let ctx = ExecutionContext { + ado_org_url: Some("https://dev.azure.com/myorg".to_string()), + ado_project: Some("MyProject".to_string()), + access_token: Some("token".to_string()), + working_directory: PathBuf::from("."), + source_directory: PathBuf::from("."), + tool_configs, + allowed_repositories: HashMap::new(), + agent_stats: None, + dry_run: false, + ..Default::default() + }; + + let exec_result = result.execute_sanitized(&ctx).await.unwrap(); + assert!(!exec_result.success); + assert!( + exec_result.message.contains("not in allowed-tags"), + "Expected allowed-tags error, got: {}", + exec_result.message + ); + } + + #[tokio::test] + async fn test_execute_accepts_tags_matching_wildcard() { + use std::collections::HashMap; + use std::path::PathBuf; + + let params = UpdateWorkItemParams { + id: 42, + title: None, + body: None, + state: None, + area_path: None, + iteration_path: None, + assignee: None, + tags: Some(vec!["agent-created".to_string()]), + }; + let mut result: UpdateWorkItemResult = params.try_into().unwrap(); + + let mut tool_configs: HashMap = HashMap::new(); + tool_configs.insert( + "update-work-item".to_string(), + serde_json::json!({ + "tags": true, + "target": "*", + "allowed-tags": ["agent-*"] + }), + ); + + // This will fail at the ADO API call, but it should pass the allowed-tags check. + // We just verify we don't get the allowed-tags error back. + let ctx = ExecutionContext { + ado_org_url: Some("https://dev.azure.com/myorg".to_string()), + ado_project: Some("MyProject".to_string()), + access_token: Some("token".to_string()), + working_directory: PathBuf::from("."), + source_directory: PathBuf::from("."), + tool_configs, + allowed_repositories: HashMap::new(), + agent_stats: None, + dry_run: false, + ..Default::default() + }; + + let exec_result = result.execute_sanitized(&ctx).await; + // Should not be an "allowed-tags" error — it will fail at the ADO HTTP call + // or succeed. Either way, the allowed-tags guard passed. + match exec_result { + Ok(r) => assert!( + !r.message.contains("not in allowed-tags"), + "Should not have failed allowed-tags check" + ), + Err(e) => assert!( + !e.to_string().contains("not in allowed-tags"), + "Should not have failed allowed-tags check" + ), + } + } }