feat(safe-outputs): add unified upload-build-artifact safe output (#380)

* feat(safe-outputs): add unified upload-build-artifact safe output

Combines the concepts from #363 (upload-artifact, current build) and #373
(upload-build-artifact, arbitrary build) into a single tool that handles
both use cases via the ADO build attachments REST API.

Key design:
- build_id is Optional — omit to target the current pipeline run
  (resolved from BUILD_BUILDID in Stage 3)
- REST API (PUT .../builds/{buildId}/attachments/{type}/{name}) for all
  cases, replacing the ##vso[artifact.upload] logging command approach
- allowed-build-ids check is skipped when targeting the current build
  (implicitly trusted)
- Full Stage 1 → Stage 3 file staging contract: validate, canonicalize,
  bounds-check, stage file with extension preservation, re-canonicalize
  and re-verify in Stage 3

Supersedes #363 and #373.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: move file read after dry-run guard and use Path::extension() for allowlist check

- Move std::fs::read after the dry_run early return to avoid reading
  up to 50 MB into memory only to discard it in dry-run mode.
- Switch extension allowlist from suffix matching on the full path
  to Path::extension() comparison, preventing false matches like
  'catalog' matching allowed-extensions: ['log'].

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs: restore cache-memory heading in safe-outputs.md

The heading was accidentally dropped when the upload-build-artifact
section was inserted, leaving the cache-memory paragraph as a stray
note under the wrong section.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: use async file read, add staged-file integrity check, clarify comments

- Switch std::fs::read to tokio::fs::read().await to avoid blocking
  the tokio runtime for files up to 50 MB.
- Add a warn!() when the live staged-file size differs from the size
  recorded at Stage 1, catching potential file tampering between stages.
- Add comments on is_valid_version() call sites noting intentional
  reuse for artifact-name and attachment-type charset validation.
- Add comment on staged filename explaining max-length reasoning.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: make staged-file size mismatch a hard failure

A size mismatch between Stage 1 and Stage 3 indicates the staged file
was modified between stages — fail hard rather than warning and
uploading potentially tampered content. Adds a test for the integrity
check.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: add attachment-type dot guard, Stage 1 size cap, executor tests

- Add starts_with('.') guard to attachment-type validation, matching
  the artifact_name check for consistency.
- Add defense-in-depth file size cap at Stage 1 (MCP) using the
  default 50 MB limit to prevent staging-disk exhaustion before
  Stage 3 enforces the operator's configured limit.
- Add three missing executor tests: extension allowlist rejection,
  artifact-name allowlist rejection, and name-prefix application.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* feat: add SHA-256 cross-stage integrity verification

Add SHA-256 hash recording at Stage 1 and verification at Stage 3 for
both file-staging safe outputs:

- upload-build-artifact: staged_sha256 field recorded after copying
  the file, verified before uploading to ADO. Catches same-size file
  swaps that the size-only check would miss.
- create-pull-request: patch_sha256 field recorded after writing the
  patch, verified before applying. Field is Optional for backward
  compatibility with existing NDJSON records.

Also adds sha256_hex() helper in upload_build_artifact module, reused
by both tools and the MCP handlers.

A separate issue (#381) tracks adding the full staging pattern
(including SHA-256) to upload-workitem-attachment, which currently
reads directly from source_directory without staging.

Also fixes: name-prefix length cap (50 chars), attachment-type
leading-dot guard, comment typo in MCP handler.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: make patch_sha256 a required field on CreatePrResult

Pipelines are versioned with their YAML — no need for backward
compatibility with old NDJSON records. Making the hash optional
created a security hole where a missing hash would skip verification
entirely. Now patch_sha256 is a required String and verification is
unconditional.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* refactor: move sha256_hex to src/hash.rs, fix prefix slice panic

- Move sha256_hex() from upload_build_artifact.rs to a dedicated
  src/hash.rs module, removing the coupling between create_pr,
  mcp.rs and the upload_build_artifact module.
- Fix potential panic in name-prefix error path: &prefix[..20]
  can panic on multi-byte UTF-8 boundaries. Use
  prefix.chars().take(20).collect() instead.
- Revert upload_build_artifact module visibility back to private.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: add SHA-256 mismatch tests, fix TOCTOU, extract is_valid_artifact_name

- Add non-dry-run SHA-256 mismatch tests for both upload-build-artifact
  and create-pull-request (the mismatch path fires before any HTTP call,
  so no credentials are needed).
- Fix TOCTOU in MCP handler: hash the source bytes before writing to
  the staging directory, instead of re-reading the staged copy.
- Extract is_valid_artifact_name() in validate.rs to decouple artifact
  name validation from version string validation, preventing silent
  breakage if is_valid_version is ever tightened.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* refactor: import DEFAULT_MAX_FILE_SIZE in MCP handler

Replace the floating STAGE1_MAX_FILE_SIZE copy with a direct import
of DEFAULT_MAX_FILE_SIZE from upload_build_artifact, ensuring the
Stage 1 cap stays in sync if the default is ever changed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: jamesadevine

Cargo.lock

@@ -34,6 +34,7 @@ rand = "0.10.1"
 base64 = "0.22.1"
 glob-match = "0.2.1"
 similar = "3.1.0"
+sha2 = "0.11.0"
 
 [dev-dependencies]
 reqwest = { version = "0.12", features = ["blocking"] }

Cargo.toml

@@ -418,6 +418,44 @@ 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
+(`PUT /_apis/build/builds/{buildId}/attachments/{type}/{name}`).
+
+**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
+posthumously decorating a finished build with a generated report, screenshot, or
+log bundle.
+
+The tool stages the file during Stage 1 (MCP) by copying it into the
+safe-outputs directory; Stage 3 reads the staged copy and uploads it via the REST
+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 `.`)
+- `file_path` - Relative path to the file in the workspace (no directory traversal)
+
+**Configuration options (front matter):**
+```yaml
+safe-outputs:
+  upload-build-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
+    attachment-type: "agent-artifact"    # Optional — {type} segment in the attachments URL (default: "agent-artifact")
+    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.
+- The default `attachment-type` is `agent-artifact` so executor contributions are visually distinguishable from the build's own artifacts.
+
 ### 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.
 

docs/safe-outputs.md

@@ -17,7 +17,8 @@ use crate::safeoutputs::{
     ExecutionContext, ExecutionResult, Executor, LinkWorkItemsResult, MissingDataResult,
     MissingToolResult, NoopResult, QueueBuildResult, ReplyToPrCommentResult,
     ReportIncompleteResult, ResolvePrThreadResult, SubmitPrReviewResult, ToolResult,
-    UpdatePrResult, UpdateWikiPageResult, UpdateWorkItemResult, UploadWorkitemAttachmentResult,
+    UpdatePrResult, UpdateWikiPageResult, UpdateWorkItemResult, UploadBuildArtifactResult,
+    UploadWorkitemAttachmentResult,
 };
 
 // Re-export memory types for use by main.rs
@@ -91,6 +92,7 @@ pub async fn execute_safe_outputs(
         AddBuildTagResult,
         CreateBranchResult,
         UpdatePrResult,
+        UploadBuildArtifactResult,
         UploadWorkitemAttachmentResult,
         SubmitPrReviewResult,
         ReplyToPrCommentResult,
@@ -273,6 +275,7 @@ pub async fn execute_safe_output(
         "add-build-tag" => AddBuildTagResult,
         "create-branch" => CreateBranchResult,
         "update-pr" => UpdatePrResult,
+        "upload-build-artifact" => UploadBuildArtifactResult,
         "upload-workitem-attachment" => UploadWorkitemAttachmentResult,
         "submit-pr-review" => SubmitPrReviewResult,
         "reply-to-pr-review-comment" => ReplyToPrCommentResult,

src/execute.rs

@@ -0,0 +1,34 @@
+//! Cryptographic hash utilities shared across the crate.
+//!
+//! Used by safe-output tools to record and verify file integrity between
+//! Stage 1 (MCP, in-sandbox) and Stage 3 (executor, outside sandbox).
+
+use sha2::{Digest, Sha256};
+
+/// Compute the SHA-256 hex digest of a byte slice.
+pub(crate) fn sha256_hex(data: &[u8]) -> String {
+    let hash = Sha256::digest(data);
+    hash.iter().map(|b| format!("{:02x}", b)).collect()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_sha256_empty() {
+        // SHA-256 of empty input is well-known.
+        assert_eq!(
+            sha256_hex(b""),
+            "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+        );
+    }
+
+    #[test]
+    fn test_sha256_hello() {
+        assert_eq!(
+            sha256_hex(b"hello"),
+            "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"
+        );
+    }
+}

src/hash.rs

@@ -7,6 +7,7 @@ mod ecosystem_domains;
 mod engine;
 mod execute;
 mod fuzzy_schedule;
+mod hash;
 mod init;
 mod logging;
 mod mcp;