fix(compile): trust generated SafeOutputs MCP for Copilot

src/compile/agentic_pipeline.rs

@@ -2674,9 +2674,10 @@ fn start_mcpg_step(
          #   - Rewrite URLs from 127.0.0.1 to host.docker.internal (AWF container needs\n\
          #     host.docker.internal to reach MCPG on the host; 127.0.0.1 is container loopback)\n\
          #   - Ensure tools: [\"*\"] on each server entry (Copilot CLI requirement)\n\
+         #   - Mark generated MCPG entries as default/trusted servers for Copilot CLI\n\
          #   - Preserve all other fields (headers, type, etc.)\n\
          jq --arg prefix \"http://$(MCP_GATEWAY_DOMAIN):$(MCP_GATEWAY_PORT)\" \\\n  \
-           '.mcpServers |= (to_entries | sort_by(.key) | map(.value.url |= sub(\"^http://[^/]+/\"; \"\\($prefix)/\") | .value.tools = [\"*\"]) | from_entries)' \\\n  \
+           '.mcpServers |= (to_entries | sort_by(.key) | map(.value.url |= sub(\"^http://[^/]+/\"; \"\\($prefix)/\") | .value.tools = [\"*\"] | .value.isDefaultServer = true) | from_entries)' \\\n  \
            \"$GATEWAY_OUTPUT\" > /tmp/awf-tools/mcp-config.json\n\
          \n\
          chmod 600 /tmp/awf-tools/mcp-config.json\n\
@@ -3679,6 +3680,24 @@ mod tests {
         assert!(msg.contains("missing `env:` key"), "got: {msg}");
     }
 
+    // ── start_mcpg_step ─────────────────────────────────────────────────────
+
+    #[test]
+    fn start_mcpg_step_marks_copilot_mcp_servers_as_default() {
+        let step = start_mcpg_step("", "", false, None).unwrap();
+
+        assert!(
+            step.script.contains(".value.tools = [\"*\"]"),
+            "Copilot mcp-config conversion should preserve wildcard tools: {}",
+            step.script
+        );
+        assert!(
+            step.script.contains(".value.isDefaultServer = true"),
+            "Copilot mcp-config conversion should mark generated MCP servers as default/trusted: {}",
+            step.script
+        );
+    }
+
     // ── split_yaml_step_sequence ───────────────────────────────────────────
 
     #[test]

src/compile/common.rs

@@ -3445,8 +3445,8 @@ mod tests {
             "wildcard bash should emit --allow-all-tools"
         );
         assert!(
-            !params.contains("--allow-tool"),
-            "no individual --allow-tool flags with --allow-all-tools"
+            params.contains("--allow-tool safeoutputs"),
+            "safeoutputs should be explicitly allowed even with --allow-all-tools"
         );
     }
 
@@ -3465,8 +3465,8 @@ mod tests {
             "\"*\" should behave same as \":*\""
         );
         assert!(
-            !params.contains("--allow-tool"),
-            "no individual --allow-tool flags with --allow-all-tools"
+            params.contains("--allow-tool safeoutputs"),
+            "safeoutputs should be explicitly allowed even with --allow-all-tools"
         );
     }
 
@@ -3511,8 +3511,8 @@ mod tests {
             "default (no bash) should emit --allow-all-tools"
         );
         assert!(
-            !params.contains("--allow-tool"),
-            "no individual --allow-tool flags with --allow-all-tools"
+            params.contains("--allow-tool safeoutputs"),
+            "safeoutputs should be explicitly allowed even with --allow-all-tools"
         );
     }
 
@@ -3555,8 +3555,8 @@ mod tests {
             "edit enabled should still emit --allow-all-paths"
         );
         assert!(
-            !params.contains("--allow-tool"),
-            "no individual --allow-tool flags"
+            params.contains("--allow-tool safeoutputs"),
+            "safeoutputs should be explicitly allowed even with --allow-all-tools"
         );
     }
 
@@ -3615,10 +3615,13 @@ mod tests {
             params.contains("--allow-all-tools"),
             "wildcard should use --allow-all-tools"
         );
-        // Should NOT add individual tool flags when --allow-all-tools is active
         assert!(
-            !params.contains("--allow-tool"),
-            "no individual tool flags with --allow-all-tools"
+            params.contains("--allow-tool safeoutputs"),
+            "safeoutputs should be explicitly allowed even with --allow-all-tools"
+        );
+        assert!(
+            !params.contains("shell(lean)"),
+            "runtime shell tools should still be covered by --allow-all-tools"
         );
     }
 
@@ -3637,6 +3640,10 @@ mod tests {
             !params.contains("--allow-tool my-tool"),
             "default (all-tools) mode should not emit individual --allow-tool for MCPs"
         );
+        assert!(
+            params.contains("--allow-tool safeoutputs"),
+            "compiler-owned safeoutputs should still get an explicit grant"
+        );
     }
 
     #[test]

src/engine.rs

@@ -439,6 +439,25 @@ impl Engine {
     }
 }
 
+fn push_unique_tool(allowed_tools: &mut Vec<String>, tool: &str) {
+    if !allowed_tools.iter().any(|existing| existing == tool) {
+        allowed_tools.push(tool.to_string());
+    }
+}
+
+fn collect_extension_allowed_tools(extension_declarations: &[Declarations]) -> Vec<String> {
+    let mut allowed_tools: Vec<String> = Vec::new();
+
+    // Tools from compiler extensions (github, safeoutputs, azure-devops, etc.)
+    for decl in extension_declarations {
+        for tool in &decl.copilot_allow_tools {
+            push_unique_tool(&mut allowed_tools, tool);
+        }
+    }
+
+    allowed_tools
+}
+
 /// Collects the list of allowed tool identifiers when bash is not in wildcard mode.
 ///
 /// Returns a flat `Vec<String>` of fully-qualified tool identifiers ready to be
@@ -449,16 +468,7 @@ fn collect_allowed_tools(
     extension_declarations: &[Declarations],
     edit_enabled: bool,
 ) -> Result<Vec<String>> {
-    let mut allowed_tools: Vec<String> = Vec::new();
-
-    // Tools from compiler extensions (github, safeoutputs, azure-devops, etc.)
-    for decl in extension_declarations {
-        for tool in &decl.copilot_allow_tools {
-            if !allowed_tools.contains(tool) {
-                allowed_tools.push(tool.clone());
-            }
-        }
-    }
+    let mut allowed_tools = collect_extension_allowed_tools(extension_declarations);
 
     // Tools from user-defined MCP servers (sorted for deterministic output).
     // Only add --allow-tool for MCPs that will actually produce an MCPG entry (i.e.,
@@ -588,10 +598,12 @@ fn copilot_args(
         .and_then(|t| t.edit)
         .unwrap_or(true);
 
-    // When --allow-all-tools is active, skip individual tool collection entirely.
-    // --allow-all-tools is a superset that permits all tool calls regardless.
+    // When --allow-all-tools is active, skip user/tool collection but keep
+    // compiler-owned extension grants. Copilot's MCP policy still requires
+    // first-party generated MCP servers (notably safeoutputs) to be explicitly
+    // permitted even when all tool calls are otherwise allowed.
     let allowed_tools: Vec<String> = if use_allow_all_tools {
-        Vec::new()
+        collect_extension_allowed_tools(extension_declarations)
     } else {
         collect_allowed_tools(front_matter, extension_declarations, edit_enabled)?
     };
@@ -651,15 +663,15 @@ fn copilot_args(
 
     if use_allow_all_tools {
         params.push("--allow-all-tools".to_string());
-    } else {
-        for tool in allowed_tools {
-            if tool.contains('(') || tool.contains(')') || tool.contains(' ') {
-                // Use double quotes - the copilot_params are embedded inside a single-quoted
-                // bash string in the AWF command, so single quotes would break quoting.
-                params.push(format!("--allow-tool \"{}\"", tool));
-            } else {
-                params.push(format!("--allow-tool {}", tool));
-            }
+    }
+
+    for tool in allowed_tools {
+        if tool.contains('(') || tool.contains(')') || tool.contains(' ') {
+            // Use double quotes - the copilot_params are embedded inside a single-quoted
+            // bash string in the AWF command, so single quotes would break quoting.
+            params.push(format!("--allow-tool \"{}\"", tool));
+        } else {
+            params.push(format!("--allow-tool {}", tool));
         }
     }
 

tests/copilot_cli_safeoutputs_tests.rs

@@ -182,6 +182,44 @@ fn extract_agent_invocation(compiled: &str) -> String {
     line[start + 1..end].to_string()
 }
 
+/// Splits the compiler-emitted Copilot command line enough for this contract
+/// test: whitespace separates words, single/double quotes group words, and
+/// backslash escapes the next character inside double quotes. It intentionally
+/// does not perform shell expansion or model every shell escape form.
+fn split_shell_words(input: &str) -> Vec<String> {
+    let mut words = Vec::new();
+    let mut current = String::new();
+    let mut chars = input.chars().peekable();
+    let mut quote = None;
+
+    while let Some(ch) = chars.next() {
+        match (quote, ch) {
+            (None, '\'') => quote = Some('\''),
+            (None, '"') => quote = Some('"'),
+            (None, ch) if ch.is_whitespace() => {
+                if !current.is_empty() {
+                    words.push(std::mem::take(&mut current));
+                }
+            }
+            (Some(q), ch) if ch == q => quote = None,
+            (Some('"'), '\\') => {
+                if let Some(next) = chars.next() {
+                    current.push(next);
+                }
+            }
+            (_, ch) => current.push(ch),
+        }
+    }
+
+    if let Some(quote) = quote {
+        panic!("unterminated {quote:?} shell quote in invocation: {input}");
+    }
+    if !current.is_empty() {
+        words.push(current);
+    }
+    words
+}
+
 fn build_local_copilot_mcp_config(
     mcpg_template: &str,
     port: u16,
@@ -327,21 +365,37 @@ fn real_copilot_cli_noop_contract() {
     );
     fs::write(&prompt_path, &prompt).expect("write prompt");
 
-    let prefix = "/tmp/awf-tools/copilot --prompt \"$(cat /tmp/awf-tools/agent-prompt.md)\" --additional-mcp-config @/tmp/awf-tools/mcp-config.json ";
-    let suffix = invocation
-        .strip_prefix(prefix)
-        .unwrap_or_else(|| panic!("unexpected compiler-emitted invocation prefix: {invocation}"));
-
     let copilot_bin =
         std::env::var("ADO_AW_COPILOT_CLI_PATH").unwrap_or_else(|_| "copilot".to_string());
+    let invocation_args = split_shell_words(&invocation);
+    assert_eq!(
+        invocation_args.first().map(String::as_str),
+        Some("/tmp/awf-tools/copilot"),
+        "unexpected compiler-emitted Copilot binary in invocation: {invocation}"
+    );
+
     let mut command = Command::new(&copilot_bin);
-    command
-        .arg("--prompt")
-        .arg(&prompt)
-        .arg("--additional-mcp-config")
-        .arg(format!("@{}", copilot_mcp_config_path.display()));
-    for arg in suffix.split_whitespace() {
-        command.arg(arg);
+    let mut args = invocation_args.iter().skip(1);
+    while let Some(arg) = args.next() {
+        match arg.as_str() {
+            "--prompt" => {
+                let _ = args.next().unwrap_or_else(|| {
+                    panic!("missing --prompt value in invocation: {invocation}")
+                });
+                command.arg("--prompt").arg(&prompt);
+            }
+            "--additional-mcp-config" => {
+                let _ = args.next().unwrap_or_else(|| {
+                    panic!("missing --additional-mcp-config value in invocation: {invocation}")
+                });
+                command
+                    .arg("--additional-mcp-config")
+                    .arg(format!("@{}", copilot_mcp_config_path.display()));
+            }
+            _ => {
+                command.arg(arg);
+            }
+        }
     }
     command.current_dir(server.output_dir.path());
     command.env("XDG_CONFIG_HOME", artifact_dir.join("xdg"));