Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/red-team-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ grep -n 'replace\|replace_with_indent' src/compile/standalone.rs src/compile/one

### Category B: Path Traversal & File System

Audit `src/execute.rs`, `src/tools/create_pr.rs`, `src/tools/memory.rs`, `src/tools/upload_attachment.rs` for:
Audit `src/execute.rs`, `src/tools/create_pr.rs`, `src/tools/memory.rs`, `src/tools/upload_workitem_attachment.rs` for:

- **Directory traversal**: Are `..` sequences fully blocked in all path inputs? Check safe output file paths, memory file paths, wiki page paths, attachment paths.
- **Bounding directory escape**: Can the MCP server's bounding directory check be bypassed via symlinks, null bytes, or Unicode normalization?
Expand All @@ -79,7 +79,7 @@ Focus files:
```bash
cat src/tools/memory.rs
cat src/tools/create_pr.rs
cat src/tools/upload_attachment.rs
cat src/tools/upload_workitem_attachment.rs
grep -rn 'Path\|PathBuf\|canonicalize\|starts_with' src/tools/
grep -rn '\.\./' src/
```
Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Every compiled pipeline runs as three sequential jobs:
│ │ ├── update_pr.rs
│ │ ├── update_wiki_page.rs
│ │ ├── update_work_item.rs
│ │ └── upload_attachment.rs
│ │ └── upload_workitem_attachment.rs
│ ├── runtimes/ # Runtime environment implementations (one dir per runtime)
│ │ ├── mod.rs # Module entry point
│ │ └── lean/ # Lean 4 theorem prover runtime
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ 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-attachment` | Uploads a workspace file as an attachment to a work item |
| `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 |
| `missing-data` | Reports required data was unavailable |
Expand Down
4 changes: 2 additions & 2 deletions docs/safe-outputs.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ safe-outputs:
max: 1 # Maximum per run (default: 1)
```

### upload-attachment
### upload-workitem-attachment
Uploads a workspace file as an attachment to an Azure DevOps work item.

**Agent parameters:**
Expand All @@ -411,7 +411,7 @@ Uploads a workspace file as an attachment to an Azure DevOps work item.
**Configuration options (front matter):**
```yaml
safe-outputs:
upload-attachment:
upload-workitem-attachment:
max-file-size: 5242880 # Maximum file size in bytes (default: 5 MB)
allowed-extensions: [] # Optional — restrict file types (e.g., [".png", ".pdf"])
comment-prefix: "[Agent] " # Optional — prefix prepended to the comment
Expand Down
2 changes: 1 addition & 1 deletion prompts/create-ado-agentic-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ tools:
| `update-work-item` | Update fields on existing work items (each field requires opt-in) | ✅ |
| `comment-on-work-item` | Add comments to work items (requires `target` scoping) | ✅ |
| `link-work-items` | Link two work items (parent/child, related, etc.) | ✅ |
| `upload-attachment` | Upload a workspace file to a work item | ✅ |
| `upload-workitem-attachment` | Upload a workspace file to a work item | ✅ |
| **Pull Requests** | | |
| `create-pull-request` | Create PRs from agent code changes | ✅ |
| `add-pr-comment` | Add a comment thread to a PR | ✅ |
Expand Down
2 changes: 1 addition & 1 deletion prompts/update-ado-agentic-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ permissions:
| Both | Agent reads; safe-outputs write |
| Neither | No ADO tokens anywhere |

If adding write-requiring safe outputs (`create-pull-request`, `create-work-item`, `comment-on-work-item`, `update-work-item`, `create-wiki-page`, `update-wiki-page`, `link-work-items`, `upload-attachment`, `create-branch`, `create-git-tag`, `add-build-tag`, `add-pr-comment`, `reply-to-pr-comment`, `resolve-pr-thread`, `submit-pr-review`, `update-pr`, `queue-build`), you **must** also add `permissions.write`. The compiler will error otherwise.
If adding write-requiring safe outputs (`create-pull-request`, `create-work-item`, `comment-on-work-item`, `update-work-item`, `create-wiki-page`, `update-wiki-page`, `link-work-items`, `upload-workitem-attachment`, `create-branch`, `create-git-tag`, `add-build-tag`, `add-pr-comment`, `reply-to-pr-comment`, `resolve-pr-thread`, `submit-pr-review`, `update-pr`, `queue-build`), you **must** also add `permissions.write`. The compiler will error otherwise.

### Adding Pre/Post Steps

Expand Down
6 changes: 3 additions & 3 deletions src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::safeoutputs::{
ExecutionContext, ExecutionResult, Executor, LinkWorkItemsResult, MissingDataResult,
MissingToolResult, NoopResult, QueueBuildResult, ReplyToPrCommentResult,
ReportIncompleteResult, ResolvePrThreadResult, SubmitPrReviewResult, ToolResult,
UpdatePrResult, UpdateWikiPageResult, UpdateWorkItemResult, UploadAttachmentResult,
UpdatePrResult, UpdateWikiPageResult, UpdateWorkItemResult, UploadWorkitemAttachmentResult,
};

// Re-export memory types for use by main.rs
Expand Down Expand Up @@ -91,7 +91,7 @@ pub async fn execute_safe_outputs(
AddBuildTagResult,
CreateBranchResult,
UpdatePrResult,
UploadAttachmentResult,
UploadWorkitemAttachmentResult,
SubmitPrReviewResult,
ReplyToPrCommentResult,
ResolvePrThreadResult,
Expand Down Expand Up @@ -262,7 +262,7 @@ pub async fn execute_safe_output(
"add-build-tag" => AddBuildTagResult,
"create-branch" => CreateBranchResult,
"update-pr" => UpdatePrResult,
"upload-attachment" => UploadAttachmentResult,
"upload-workitem-attachment" => UploadWorkitemAttachmentResult,
"submit-pr-review" => SubmitPrReviewResult,
"reply-to-pr-review-comment" => ReplyToPrCommentResult,
"resolve-pr-thread" => ResolvePrThreadResult,
Expand Down
12 changes: 6 additions & 6 deletions src/mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::safeoutputs::{
QueueBuildResult, SubmitPrReviewParams, SubmitPrReviewResult, ToolResult,
UpdatePrParams, UpdatePrResult,
UpdateWorkItemParams, UpdateWorkItemResult,
UploadAttachmentParams, UploadAttachmentResult,
UploadWorkitemAttachmentParams, UploadWorkitemAttachmentResult,
anyhow_to_mcp_error,
};

Expand Down Expand Up @@ -960,21 +960,21 @@ Changes will be applied during safe output processing."
}

#[tool(
name = "upload-attachment",
name = "upload-workitem-attachment",
description = "Upload a file attachment to an Azure DevOps work item. The file will be \
uploaded and linked during safe output processing. File size and type restrictions may apply."
)]
async fn upload_attachment(
async fn upload_workitem_attachment(
&self,
params: Parameters<UploadAttachmentParams>,
params: Parameters<UploadWorkitemAttachmentParams>,
) -> Result<CallToolResult, McpError> {
info!(
"Tool called: upload-attachment - work item #{} file '{}'",
"Tool called: upload-workitem-attachment - work item #{} file '{}'",
params.0.work_item_id, params.0.file_path
);
let mut sanitized = params.0;
sanitized.comment = sanitized.comment.map(|c| sanitize_text(&c));
let result: UploadAttachmentResult = sanitized.try_into()?;
let result: UploadWorkitemAttachmentResult = sanitized.try_into()?;
self.write_safe_output_file(&result).await
.map_err(|e| anyhow_to_mcp_error(anyhow::anyhow!("Failed to write safe output: {}", e)))?;
Ok(CallToolResult::success(vec![Content::text(format!(
Expand Down
10 changes: 5 additions & 5 deletions src/safeoutputs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = tool_names![
AddBuildTagResult,
CreateBranchResult,
UpdatePrResult,
UploadAttachmentResult,
UploadWorkitemAttachmentResult,
SubmitPrReviewResult,
ReplyToPrCommentResult,
ResolvePrThreadResult,
Expand Down Expand Up @@ -75,7 +75,7 @@ pub const ALL_KNOWN_SAFE_OUTPUTS: &[&str] = all_safe_output_names![
AddBuildTagResult,
CreateBranchResult,
UpdatePrResult,
UploadAttachmentResult,
UploadWorkitemAttachmentResult,
SubmitPrReviewResult,
ReplyToPrCommentResult,
ResolvePrThreadResult,
Expand Down Expand Up @@ -262,7 +262,7 @@ mod submit_pr_review;
mod update_pr;
mod update_wiki_page;
mod update_work_item;
mod upload_attachment;
mod upload_workitem_attachment;

pub use add_build_tag::*;
pub use add_pr_comment::*;
Expand All @@ -287,7 +287,7 @@ pub use submit_pr_review::*;
pub use update_pr::*;
pub use update_wiki_page::*;
pub use update_work_item::*;
pub use upload_attachment::*;
pub use upload_workitem_attachment::*;

#[cfg(test)]
mod tests {
Expand Down Expand Up @@ -344,7 +344,7 @@ mod tests {
assert!(AddBuildTagResult::REQUIRES_WRITE);
assert!(CreateBranchResult::REQUIRES_WRITE);
assert!(UpdatePrResult::REQUIRES_WRITE);
assert!(UploadAttachmentResult::REQUIRES_WRITE);
assert!(UploadWorkitemAttachmentResult::REQUIRES_WRITE);
assert!(SubmitPrReviewResult::REQUIRES_WRITE);
assert!(ReplyToPrCommentResult::REQUIRES_WRITE);
assert!(ResolvePrThreadResult::REQUIRES_WRITE);
Expand Down
Loading
Loading