diff --git a/AGENTS.md b/AGENTS.md index 84a93381..309ba85b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,6 +96,8 @@ Every compiled pipeline runs as three sequential jobs: │ │ ├── update_pr.rs │ │ ├── update_wiki_page.rs │ │ ├── update_work_item.rs +│ │ ├── upload_build_attachment.rs +│ │ ├── upload_pipeline_artifact.rs │ │ └── upload_workitem_attachment.rs │ ├── runtimes/ # Runtime environment implementations (one dir per runtime) │ │ ├── mod.rs # Module entry point diff --git a/README.md b/README.md index f1fb9d85..6ac9419c 100644 --- a/README.md +++ b/README.md @@ -389,6 +389,8 @@ actions, and the executor processes them after threat analysis. | `create-git-tag` | Creates a git tag on a repository ref | | `create-branch` | Creates a new branch from an existing ref | | `add-build-tag` | Adds a tag to an ADO build | +| `upload-build-attachment` | Attaches a file to a build (accessible via REST API or custom extension only) | +| `upload-pipeline-artifact` | Publishes a file as a pipeline artifact visible in the ADO Artifacts tab | | `upload-workitem-attachment` | Uploads a workspace file as an attachment to a work item | | `report-incomplete` | Reports that a task could not be completed | | `noop` | Reports no action was needed | diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index d8dfb6c8..18274054 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -418,11 +418,18 @@ safe-outputs: max: 1 # Maximum per run (default: 1) ``` -### upload-build-artifact -Attaches a workspace file to an Azure DevOps build as a build attachment via the -ADO build attachments REST API +### upload-build-attachment + +Attaches a workspace file to an Azure DevOps build as a **build attachment** via +the ADO build attachments REST API (`PUT /_apis/build/builds/{buildId}/attachments/{type}/{name}`). +> **Important:** Build attachments are **not visible** in the standard Azure +> DevOps build summary UI. They are only accessible via the REST API or through +> a custom Azure DevOps extension that registers a tab matching the +> `attachment-type` value. For artifacts that should appear in the **Artifacts +> tab**, use [`upload-pipeline-artifact`](#upload-pipeline-artifact) instead. + **Omit `build_id` to target the current pipeline run** — the executor resolves the build ID from the `BUILD_BUILDID` environment variable automatically. When `build_id` is provided, the file is attached to that specific build — useful for @@ -435,13 +442,13 @@ API. **Agent parameters:** - `build_id` *(optional)* - Target build ID. Omit to attach to the current pipeline run. Must be positive when specified. -- `artifact_name` - Artifact name (1–100 chars, alphanumeric / `-` / `_` / `.`, no leading `.`) +- `artifact_name` - Attachment name (1–100 chars, alphanumeric / `-` / `_` / `.`, no leading `.`) - `file_path` - Relative path to the file in the workspace (no directory traversal) **Configuration options (front matter):** ```yaml safe-outputs: - upload-build-artifact: + upload-build-attachment: max-file-size: 52428800 # Maximum file size in bytes (default: 50 MB) allowed-extensions: [] # Optional — restrict file types (e.g., [".png", ".pdf", ".log"]) allowed-artifact-names: [] # Optional — restrict names (suffix `*` = prefix match) @@ -454,7 +461,53 @@ safe-outputs: **Notes:** - Single-file only; directory uploads are not supported. - When `build_id` is omitted and `allowed-build-ids` is configured, the allow-list check is skipped — the current build is implicitly trusted. -- The default `attachment-type` is `agent-artifact` so executor contributions are visually distinguishable from the build's own artifacts. + +**About `attachment-type`:** This is the `{type}` segment in the ADO build +attachments URL (`PUT .../attachments/{type}/{name}`). It acts as a category +label. Azure DevOps extensions can register to display attachments of a specific +type — for example, the built-in code coverage extension displays attachments +with type `CodeCoverageSummary`. The default `agent-artifact` is a custom type; +without a matching ADO extension installed, attachments with this type are only +accessible via the REST API. Change this only if you have a custom extension +that displays attachments of a specific type. Most users should use +[`upload-pipeline-artifact`](#upload-pipeline-artifact) for user-visible +artifacts instead. + +### upload-pipeline-artifact + +Publishes a workspace file as an Azure DevOps **pipeline artifact** that appears +in the **Artifacts tab** of the build summary page. Uses the ADO build artifacts +REST API (container creation + file upload + artifact association). + +**Omit `build_id` to target the current pipeline run** — the executor resolves +the build ID from the `BUILD_BUILDID` environment variable automatically. When +`build_id` is provided, the artifact is published to that specific build. + +The tool stages the file during Stage 1 (MCP) by copying it into the +safe-outputs directory; Stage 3 reads the staged copy and executes the three-step +REST API flow to create the artifact. + +**Agent parameters:** +- `build_id` *(optional)* - Target build ID. Omit to publish to the current pipeline run. Must be positive when specified. +- `artifact_name` - Artifact name shown in the Artifacts tab (1–100 chars, alphanumeric / `-` / `_` / `.`, no leading `.`) +- `file_path` - Relative path to the file in the workspace (no directory traversal) + +**Configuration options (front matter):** +```yaml +safe-outputs: + upload-pipeline-artifact: + max-file-size: 52428800 # Maximum file size in bytes (default: 50 MB) + allowed-extensions: [] # Optional — restrict file types (e.g., [".png", ".pdf", ".log"]) + allowed-artifact-names: [] # Optional — restrict names (suffix `*` = prefix match) + allowed-build-ids: [] # Optional — restrict target builds (skipped when targeting current build) + name-prefix: "" # Optional — prepended to the agent-supplied artifact name + max: 3 # Maximum per run (default: 3) +``` + +**Notes:** +- Single-file only; directory uploads are not supported. +- When `build_id` is omitted and `allowed-build-ids` is configured, the allow-list check is skipped — the current build is implicitly trusted. +- Requires `SYSTEM_TEAMPROJECTID` to be available in the execution environment (set automatically by Azure DevOps). ### cache-memory (moved to `tools:`) Memory is now configured as a first-class tool under `tools: cache-memory:` instead of `safe-outputs: memory:`. See the [Cache Memory section](./tools.md#cache-memory-cache-memory) in `docs/tools.md` for details. diff --git a/src/execute.rs b/src/execute.rs index 743af2fd..02223b0d 100644 --- a/src/execute.rs +++ b/src/execute.rs @@ -18,8 +18,8 @@ use crate::safeoutputs::{ ExecutionContext, ExecutionResult, Executor, LinkWorkItemsResult, MissingDataResult, MissingToolResult, NoopResult, QueueBuildResult, ReplyToPrCommentResult, ReportIncompleteResult, ResolvePrThreadResult, SubmitPrReviewResult, ToolResult, - UpdatePrResult, UpdateWikiPageResult, UpdateWorkItemResult, UploadBuildArtifactResult, - UploadWorkitemAttachmentResult, + UpdatePrResult, UpdateWikiPageResult, UpdateWorkItemResult, UploadBuildAttachmentResult, + UploadPipelineArtifactResult, UploadWorkitemAttachmentResult, }; // Re-export memory types for use by main.rs @@ -93,7 +93,8 @@ pub async fn execute_safe_outputs( AddBuildTagResult, CreateBranchResult, UpdatePrResult, - UploadBuildArtifactResult, + UploadBuildAttachmentResult, + UploadPipelineArtifactResult, UploadWorkitemAttachmentResult, SubmitPrReviewResult, ReplyToPrCommentResult, @@ -346,7 +347,8 @@ async fn dispatch_resource_tools( "create-git-tag" => CreateGitTagResult, "add-build-tag" => AddBuildTagResult, "create-branch" => CreateBranchResult, - "upload-build-artifact" => UploadBuildArtifactResult, + "upload-build-attachment" => UploadBuildAttachmentResult, + "upload-pipeline-artifact" => UploadPipelineArtifactResult, }) } diff --git a/src/mcp.rs b/src/mcp.rs index 0b796584..a72ca3d6 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -27,7 +27,8 @@ use crate::safeoutputs::{ QueueBuildResult, SubmitPrReviewParams, SubmitPrReviewResult, ToolResult, UpdatePrParams, UpdatePrResult, UpdateWorkItemParams, UpdateWorkItemResult, - UploadBuildArtifactParams, UploadBuildArtifactResult, DEFAULT_MAX_FILE_SIZE, + UploadBuildAttachmentParams, UploadBuildAttachmentResult, DEFAULT_MAX_FILE_SIZE, + UploadPipelineArtifactParams, UploadPipelineArtifactResult, PIPELINE_ARTIFACT_DEFAULT_MAX_FILE_SIZE, UploadWorkitemAttachmentParams, UploadWorkitemAttachmentResult, anyhow_to_mcp_error, }; @@ -990,21 +991,22 @@ uploaded and linked during safe output processing. File size and type restrictio } #[tool( - name = "upload-build-artifact", - description = "Attach a workspace file to an Azure DevOps build as a build attachment. \ -Omit `build_id` to target the current pipeline run (the executor resolves it from the \ -BUILD_BUILDID environment variable automatically). When `build_id` is provided, the file is \ + name = "upload-build-attachment", + description = "Attach a workspace file to an Azure DevOps build as a build attachment via \ +the ADO build attachments REST API. Build attachments are NOT visible in the standard ADO UI — \ +they are only accessible via the REST API or a custom Azure DevOps extension. For files that \ +should appear in the Artifacts tab, use upload-pipeline-artifact instead. \ +Omit `build_id` to target the current pipeline run. When `build_id` is provided, the file is \ attached to that specific build — useful for posthumously decorating a finished build with a \ -generated report, screenshot, or log bundle. The file will be staged now and uploaded via the \ -ADO build attachments REST API during safe output processing. File size, extension, \ -artifact-name and build-id restrictions may apply per the workflow's safe-outputs config." +generated report or log bundle. File size, extension, artifact-name and build-id restrictions \ +may apply per the workflow's safe-outputs config." )] - async fn upload_build_artifact( + async fn upload_build_attachment( &self, - params: Parameters, + params: Parameters, ) -> Result { info!( - "Tool called: upload-build-artifact - artifact '{}' file '{}' build {:?}", + "Tool called: upload-build-attachment - artifact '{}' file '{}' build {:?}", params.0.artifact_name, params.0.file_path, params.0.build_id ); @@ -1038,26 +1040,26 @@ artifact-name and build-id restrictions may apply per the workflow's safe-output ))); } - // Reject directories — upload-build-artifact is single-file only. + // Reject directories — upload-build-attachment is single-file only. let metadata = tokio::fs::metadata(&canonical).await.map_err(|e| { anyhow_to_mcp_error(anyhow::anyhow!("Failed to stat '{}': {}", params.0.file_path, e)) })?; if metadata.is_dir() { return Err(anyhow_to_mcp_error(anyhow::anyhow!( - "File '{}' is a directory; upload-build-artifact only supports single files", + "File '{}' is a directory; upload-build-attachment only supports single files", params.0.file_path ))); } - let file_size = metadata.len(); + let metadata_size = metadata.len(); // Defense-in-depth: reject files exceeding the default max size at // Stage 1 to prevent a misbehaving agent from filling the staging // disk before Stage 3 gets a chance to enforce the operator's limit. - if file_size > DEFAULT_MAX_FILE_SIZE { + if metadata_size > DEFAULT_MAX_FILE_SIZE { return Err(anyhow_to_mcp_error(anyhow::anyhow!( "File '{}' is {} bytes, exceeding the maximum staging size of {} bytes", params.0.file_path, - file_size, + metadata_size, DEFAULT_MAX_FILE_SIZE ))); } @@ -1082,13 +1084,13 @@ artifact-name and build-id restrictions may apply per the workflow's safe-output // max length (~140 chars) is well within filesystem limits. let staged_filename = if extension.is_empty() { format!( - "upload-build-artifact-{}-{}", + "upload-build-attachment-{}-{}", params.0.artifact_name, generate_short_id() ) } else { format!( - "upload-build-artifact-{}-{}.{}", + "upload-build-attachment-{}-{}.{}", params.0.artifact_name, generate_short_id(), extension @@ -1106,6 +1108,10 @@ artifact-name and build-id restrictions may apply per the workflow's safe-output )) })?; let staged_sha256 = crate::hash::sha256_hex(&source_bytes); + // Use the actual byte count rather than the earlier metadata.len() so + // that the recorded size matches the staged content exactly, closing + // a TOCTOU window if the source file changes between stat and read. + let file_size = source_bytes.len() as u64; let staged_path = self.output_directory.join(&staged_filename); tokio::fs::write(&staged_path, &source_bytes).await.map_err(|e| { @@ -1116,7 +1122,7 @@ artifact-name and build-id restrictions may apply per the workflow's safe-output )) })?; - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( params.0.build_id, params.0.artifact_name.clone(), params.0.file_path.clone(), @@ -1137,6 +1143,135 @@ artifact-name and build-id restrictions may apply per the workflow's safe-output ))])) } + #[tool( + name = "upload-pipeline-artifact", + description = "Publish a workspace file as an Azure DevOps pipeline artifact that appears \ +in the Artifacts tab of the build summary page — visible to all users viewing the build. Use \ +this tool when you want users to be able to find and download the file from the ADO UI. \ +Omit `build_id` to target the current pipeline run. When `build_id` is provided, the artifact \ +is published to that specific build. File size, extension, artifact-name and build-id \ +restrictions may apply per the workflow's safe-outputs config." + )] + async fn upload_pipeline_artifact( + &self, + params: Parameters, + ) -> Result { + info!( + "Tool called: upload-pipeline-artifact - artifact '{}' file '{}' build {:?}", + params.0.artifact_name, params.0.file_path, params.0.build_id + ); + + crate::safeoutputs::Validate::validate(¶ms.0).map_err(anyhow_to_mcp_error)?; + + let resolved = self.bounding_directory.join(¶ms.0.file_path); + let canonical = resolved.canonicalize().map_err(|e| { + anyhow_to_mcp_error(anyhow::anyhow!( + "File '{}' could not be located inside the workspace: {}", + params.0.file_path, + e + )) + })?; + let canonical_root = self.bounding_directory.canonicalize().map_err(|e| { + anyhow_to_mcp_error(anyhow::anyhow!( + "Failed to canonicalize bounding directory: {}", + e + )) + })?; + if !canonical.starts_with(&canonical_root) { + return Err(anyhow_to_mcp_error(anyhow::anyhow!( + "File '{}' resolves outside the workspace (symlink escape)", + params.0.file_path + ))); + } + + let metadata = tokio::fs::metadata(&canonical).await.map_err(|e| { + anyhow_to_mcp_error(anyhow::anyhow!("Failed to stat '{}': {}", params.0.file_path, e)) + })?; + if metadata.is_dir() { + return Err(anyhow_to_mcp_error(anyhow::anyhow!( + "File '{}' is a directory; upload-pipeline-artifact only supports single files", + params.0.file_path + ))); + } + let metadata_size = metadata.len(); + + if metadata_size > PIPELINE_ARTIFACT_DEFAULT_MAX_FILE_SIZE { + return Err(anyhow_to_mcp_error(anyhow::anyhow!( + "File '{}' is {} bytes, exceeding the maximum staging size of {} bytes", + params.0.file_path, + metadata_size, + PIPELINE_ARTIFACT_DEFAULT_MAX_FILE_SIZE + ))); + } + + let extension = std::path::Path::new(¶ms.0.file_path) + .extension() + .and_then(|s| s.to_str()) + .map(|s| { + s.chars() + .filter(|c| c.is_ascii_alphanumeric()) + .take(16) + .collect::() + }) + .unwrap_or_default(); + let staged_filename = if extension.is_empty() { + format!( + "upload-pipeline-artifact-{}-{}", + params.0.artifact_name, + generate_short_id() + ) + } else { + format!( + "upload-pipeline-artifact-{}-{}.{}", + params.0.artifact_name, + generate_short_id(), + extension + ) + }; + + let source_bytes = tokio::fs::read(&canonical).await.map_err(|e| { + anyhow_to_mcp_error(anyhow::anyhow!( + "Failed to read source file '{}': {}", + params.0.file_path, + e + )) + })?; + let staged_sha256 = crate::hash::sha256_hex(&source_bytes); + // Use the actual byte count rather than the earlier metadata.len() so + // that the recorded size matches the staged content exactly, closing + // a TOCTOU window if the source file changes between stat and read. + let file_size = source_bytes.len() as u64; + + let staged_path = self.output_directory.join(&staged_filename); + tokio::fs::write(&staged_path, &source_bytes).await.map_err(|e| { + anyhow_to_mcp_error(anyhow::anyhow!( + "Failed to stage file '{}' into safe-outputs directory: {}", + params.0.file_path, + e + )) + })?; + + let result = UploadPipelineArtifactResult::new( + params.0.build_id, + params.0.artifact_name.clone(), + params.0.file_path.clone(), + staged_filename.clone(), + file_size, + staged_sha256, + ); + self.write_safe_output_file(&result).await + .map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?; + + let build_desc = match params.0.build_id { + Some(id) => format!("build #{}", id), + None => "the current build".to_string(), + }; + Ok(CallToolResult::success(vec![Content::text(format!( + "Pipeline artifact '{}' queued from file '{}' ({} bytes) for {}. The artifact will appear in the Artifacts tab after safe output processing.", + result.artifact_name, result.file_path, file_size, build_desc + ))])) + } + #[tool( name = "submit-pr-review", description = "Submit a pull request review with a decision (approve, request-changes, \ diff --git a/src/safeoutputs/create_pr.rs b/src/safeoutputs/create_pr.rs index 154b4fa1..f571892e 100644 --- a/src/safeoutputs/create_pr.rs +++ b/src/safeoutputs/create_pr.rs @@ -2444,6 +2444,7 @@ new file mode 100755 ado_org_url: Some("https://dev.azure.com/test".to_string()), ado_organization: Some("test".to_string()), ado_project: Some("TestProject".to_string()), + ado_project_id: None, access_token: Some("fake-token".to_string()), source_directory: dir.path().to_path_buf(), working_directory: dir.path().to_path_buf(), diff --git a/src/safeoutputs/mod.rs b/src/safeoutputs/mod.rs index 1331e1ef..fccfacd3 100644 --- a/src/safeoutputs/mod.rs +++ b/src/safeoutputs/mod.rs @@ -42,7 +42,8 @@ pub const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = tool_names![ AddBuildTagResult, CreateBranchResult, UpdatePrResult, - UploadBuildArtifactResult, + UploadBuildAttachmentResult, + UploadPipelineArtifactResult, UploadWorkitemAttachmentResult, SubmitPrReviewResult, ReplyToPrCommentResult, @@ -76,7 +77,8 @@ pub const ALL_KNOWN_SAFE_OUTPUTS: &[&str] = all_safe_output_names![ AddBuildTagResult, CreateBranchResult, UpdatePrResult, - UploadBuildArtifactResult, + UploadBuildAttachmentResult, + UploadPipelineArtifactResult, UploadWorkitemAttachmentResult, SubmitPrReviewResult, ReplyToPrCommentResult, @@ -264,7 +266,8 @@ mod submit_pr_review; mod update_pr; mod update_wiki_page; mod update_work_item; -mod upload_build_artifact; +mod upload_build_attachment; +mod upload_pipeline_artifact; mod upload_workitem_attachment; pub use add_build_tag::*; @@ -290,7 +293,8 @@ pub use submit_pr_review::*; pub use update_pr::*; pub use update_wiki_page::*; pub use update_work_item::*; -pub use upload_build_artifact::*; +pub use upload_build_attachment::*; +pub use upload_pipeline_artifact::*; pub use upload_workitem_attachment::*; #[cfg(test)] @@ -348,7 +352,8 @@ mod tests { assert!(AddBuildTagResult::REQUIRES_WRITE); assert!(CreateBranchResult::REQUIRES_WRITE); assert!(UpdatePrResult::REQUIRES_WRITE); - assert!(UploadBuildArtifactResult::REQUIRES_WRITE); + assert!(UploadBuildAttachmentResult::REQUIRES_WRITE); + assert!(UploadPipelineArtifactResult::REQUIRES_WRITE); assert!(UploadWorkitemAttachmentResult::REQUIRES_WRITE); assert!(SubmitPrReviewResult::REQUIRES_WRITE); assert!(ReplyToPrCommentResult::REQUIRES_WRITE); diff --git a/src/safeoutputs/result.rs b/src/safeoutputs/result.rs index 5de9f2c3..b9ffb57a 100644 --- a/src/safeoutputs/result.rs +++ b/src/safeoutputs/result.rs @@ -42,6 +42,8 @@ pub struct ExecutionContext { pub ado_organization: Option, /// Azure DevOps project name pub ado_project: Option, + /// Azure DevOps project GUID (`SYSTEM_TEAMPROJECTID`) + pub ado_project_id: Option, /// Personal access token or system access token pub access_token: Option, /// Working directory for file operations (safe outputs directory) @@ -159,6 +161,7 @@ impl ExecutionContext { ado_org_url, ado_organization, ado_project: env("SYSTEM_TEAMPROJECT"), + ado_project_id: env("SYSTEM_TEAMPROJECTID"), access_token: env("SYSTEM_ACCESSTOKEN").or_else(|| env("AZURE_DEVOPS_EXT_PAT")), working_directory: std::env::current_dir().unwrap_or_default(), source_directory, diff --git a/src/safeoutputs/upload_build_artifact.rs b/src/safeoutputs/upload_build_attachment.rs similarity index 90% rename from src/safeoutputs/upload_build_artifact.rs rename to src/safeoutputs/upload_build_attachment.rs index e84b0abc..6b80dc44 100644 --- a/src/safeoutputs/upload_build_artifact.rs +++ b/src/safeoutputs/upload_build_attachment.rs @@ -1,4 +1,4 @@ -//! Upload build artifact safe output tool. +//! Upload build attachment safe output tool. //! //! Lets an agent propose attaching a workspace file to an Azure DevOps build //! via the **build attachments** REST API @@ -37,7 +37,7 @@ use anyhow::{Context, ensure}; /// Parameters for attaching a workspace file to an ADO build. #[derive(Deserialize, JsonSchema)] -pub struct UploadBuildArtifactParams { +pub struct UploadBuildAttachmentParams { /// The build ID to attach the file to. **Omit to target the current /// pipeline run** — the executor resolves the build ID from the /// `BUILD_BUILDID` environment variable automatically. When provided, @@ -56,7 +56,7 @@ pub struct UploadBuildArtifactParams { pub file_path: String, } -impl Validate for UploadBuildArtifactParams { +impl Validate for UploadBuildAttachmentParams { fn validate(&self) -> anyhow::Result<()> { // build_id: if present, must be positive. if let Some(id) = self.build_id { @@ -100,13 +100,13 @@ impl Validate for UploadBuildArtifactParams { } } -/// Internal params struct mirroring `UploadBuildArtifactResult` fields for the +/// Internal params struct mirroring `UploadBuildAttachmentResult` fields for the /// `tool_result!` macro's `TryFrom` plumbing. The actual MCP parameters come -/// from `UploadBuildArtifactParams`; this struct only exists so the macro can +/// from `UploadBuildAttachmentParams`; this struct only exists so the macro can /// wire up `Validate`/`TryFrom` while the real construction happens in MCP via -/// `UploadBuildArtifactResult::new()` after the file is staged. +/// `UploadBuildAttachmentResult::new()` after the file is staged. #[derive(Deserialize, JsonSchema)] -struct UploadBuildArtifactResultFields { +struct UploadBuildAttachmentResultFields { build_id: Option, artifact_name: String, file_path: String, @@ -115,15 +115,15 @@ struct UploadBuildArtifactResultFields { staged_sha256: String, } -impl Validate for UploadBuildArtifactResultFields {} +impl Validate for UploadBuildAttachmentResultFields {} tool_result! { - name = "upload-build-artifact", + name = "upload-build-attachment", write = true, - params = UploadBuildArtifactResultFields, + params = UploadBuildAttachmentResultFields, default_max = 3, /// Result of attaching a workspace file to an ADO build. - pub struct UploadBuildArtifactResult { + pub struct UploadBuildAttachmentResult { /// Build ID the file should be attached to. `None` means "current /// build" — resolved at execution time from `BUILD_BUILDID`. build_id: Option, @@ -147,14 +147,14 @@ tool_result! { } } -impl SanitizeContent for UploadBuildArtifactResult { +impl SanitizeContent for UploadBuildAttachmentResult { fn sanitize_content_fields(&mut self) { // All textual fields are strictly validated to safe charsets; no // additional textual sanitization is required. } } -impl UploadBuildArtifactResult { +impl UploadBuildAttachmentResult { /// Construct a result after the agent's file has been staged into the /// safe-outputs directory. pub fn new( @@ -177,18 +177,18 @@ impl UploadBuildArtifactResult { } } -/// Default maximum file size for upload-build-artifact (50 MB). +/// Default maximum file size for upload-build-attachment (50 MB). /// Also used by the MCP handler as the Stage 1 staging cap. pub const DEFAULT_MAX_FILE_SIZE: u64 = 50 * 1024 * 1024; const DEFAULT_ATTACHMENT_TYPE: &str = "agent-artifact"; -/// Configuration for the upload-build-artifact tool (specified in front +/// Configuration for the upload-build-attachment tool (specified in front /// matter). /// /// Example front matter: /// ```yaml /// safe-outputs: -/// upload-build-artifact: +/// upload-build-attachment: /// max-file-size: 52428800 /// allowed-extensions: /// - .png @@ -204,7 +204,7 @@ const DEFAULT_ATTACHMENT_TYPE: &str = "agent-artifact"; /// max: 5 /// ``` #[derive(Debug, Clone, SanitizeConfig, Serialize, Deserialize)] -pub struct UploadBuildArtifactConfig { +pub struct UploadBuildAttachmentConfig { /// Maximum file size in bytes (default: 50 MB). #[serde(default = "default_max_file_size", rename = "max-file-size")] pub max_file_size: u64, @@ -241,7 +241,7 @@ fn default_max_file_size() -> u64 { DEFAULT_MAX_FILE_SIZE } -impl Default for UploadBuildArtifactConfig { +impl Default for UploadBuildAttachmentConfig { fn default() -> Self { Self { max_file_size: DEFAULT_MAX_FILE_SIZE, @@ -255,7 +255,7 @@ impl Default for UploadBuildArtifactConfig { } #[async_trait::async_trait] -impl Executor for UploadBuildArtifactResult { +impl Executor for UploadBuildAttachmentResult { fn dry_run_summary(&self) -> String { match self.build_id { Some(id) => format!( @@ -291,11 +291,11 @@ impl Executor for UploadBuildArtifactResult { if self.build_id.is_none() { " (current build)" } else { "" } ); debug!( - "upload-build-artifact: build_id={}, artifact_name='{}', file_path='{}'", + "upload-build-attachment: build_id={}, artifact_name='{}', file_path='{}'", effective_build_id, self.artifact_name, self.file_path ); - let config: UploadBuildArtifactConfig = ctx.get_tool_config("upload-build-artifact"); + let config: UploadBuildAttachmentConfig = ctx.get_tool_config("upload-build-attachment"); debug!("Max file size: {} bytes", config.max_file_size); debug!("Allowed extensions: {:?}", config.allowed_extensions); debug!("Allowed artifact names: {:?}", config.allowed_artifact_names); @@ -429,7 +429,7 @@ impl Executor for UploadBuildArtifactResult { let metadata = std::fs::metadata(&canonical).context("Failed to read file metadata")?; if metadata.is_dir() { return Ok(ExecutionResult::failure(format!( - "Staged path '{}' is a directory; upload-build-artifact only supports single files", + "Staged path '{}' is a directory; upload-build-attachment only supports single files", self.staged_file ))); } @@ -576,15 +576,15 @@ mod tests { #[test] fn test_result_has_correct_name() { - assert_eq!(UploadBuildArtifactResult::NAME, "upload-build-artifact"); + assert_eq!(UploadBuildAttachmentResult::NAME, "upload-build-attachment"); } fn make_params( build_id: Option, artifact_name: &str, file_path: &str, - ) -> UploadBuildArtifactParams { - UploadBuildArtifactParams { + ) -> UploadBuildAttachmentParams { + UploadBuildAttachmentParams { build_id, artifact_name: artifact_name.to_string(), file_path: file_path.to_string(), @@ -605,7 +605,7 @@ mod tests { fn test_params_deserializes_with_build_id() { let json = r#"{"build_id": 1234, "artifact_name": "agent-report", "file_path": "out/report.pdf"}"#; - let params: UploadBuildArtifactParams = serde_json::from_str(json).unwrap(); + let params: UploadBuildAttachmentParams = serde_json::from_str(json).unwrap(); assert_eq!(params.build_id, Some(1234)); assert_eq!(params.artifact_name, "agent-report"); assert_eq!(params.file_path, "out/report.pdf"); @@ -614,7 +614,7 @@ mod tests { #[test] fn test_params_deserializes_without_build_id() { let json = r#"{"artifact_name": "agent-report", "file_path": "out/report.pdf"}"#; - let params: UploadBuildArtifactParams = serde_json::from_str(json).unwrap(); + let params: UploadBuildAttachmentParams = serde_json::from_str(json).unwrap(); assert_eq!(params.build_id, None); assert_eq!(params.artifact_name, "agent-report"); assert_eq!(params.file_path, "out/report.pdf"); @@ -720,41 +720,41 @@ mod tests { #[test] fn test_result_serializes_correctly_with_build_id() { - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1234), "agent-report".to_string(), "out/report.pdf".to_string(), - "upload-build-artifact-agent-report-1234.bin".to_string(), + "upload-build-attachment-agent-report-1234.bin".to_string(), 42, DUMMY_HASH.to_string(), ); let json = serde_json::to_string(&result).unwrap(); - assert!(json.contains(r#""name":"upload-build-artifact""#)); + assert!(json.contains(r#""name":"upload-build-attachment""#)); assert!(json.contains(r#""build_id":1234"#)); assert!(json.contains(r#""artifact_name":"agent-report""#)); assert!(json.contains(r#""file_path":"out/report.pdf""#)); - assert!(json.contains(r#""staged_file":"upload-build-artifact-agent-report-1234.bin""#)); + assert!(json.contains(r#""staged_file":"upload-build-attachment-agent-report-1234.bin""#)); } #[test] fn test_result_serializes_correctly_without_build_id() { - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( None, "agent-report".to_string(), "out/report.pdf".to_string(), - "upload-build-artifact-agent-report-1234.bin".to_string(), + "upload-build-attachment-agent-report-1234.bin".to_string(), 42, DUMMY_HASH.to_string(), ); let json = serde_json::to_string(&result).unwrap(); - assert!(json.contains(r#""name":"upload-build-artifact""#)); + assert!(json.contains(r#""name":"upload-build-attachment""#)); assert!(json.contains(r#""build_id":null"#)); assert!(json.contains(r#""artifact_name":"agent-report""#)); } #[test] fn test_config_defaults() { - let config = UploadBuildArtifactConfig::default(); + let config = UploadBuildAttachmentConfig::default(); assert_eq!(config.max_file_size, 50 * 1024 * 1024); assert!(config.allowed_extensions.is_empty()); assert!(config.allowed_artifact_names.is_empty()); @@ -779,7 +779,7 @@ allowed-build-ids: name-prefix: "agent-" attachment-type: "agent-artifact" "#; - let config: UploadBuildArtifactConfig = serde_yaml::from_str(yaml).unwrap(); + let config: UploadBuildAttachmentConfig = serde_yaml::from_str(yaml).unwrap(); assert_eq!(config.max_file_size, 1_048_576); assert_eq!(config.allowed_extensions, vec![".png", ".pdf"]); assert_eq!(config.allowed_artifact_names, vec!["agent-*", "report"]); @@ -793,6 +793,7 @@ attachment-type: "agent-artifact" ado_org_url: None, ado_organization: None, ado_project: None, + ado_project_id: None, access_token: None, source_directory: working_directory.clone(), working_directory, @@ -822,10 +823,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_reads_staged_file_with_explicit_build_id() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-deadbeef.pdf"; + let staged = "upload-build-attachment-agent-report-deadbeef.pdf"; std::fs::write(dir.path().join(staged), b"%PDF-1.4 hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1234), "agent-report".to_string(), "out/report.pdf".to_string(), @@ -844,10 +845,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_resolves_current_build_when_build_id_omitted() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-feedf00d.pdf"; + let staged = "upload-build-attachment-agent-report-feedf00d.pdf"; std::fs::write(dir.path().join(staged), b"%PDF-1.4 hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( None, "agent-report".to_string(), "out/report.pdf".to_string(), @@ -867,10 +868,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_fails_when_build_id_omitted_and_not_in_env() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-cafef00d.pdf"; + let staged = "upload-build-attachment-agent-report-cafef00d.pdf"; std::fs::write(dir.path().join(staged), b"hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( None, "agent-report".to_string(), "out/report.pdf".to_string(), @@ -891,7 +892,7 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_rejects_missing_staged_file() { let dir = tempfile::tempdir().unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1234), "agent-report".to_string(), "out/report.pdf".to_string(), @@ -911,10 +912,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_rejects_disallowed_build_id() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-cafef00d.pdf"; + let staged = "upload-build-attachment-agent-report-cafef00d.pdf"; std::fs::write(dir.path().join(staged), b"hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(999), "agent-report".to_string(), "out/report.pdf".to_string(), @@ -924,7 +925,7 @@ attachment-type: "agent-artifact" ); let mut ctx = make_ctx(dir.path().to_path_buf(), true); ctx.tool_configs.insert( - "upload-build-artifact".to_string(), + "upload-build-attachment".to_string(), serde_json::json!({ "allowed-build-ids": [100, 200] }), ); let outcome = result.execute_impl(&ctx).await.unwrap(); @@ -939,10 +940,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_skips_build_id_allowlist_for_current_build() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-aabbccdd.pdf"; + let staged = "upload-build-attachment-agent-report-aabbccdd.pdf"; std::fs::write(dir.path().join(staged), b"hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( None, // targeting current build "agent-report".to_string(), "out/report.pdf".to_string(), @@ -953,7 +954,7 @@ attachment-type: "agent-artifact" let mut ctx = make_ctx(dir.path().to_path_buf(), true); ctx.build_id = Some(999); // current build not in allowed list ctx.tool_configs.insert( - "upload-build-artifact".to_string(), + "upload-build-attachment".to_string(), serde_json::json!({ "allowed-build-ids": [100, 200] }), ); let outcome = result.execute_impl(&ctx).await.unwrap(); @@ -967,10 +968,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_accepts_allowed_build_id() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-feedf00d.pdf"; + let staged = "upload-build-attachment-agent-report-feedf00d.pdf"; std::fs::write(dir.path().join(staged), b"hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(100), "agent-report".to_string(), "out/report.pdf".to_string(), @@ -980,7 +981,7 @@ attachment-type: "agent-artifact" ); let mut ctx = make_ctx(dir.path().to_path_buf(), true); ctx.tool_configs.insert( - "upload-build-artifact".to_string(), + "upload-build-attachment".to_string(), serde_json::json!({ "allowed-build-ids": [100, 200] }), ); let outcome = result.execute_impl(&ctx).await.unwrap(); @@ -990,10 +991,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_rejects_oversized_file() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-big-deadbeef.bin"; + let staged = "upload-build-attachment-agent-big-deadbeef.bin"; std::fs::write(dir.path().join(staged), vec![0u8; 1024]).unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1), "agent-big".to_string(), "out/big.bin".to_string(), @@ -1003,7 +1004,7 @@ attachment-type: "agent-artifact" ); let mut ctx = make_ctx(dir.path().to_path_buf(), true); ctx.tool_configs.insert( - "upload-build-artifact".to_string(), + "upload-build-attachment".to_string(), serde_json::json!({ "max-file-size": 100 }), ); let outcome = result.execute_impl(&ctx).await.unwrap(); @@ -1018,10 +1019,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_rejects_disallowed_extension() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-aabb1122.exe"; + let staged = "upload-build-attachment-agent-report-aabb1122.exe"; std::fs::write(dir.path().join(staged), b"MZ hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1), "agent-report".to_string(), "out/report.exe".to_string(), @@ -1031,7 +1032,7 @@ attachment-type: "agent-artifact" ); let mut ctx = make_ctx(dir.path().to_path_buf(), true); ctx.tool_configs.insert( - "upload-build-artifact".to_string(), + "upload-build-attachment".to_string(), serde_json::json!({ "allowed-extensions": [".pdf", ".png"] }), ); let outcome = result.execute_impl(&ctx).await.unwrap(); @@ -1046,10 +1047,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_rejects_disallowed_artifact_name() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-evil-report-ccdd3344.pdf"; + let staged = "upload-build-attachment-evil-report-ccdd3344.pdf"; std::fs::write(dir.path().join(staged), b"hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1), "evil-report".to_string(), "out/report.pdf".to_string(), @@ -1059,7 +1060,7 @@ attachment-type: "agent-artifact" ); let mut ctx = make_ctx(dir.path().to_path_buf(), true); ctx.tool_configs.insert( - "upload-build-artifact".to_string(), + "upload-build-attachment".to_string(), serde_json::json!({ "allowed-artifact-names": ["agent-*", "safe-report"] }), ); let outcome = result.execute_impl(&ctx).await.unwrap(); @@ -1074,10 +1075,10 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_applies_name_prefix() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-report-eeff5566.pdf"; + let staged = "upload-build-attachment-report-eeff5566.pdf"; std::fs::write(dir.path().join(staged), b"hello").unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1), "report".to_string(), "out/report.pdf".to_string(), @@ -1087,7 +1088,7 @@ attachment-type: "agent-artifact" ); let mut ctx = make_ctx(dir.path().to_path_buf(), true); ctx.tool_configs.insert( - "upload-build-artifact".to_string(), + "upload-build-attachment".to_string(), serde_json::json!({ "name-prefix": "agent-" }), ); let outcome = result.execute_impl(&ctx).await.unwrap(); @@ -1103,11 +1104,11 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_rejects_tampered_staged_file() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-tampered.pdf"; + let staged = "upload-build-attachment-agent-report-tampered.pdf"; // Write 100 bytes but record 50 in the result — simulates tampering. std::fs::write(dir.path().join(staged), vec![0u8; 100]).unwrap(); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1), "agent-report".to_string(), "out/report.pdf".to_string(), @@ -1128,14 +1129,14 @@ attachment-type: "agent-artifact" #[tokio::test] async fn test_executor_rejects_sha256_mismatch() { let dir = tempfile::tempdir().unwrap(); - let staged = "upload-build-artifact-agent-report-sha-mismatch.pdf"; + let staged = "upload-build-attachment-agent-report-sha-mismatch.pdf"; let content = b"real file content"; std::fs::write(dir.path().join(staged), content).unwrap(); // Record the hash of different content — same size but wrong hash. let wrong_hash = crate::hash::sha256_hex(b"wrong file content"); - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(1), "agent-report".to_string(), "out/report.pdf".to_string(), @@ -1161,7 +1162,7 @@ attachment-type: "agent-artifact" #[test] fn test_dry_run_summary_with_build_id() { - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( Some(42), "report".to_string(), "out/report.pdf".to_string(), @@ -1176,7 +1177,7 @@ attachment-type: "agent-artifact" #[test] fn test_dry_run_summary_without_build_id() { - let result = UploadBuildArtifactResult::new( + let result = UploadBuildAttachmentResult::new( None, "report".to_string(), "out/report.pdf".to_string(), diff --git a/src/safeoutputs/upload_pipeline_artifact.rs b/src/safeoutputs/upload_pipeline_artifact.rs new file mode 100644 index 00000000..172f457f --- /dev/null +++ b/src/safeoutputs/upload_pipeline_artifact.rs @@ -0,0 +1,855 @@ +//! Upload pipeline artifact safe output tool. +//! +//! Lets an agent propose publishing a workspace file as an Azure DevOps build +//! artifact via the **build artifacts** REST API. Unlike build attachments +//! (which are invisible in the standard UI without a custom extension), +//! build artifacts published through this tool appear in the **Artifacts tab** +//! of the build summary page in Azure DevOps. +//! +//! The upload is a three-step REST flow: +//! +//! 1. **Create container** — `POST /_apis/resources/containers` returns a +//! `containerId`. +//! 2. **Upload file** — `PUT /_apis/resources/containers/{containerId}` sends +//! the file bytes with `itemPath` and `scope` query params. +//! 3. **Associate artifact** — `POST /_apis/build/builds/{buildId}/artifacts` +//! links the container to the build as a named artifact. +//! +//! The flow mirrors `upload-build-attachment`: +//! +//! * **Stage 1 (MCP, in the agent sandbox):** the MCP server validates the +//! agent-supplied params, resolves `file_path` against the agent's workspace, +//! rejects symlink escapes / directories, and **copies the file** into the +//! safe-outputs `output_directory` under a generated unique name. +//! * **Stage 3 (executor, outside the sandbox):** reads the staged file from +//! `ctx.working_directory.join(staged_file)`, applies operator-supplied +//! limits (`max-file-size`, `allowed-extensions`, `allowed-artifact-names`, +//! `allowed-build-ids`, `name-prefix`), resolves the target build ID, and +//! executes the three-step upload flow. + +use ado_aw_derive::SanitizeConfig; +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::SanitizeContent; +use crate::safeoutputs::{ExecutionContext, ExecutionResult, Executor, Validate}; +use crate::tool_result; +use crate::validate::{is_safe_path_segment, is_valid_artifact_name}; +use anyhow::{Context, ensure}; + +/// Parameters for publishing a workspace file as an ADO pipeline artifact. +#[derive(Deserialize, JsonSchema)] +pub struct UploadPipelineArtifactParams { + /// The build ID to publish the artifact to. **Omit to target the current + /// pipeline run** — the executor resolves the build ID from the + /// `BUILD_BUILDID` environment variable automatically. When provided, + /// must be a positive integer. + pub build_id: Option, + + /// The artifact name shown in the Artifacts tab. ADO requires a non-empty + /// name made of alphanumerics, `-`, `_`, or `.`. Must be 1-100 characters + /// and must not start with `.`. + pub artifact_name: String, + + /// Path to the file in the workspace to publish. Must be a relative path + /// with no directory traversal, no absolute prefix, and no `.git` + /// segments. + pub file_path: String, +} + +impl Validate for UploadPipelineArtifactParams { + fn validate(&self) -> anyhow::Result<()> { + if let Some(id) = self.build_id { + ensure!(id > 0, "build_id must be positive when specified"); + } + + ensure!( + self.artifact_name.len() <= 100, + "artifact_name must be at most 100 characters" + ); + ensure!( + !self.artifact_name.starts_with('.'), + "artifact_name must not start with '.'" + ); + ensure!( + is_valid_artifact_name(&self.artifact_name), + "artifact_name must be non-empty and contain only alphanumeric characters, '-', '_' or '.'" + ); + + ensure!(!self.file_path.is_empty(), "file_path must not be empty"); + ensure!( + !self.file_path.contains('\0'), + "file_path must not contain null bytes" + ); + ensure!( + !self.file_path.contains(':'), + "file_path must not contain ':'" + ); + for component in self.file_path.split(['/', '\\']) { + ensure!( + is_safe_path_segment(component), + "file_path component '{}' is not a safe path segment (no empty, '..', or leading '.' allowed)", + component + ); + } + Ok(()) + } +} + +/// Internal params struct for the `tool_result!` macro's `TryFrom` plumbing. +#[derive(Deserialize, JsonSchema)] +struct UploadPipelineArtifactResultFields { + build_id: Option, + artifact_name: String, + file_path: String, + staged_file: String, + file_size: u64, + staged_sha256: String, +} + +impl Validate for UploadPipelineArtifactResultFields {} + +tool_result! { + name = "upload-pipeline-artifact", + write = true, + params = UploadPipelineArtifactResultFields, + default_max = 3, + /// Result of publishing a workspace file as an ADO pipeline artifact. + pub struct UploadPipelineArtifactResult { + /// Build ID the artifact should be published to. `None` means "current + /// build" — resolved at execution time from `BUILD_BUILDID`. + build_id: Option, + /// Artifact name as proposed by the agent (pre-prefix). + artifact_name: String, + /// Original file path proposed by the agent. + file_path: String, + /// Filename of the staged copy inside the safe-outputs directory. + staged_file: String, + /// Size in bytes of the staged file at copy time. + file_size: u64, + /// SHA-256 hex digest of the staged file recorded at copy time. + staged_sha256: String, + } +} + +impl SanitizeContent for UploadPipelineArtifactResult { + fn sanitize_content_fields(&mut self) { + // All textual fields are strictly validated to safe charsets. + } +} + +impl UploadPipelineArtifactResult { + /// Construct a result after the agent's file has been staged. + pub fn new( + build_id: Option, + artifact_name: String, + file_path: String, + staged_file: String, + file_size: u64, + staged_sha256: String, + ) -> Self { + Self { + name: ::NAME.to_string(), + build_id, + artifact_name, + file_path, + staged_file, + file_size, + staged_sha256, + } + } +} + +/// Default maximum file size (50 MB). +pub const PIPELINE_ARTIFACT_DEFAULT_MAX_FILE_SIZE: u64 = 50 * 1024 * 1024; + +/// Configuration for the upload-pipeline-artifact tool (specified in front +/// matter). +#[derive(Debug, Clone, SanitizeConfig, Serialize, Deserialize)] +pub struct UploadPipelineArtifactConfig { + /// Maximum file size in bytes (default: 50 MB). + #[serde(default = "default_pipeline_max_file_size", rename = "max-file-size")] + pub max_file_size: u64, + + /// Allowed file extensions (e.g., `[".png", ".pdf"]`). Empty means all + /// extensions are allowed. + #[serde(default, rename = "allowed-extensions")] + pub allowed_extensions: Vec, + + /// Restrict which artifact names may be published. Empty means any name + /// (subject to charset rules) is allowed. Entries ending with `*` match + /// by prefix; otherwise the comparison is exact. + #[serde(default, rename = "allowed-artifact-names")] + pub allowed_artifact_names: Vec, + + /// Restrict which build IDs the agent may publish to. Empty means any + /// build ID accessible to the executor's token is allowed. This check + /// is skipped when `build_id` is omitted (targeting the current build). + #[serde(default, rename = "allowed-build-ids")] + pub allowed_build_ids: Vec, + + /// Prefix prepended to the agent-supplied artifact name before publishing. + #[serde(default, rename = "name-prefix")] + pub name_prefix: Option, +} + +fn default_pipeline_max_file_size() -> u64 { + PIPELINE_ARTIFACT_DEFAULT_MAX_FILE_SIZE +} + +impl Default for UploadPipelineArtifactConfig { + fn default() -> Self { + Self { + max_file_size: PIPELINE_ARTIFACT_DEFAULT_MAX_FILE_SIZE, + allowed_extensions: Vec::new(), + allowed_artifact_names: Vec::new(), + allowed_build_ids: Vec::new(), + name_prefix: None, + } + } +} + +#[async_trait::async_trait] +impl Executor for UploadPipelineArtifactResult { + fn dry_run_summary(&self) -> String { + match self.build_id { + Some(id) => format!( + "publish '{}' as pipeline artifact '{}' on build #{}", + self.file_path, self.artifact_name, id + ), + None => format!( + "publish '{}' as pipeline artifact '{}' on current build", + self.file_path, self.artifact_name + ), + } + } + + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { + let effective_build_id: i64 = match self.build_id { + Some(id) => id, + None => { + let current = ctx.build_id.context( + "build_id was not specified and BUILD_BUILDID is not set — \ + cannot determine which build to publish the artifact to", + )?; + i64::try_from(current).context("BUILD_BUILDID value overflows i64")? + } + }; + + info!( + "Publishing '{}' as pipeline artifact '{}' on build #{}{}", + self.file_path, + self.artifact_name, + effective_build_id, + if self.build_id.is_none() { " (current build)" } else { "" } + ); + + let config: UploadPipelineArtifactConfig = ctx.get_tool_config("upload-pipeline-artifact"); + + // ── Build-ID allow-list ────────────────────────────────────────── + if self.build_id.is_some() + && !config.allowed_build_ids.is_empty() + && !config.allowed_build_ids.contains(&effective_build_id) + { + return Ok(ExecutionResult::failure(format!( + "Build ID {} is not in the allowed-build-ids list", + effective_build_id + ))); + } + + // ── Name-prefix ───────────────────────────────────────────────── + if let Some(prefix) = &config.name_prefix { + if prefix.len() > 50 { + return Ok(ExecutionResult::failure(format!( + "name-prefix '{}...' is too long ({} chars, max 50)", + prefix.chars().take(20).collect::(), + prefix.len() + ))); + } + } + let final_name = match &config.name_prefix { + Some(prefix) => format!("{}{}", prefix, self.artifact_name), + None => self.artifact_name.clone(), + }; + if final_name.starts_with('.') || final_name.len() > 100 || !is_valid_artifact_name(&final_name) { + return Ok(ExecutionResult::failure(format!( + "Resolved artifact name '{}' is not a valid Azure DevOps artifact name", + final_name + ))); + } + + // ── Artifact-name allow-list ───────────────────────────────────── + if !config.allowed_artifact_names.is_empty() { + let allowed = config.allowed_artifact_names.iter().any(|pattern| { + if let Some(prefix) = pattern.strip_suffix('*') { + final_name.starts_with(prefix) + } else { + *pattern == final_name + } + }); + if !allowed { + return Ok(ExecutionResult::failure(format!( + "Artifact name '{}' is not in the allowed list", + final_name + ))); + } + } + + // ── Extension allow-list ───────────────────────────────────────── + if !config.allowed_extensions.is_empty() { + let file_ext = std::path::Path::new(&self.file_path) + .extension() + .and_then(|e| e.to_str()) + .unwrap_or(""); + let has_valid_ext = config.allowed_extensions.iter().any(|ext| { + ext.trim_start_matches('.').eq_ignore_ascii_case(file_ext) + }); + if !has_valid_ext { + return Ok(ExecutionResult::failure(format!( + "File '{}' has an extension not in the allowed list: {:?}", + self.file_path, config.allowed_extensions + ))); + } + } + + // ── Staged file resolution ─────────────────────────────────────── + let staged_path = ctx.working_directory.join(&self.staged_file); + let canonical = staged_path.canonicalize().context( + "Failed to canonicalize staged file path — file may be missing or contains broken symlinks", + )?; + let canonical_base = ctx + .working_directory + .canonicalize() + .context("Failed to canonicalize working directory")?; + if !canonical.starts_with(&canonical_base) { + return Ok(ExecutionResult::failure(format!( + "Staged file '{}' resolves outside the safe-outputs directory", + self.staged_file + ))); + } + let metadata = std::fs::metadata(&canonical).context("Failed to read file metadata")?; + if metadata.is_dir() { + return Ok(ExecutionResult::failure(format!( + "Staged path '{}' is a directory; upload-pipeline-artifact only supports single files", + self.staged_file + ))); + } + let file_size = metadata.len(); + + // ── Integrity checks ───────────────────────────────────────────── + if file_size != self.file_size { + return Ok(ExecutionResult::failure(format!( + "Staged file size ({} bytes) differs from size recorded at Stage 1 ({} bytes) — \ + the file may have been modified between stages", + file_size, self.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 + ))); + } + + if ctx.dry_run { + return Ok(ExecutionResult::success(format!( + "[dry-run] would publish '{}' ({} bytes) as pipeline artifact '{}' on build #{}{}", + self.file_path, + file_size, + final_name, + effective_build_id, + if self.build_id.is_none() { " (current build)" } else { "" } + ))); + } + + let file_bytes = tokio::fs::read(&canonical).await.context("Failed to read file contents")?; + + let live_hash = crate::hash::sha256_hex(&file_bytes); + if live_hash != self.staged_sha256 { + return Ok(ExecutionResult::failure(format!( + "Staged file SHA-256 mismatch: expected {} (recorded at Stage 1), got {} — \ + the file may have been tampered with between stages", + self.staged_sha256, live_hash + ))); + } + + // ── Resolve ADO 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 project_id = ctx + .ado_project_id + .as_ref() + .context("SYSTEM_TEAMPROJECTID not set — required for pipeline artifact upload")?; + let token = ctx + .access_token + .as_ref() + .context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?; + + let client = reqwest::Client::new(); + + // Derive a filename from the original file path for use as the + // itemPath inside the container (e.g. "report.pdf" from "out/report.pdf"). + let filename = std::path::Path::new(&self.file_path) + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or(&self.staged_file); + + // ── Step 1: Create container ───────────────────────────────────── + let container_url = format!( + "{}/_apis/resources/containers?api-version=7.1-preview.4", + org_url.trim_end_matches('/'), + ); + debug!("Creating container for artifact '{}': {}", final_name, container_url); + + let container_body = serde_json::json!({ + "name": final_name, + "scope": project_id, + }); + let container_resp = client + .post(&container_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&container_body) + .send() + .await + .context("Failed to create artifact container")?; + + if !container_resp.status().is_success() { + let status = container_resp.status(); + let error_body = container_resp + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to create artifact container (HTTP {}): {}", + status, error_body + ))); + } + let container_json: serde_json::Value = container_resp + .json() + .await + .context("Failed to parse container creation response")?; + let container_id = container_json + .get("containerId") + .and_then(|v| v.as_u64()) + .context("Container creation response missing 'containerId'")?; + debug!("Container created: id={}", container_id); + + // ── Step 2: Upload file to container ───────────────────────────── + let upload_url = format!( + "{}/_apis/resources/containers/{}?itemPath={}/{}&scope={}&api-version=7.1-preview.4", + org_url.trim_end_matches('/'), + container_id, + utf8_percent_encode(&final_name, PATH_SEGMENT), + utf8_percent_encode(filename, PATH_SEGMENT), + utf8_percent_encode(project_id, PATH_SEGMENT), + ); + debug!("Uploading {} bytes to container: {}", file_size, upload_url); + + let upload_resp = client + .put(&upload_url) + .header("Content-Type", "application/octet-stream") + .basic_auth("", Some(token)) + .body(file_bytes) + .send() + .await + .context("Failed to upload file to artifact container")?; + + if !upload_resp.status().is_success() { + let status = upload_resp.status(); + let error_body = upload_resp + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + return Ok(ExecutionResult::failure(format!( + "Failed to upload file to container (HTTP {}): {}", + status, error_body + ))); + } + debug!("File uploaded to container {}", container_id); + + // ── Step 3: Associate artifact with build ──────────────────────── + let artifact_url = format!( + "{}/{}/_apis/build/builds/{}/artifacts?api-version=7.1", + org_url.trim_end_matches('/'), + utf8_percent_encode(project, PATH_SEGMENT), + effective_build_id, + ); + debug!("Associating artifact '{}' with build #{}: {}", final_name, effective_build_id, artifact_url); + + let artifact_body = serde_json::json!({ + "name": final_name, + "resource": { + "data": format!("#/{}/{}", container_id, final_name), + "type": "container", + }, + }); + let artifact_resp = client + .post(&artifact_url) + .header("Content-Type", "application/json") + .basic_auth("", Some(token)) + .json(&artifact_body) + .send() + .await + .context("Failed to associate artifact with build")?; + + if artifact_resp.status().is_success() { + let resp_body: serde_json::Value = artifact_resp.json().await.unwrap_or_else(|e| { + warn!( + "Pipeline artifact created for build #{} but the response JSON could not be parsed: {} — proceeding without download URL", + effective_build_id, e + ); + serde_json::Value::Null + }); + let download_url = resp_body + .get("resource") + .and_then(|r| r.get("downloadUrl")) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()); + info!( + "Published '{}' as pipeline artifact '{}' on build #{}", + self.file_path, final_name, effective_build_id + ); + + Ok(ExecutionResult::success_with_data( + format!( + "Published '{}' as pipeline artifact '{}' on build #{}", + self.file_path, final_name, effective_build_id + ), + serde_json::json!({ + "build_id": effective_build_id, + "artifact_name": final_name, + "file_path": self.file_path, + "size_bytes": file_size, + "download_url": download_url, + "project": project, + }), + )) + } else { + let status = artifact_resp.status(); + let error_body = artifact_resp + .text() + .await + .unwrap_or_else(|_| "Unknown error".to_string()); + Ok(ExecutionResult::failure(format!( + "Failed to associate artifact with build #{} (HTTP {}): {}", + effective_build_id, status, error_body + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::safeoutputs::ToolResult; + + #[test] + fn test_result_has_correct_name() { + assert_eq!(UploadPipelineArtifactResult::NAME, "upload-pipeline-artifact"); + } + + fn make_params( + build_id: Option, + artifact_name: &str, + file_path: &str, + ) -> UploadPipelineArtifactParams { + UploadPipelineArtifactParams { + build_id, + artifact_name: artifact_name.to_string(), + file_path: file_path.to_string(), + } + } + + const DUMMY_HASH: &str = "0000000000000000000000000000000000000000000000000000000000000000"; + + #[test] + fn test_params_validate_accepts_valid() { + assert!(make_params(Some(1), "agent-report", "out/report.pdf") + .validate() + .is_ok()); + assert!(make_params(None, "agent-report", "out/report.pdf") + .validate() + .is_ok()); + } + + #[test] + fn test_validation_rejects_zero_build_id() { + assert!(make_params(Some(0), "report", "out/report.pdf") + .validate() + .is_err()); + } + + #[test] + fn test_validation_rejects_negative_build_id() { + assert!(make_params(Some(-1), "report", "out/report.pdf") + .validate() + .is_err()); + } + + #[test] + fn test_validation_rejects_empty_artifact_name() { + assert!(make_params(None, "", "out/report.pdf").validate().is_err()); + } + + #[test] + fn test_validation_rejects_artifact_name_with_spaces() { + assert!(make_params(None, "my report", "out/report.pdf") + .validate() + .is_err()); + } + + #[test] + fn test_validation_rejects_leading_dot_artifact_name() { + assert!(make_params(None, ".hidden", "out/report.pdf") + .validate() + .is_err()); + } + + #[test] + fn test_validation_rejects_long_artifact_name() { + let long_name = "a".repeat(101); + assert!(make_params(None, &long_name, "out/report.pdf") + .validate() + .is_err()); + } + + #[test] + fn test_validation_rejects_empty_file_path() { + assert!(make_params(None, "report", "").validate().is_err()); + } + + #[test] + fn test_validation_rejects_traversal_in_file_path() { + assert!(make_params(None, "report", "../etc/passwd") + .validate() + .is_err()); + } + + #[test] + fn test_validation_rejects_null_bytes_in_file_path() { + assert!(make_params(None, "report", "out/report\0.pdf") + .validate() + .is_err()); + } + + #[test] + fn test_validation_rejects_colon_in_file_path() { + assert!(make_params(None, "report", "C:\\out\\report.pdf") + .validate() + .is_err()); + } + + #[test] + fn test_dry_run_summary() { + let result = UploadPipelineArtifactResult::new( + None, + "agent-report".to_string(), + "out/report.pdf".to_string(), + "staged-abc123.pdf".to_string(), + 1024, + DUMMY_HASH.to_string(), + ); + assert!(result.dry_run_summary().contains("agent-report")); + assert!(result.dry_run_summary().contains("current build")); + + let result_with_id = UploadPipelineArtifactResult::new( + Some(42), + "agent-report".to_string(), + "out/report.pdf".to_string(), + "staged-abc123.pdf".to_string(), + 1024, + DUMMY_HASH.to_string(), + ); + assert!(result_with_id.dry_run_summary().contains("build #42")); + } + + #[test] + fn test_config_defaults() { + let config = UploadPipelineArtifactConfig::default(); + assert_eq!(config.max_file_size, PIPELINE_ARTIFACT_DEFAULT_MAX_FILE_SIZE); + assert!(config.allowed_extensions.is_empty()); + assert!(config.allowed_artifact_names.is_empty()); + assert!(config.allowed_build_ids.is_empty()); + assert!(config.name_prefix.is_none()); + } + + #[test] + fn test_config_deserializes_from_yaml() { + let yaml = r#" +max-file-size: 10485760 +allowed-extensions: + - .pdf + - .log +allowed-artifact-names: + - agent-* +name-prefix: "ci-" +"#; + let config: UploadPipelineArtifactConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.max_file_size, 10485760); + assert_eq!(config.allowed_extensions, vec![".pdf", ".log"]); + assert_eq!(config.allowed_artifact_names, vec!["agent-*"]); + assert_eq!(config.name_prefix.as_deref(), Some("ci-")); + } + + #[tokio::test] + async fn test_dry_run_succeeds() { + let dir = tempfile::tempdir().unwrap(); + let staged = dir.path().join("staged-file.pdf"); + let content = b"test content"; + std::fs::write(&staged, content).unwrap(); + let hash = crate::hash::sha256_hex(content); + + let result = UploadPipelineArtifactResult::new( + None, + "agent-report".to_string(), + "out/report.pdf".to_string(), + "staged-file.pdf".to_string(), + content.len() as u64, + hash, + ); + + let ctx = ExecutionContext { + working_directory: dir.path().to_path_buf(), + build_id: Some(123), + dry_run: true, + ..Default::default() + }; + + let exec_result = result.execute_impl(&ctx).await.unwrap(); + assert!(exec_result.success); + assert!(exec_result.message.contains("[dry-run]")); + assert!(exec_result.message.contains("agent-report")); + } + + #[tokio::test] + async fn test_rejects_file_size_mismatch() { + let dir = tempfile::tempdir().unwrap(); + let staged = dir.path().join("staged-file.pdf"); + std::fs::write(&staged, b"test content").unwrap(); + + let result = UploadPipelineArtifactResult::new( + None, + "report".to_string(), + "out/report.pdf".to_string(), + "staged-file.pdf".to_string(), + 9999, // wrong size + DUMMY_HASH.to_string(), + ); + + let ctx = ExecutionContext { + working_directory: dir.path().to_path_buf(), + build_id: Some(123), + dry_run: false, + ..Default::default() + }; + + let exec_result = result.execute_impl(&ctx).await.unwrap(); + assert!(!exec_result.success); + assert!(exec_result.message.contains("differs from size")); + } + + #[tokio::test] + async fn test_rejects_exceeding_max_file_size() { + let dir = tempfile::tempdir().unwrap(); + let staged = dir.path().join("staged-file.pdf"); + let content = b"x"; + std::fs::write(&staged, content).unwrap(); + + let result = UploadPipelineArtifactResult::new( + None, + "report".to_string(), + "out/report.pdf".to_string(), + "staged-file.pdf".to_string(), + 1, + crate::hash::sha256_hex(content), + ); + + let mut ctx = ExecutionContext { + working_directory: dir.path().to_path_buf(), + build_id: Some(123), + dry_run: false, + ..Default::default() + }; + // Set max-file-size to 0 to trigger rejection + ctx.tool_configs.insert( + "upload-pipeline-artifact".to_string(), + serde_json::json!({"max-file-size": 0}), + ); + + let exec_result = result.execute_impl(&ctx).await.unwrap(); + assert!(!exec_result.success); + assert!(exec_result.message.contains("exceeds maximum")); + } + + #[tokio::test] + async fn test_rejects_disallowed_build_id() { + let dir = tempfile::tempdir().unwrap(); + let staged = dir.path().join("staged-file.pdf"); + std::fs::write(&staged, b"test").unwrap(); + + let result = UploadPipelineArtifactResult::new( + Some(999), + "report".to_string(), + "out/report.pdf".to_string(), + "staged-file.pdf".to_string(), + 4, + DUMMY_HASH.to_string(), + ); + + let mut ctx = ExecutionContext { + working_directory: dir.path().to_path_buf(), + dry_run: false, + ..Default::default() + }; + ctx.tool_configs.insert( + "upload-pipeline-artifact".to_string(), + serde_json::json!({"allowed-build-ids": [123, 456]}), + ); + + let exec_result = result.execute_impl(&ctx).await.unwrap(); + assert!(!exec_result.success); + assert!(exec_result.message.contains("not in the allowed-build-ids")); + } + + #[tokio::test] + async fn test_rejects_disallowed_extension() { + let dir = tempfile::tempdir().unwrap(); + let staged = dir.path().join("staged-file.exe"); + let content = b"test"; + std::fs::write(&staged, content).unwrap(); + + let result = UploadPipelineArtifactResult::new( + None, + "report".to_string(), + "out/report.exe".to_string(), + "staged-file.exe".to_string(), + content.len() as u64, + crate::hash::sha256_hex(content), + ); + + let mut ctx = ExecutionContext { + working_directory: dir.path().to_path_buf(), + build_id: Some(123), + dry_run: false, + ..Default::default() + }; + ctx.tool_configs.insert( + "upload-pipeline-artifact".to_string(), + serde_json::json!({"allowed-extensions": [".pdf", ".log"]}), + ); + + let exec_result = result.execute_impl(&ctx).await.unwrap(); + assert!(!exec_result.success); + assert!(exec_result.message.contains("not in the allowed list")); + } +}