Skip to content

Commit 1cddfb3

Browse files
feat: swap to using aw-mcpg (#19)
* refactor: remove custom MCP firewall, metadata, and built-in MCP concept Remove the custom MCP firewall proxy (src/mcp_firewall.rs), embedded tool metadata (src/mcp_metadata.rs, mcp-metadata.json), and all firewall tests. The copilot CLI no longer has built-in MCPs, so: - Replace BUILTIN_MCPS constant with is_custom_mcp() helper - Remove --disable-builtin-mcps, --disable-mcp-server, --mcp from copilot params - Remove mcp-firewall CLI subcommand - Simplify create wizard to MCP-level selection (no tool-level metadata) - Update 1ES compiler to use is_custom_mcp() check - Remove terminal_size dependency (only used by deleted MCP tool selector) All MCPs are now handled through the MCP Gateway (MCPG) instead of the custom firewall proxy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat: add MCP Gateway (MCPG) support with SafeOutputs HTTP server Add infrastructure for the gh-aw-mcpg gateway integration: - Add MCPG_VERSION and MCPG_IMAGE constants in common.rs - Add {{ mcpg_version }} and {{ mcpg_image }} template markers - Replace generate_firewall_config() with generate_mcpg_config() that produces MCPG-compatible JSON (mcpServers + gateway sections) - SafeOutputs always included as HTTP backend via host.docker.internal - Custom MCPs (with command:) become stdio servers in MCPG config - Add host.docker.internal to CORE_ALLOWED_HOSTS for AWF container to reach host-side MCPG - Add mcp-http subcommand: serves SafeOutputs over HTTP using rmcp's StreamableHttpService with axum, API key auth, and health endpoint - Add axum and rmcp transport-streamable-http-server dependencies - Remove terminal_size dependency (no longer used) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat: rewrite base.yml template for MCPG integration Replace the legacy MCP firewall pipeline steps with MCPG architecture: - Replace 'Prepare MCP firewall config' with 'Prepare MCPG config' - Replace stdio MCP config (safeoutputs + mcp-firewall) with HTTP config pointing copilot to MCPG via host.docker.internal - Add 'Start SafeOutputs HTTP server' step (background process on host) - Add 'Start MCP Gateway (MCPG)' step (Docker container on host network) - Add 'Stop MCPG and SafeOutputs' cleanup step (condition: always) - Add --enable-host-access to AWF invocation for container-to-host access - Pre-pull MCPG Docker image alongside AWF images - Update step display names and comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: update compiler tests for MCPG migration - Add assertions for MCPG Docker image reference in compiled output - Add assertions for host.docker.internal and --enable-host-access - Add assertions verifying no legacy mcp-firewall references - Add template structure checks for mcpg_config, mcpg_image, mcpg_version markers - Verify template no longer contains mcp-firewall-config or MCP_FIREWALL_EOF - Update fixture test to not require built-in MCP assertions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: update AGENTS.md for MCPG migration - Update architecture file tree (remove mcp_firewall.rs, mcp_metadata.rs, mcp-metadata.json) - Replace {{ firewall_config }} marker docs with {{ mcpg_config }} - Add {{ mcpg_version }} and {{ mcpg_image }} marker documentation - Update {{ agency_params }} to remove MCP-related flags - Replace mcp-firewall CLI docs with mcp-http subcommand - Remove 'Built-in MCP Servers' section (no built-in MCPs exist) - Simplify MCP Configuration to custom servers only - Add host.docker.internal to allowed domains table - Replace entire MCP Firewall section with MCP Gateway (MCPG) docs - Update standalone target description for MCPG - Update front matter example to remove built-in MCP references - Add gh-aw-mcpg and gh-aw-firewall to References Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review feedback on MCPG integration - Always call generate_mcpg_config() regardless of mcp_servers being empty — safeoutputs must always be present in the MCPG config - Pre-initialize SafeOutputs outside the StreamableHttpService factory closure to avoid block_on() panic on a Tokio worker thread - Add failure guards to readiness wait loops in base.yml: both SafeOutputs and MCPG now fail the pipeline step explicitly if they don't become ready within 30 seconds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden security per PR review feedback - Move host.docker.internal from CORE_ALLOWED_HOSTS to standalone compiler's generate_allowed_domains — only pipelines using MCPG need host access from the AWF container - Replace weak time-based API key fallback with /dev/urandom (32 bytes); time-based value retained only as last-resort if urandom is unavailable - Remove SAFE_OUTPUTS_API_KEY println to avoid leaking the secret to log files; the pipeline already knows the key it generated - Add security comment explaining why Docker socket mount is required for MCPG (spawns stdio MCP servers as sibling containers) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: improve code quality per PR review feedback - Extract MCPG_PORT constant alongside MCPG_VERSION/MCPG_IMAGE to avoid hardcoded port 80 in generated config - Propagate MCPG config serialization error with ? instead of silently falling back to broken JSON missing the gateway section - Omit empty args array from MCPG config for cleaner output (consistent with existing env/tools handling) - Add warning in 1ES compiler when non-custom MCPs fall back to convention-based service connection names that may not exist Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address round 3 PR review feedback - Guard reserved 'safeoutputs' name in generate_mcpg_config to prevent user-defined MCPs from overwriting the safe outputs HTTP backend - Log MCPG config template before API key substitution to avoid leaking MCP_GATEWAY_API_KEY to pipeline logs (ADO secret masking only applies in subsequent steps, not the current step's output) - Clarify allowed_hosts.rs comment: host.docker.internal is always added for standalone pipelines (not conditionally) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden SafeOutputs HTTP server security - Use subtle::ConstantTimeEq for API key comparison to prevent timing side-channel attacks from a compromised AWF container - Fail loudly (panic) when /dev/urandom is unavailable instead of silently generating a weak time-based key - Add doc comment on SafeOutputs Clone confirming concurrent safety: only contains immutable PathBuf fields, file I/O opens fresh Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: improve MCPG reliability and diagnostics - Pre-cleanup stale mcpg container before docker run to prevent retry failures when a previous run was interrupted (OOM/SIGKILL leaves container behind despite --rm) - Differentiate duplicate warning messages for MCPs without commands: options-but-no-command vs boolean-enabled (helps users understand migration path from removed built-in MCPs) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: improve SafeOutputs HTTP server robustness - Replace panic! with anyhow bail via .context() for /dev/urandom failure — panics in async contexts crash the Tokio task instead of propagating cleanly through the Result chain - Bind to 127.0.0.1 instead of 0.0.0.0 — MCPG runs with --network host so localhost is sufficient, reducing exposure on shared agents Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: correct MCPG networking for Linux host mode - SafeOutputs URL in MCPG config changed from host.docker.internal to localhost — MCPG runs with --network host on Linux where host.docker.internal is not auto-injected in host network mode - Gateway domain remains host.docker.internal (used by AWF container which runs in bridge network mode with --enable-host-access) - Add docker rm -f pre-cleanup before MCPG container start to handle interrupted retry scenarios - Differentiate MCP warning messages for options-without-command vs boolean-enabled formats Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: add MCPG client auth and generate API key early MCPG enforces authentication on incoming client requests. The agent's mcp-config.json now includes an Authorization header with the gateway API key. To support this, the MCP_GATEWAY_API_KEY is generated in the 'Prepare MCPG config' step (as an ADO secret variable) instead of the 'Start MCP Gateway' step, making it available when writing both the MCPG server config and the agent's client config. Also updates MCPG startup step to reference the ADO variable via $(MCP_GATEWAY_API_KEY) instead of the shell-local variable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: minor code quality improvements from review - Replace Response::builder().unwrap() with IntoResponse tuple for the 401 response in auth middleware (avoids unwrap on user path) - Case-insensitive safeoutputs name reservation check to prevent collision via 'SafeOutputs' or 'SAFEOUTPUTS' variants - Fix typo: 'required' → 'require' in user-visible warning message Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: inline MCPG image URL in template The MCPG image name (ghcr.io/github/gh-aw-mcpg) is a static string that belongs in the template, not compiled into the binary. Only the version number needs to be a compiled-in constant for template replacement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: pipeline template and docs fixes for SafeOutputs/MCPG - Add mkdir -p for staging/logs before SafeOutputs starts (prevents nohup redirect failure on every pipeline run) - Fix AGENTS.md SafeOutputs URL: host.docker.internal -> localhost (MCPG runs with --network host, so localhost is correct) - Remove unused MCPG_PID pipeline variable (cleanup uses docker stop by container name) - Add unit test for safeoutputs reserved-name collision guard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add tests for mcpg * Merge main --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4ba6706 commit 1cddfb3

18 files changed

Lines changed: 1919 additions & 5818 deletions

AGENTS.md

Lines changed: 100 additions & 120 deletions
Large diffs are not rendered by default.

Cargo.lock

Lines changed: 375 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@ serde = { version = "1.0.228", features = ["derive"] }
1313
serde_yaml = "0.9.34"
1414
serde_json = "1.0.149"
1515
schemars = "1.2"
16-
rmcp = { version = "0.8.0", features = ["server", "transport-io"] }
16+
rmcp = { version = "0.8.0", features = [
17+
"server",
18+
"transport-io",
19+
"transport-streamable-http-server",
20+
] }
1721
percent-encoding = "2.3"
18-
reqwest = { version = "0.12", features = ["json"] }
22+
reqwest = { version = "0.12", features = ["json", "blocking"] }
1923
tempfile = "3"
2024
tokio = { version = "1.43", features = ["full"] }
2125
log = "0.4"
@@ -24,3 +28,5 @@ regex-lite = "0.1"
2428
inquire = { version = "0.9.2", features = ["editor"] }
2529
terminal_size = "0.4.3"
2630
url = "2.5.8"
31+
axum = { version = "0.8.8", features = ["tokio"] }
32+
subtle = "2.6.1"

mcp-metadata.json

Lines changed: 0 additions & 4176 deletions
This file was deleted.

src/allowed_hosts.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ pub static CORE_ALLOWED_HOSTS: &[&str] = &[
5454
// ===== Agency / Copilot configuration =====
5555
"config.edge.skype.com",
5656
// Note: 168.63.129.16 (Azure DNS) is handled separately as it's an IP
57+
// Note: host.docker.internal is NOT in CORE — it's always added by the
58+
// standalone compiler in generate_allowed_domains (standalone always uses
59+
// MCPG, which needs host access from the AWF container).
5760
];
5861

5962
/// Hosts required by specific MCP servers.

src/compile/common.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33
use anyhow::{Context, Result};
44

55
use super::types::{FrontMatter, Repository, TriggerConfig};
6+
use crate::compile::types::McpConfig;
67
use crate::fuzzy_schedule;
7-
use crate::mcp_metadata::McpMetadataFile;
88

9-
/// Check if an MCP name is a built-in (known to the Copilot CLI via mcp-metadata.json)
10-
pub fn is_builtin_mcp(name: &str) -> bool {
11-
let metadata = McpMetadataFile::bundled();
12-
metadata.get(name).map(|m| m.builtin).unwrap_or(false)
9+
/// Check if an MCP has a custom command (i.e., is not just a name-based reference).
10+
/// All MCPs now require explicit command configuration — there are no built-in MCPs
11+
/// in the copilot CLI.
12+
pub fn is_custom_mcp(config: &McpConfig) -> bool {
13+
matches!(config, McpConfig::WithOptions(opts) if opts.command.is_some())
1314
}
1415

1516
/// Parse the markdown file and extract front matter and body
@@ -303,10 +304,6 @@ pub fn generate_copilot_params(front_matter: &FrontMatter) -> String {
303304
allowed_tools.push(format!("shell({})", cmd));
304305
}
305306

306-
let metadata = McpMetadataFile::bundled();
307-
let mut disallowed_mcps: Vec<&str> = metadata.mcp_names();
308-
disallowed_mcps.sort();
309-
310307
let mut params = Vec::new();
311308

312309
params.push(format!("--model {}", front_matter.engine.model()));
@@ -340,10 +337,6 @@ pub fn generate_copilot_params(front_matter: &FrontMatter) -> String {
340337
}
341338
}
342339

343-
for mcp in disallowed_mcps {
344-
params.push(format!("--disable-mcp-server {}", mcp));
345-
}
346-
347340
params.join(" ")
348341
}
349342

@@ -506,6 +499,14 @@ pub fn generate_header_comment(input_path: &std::path::Path) -> String {
506499
)
507500
}
508501

502+
/// Docker image and version for the MCP Gateway (gh-aw-mcpg).
503+
/// Update this when upgrading to a new MCPG release.
504+
/// See: https://github.com/github/gh-aw-mcpg/releases
505+
pub const MCPG_VERSION: &str = "0.1.9";
506+
507+
/// Default port MCPG listens on inside the container (host network mode).
508+
pub const MCPG_PORT: u16 = 80;
509+
509510
/// Generate source path for the execute command.
510511
///
511512
/// Returns a path using `{{ workspace }}` as the base, which gets resolved

src/compile/onees.rs

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ use std::path::Path;
1717

1818
use super::Compiler;
1919
use super::common::{
20-
self, AWF_VERSION, COPILOT_CLI_VERSION, DEFAULT_POOL, compute_effective_workspace, generate_copilot_params,
20+
self, AWF_VERSION, COPILOT_CLI_VERSION, DEFAULT_POOL, compute_effective_workspace,
2121
generate_acquire_ado_token, generate_checkout_self, generate_checkout_steps,
22-
generate_ci_trigger, generate_copilot_ado_env, generate_executor_ado_env,
23-
generate_header_comment, generate_job_timeout, generate_pipeline_path,
24-
generate_pipeline_resources, generate_pr_trigger, generate_repositories,
25-
generate_schedule, generate_source_path, generate_working_directory,
26-
replace_with_indent, validate_comment_target, validate_update_work_item_target,
27-
validate_write_permissions, validate_submit_pr_review_events,
28-
validate_update_pr_votes, validate_resolve_pr_thread_statuses,
22+
generate_ci_trigger, generate_copilot_ado_env, generate_copilot_params,
23+
generate_executor_ado_env, generate_header_comment, generate_job_timeout,
24+
generate_pipeline_path, generate_pipeline_resources, generate_pr_trigger,
25+
generate_repositories, generate_schedule, generate_source_path, generate_working_directory,
26+
is_custom_mcp, replace_with_indent, validate_comment_target,
27+
validate_resolve_pr_thread_statuses, validate_submit_pr_review_events,
28+
validate_update_pr_votes, validate_update_work_item_target, validate_write_permissions,
2929
};
3030
use super::types::{FrontMatter, McpConfig};
3131

@@ -120,18 +120,30 @@ displayName: "Finalize""#,
120120

121121
// Generate service connection token acquisition steps and env vars
122122
let acquire_read_token = generate_acquire_ado_token(
123-
front_matter.permissions.as_ref().and_then(|p| p.read.as_deref()),
123+
front_matter
124+
.permissions
125+
.as_ref()
126+
.and_then(|p| p.read.as_deref()),
124127
"SC_READ_TOKEN",
125128
);
126129
let copilot_ado_env = generate_copilot_ado_env(
127-
front_matter.permissions.as_ref().and_then(|p| p.read.as_deref()),
130+
front_matter
131+
.permissions
132+
.as_ref()
133+
.and_then(|p| p.read.as_deref()),
128134
);
129135
let acquire_write_token = generate_acquire_ado_token(
130-
front_matter.permissions.as_ref().and_then(|p| p.write.as_deref()),
136+
front_matter
137+
.permissions
138+
.as_ref()
139+
.and_then(|p| p.write.as_deref()),
131140
"SC_WRITE_TOKEN",
132141
);
133142
let executor_ado_env = generate_executor_ado_env(
134-
front_matter.permissions.as_ref().and_then(|p| p.write.as_deref()),
143+
front_matter
144+
.permissions
145+
.as_ref()
146+
.and_then(|p| p.write.as_deref()),
135147
);
136148

137149
// Validate that write-requiring safe-outputs have a write service connection
@@ -222,24 +234,47 @@ fn generate_agent_context_root(effective_workspace: &str) -> String {
222234
}
223235
}
224236

225-
/// Generate MCP configuration for 1ES templates
237+
/// Generate MCP configuration for 1ES templates.
238+
///
239+
/// In 1ES, MCPs require service connections. Only MCPs with explicit
240+
/// `service_connection` configuration or custom commands are included.
226241
fn generate_mcp_configuration(mcps: &HashMap<String, McpConfig>) -> String {
227242
let mut mcp_entries: Vec<_> = mcps
228243
.iter()
229244
.filter_map(|(name, config)| {
230245
let (is_enabled, opts) = match config {
231246
McpConfig::Enabled(enabled) => (*enabled, None),
232-
McpConfig::WithOptions(o) => (o.command.is_none(), Some(o)), // Custom MCPs not supported
247+
McpConfig::WithOptions(o) => (true, Some(o)),
233248
};
234249

235-
if !is_enabled || !common::is_builtin_mcp(name) {
250+
if !is_enabled {
236251
return None;
237252
}
238253

239-
// Use explicit service connection or generate default
254+
// Custom MCPs with command: not supported in 1ES (needs service connection)
255+
if is_custom_mcp(config) {
256+
log::warn!(
257+
"MCP '{}' uses custom command — not supported in 1ES target (requires service connection)",
258+
name
259+
);
260+
return None;
261+
}
262+
263+
// Use explicit service connection or generate default.
264+
// Warn when falling back to the naming convention — the generated
265+
// service connection reference may not exist in the ADO project.
240266
let service_connection = opts
241267
.and_then(|o| o.service_connection.clone())
242-
.unwrap_or_else(|| format!("mcp-{}-service-connection", name));
268+
.unwrap_or_else(|| {
269+
let default = format!("mcp-{}-service-connection", name);
270+
log::warn!(
271+
"MCP '{}' has no explicit service connection in 1ES target — \
272+
assuming '{}' exists",
273+
name,
274+
default,
275+
);
276+
default
277+
});
243278

244279
Some((name.clone(), service_connection))
245280
})

0 commit comments

Comments
 (0)