diff --git a/AGENTS.md b/AGENTS.md index 64f4013e..85448862 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,8 +34,11 @@ Every compiled pipeline runs as three sequential jobs: sandbox with a read-only ADO token. The agent produces *safe-output proposals* (e.g. "create this PR", "comment on this work item") rather than acting directly. -2. **Detection (Stage 2)** — a separate agent inspects Stage 1's proposals for - prompt injection, secret leaks, and other threats. +2. **Detection (Stage 2)** — by default, a separate agent inspects Stage 1's + proposals for prompt injection, secret leaks, and other threats. Authors can + configure or explicitly disable AI analysis under + `safe-outputs.threat-detection`; the Detection job remains as the pipeline + boundary even when analysis is disabled. 3. **SafeOutputs (Stage 3)** — a non-agent executor applies approved safe outputs using a write-capable ADO token that the agent never sees. diff --git a/docs/audit.md b/docs/audit.md index 9b92111c..acbaf5f0 100644 --- a/docs/audit.md +++ b/docs/audit.md @@ -71,7 +71,7 @@ Current top-level keys include the following. Optional sections are omitted from | Key | Source | | --- | --- | -| `overview` | ADO build metadata + `aw_info.json` (engine, model, agent name, source, target). | +| `overview` | ADO build metadata + `aw_info.json` (engine, model, optional threat-detection enabled/engine/model, agent name, source, target). | | `task_domain` | Audit heuristics over the run's prompts and outputs. | | `behavior_fingerprint` | Higher-level audit heuristics over the run's behavior. | | `agentic_assessments` | Higher-level audit assessments emitted by the analyzers. | diff --git a/docs/engine.md b/docs/engine.md index e8f8491f..f7c14107 100644 --- a/docs/engine.md +++ b/docs/engine.md @@ -44,6 +44,51 @@ The `timeout-minutes` field sets a wall-clock limit (in minutes) for the entire When omitted, Azure DevOps uses its default job timeout (60 minutes). When set, the compiler emits `timeoutInMinutes: ` on the agentic job. +### Detection-specific engine overlay + +`safe-outputs.threat-detection.engine` can override the Copilot configuration +used by the separate Detection job: + +```yaml +engine: + id: copilot + model: claude-opus-4.7 + version: "1.0.70" + args: [--reasoning-effort=high] + env: + SHARED_SETTING: agent + +safe-outputs: + create-pull-request: {} + threat-detection: + engine: + model: gpt-5-mini + args: [] # explicitly clear inherited args + env: + SHARED_SETTING: detection + DETECTION_ONLY: "true" +``` + +The nested block accepts the same Copilot engine fields as top-level `engine:`. +Omitted values inherit from the top-level configuration. Scalar, command, +provider, and auth values supplied in the nested block replace inherited +values. Detection `env` merges by key with nested values winning; `args` +inherits when absent and replaces the complete inherited list when present. +When a Detection engine overlay is present, `--model` and `--api-target` are +rejected inside the effective args list; use the structured fields so metadata +and firewall hosts cannot drift. Set nested `args: []` to clear an inherited +override. + +The effective Detection config drives its own binary installation, invocation, +timeout, GitHub App/provider token minting, BYOK environment and credential +isolation, API target, and AWF network hosts. It never mutates the Agent config. +Detection inherits top-level `engine.timeout-minutes`; a nested +`timeout-minutes` overrides it for Detection only. + +ado-aw currently supports only engine ID `copilot`; a different nested ID fails +at compile time. See [Threat detection](safe-outputs.md#threat-detection-threat-detection) +for prompt, enable/disable, and custom-step behavior. + ### GitHub App-backed Copilot engine auth By default the Copilot engine authenticates with the `GITHUB_TOKEN` pipeline @@ -240,10 +285,11 @@ sidecar active: placeholder override. This isolation applies to **both** the Agent stage and the Detection -(threat-analysis) stage: the detection Copilot run inherits the same -`COPILOT_PROVIDER_*` routing and api-proxy credential isolation, so it reaches -the same external provider without exposing the credential (matching gh-aw, -whose detection engine config inherits the main engine's `env`). +(threat-analysis) stage. By default Detection uses the top-level provider +routing. An explicit +`safe-outputs.threat-detection.engine.provider` can select a different provider; +its credential exclusion and required firewall host are resolved independently +from the Agent. #### Network allowlist diff --git a/docs/front-matter.md b/docs/front-matter.md index ee7bb27c..f7c9a336 100644 --- a/docs/front-matter.md +++ b/docs/front-matter.md @@ -102,6 +102,16 @@ safe-outputs: # optional per-tool configuration for safe output artifact-link: # optional: link work item to repository branch enabled: true branch: main + threat-detection: # section-level Detection configuration + enabled: true # boolean only; false keeps a pass-through Detection job + prompt: | # appended to the fixed detector prompt + Focus on unsafe deserialization and authentication bypasses. + engine: # optional overlay on top-level engine: + model: gpt-5-mini # currently only the copilot engine ID is supported + steps: # trusted ADO steps before AI analysis + - bash: echo "Prepare detector" + post-steps: # trusted ADO steps after AI analysis + - bash: echo "Run additional scanner" on: # trigger configuration (unified under on: key) schedule: daily around 14:00 # fuzzy schedule - see docs/schedule-syntax.md pipeline: @@ -247,11 +257,13 @@ runtime — write it as clear, structured natural-language instructions. > report on pipeline failures and surfaces diagnostic signals. See > [`docs/conclusion.md`](conclusion.md). -## Inline step validation (`setup` / `steps` / `post-steps` / `teardown`) +## Inline step validation -Inline steps are authored as raw Azure DevOps YAML and are emitted into the -generated pipeline **verbatim** (a passthrough). For steps that invoke a -built-in ADO task the compiler also knows (e.g. `CopyFiles@2`, `Docker@2`, +Inline steps under `setup`, `steps`, `post-steps`, `teardown`, +`safe-outputs.threat-detection.steps`, and +`safe-outputs.threat-detection.post-steps` are authored as raw Azure DevOps YAML +and emitted into the generated pipeline **verbatim** (a passthrough). For steps +that invoke a built-in ADO task the compiler also knows (e.g. `CopyFiles@2`, `Docker@2`, `DotNetCoreCLI@2`, and most other first-party tasks), `ado-aw lint` performs an **advisory** validation of the `inputs:` mapping against the task's typed schema — checking for missing required inputs, unknown input keys, bad diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index 1f2589e1..baee7bef 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -37,6 +37,79 @@ safe-outputs: Safe output configurations are passed to Stage 3 execution and used when processing safe outputs. +### Threat detection (`threat-detection`) + +Threat Detection runs between the Agent and SafeOutputs jobs. Configuration +lives under `safe-outputs:` because it controls the gate protecting safe-output +execution, matching gh-aw. + +```yaml +safe-outputs: + create-pull-request: {} + threat-detection: + enabled: true + prompt: | + Also check for: + - authentication bypasses + - unsafe deserialization + - hardcoded credentials + engine: + id: copilot + model: gpt-5-mini + args: [--reasoning-effort=high] + env: + DETECTION_MODE: strict + steps: + - bash: ./scripts/prepare-security-scanner.sh + displayName: Prepare security scanner + post-steps: + - bash: ./scripts/run-security-scanner.sh + displayName: Run security scanner +``` + +| Field | Type | Default | Description | +|---|---|---|---| +| `enabled` | boolean | `true` | Run AI threat analysis. Runtime expressions are not supported. | +| `prompt` | string | *(none)* | Literal additional instructions appended under `## Additional Instructions`. ADO expressions (`$(...)`, `${{ ... }}`, `$[...]`) are rejected. The fixed detector prompt and `THREAT_DETECTION_RESULT:` contract are never replaced. | +| `engine` | string/object | top-level `engine` | Detection-specific Copilot configuration overlay. Non-Copilot IDs remain unsupported. | +| `steps` | ADO step list | `[]` | Trusted host-side steps after artifact/prompt preparation and before credential minting + AI execution. | +| `post-steps` | ADO step list | `[]` | Trusted host-side steps after AI execution and token revocation, before verdict parsing. | + +Boolean shorthand is also supported: + +```yaml +safe-outputs: + create-pull-request: {} + threat-detection: false +``` + +When disabled, the Detection job remains in the pipeline as a pass-through: it +copies the Agent artifact to `analyzed_outputs`, publishes +`threatAnalysis.SafeToProcess=true`, and still detects approval-gated proposals. +Safe outputs therefore proceed without AI analysis, but manual-review gates and +the existing job/artifact graph remain intact. Custom Detection `steps` and +`post-steps` do not run while disabled. + +The engine overlay inherits top-level settings unless a nested value is +supplied. Detection `env` merges by key (Detection wins); `args` inherits when +omitted and replaces the inherited list when supplied, so `args: []` clears +top-level arguments for Detection. Install version, command, auth/provider, +BYOK credential isolation, and firewall hosts are all resolved from the +effective Detection engine. + +Detection inherits the top-level `engine.timeout-minutes`. Set +`safe-outputs.threat-detection.engine.timeout-minutes` to override that limit +for Detection only. + +`steps` and `post-steps` run outside AWF and are trusted with the checked-out +repository and Detection artifacts. A failing custom step fails Detection and +blocks SafeOutputs. Use `pool.overrides.detection` to select a different +Detection pool; there is no duplicate `runs-on` key. + +Not currently supported: runtime/expression-controlled enablement, +`continue-on-error`, `engine: false` custom-scanner-only mode, and a +Detection-specific AI-credit budget. + ### Manual review (`require-approval`) High-impact safe outputs can be gated behind a human approval step @@ -120,8 +193,8 @@ apply one note to every tool. low-impact tool (e.g. `add-pr-comment`) non-gated so the automatic split job is created. -The Detection threat gate always runs first, so a flagged run applies nothing — -automatic or reviewed. +The Detection job always runs first. When AI threat analysis is enabled, a +flagged run applies nothing — automatic or reviewed. > **Trust boundary note for `pool.overrides:`:** When `pool.overrides:` is used > to move Detection, SafeOutputs, or Conclusion onto a different **self-hosted** diff --git a/src/audit/analyzers/otel.rs b/src/audit/analyzers/otel.rs index eb5050da..ebd513f5 100644 --- a/src/audit/analyzers/otel.rs +++ b/src/audit/analyzers/otel.rs @@ -105,7 +105,7 @@ mod tests { use super::*; use tempfile::TempDir; - const AW_INFO_JSON: &str = r#"{"schema":"ado-aw/aw_info/1","engine":"copilot","model":"claude-sonnet-4.5","agent_name":"test","target":"standalone","source":"agents/test.md","compiler_version":"0.30.0"}"#; + const AW_INFO_JSON: &str = r#"{"schema":"ado-aw/aw_info/1","engine":"copilot","model":"claude-sonnet-4.5","threat_detection_enabled":false,"detection_engine":"copilot","detection_model":"gpt-5-mini","agent_name":"test","target":"standalone","source":"agents/test.md","compiler_version":"0.30.0"}"#; const COPILOT_OTEL_FIXTURE: &str = include_str!("../../../tests/fixtures/copilot-otel.jsonl"); async fn write_file(path: &Path, contents: &str) { @@ -141,6 +141,9 @@ mod tests { let aw_info = analysis.aw_info.expect("expected aw_info"); assert_eq!(aw_info.engine.as_deref(), Some("copilot")); assert_eq!(aw_info.model.as_deref(), Some("claude-sonnet-4.5")); + assert_eq!(aw_info.threat_detection_enabled, Some(false)); + assert_eq!(aw_info.detection_engine.as_deref(), Some("copilot")); + assert_eq!(aw_info.detection_model.as_deref(), Some("gpt-5-mini")); assert_eq!(aw_info.compiler_version.as_deref(), Some("0.30.0")); let engine_config = analysis.engine_config.expect("expected engine config"); diff --git a/src/audit/model.rs b/src/audit/model.rs index 07b96931..5f4897b7 100644 --- a/src/audit/model.rs +++ b/src/audit/model.rs @@ -164,6 +164,15 @@ pub struct AwInfo { /// Model identifier used by the agent runtime. #[serde(skip_serializing_if = "Option::is_none")] pub model: Option, + /// Whether AI threat detection was enabled for this workflow. + #[serde(skip_serializing_if = "Option::is_none")] + pub threat_detection_enabled: Option, + /// Engine identifier used by the Detection job when explicitly configured. + #[serde(skip_serializing_if = "Option::is_none")] + pub detection_engine: Option, + /// Model identifier used by the Detection job when explicitly configured. + #[serde(skip_serializing_if = "Option::is_none")] + pub detection_model: Option, /// Agent name emitted by the compiled workflow metadata. #[serde(skip_serializing_if = "Option::is_none")] pub agent_name: Option, @@ -919,6 +928,7 @@ mod tests { source: Some(String::from("agents/security-scan.md")), target: Some(String::from("standalone")), compiler_version: Some(String::from("0.30.2")), + ..AwInfo::default() }), }, task_domain: Some(TaskDomainInfo { diff --git a/src/audit/render/console.rs b/src/audit/render/console.rs index 76f37cd5..f64e1d83 100644 --- a/src/audit/render/console.rs +++ b/src/audit/render/console.rs @@ -93,6 +93,19 @@ fn render_overview_section( push_opt_owned_row(&mut rows, "engine", engine); push_opt_owned_row(&mut rows, "model", model); + if let Some(enabled) = aw_info.and_then(|info| info.threat_detection_enabled) { + rows.push(("threat_detection_enabled".to_string(), enabled.to_string())); + } + push_opt_owned_row( + &mut rows, + "detection_engine", + aw_info.and_then(|info| info.detection_engine.clone()), + ); + push_opt_owned_row( + &mut rows, + "detection_model", + aw_info.and_then(|info| info.detection_model.clone()), + ); push_opt_owned_row( &mut rows, "agent", @@ -1171,6 +1184,7 @@ By threat: source: Some("agents/my-agent.md".to_string()), target: Some("standalone".to_string()), compiler_version: Some("0.30.2".to_string()), + ..AwInfo::default() }), }, task_domain: Some(TaskDomainInfo { diff --git a/src/audit/render/json.rs b/src/audit/render/json.rs index 24b279d7..c247beac 100644 --- a/src/audit/render/json.rs +++ b/src/audit/render/json.rs @@ -65,6 +65,7 @@ mod tests { source: Some(String::from("agents/security-scan.md")), target: Some(String::from("standalone")), compiler_version: Some(String::from("0.30.2")), + ..AwInfo::default() }), }, task_domain: Some(TaskDomainInfo { diff --git a/src/compile/agentic_pipeline.rs b/src/compile/agentic_pipeline.rs index 859d207e..f069def0 100644 --- a/src/compile/agentic_pipeline.rs +++ b/src/compile/agentic_pipeline.rs @@ -90,9 +90,9 @@ use super::ir::{ PrTrigger, RepositoryResource, Resources, Schedule, Triggers, }; use super::types::{ - ApprovalConfig, ApprovalOnTimeout, CheckoutFetchOpts, FrontMatter, OnConfig, + ApprovalConfig, ApprovalOnTimeout, CheckoutFetchOpts, EngineConfig, FrontMatter, OnConfig, PipelineArtifactConfig, PrMode, ProviderToken, Repository as RepoCfg, SELF_CHECKOUT_ALIAS, - SupplyChainConfig, + SupplyChainConfig, ThreatDetectionConfig, }; /// The `safe-outputs:` key for the create-pull-request tool. Matches the kebab @@ -129,6 +129,9 @@ pub(crate) fn build_pipeline_context( debug_pipeline: bool, prefix: Option<&str>, ) -> Result { + let threat_detection = front_matter.threat_detection_config()?; + let detection_engine_config = front_matter.effective_detection_engine(&threat_detection); + // ─── Validations (reuse all shared validators) ──────────────── common::validate_front_matter_identity(front_matter)?; common::validate_variable_groups(front_matter)?; @@ -138,6 +141,8 @@ pub(crate) fn build_pipeline_context( ctx.ado_context.as_ref().map(|c| c.repo_name.as_str()), )?; common::validate_safe_outputs_keys(front_matter)?; + front_matter + .validate_threat_detection_config(&threat_detection, &detection_engine_config)?; front_matter.validate_require_approval()?; common::validate_comment_target(front_matter)?; common::validate_update_work_item_target(front_matter)?; @@ -179,6 +184,7 @@ pub(crate) fn build_pipeline_context( )?; let compiler_version = env!("CARGO_PKG_VERSION").to_string(); + let detection_engine = crate::engine::get_engine(detection_engine_config.engine_id())?; let engine_run = ctx.engine.invocation( ctx.front_matter, @@ -186,7 +192,8 @@ pub(crate) fn build_pipeline_context( "/tmp/awf-tools/agent-prompt.md", Some("/tmp/awf-tools/mcp-config.json"), )?; - let engine_run_detection = ctx.engine.invocation( + let engine_run_detection = detection_engine.invocation_with_config( + &detection_engine_config, ctx.front_matter, &extension_declarations, "/tmp/awf-tools/threat-analysis-prompt.md", @@ -195,6 +202,15 @@ pub(crate) fn build_pipeline_context( let engine_install_steps_yaml = ctx.engine .install_steps(&front_matter.engine, &front_matter.target, ctx.ado_org())?; + let detection_engine_install_steps_yaml = if threat_detection.is_enabled() { + detection_engine.install_steps( + &detection_engine_config, + &front_matter.target, + ctx.ado_org(), + )? + } else { + String::new() + }; let engine_log_dir = ctx.engine.log_dir().to_string(); let mut engine_env = ctx.engine.env(&front_matter.engine)?; @@ -224,11 +240,23 @@ pub(crate) fn build_pipeline_context( { byom_exclude_keys.push(crate::compile::types::PROVIDER_BEARER_TOKEN_VAR.to_string()); } - // Provider-only env subset for the Detection step, so the threat-analysis - // Copilot run inherits the same BYOM/BYOK routing + credential isolation as - // the main agent (mirrors gh-aw's detection engine-config Env inheritance). - let detection_provider_env = if is_copilot { - crate::engine::copilot_provider_env(&front_matter.engine)? + let detection_is_copilot = matches!(detection_engine, crate::engine::Engine::Copilot); + let mut detection_byom_exclude_keys = if detection_is_copilot { + crate::engine::copilot_byom_credential_keys(&detection_engine_config) + } else { + Vec::new() + }; + if detection_is_copilot + && detection_engine_config + .provider() + .and_then(|p| p.token.as_ref()) + .is_some() + { + detection_byom_exclude_keys + .push(crate::compile::types::PROVIDER_BEARER_TOKEN_VAR.to_string()); + }; + let detection_engine_env = if detection_is_copilot { + crate::engine::copilot_detection_env(&detection_engine_config)? } else { Vec::new() }; @@ -247,6 +275,16 @@ pub(crate) fn build_pipeline_context( // AWF mounts + allowlist let allowed_domains = common::generate_allowed_domains(front_matter, extensions, &extension_declarations)?; + let detection_allowed_domains = if threat_detection.engine.is_some() { + common::generate_allowed_domains_for_engine( + front_matter, + extensions, + &extension_declarations, + &detection_engine_config, + )? + } else { + allowed_domains.clone() + }; let awf_mounts = common::generate_awf_mounts(extensions, &extension_declarations); let awf_path_step_yaml = common::generate_awf_path_step(&awf_paths); @@ -262,6 +300,10 @@ pub(crate) fn build_pipeline_context( // legacy template fold substitutes — do the same eagerly so step // bodies receive a fully-resolved scalar. let source_path_raw = common::generate_source_path(input_path); + let source_path_suffix = source_path_raw + .strip_prefix("{{ trigger_repo_directory }}/") + .expect("generate_source_path always emits the compiler-owned trigger-repo prefix"); + crate::validate::reject_pipeline_injection(source_path_suffix, "workflow source path")?; let source_path = source_path_raw.replace("{{ trigger_repo_directory }}", &trigger_repo_directory); let pipeline_path = common::generate_pipeline_path(output_path); @@ -345,11 +387,15 @@ pub(crate) fn build_pipeline_context( trigger_repo_directory: trigger_repo_directory.clone(), compiler_version: compiler_version.clone(), engine_install_steps_yaml, + detection_engine_install_steps_yaml, engine_run, engine_run_detection, + detection_engine_config, + threat_detection, engine_env, engine_log_dir, allowed_domains, + detection_allowed_domains, awf_mounts, awf_path_step_yaml, mcpg_config_json, @@ -364,7 +410,8 @@ pub(crate) fn build_pipeline_context( agent_content_value, debug_pipeline, byom_exclude_keys, - detection_provider_env, + detection_byom_exclude_keys, + detection_engine_env, }; // ─── Build jobs ─────────────────────────────────────────────── @@ -515,13 +562,17 @@ pub(crate) struct StandaloneCtx { /// impl. A future `Engine::install_steps_typed` would lift this /// to typed steps. pub(crate) engine_install_steps_yaml: String, + pub(crate) detection_engine_install_steps_yaml: String, pub(crate) engine_run: String, pub(crate) engine_run_detection: String, + pub(crate) detection_engine_config: EngineConfig, + pub(crate) threat_detection: ThreatDetectionConfig, /// Composed engine env block — `KEY: VALUE` lines, one per line. /// Carried as a string and re-parsed during step emission. pub(crate) engine_env: String, pub(crate) engine_log_dir: String, pub(crate) allowed_domains: String, + pub(crate) detection_allowed_domains: String, /// `--mount` flags for AWF (or `\` placeholder when no mounts). pub(crate) awf_mounts: String, /// `awf_path_step` YAML body (or empty when no path prepends). @@ -548,10 +599,9 @@ pub(crate) struct StandaloneCtx { /// Actual provider credential env keys present to pass to AWF `--exclude-env`; /// empty for non-BYOM. AWF's API proxy itself is always enabled. pub(crate) byom_exclude_keys: Vec, - /// Provider-only (`COPILOT_PROVIDER_*`) subset of `engine.env` as validated - /// raw `(key, value)` pairs (empty when none). Applied to the Detection - /// (threat-analysis) step so it inherits BYOM provider routing + isolation. - pub(crate) detection_provider_env: Vec<(String, String)>, + pub(crate) detection_byom_exclude_keys: Vec, + /// Validated inherited/overridden custom env for Detection. + pub(crate) detection_engine_env: Vec<(String, String)>, } // ───────────────────────────────────────────────────────────────────── @@ -1134,8 +1184,8 @@ fn agent_job_variables_hoist( /// emitted. Today only the GitHub App token step needs it; future /// detection-only bundle consumers should `||` their own condition in here /// rather than adding a second `install_and_download_steps_typed` call. -fn detection_job_needs_ado_script_bundle(front_matter: &FrontMatter) -> bool { - front_matter.engine.github_app_token().is_some() +fn detection_job_needs_ado_script_bundle(engine_config: &EngineConfig) -> bool { + engine_config.github_app_token().is_some() } fn build_detection_job( @@ -1152,95 +1202,93 @@ fn build_detection_job( condition: None, })); - // Engine install - push_raw_yaml_if_nonempty(&mut steps, &cfg.engine_install_steps_yaml)?; - // One NuGetAuthenticate@1 for the whole Detection job (feed mirror). - if let Some(auth) = feed_auth_step(front_matter.supply_chain()) { - steps.push(auth); - } - // Download compiler - steps.extend(download_compiler_step( - &cfg.compiler_version, - front_matter.supply_chain(), - )); - // DockerInstaller - steps.push(Step::Task(DockerInstaller::new("26.1.4").into_step())); - // Download AWF - steps.extend(download_awf_step(front_matter.supply_chain())); - // Pre-pull AWF (no MCPG image for detection). - steps.extend(prepull_images_step(false, front_matter.supply_chain())); // Prepare safe outputs for analysis steps.push(Step::Bash(prepare_safe_outputs_for_analysis( &cfg.working_directory, ))); - // Prepare threat analysis prompt - // include_str! may carry CRLF line endings on Windows; normalise to LF - // so the resulting block scalar emits cleanly. Then substitute the - // template markers the threat prompt embeds (source_path, agent_name, - // agent_description, working_directory) — these match the legacy - // template fold's behaviour. - let threat_prompt_raw = include_str!("../data/threat-analysis.md"); - let threat_prompt = threat_prompt_raw - .replace("\r\n", "\n") - .replace("{{ source_path }}", &cfg.source_path) - .replace("{{ agent_name }}", &cfg.agent_display_name) - .replace("{{ agent_description }}", &front_matter.description) - .replace("{{ working_directory }}", &cfg.working_directory); - steps.push(Step::Bash(prepare_threat_analysis_prompt_step( - &threat_prompt, - )?)); - // Setup compiler - steps.push(Step::Bash(setup_compiler_step())); - // When GitHub App auth is configured, mint the installation token - // immediately before the threat-analysis Copilot run. Unlike the Agent job - // (whose bundle download is staged by the ado-script extension's - // agent-prepare phase, gated on `github_app_token_active`), the Detection - // job has no extension-prepare phase to piggyback on, so it stages the - // bundle self-contained — but exactly once, gated on the single - // `detection_job_needs_ado_script_bundle` predicate below so future - // detection-only bundle consumers OR into one download rather than adding a - // second (mirroring the Agent-job predicate in `AdoScriptExtension`). - if detection_job_needs_ado_script_bundle(front_matter) { - steps.extend( - super::extensions::ado_script::install_and_download_steps_typed( - front_matter.supply_chain(), - ), - ); - } - if let Some(app_token) = front_matter.engine.github_app_token() { - steps.push(super::extensions::ado_script::github_app_token_step_typed( - app_token, - )?); - } - // Mint the external provider token in-job (same-job secret) before the - // threat-analysis Copilot run, mirroring the Agent job. - if let Some(token) = front_matter - .engine - .provider() - .and_then(|p| p.token.as_ref()) - { - steps.push(Step::Task(provider_token_mint_step(token))); - } - // Run threat analysis - steps.push(Step::Bash(run_threat_analysis_step( - &cfg.allowed_domains, - &cfg.working_directory, - &cfg.engine_run_detection, - &cfg.byom_exclude_keys, - &cfg.detection_provider_env, - crate::engine::github_token_source_var(&front_matter.engine), - front_matter.supply_chain(), - )?)); - // Revoke the GitHub App token (best-effort, always) after threat analysis. - if let Some(app_token) = front_matter.engine.github_app_token() - && !app_token.skip_token_revocation - { - steps.push(super::extensions::ado_script::github_app_token_revoke_step_typed(app_token)?); + if cfg.threat_detection.is_enabled() { + // Detection gets its own effective engine install/config path. + push_raw_yaml_if_nonempty(&mut steps, &cfg.detection_engine_install_steps_yaml)?; + if let Some(auth) = feed_auth_step(front_matter.supply_chain()) { + steps.push(auth); + } + steps.extend(download_compiler_step( + &cfg.compiler_version, + front_matter.supply_chain(), + )); + steps.push(Step::Task(DockerInstaller::new("26.1.4").into_step())); + steps.extend(download_awf_step(front_matter.supply_chain())); + steps.extend(prepull_images_step(false, front_matter.supply_chain())); + + // include_str! may carry CRLF line endings on Windows; normalise to LF + // before marker substitution and appending operator instructions. + let threat_prompt_raw = include_str!("../data/threat-analysis.md"); + let mut threat_prompt = threat_prompt_raw + .replace("\r\n", "\n") + .replace("{{ source_path }}", &cfg.source_path) + .replace("{{ agent_name }}", &cfg.agent_display_name) + .replace("{{ agent_description }}", &front_matter.description) + .replace("{{ working_directory }}", &cfg.working_directory); + if let Some(custom_prompt) = cfg + .threat_detection + .prompt + .as_deref() + .filter(|prompt| !prompt.is_empty()) + { + threat_prompt.push_str("\n\n## Additional Instructions\n\n"); + threat_prompt.push_str(&custom_prompt.replace("\r\n", "\n")); + } + steps.push(Step::Bash(prepare_threat_analysis_prompt_step(&threat_prompt)?)); + steps.push(Step::Bash(setup_compiler_step())); + + // Stage auth support before custom pre-steps, but mint credentials only + // after them so trusted setup code receives the least privilege needed. + if detection_job_needs_ado_script_bundle(&cfg.detection_engine_config) { + steps.extend( + super::extensions::ado_script::install_and_download_steps_typed( + front_matter.supply_chain(), + ), + ); + } + for user_step in &cfg.threat_detection.steps { + steps.push(Step::RawYaml(step_to_raw_yaml_string(user_step)?)); + } + + if let Some(app_token) = cfg.detection_engine_config.github_app_token() { + steps.push(super::extensions::ado_script::github_app_token_step_typed( + app_token, + )?); + } + if let Some(token) = cfg + .detection_engine_config + .provider() + .and_then(|provider| provider.token.as_ref()) + { + steps.push(Step::Task(provider_token_mint_step(token))); + } + steps.push(Step::Bash(run_threat_analysis_step( + &cfg.detection_allowed_domains, + &cfg.working_directory, + &cfg.engine_run_detection, + &cfg.detection_byom_exclude_keys, + &cfg.detection_engine_env, + crate::engine::github_token_source_var(&cfg.detection_engine_config), + front_matter.supply_chain(), + )?)); + if let Some(app_token) = cfg.detection_engine_config.github_app_token() + && !app_token.skip_token_revocation + { + steps.push(super::extensions::ado_script::github_app_token_revoke_step_typed(app_token)?); + } + for user_step in &cfg.threat_detection.post_steps { + steps.push(Step::RawYaml(step_to_raw_yaml_string(user_step)?)); + } + steps.push(Step::Bash(prepare_analyzed_outputs_step())); + steps.push(Step::Bash(evaluate_threat_analysis_step())); + } else { + steps.push(Step::Bash(prepare_analyzed_outputs_passthrough_step())); + steps.push(Step::Bash(threat_analysis_disabled_step())); } - // Prepare analyzed outputs - steps.push(Step::Bash(prepare_analyzed_outputs_step())); - // Evaluate threat analysis — DECLARES TYPED OUTPUT - steps.push(Step::Bash(evaluate_threat_analysis_step())); // When manual review is configured, detect whether the agent actually // proposed any approval-gated outputs — DECLARES TYPED OUTPUT. The // ManualReview gate is conditioned on this so the run never pauses for a @@ -1252,8 +1300,9 @@ fn build_detection_job( &reviewed_tools, ))); } - // Copy logs - steps.push(Step::Bash(copy_logs_step(&cfg.engine_log_dir, true))); + if cfg.threat_detection.is_enabled() { + steps.push(Step::Bash(copy_logs_step(&cfg.engine_log_dir, true))); + } // Publish steps.push(Step::Publish(PublishStep { path: "$(Agent.TempDirectory)/analyzed_outputs".to_string(), @@ -1266,6 +1315,11 @@ fn build_detection_job( "Detection", cfg.pools.detection.clone(), ); + if cfg.threat_detection.is_enabled() + && let Some(minutes) = cfg.detection_engine_config.timeout_minutes() + { + job.timeout = Some(std::time::Duration::from_secs(60 * minutes as u64)); + } job.steps = steps; Ok(job) } @@ -3199,7 +3253,7 @@ fn run_threat_analysis_step( working_directory: &str, engine_run_detection: &str, byom_exclude_keys: &[String], - detection_provider_env: &[(String, String)], + detection_engine_env: &[(String, String)], github_token_var: &str, supply_chain: Option<&SupplyChainConfig>, ) -> Result { @@ -3243,10 +3297,9 @@ fn run_threat_analysis_step( "GITHUB_READ_ONLY", EnvValue::RawYamlScalar(serde_yaml::Value::Number(1.into())), ); - // BYOM/BYOK: apply the COPILOT_PROVIDER_* env so the detection Copilot run - // routes to the same external provider as the main agent. Classify each raw - // value directly (macro → PipelineVar, else Literal) — no YAML round-trip. - for (k, raw) in detection_provider_env { + // Apply validated Detection engine env directly (macro → PipelineVar, else + // Literal) without a YAML render/reparse round-trip. + for (k, raw) in detection_engine_env { step = step.with_env(k.clone(), env_value_from_str(raw)); } Ok(step) @@ -3285,6 +3338,27 @@ fn prepare_analyzed_outputs_step() -> BashStep { bash("Prepare analyzed outputs", script).with_condition(Condition::Always) } +fn prepare_analyzed_outputs_passthrough_step() -> BashStep { + let script = "set -eo pipefail\n\ + mkdir -p \"$(Agent.TempDirectory)/analyzed_outputs\"\n\ + cp -a \"$(Pipeline.Workspace)/agent_outputs_$(Build.BuildId)/.\" \ + \"$(Agent.TempDirectory)/analyzed_outputs/\"\n\ + echo \"AI threat detection is disabled; copied Agent outputs unchanged.\"\n"; + bash("Prepare analyzed outputs (detection disabled)", script) +} + +fn threat_analysis_disabled_step() -> BashStep { + let script = "echo \"AI threat detection was explicitly disabled by workflow configuration.\"\n\ + echo \"##vso[task.setvariable variable=SafeToProcess;isOutput=true]true\"\n\ + echo \"SafeToProcess set to: true\"\n"; + bash("Bypass AI threat analysis", script) + .with_id( + StepId::new("threatAnalysis") + .expect("threatAnalysis is a valid StepId — see StepId::new contract"), + ) + .with_output(OutputDecl::new("SafeToProcess")) +} + fn evaluate_threat_analysis_step() -> BashStep { let script = "SAFE_TO_PROCESS=\"false\"\n\ JSON_FILE=\"$(Agent.TempDirectory)/analyzed_outputs/threat-analysis.json\"\n\ @@ -3954,6 +4028,8 @@ mod tests { fn build_jobs(source: &str) -> Vec { let fm = parse_and_resolve(source); + let threat_detection = fm.threat_detection_config().unwrap(); + let detection_engine_config = fm.effective_detection_engine(&threat_detection); let ctx = super::super::extensions::CompileContext::for_test(&fm); let extensions = super::super::extensions::collect_extensions(&fm); let decls: Vec<_> = extensions @@ -3995,11 +4071,15 @@ mod tests { ), compiler_version: "0.0.0-test".to_string(), engine_install_steps_yaml: String::new(), + detection_engine_install_steps_yaml: String::new(), engine_run: String::new(), engine_run_detection: String::new(), + detection_engine_config, + threat_detection, engine_env: "env:\n GITHUB_TOKEN: $(GITHUB_TOKEN)\n".to_string(), engine_log_dir: "/tmp/logs".to_string(), allowed_domains: String::new(), + detection_allowed_domains: String::new(), awf_mounts: "\\".to_string(), awf_path_step_yaml: String::new(), mcpg_config_json: "{}".to_string(), @@ -4014,7 +4094,8 @@ mod tests { agent_content_value: String::new(), debug_pipeline: false, byom_exclude_keys: vec![], - detection_provider_env: vec![], + detection_byom_exclude_keys: vec![], + detection_engine_env: vec![], }; build_canonical_jobs( &fm, @@ -4032,6 +4113,118 @@ mod tests { jobs.iter().find(|j| j.id.as_ref() == id).map(|j| &j.pool) } + #[test] + fn threat_detection_enabled_and_disabled_match_expected_ir_graph() { + use std::collections::{BTreeMap, BTreeSet}; + + use super::super::ir::{ + Pipeline, PipelineBody, PipelineShape, Resources, Triggers, + graph::build_graph, + }; + + let common = concat!( + "name: test\n", + "description: test\n", + "safe-outputs:\n", + " require-approval: true\n", + " create-pull-request: {}\n", + " add-pr-comment:\n", + " require-approval: false\n", + ); + let enabled = format!("---\n{common} threat-detection: true\n---\nbody\n"); + let disabled = format!( + "---\n{common} threat-detection:\n enabled: false\n steps:\n \ + - bash: echo should-not-run\n displayName: SHOULD_NOT_RUN_PRE\n \ + post-steps:\n - bash: echo should-not-run\n displayName: \ + SHOULD_NOT_RUN_POST\n---\nbody\n" + ); + + let enabled_jobs = build_jobs(&enabled); + let disabled_jobs = build_jobs(&disabled); + + let graph_for = |jobs| { + let pipeline = Pipeline { + name: "test".to_string(), + parameters: vec![], + resources: Resources::default(), + triggers: Triggers::default(), + variables: vec![], + body: PipelineBody::Jobs(jobs), + shape: PipelineShape::Standalone, + }; + build_graph(&pipeline).unwrap() + }; + let enabled_graph = graph_for(enabled_jobs.clone()); + let disabled_graph = graph_for(disabled_jobs.clone()); + + let job = |id: &str| JobId::new(id).unwrap(); + let expected_edges = BTreeSet::from([ + (job("Conclusion"), job("Agent")), + (job("Conclusion"), job("Detection")), + (job("Conclusion"), job("SafeOutputs")), + (job("Conclusion"), job("SafeOutputs_Reviewed")), + (job("Detection"), job("Agent")), + (job("ManualReview"), job("Agent")), + (job("ManualReview"), job("Detection")), + (job("SafeOutputs"), job("Agent")), + (job("SafeOutputs"), job("Detection")), + (job("SafeOutputs_Reviewed"), job("Agent")), + (job("SafeOutputs_Reviewed"), job("Detection")), + (job("SafeOutputs_Reviewed"), job("ManualReview")), + ]); + let expected_outputs = BTreeMap::from([ + ( + StepId::new("reviewedProposals").unwrap(), + BTreeSet::from(["HasReviewedProposals".to_string()]), + ), + ( + StepId::new("threatAnalysis").unwrap(), + BTreeSet::from(["SafeToProcess".to_string()]), + ), + ]); + + for graph in [&enabled_graph, &disabled_graph] { + assert_eq!(graph.job_edges, expected_edges); + assert!(graph.stage_edges.is_empty()); + assert_eq!(graph.outputs_needing_is_output, expected_outputs); + assert_eq!(graph.step_locations.len(), 2); + for (step, outputs) in &expected_outputs { + let location = graph.step_locations.get(step).unwrap(); + assert_eq!(location.job, job("Detection")); + assert_eq!(&location.outputs, outputs); + } + } + + let disabled_detection = disabled_jobs + .iter() + .find(|job| job.id.as_ref() == "Detection") + .unwrap(); + assert!(disabled_detection.steps.iter().any(|step| { + matches!(step, Step::Bash(step) if step.display_name == "Bypass AI threat analysis") + })); + assert!(!disabled_detection.steps.iter().any(|step| { + matches!(step, Step::RawYaml(raw) if raw.contains("SHOULD_NOT_RUN")) + })); + + let enabled_detection = enabled_jobs + .iter() + .find(|job| job.id.as_ref() == "Detection") + .unwrap(); + let reviewed_index = enabled_detection + .steps + .iter() + .position(|step| step.id().is_some_and(|id| id.as_ref() == "reviewedProposals")) + .unwrap(); + let copy_logs_index = enabled_detection + .steps + .iter() + .position(|step| { + matches!(step, Step::Bash(step) if step.display_name == "Copy logs to output directory") + }) + .unwrap(); + assert!(reviewed_index < copy_logs_index); + } + #[test] fn pool_overrides_detection_only_flows_to_compiled_job() { let source = concat!( diff --git a/src/compile/common.rs b/src/compile/common.rs index 74920512..65846e14 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -2678,6 +2678,23 @@ pub fn generate_allowed_domains( front_matter: &FrontMatter, extensions: &[super::extensions::Extension], extension_declarations: &[Declarations], +) -> Result { + generate_allowed_domains_for_engine( + front_matter, + extensions, + extension_declarations, + &front_matter.engine, + ) +} + +/// Generate the AWF allowlist using an explicit engine configuration. +/// +/// Detection uses this when its engine overlay changes API/provider hosts. +pub fn generate_allowed_domains_for_engine( + front_matter: &FrontMatter, + extensions: &[super::extensions::Extension], + extension_declarations: &[Declarations], + engine_config: &crate::compile::types::EngineConfig, ) -> Result { // Collect enabled MCP names (user-defined MCPs, not first-party tools) let enabled_mcps: Vec = front_matter @@ -2728,13 +2745,13 @@ pub fn generate_allowed_domains( // Add engine-required hosts (e.g., GHES/GHEC api-target hostname). // The engine resolves its config and returns additional hosts that AWF must allow. - let engine = crate::engine::get_engine(front_matter.engine.engine_id())?; - for host in engine.required_hosts(&front_matter.engine) { + let engine = crate::engine::get_engine(engine_config.engine_id())?; + for host in engine.required_hosts(engine_config) { hosts.insert(host); } // Surface non-fatal engine network warnings (e.g. a literal but malformed // COPILOT_PROVIDER_BASE_URL whose host could not be resolved and added). - for warning in engine.network_host_warnings(&front_matter.engine) { + for warning in engine.network_host_warnings(engine_config) { eprintln!("warning: {warning}"); } diff --git a/src/compile/extensions/ado_aw_marker.rs b/src/compile/extensions/ado_aw_marker.rs index 55b8fecc..7127b550 100644 --- a/src/compile/extensions/ado_aw_marker.rs +++ b/src/compile/extensions/ado_aw_marker.rs @@ -54,7 +54,7 @@ impl CompilerExtension for AdoAwMarkerExtension { /// Returns the two Agent-job prepare steps as typed /// `Step::Bash(BashStep)` values. fn declarations(&self, ctx: &CompileContext) -> anyhow::Result { - let Some(metadata) = CompileMetadata::from_ctx(ctx) else { + let Some(metadata) = CompileMetadata::from_ctx(ctx)? else { return Ok(Declarations::default()); }; let agent_prepare_steps = vec![ @@ -116,12 +116,41 @@ struct CompileMetadata { engine: String, model: String, agent_name: String, + threat_detection_enabled: Option, + detection_engine: Option, + detection_model: Option, } impl CompileMetadata { - fn from_ctx(ctx: &CompileContext) -> Option { - let input_path = ctx.input_path?; - Some(Self { + fn from_ctx(ctx: &CompileContext) -> anyhow::Result> { + let Some(input_path) = ctx.input_path else { + return Ok(None); + }; + let explicit_threat_detection = ctx + .front_matter + .safe_outputs + .contains_key(crate::compile::types::THREAT_DETECTION_KEY); + let (threat_detection_enabled, detection_engine, detection_model) = + if explicit_threat_detection { + let config = ctx.front_matter.threat_detection_config()?; + let (engine, model) = if config.engine.is_some() { + let effective = ctx.front_matter.effective_detection_engine(&config); + let engine = crate::engine::get_engine(effective.engine_id())?; + let model = match engine { + crate::engine::Engine::Copilot => effective + .model() + .unwrap_or(crate::engine::DEFAULT_COPILOT_MODEL) + .to_string(), + }; + (Some(effective.engine_id().to_string()), Some(model)) + } else { + (None, None) + }; + (Some(config.is_enabled()), engine, model) + } else { + (None, None, None) + }; + Ok(Some(Self { source: super::super::common::normalize_source_path(input_path), org: ctx .ado_org() @@ -144,7 +173,10 @@ impl CompileMetadata { .to_string(), }, agent_name: ctx.agent_name.to_string(), - }) + threat_detection_enabled, + detection_engine, + detection_model, + })) } fn marker_json(&self) -> String { @@ -160,7 +192,7 @@ impl CompileMetadata { } fn aw_info_json(&self) -> String { - serde_json::to_string(&serde_json::json!({ + let mut value = serde_json::json!({ "schema": "ado-aw/aw_info/1", "source": &self.source, "org": &self.org, @@ -174,8 +206,29 @@ impl CompileMetadata { "source_version": "$(Build.SourceVersion)", "source_branch": "$(Build.SourceBranch)", "build_definition_id": "$(System.DefinitionId)", - })) - .unwrap() + }); + let object = value + .as_object_mut() + .expect("aw_info metadata is always a JSON object"); + if let Some(enabled) = self.threat_detection_enabled { + object.insert( + "threat_detection_enabled".to_string(), + serde_json::Value::Bool(enabled), + ); + } + if let Some(engine) = &self.detection_engine { + object.insert( + "detection_engine".to_string(), + serde_json::Value::String(engine.clone()), + ); + } + if let Some(model) = &self.detection_model { + object.insert( + "detection_model".to_string(), + serde_json::Value::String(model.clone()), + ); + } + serde_json::to_string(&value).unwrap() } } @@ -352,6 +405,70 @@ mod tests { "step missing build_definition_id macro:\n{}", step.script ); + assert!(!step.script.contains("detection_model")); + assert!(!step.script.contains("threat_detection_enabled")); + } + + #[test] + fn explicit_threat_detection_emits_detector_metadata() { + let fm = parse_fm( + "name: t\ndescription: x\nengine:\n id: copilot\n model: agent-model\n\ + safe-outputs:\n threat-detection:\n enabled: false\n engine:\n \ + model: detector-model\n", + ); + let input_path = Path::new("agents/foo.md"); + let ctx = CompileContext { + agent_name: &fm.name, + front_matter: &fm, + ado_context: None, + engine: crate::engine::Engine::Copilot, + compile_dir: None, + input_path: Some(input_path), + }; + let steps = agent_prepare_steps(&ctx); + let step = bash_step(&steps[1]); + assert!( + step.script + .contains("\"threat_detection_enabled\":false"), + "{}", + step.script + ); + assert!( + step.script.contains("\"detection_engine\":\"copilot\""), + "{}", + step.script + ); + assert!( + step.script + .contains("\"detection_model\":\"detector-model\""), + "{}", + step.script + ); + } + + #[test] + fn explicit_default_threat_detection_emits_enabled_state_only() { + let fm = parse_fm( + "name: t\ndescription: x\nsafe-outputs:\n threat-detection: true\n", + ); + let input_path = Path::new("agents/foo.md"); + let ctx = CompileContext { + agent_name: &fm.name, + front_matter: &fm, + ado_context: None, + engine: crate::engine::Engine::Copilot, + compile_dir: None, + input_path: Some(input_path), + }; + let steps = agent_prepare_steps(&ctx); + let step = bash_step(&steps[1]); + assert!( + step.script.contains("\"threat_detection_enabled\":true"), + "{}", + step.script + ); + assert!(!step.script.contains("\"detection_engine\"")); + assert!(!step.script.contains("\"detection_model\"")); } #[test] diff --git a/src/compile/ir/tasks/parse.rs b/src/compile/ir/tasks/parse.rs index 0144f6dc..ae2bdba4 100644 --- a/src/compile/ir/tasks/parse.rs +++ b/src/compile/ir/tasks/parse.rs @@ -156,8 +156,7 @@ pub fn validate_task_step(step: &Value) -> Option> { /// synthesised the step attribute and fix the finding. #[derive(Debug, Clone, PartialEq, Eq)] pub struct TaskStepFinding { - /// Which front-matter step list the step came from: `"setup"`, `"steps"`, - /// `"post-steps"`, or `"teardown"`. + /// Which front-matter step list the step came from. pub list: &'static str, /// Zero-based index of the step within that list. pub index: usize, @@ -180,13 +179,20 @@ pub fn validate_front_matter_task_steps( post_steps: &[Value], teardown: &[Value], ) -> Vec { - let mut findings = Vec::new(); - for (list, values) in [ + validate_named_task_step_lists(&[ ("setup", setup), ("steps", steps), ("post-steps", post_steps), ("teardown", teardown), - ] { + ]) +} + +/// Validate arbitrary named front-matter step lists. +pub fn validate_named_task_step_lists( + lists: &[(&'static str, &[Value])], +) -> Vec { + let mut findings = Vec::new(); + for &(list, values) in lists { for (index, step) in values.iter().enumerate() { if let Some(Err(message)) = validate_task_step(step) { // A `Some(_)` result guarantees `validate_task_step` already diff --git a/src/compile/path_layout_check.rs b/src/compile/path_layout_check.rs index 90ddf96e..5deedc7c 100644 --- a/src/compile/path_layout_check.rs +++ b/src/compile/path_layout_check.rs @@ -69,6 +69,7 @@ pub(crate) fn collect_path_layout_warnings(front_matter: &FrontMatter, markdown_ // in a non-step field (e.g. `description:`) is harmless and intentionally // not flagged here. let mut step_scalars: Vec<&str> = Vec::new(); + let threat_detection = front_matter.threat_detection_config().ok(); for block in [ &front_matter.setup, &front_matter.steps, @@ -79,6 +80,16 @@ pub(crate) fn collect_path_layout_warnings(front_matter: &FrontMatter, markdown_ collect_string_scalars(value, &mut step_scalars); } } + // Invalid threat-detection config is reported by the normal compile + // validation immediately after this advisory pass; only valid nested step + // lists participate in path warnings. + if let Some(threat_detection) = &threat_detection { + for block in [&threat_detection.steps, &threat_detection.post_steps] { + for value in block { + collect_string_scalars(value, &mut step_scalars); + } + } + } for scalar in &step_scalars { for seg in sources_dir_segments(scalar) { if declared_not_checked_out.contains(&seg.as_str()) { @@ -254,6 +265,19 @@ mod tests { assert!(w[0].contains("other"), "{w:?}"); } + #[test] + fn warns_on_nested_threat_detection_step_path() { + let fm = fm( + "name: a\ndescription: d\nrepos:\n - name: org/other\n checkout: false\n\ + safe-outputs:\n threat-detection:\n post-steps:\n - script: cat \ + $(Build.SourcesDirectory)/other/file\n", + ); + let w = collect_path_layout_warnings(&fm, "body"); + assert_eq!(w.len(), 1, "{w:?}"); + assert!(w[0].contains("`checkout: false`"), "{w:?}"); + assert!(w[0].contains("other"), "{w:?}"); + } + #[test] fn no_warning_for_checked_out_alias_reference() { let fm = fm( diff --git a/src/compile/types.rs b/src/compile/types.rs index 1b38b280..f2a1c7b8 100644 --- a/src/compile/types.rs +++ b/src/compile/types.rs @@ -347,7 +347,7 @@ impl SanitizeConfigTrait for EngineConfig { } } -#[derive(Debug, Deserialize, Clone, SanitizeConfig)] +#[derive(Debug, Deserialize, Clone, Default, SanitizeConfig)] pub struct EngineOptions { /// Engine identifier (e.g., "copilot"). Defaults to "copilot" when omitted. #[serde(default)] @@ -401,6 +401,174 @@ pub struct EngineOptions { pub provider: Option, } +/// Detection-specific engine configuration layered over the top-level +/// [`EngineConfig`]. +/// +/// Collection fields are optional so the compiler can distinguish omission +/// (inherit the top-level value) from an explicitly empty list/map. `args: []` +/// therefore clears inherited CLI arguments, while `env` merges by key with +/// Detection values taking precedence. +#[derive(Debug, Deserialize, Clone, Default)] +#[serde(deny_unknown_fields)] +pub struct DetectionEngineOptions { + #[serde(default)] + pub id: Option, + #[serde(default)] + pub model: Option, + #[serde(default)] + pub version: Option, + #[serde(default)] + pub agent: Option, + #[serde(default, rename = "api-target")] + pub api_target: Option, + #[serde(default)] + pub args: Option>, + #[serde(default)] + pub env: Option>, + #[serde(default)] + pub command: Option, + #[serde(default, rename = "timeout-minutes")] + pub timeout_minutes: Option, + #[serde(default, rename = "github-app-token")] + pub github_app_token: Option, + #[serde(default)] + pub provider: Option, +} + +/// String or object form accepted by +/// `safe-outputs.threat-detection.engine`. +#[derive(Debug, Deserialize, Clone)] +#[serde(untagged)] +pub enum DetectionEngineConfig { + Simple(String), + Full(Box), +} + +impl DetectionEngineConfig { + /// Apply this override to the top-level engine configuration. + pub fn resolve(&self, base: &EngineConfig) -> EngineConfig { + let base_id = base.engine_id().to_string(); + let mut options = match base { + EngineConfig::Simple(id) => EngineOptions { + id: Some(id.clone()), + ..EngineOptions::default() + }, + EngineConfig::Full(options) => (**options).clone(), + }; + + match self { + DetectionEngineConfig::Simple(id) => { + options.id = Some(id.clone()); + } + DetectionEngineConfig::Full(overlay) => { + if let Some(value) = &overlay.id { + options.id = Some(value.clone()); + } + if let Some(value) = &overlay.model { + options.model = Some(value.clone()); + } + if let Some(value) = &overlay.version { + options.version = Some(value.clone()); + } + if let Some(value) = &overlay.agent { + options.agent = Some(value.clone()); + } + if let Some(value) = &overlay.api_target { + options.api_target = Some(value.clone()); + } + if let Some(value) = &overlay.args { + options.args = value.clone(); + } + if overlay.provider.is_some() { + // A typed Detection provider replaces legacy top-level + // COPILOT_PROVIDER_* env configuration. Remove inherited + // keys before merging the nested env so conflicts authored + // inside the overlay still reach validation and fail. + if let Some(env) = &mut options.env { + env.retain(|key, _| !key.starts_with("COPILOT_PROVIDER_")); + } + } + if let Some(value) = &overlay.env { + let mut merged = options.env.take().unwrap_or_default(); + merged.extend(value.clone()); + options.env = Some(merged); + } + if let Some(value) = &overlay.command { + options.command = Some(value.clone()); + } + if let Some(value) = overlay.timeout_minutes { + options.timeout_minutes = Some(value); + } + if let Some(value) = &overlay.github_app_token { + options.github_app_token = Some(value.clone()); + } + if let Some(value) = &overlay.provider { + options.provider = Some(value.clone()); + } + } + } + + let effective_id = options.id.as_deref().unwrap_or("copilot"); + if base_id == "copilot" && effective_id != base_id { + if let Some(env) = &mut options.env { + env.retain(|key, _| !key.starts_with("COPILOT_PROVIDER_")); + } + options.provider = None; + options.github_app_token = None; + } + + EngineConfig::Full(Box::new(options)) + } +} + +/// Object form of `safe-outputs.threat-detection`. +#[derive(Debug, Deserialize, Clone, Default)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +pub struct ThreatDetectionConfig { + /// Whether AI threat analysis runs. Defaults to true. + #[serde(default)] + pub enabled: Option, + /// Additional operator instructions appended to the fixed detector prompt. + #[serde(default)] + pub prompt: Option, + /// Detection-specific engine settings layered over top-level `engine:`. + #[serde(default)] + pub engine: Option, + /// Trusted raw ADO steps run before AI analysis. + #[serde(default)] + pub steps: Vec, + /// Trusted raw ADO steps run after AI analysis. + #[serde(default)] + pub post_steps: Vec, +} + +impl ThreatDetectionConfig { + pub fn is_enabled(&self) -> bool { + self.enabled.unwrap_or(true) + } +} + +/// Boolean shorthand or object form accepted by +/// `safe-outputs.threat-detection`. +#[derive(Debug, Deserialize, Clone)] +#[serde(untagged)] +pub enum ThreatDetectionSetting { + Bool(bool), + Config(ThreatDetectionConfig), +} + +impl ThreatDetectionSetting { + fn into_config(self) -> ThreatDetectionConfig { + match self { + ThreatDetectionSetting::Bool(enabled) => ThreatDetectionConfig { + enabled: Some(enabled), + ..ThreatDetectionConfig::default() + }, + ThreatDetectionSetting::Config(config) => config, + } + } +} + /// GitHub App-backed Copilot engine authentication configuration. /// /// Mirrors gh-aw's `create-github-app-token` model, adapted to Azure DevOps. @@ -1194,7 +1362,8 @@ impl FrontMatter { /// itself rather than naming a safe-output tool. These must never be treated /// as tool names (e.g. in `--enabled-tools`, Stage-3 budgets, or unknown-key /// validation). -pub const SAFE_OUTPUT_RESERVED_KEYS: &[&str] = &["require-approval"]; +pub const THREAT_DETECTION_KEY: &str = "threat-detection"; +pub const SAFE_OUTPUT_RESERVED_KEYS: &[&str] = &["require-approval", THREAT_DETECTION_KEY]; /// Automatic action a manual-validation gate takes when its pending period /// elapses with no human response. Mirrors `ManualValidation@1`'s `onTimeout`. @@ -1286,6 +1455,87 @@ impl FrontMatter { self.safe_output_tool_names().next().is_some() } + /// Parse the section-level threat-detection configuration. + /// + /// An absent or null key preserves the current enabled-by-default behavior. + /// The returned prompt is sanitized for pipeline-command/control-character + /// safety before it is written to and echoed from the Detection job. + pub fn threat_detection_config(&self) -> anyhow::Result { + let Some(value) = self.safe_outputs.get(THREAT_DETECTION_KEY) else { + return Ok(ThreatDetectionConfig::default()); + }; + if value.is_null() { + return Ok(ThreatDetectionConfig::default()); + } + + let setting = + serde_json::from_value::(value.clone()).map_err(|e| { + anyhow::anyhow!( + "safe-outputs.threat-detection has invalid configuration: {e}\n\n\ + Use a boolean or an object with the keys: enabled, prompt, engine, \ + steps, post-steps. See docs/safe-outputs.md." + ) + })?; + let mut config = setting.into_config(); + if let Some(prompt) = &mut config.prompt { + let sanitized = crate::sanitize::sanitize_config(prompt); + if crate::validate::contains_ado_expression(&sanitized) { + anyhow::bail!( + "safe-outputs.threat-detection.prompt contains an ADO expression \ + ('${{{{', '$(', or '$['), which is not allowed. Detection prompt \ + instructions must be literal text." + ); + } + *prompt = sanitized; + } + Ok(config) + } + + /// Resolve the effective Detection engine without mutating the Agent engine + /// configuration. + pub fn effective_detection_engine( + &self, + config: &ThreatDetectionConfig, + ) -> EngineConfig { + let mut effective = config + .engine + .as_ref() + .map(|overlay| overlay.resolve(&self.engine)) + .unwrap_or_else(|| self.engine.clone()); + effective.sanitize_config_fields(); + effective + } + + /// Validate an already-parsed threat-detection config and resolved engine. + pub fn validate_threat_detection_config( + &self, + config: &ThreatDetectionConfig, + effective: &EngineConfig, + ) -> anyhow::Result<()> { + crate::engine::validate_engine_feature_support(effective)?; + crate::engine::get_engine(effective.engine_id())?; + if config.engine.is_some() { + for arg in effective.args() { + if arg == "--model" + || arg.starts_with("--model=") + || arg == "--api-target" + || arg.starts_with("--api-target=") + { + anyhow::bail!( + "safe-outputs.threat-detection effective engine args contain '{arg}', \ + which would desynchronize Detection metadata or firewall hosts. Use \ + threat-detection.engine.model / api-target instead; set nested \ + `args: []` first if the flag is inherited from top-level engine.args." + ); + } + } + } + if let Some(app) = effective.github_app_token() { + app.validate()?; + } + Ok(()) + } + /// The parsed, sanitized `create-pull-request` config, or `None` when the /// tool is not configured. Mirrors Stage 3's `ExecutionContext::get_tool_config` /// (deserialize + `sanitize_config_fields`) so the compiler resolves per-repo @@ -2776,6 +3026,12 @@ impl SanitizeConfigTrait for LabelFilter { mod tests { use super::*; + fn validate_threat_detection(fm: &FrontMatter) -> anyhow::Result<()> { + let config = fm.threat_detection_config()?; + let effective = fm.effective_detection_engine(&config); + fm.validate_threat_detection_config(&config, &effective) + } + // ─── SupplyChainConfig deserialization + resolution ────────────────────── fn parse_supply_chain(yaml: &str) -> SupplyChainConfig { @@ -3104,6 +3360,234 @@ timeout-minutes: 60 assert_eq!(env.get("AWS_REGION").unwrap(), "us-west-2"); } + // ─── ThreatDetectionConfig ────────────────────────────────────────────── + + #[test] + fn threat_detection_absent_defaults_enabled() { + let (fm, _) = + super::super::common::parse_markdown("---\nname: test\ndescription: test\n---\n") + .unwrap(); + let config = fm.threat_detection_config().unwrap(); + assert!(config.is_enabled()); + assert!(config.prompt.is_none()); + assert!(config.engine.is_none()); + assert!(config.steps.is_empty()); + assert!(config.post_steps.is_empty()); + } + + #[test] + fn threat_detection_boolean_shorthand_and_reserved_key() { + let source = "---\nname: test\ndescription: test\nsafe-outputs:\n \ + threat-detection: false\n noop: {}\n---\n"; + let (fm, _) = super::super::common::parse_markdown(source).unwrap(); + let config = fm.threat_detection_config().unwrap(); + assert!(!config.is_enabled()); + assert_eq!( + fm.safe_output_tool_names().cloned().collect::>(), + vec!["noop".to_string()] + ); + } + + #[test] + fn threat_detection_object_appends_sanitized_prompt_and_steps() { + let source = r#"--- +name: test +description: test +safe-outputs: + threat-detection: + prompt: "Inspect this ##vso[task.setvariable variable=x]payload" + steps: + - bash: echo pre + post-steps: + - bash: echo post +--- +"#; + let (fm, _) = super::super::common::parse_markdown(source).unwrap(); + let config = fm.threat_detection_config().unwrap(); + assert!(config.is_enabled()); + assert_eq!(config.steps.len(), 1); + assert_eq!(config.post_steps.len(), 1); + let prompt = config.prompt.unwrap(); + assert!(!prompt.contains("##vso[task.setvariable")); + assert!(prompt.contains("`##vso[`")); + } + + #[test] + fn threat_detection_engine_overlay_merges_env_and_replaces_args() { + let source = r#"--- +name: test +description: test +engine: + id: copilot + model: base-model + version: "1.0.0" + args: [--base] + env: + KEEP: base + SHARED: base +safe-outputs: + threat-detection: + engine: + model: detection-model + args: [] + env: + SHARED: detection + EXTRA: value +--- +"#; + let (fm, _) = super::super::common::parse_markdown(source).unwrap(); + let config = fm.threat_detection_config().unwrap(); + let effective = fm.effective_detection_engine(&config); + assert_eq!(effective.engine_id(), "copilot"); + assert_eq!(effective.model(), Some("detection-model")); + assert_eq!(effective.version(), Some("1.0.0")); + assert!(effective.args().is_empty()); + let env = effective.env().unwrap(); + assert_eq!(env.get("KEEP").map(String::as_str), Some("base")); + assert_eq!(env.get("SHARED").map(String::as_str), Some("detection")); + assert_eq!(env.get("EXTRA").map(String::as_str), Some("value")); + } + + #[test] + fn threat_detection_typed_provider_replaces_inherited_legacy_provider_env() { + let source = r#"--- +name: test +description: test +engine: + id: copilot + env: + KEEP: value + COPILOT_PROVIDER_BASE_URL: https://legacy.example.com/v1 + COPILOT_PROVIDER_API_KEY: $(LEGACY_KEY) +safe-outputs: + threat-detection: + engine: + provider: + base-url: https://detector.example.com/v1 + api-key: $(DETECTOR_KEY) +--- +"#; + let (fm, _) = super::super::common::parse_markdown(source).unwrap(); + let config = fm.threat_detection_config().unwrap(); + let effective = fm.effective_detection_engine(&config); + let env = effective.env().unwrap(); + assert_eq!(env.get("KEEP").map(String::as_str), Some("value")); + assert!(!env.keys().any(|key| key.starts_with("COPILOT_PROVIDER_"))); + validate_threat_detection(&fm).unwrap(); + + let conflict = r#"--- +name: test +description: test +safe-outputs: + threat-detection: + engine: + env: + COPILOT_PROVIDER_BASE_URL: https://conflict.example.com/v1 + provider: + base-url: https://detector.example.com/v1 + api-key: $(DETECTOR_KEY) +--- +"#; + let (fm, _) = super::super::common::parse_markdown(conflict).unwrap(); + assert!(validate_threat_detection(&fm).is_err()); + } + + #[test] + fn threat_detection_engine_switch_drops_inherited_copilot_credentials() { + let source = r#"--- +name: test +description: test +engine: + id: copilot + env: + KEEP: value + COPILOT_PROVIDER_BASE_URL: https://legacy.example.com/v1 + COPILOT_PROVIDER_API_KEY: $(LEGACY_KEY) +safe-outputs: + threat-detection: + engine: claude +--- +"#; + let (fm, _) = super::super::common::parse_markdown(source).unwrap(); + let config = fm.threat_detection_config().unwrap(); + let effective = fm.effective_detection_engine(&config); + let env = effective.env().unwrap(); + assert_eq!(env.get("KEEP").map(String::as_str), Some("value")); + assert!(!env.keys().any(|key| key.starts_with("COPILOT_PROVIDER_"))); + assert!(effective.provider().is_none()); + assert!(effective.github_app_token().is_none()); + } + + #[test] + fn threat_detection_engine_overlay_rejects_model_and_api_target_args() { + for arg in ["--model=other", "--api-target=other.example.com"] { + let source = format!( + "---\nname: test\ndescription: test\nengine:\n id: copilot\n \ + args:\n - {arg}\nsafe-outputs:\n threat-detection:\n \ + engine:\n model: detector\n---\n" + ); + let (fm, _) = super::super::common::parse_markdown(&source).unwrap(); + let error = validate_threat_detection(&fm).unwrap_err().to_string(); + assert!(error.contains("desynchronize"), "{error}"); + } + + let source = "---\nname: test\ndescription: test\nengine:\n id: copilot\n \ + args:\n - --model=other\nsafe-outputs:\n \ + threat-detection: true\n---\n"; + let (fm, _) = super::super::common::parse_markdown(source).unwrap(); + validate_threat_detection(&fm).unwrap(); + + let source = "---\nname: test\ndescription: test\nengine:\n id: copilot\n \ + args:\n - --model=other\nsafe-outputs:\n threat-detection:\n \ + engine:\n model: detector\n args: []\n---\n"; + let (fm, _) = super::super::common::parse_markdown(source).unwrap(); + validate_threat_detection(&fm).unwrap(); + } + + #[test] + fn threat_detection_rejects_unknown_fields_and_engines() { + let unknown = "---\nname: test\ndescription: test\nsafe-outputs:\n \ + threat-detection:\n promtp: typo\n---\n"; + let (fm, _) = super::super::common::parse_markdown(unknown).unwrap(); + let err = fm.threat_detection_config().unwrap_err().to_string(); + assert!(err.contains("promtp") || err.contains("invalid configuration")); + + let unsupported = "---\nname: test\ndescription: test\nsafe-outputs:\n \ + threat-detection:\n engine: claude\n---\n"; + let (fm, _) = super::super::common::parse_markdown(unsupported).unwrap(); + let err = validate_threat_detection(&fm).unwrap_err().to_string(); + assert!(err.contains("Unsupported engine 'claude'"), "{err}"); + } + + #[test] + fn threat_detection_rejects_ado_expressions_in_prompt() { + for expression in [ + "$(MALICIOUS_MULTILINE_VARIABLE)", + "${{ variables.secret }}", + "$[variables.runtime]", + ] { + let source = format!( + "---\nname: test\ndescription: test\nsafe-outputs:\n \ + threat-detection:\n prompt: \"{expression}\"\n---\n" + ); + let (fm, _) = super::super::common::parse_markdown(&source).unwrap(); + let err = fm.threat_detection_config().unwrap_err().to_string(); + assert!(err.contains("literal text"), "{err}"); + } + + let reconstructed = r#"--- +name: test +description: test +safe-outputs: + threat-detection: + prompt: "$\0(SECRET)" +--- +"#; + let (fm, _) = super::super::common::parse_markdown(reconstructed).unwrap(); + let err = fm.threat_detection_config().unwrap_err().to_string(); + assert!(err.contains("literal text"), "{err}"); + } + // ─── GithubAppTokenConfig ──────────────────────────────────────────── #[test] diff --git a/src/engine.rs b/src/engine.rs index 02180e5b..83481018 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -300,9 +300,24 @@ impl Engine { &self, front_matter: &FrontMatter, extension_declarations: &[Declarations], + ) -> Result { + self.args_with_config( + &front_matter.engine, + front_matter, + extension_declarations, + ) + } + + /// Generate CLI arguments using an explicit engine configuration while + /// retaining the workflow's tool/MCP policy. + pub fn args_with_config( + &self, + engine_config: &EngineConfig, + front_matter: &FrontMatter, + extension_declarations: &[Declarations], ) -> Result { match self { - Engine::Copilot => copilot_args(front_matter, extension_declarations), + Engine::Copilot => copilot_args(engine_config, front_matter, extension_declarations), } } @@ -412,9 +427,37 @@ impl Engine { mcp_config_path: Option<&str>, ) -> Result { let args = self.args(front_matter, extension_declarations)?; + self.invocation_with_args( + &front_matter.engine, + prompt_path, + mcp_config_path, + &args, + ) + } + + /// Generate an invocation using an explicit engine configuration. + pub fn invocation_with_config( + &self, + engine_config: &EngineConfig, + front_matter: &FrontMatter, + extension_declarations: &[Declarations], + prompt_path: &str, + mcp_config_path: Option<&str>, + ) -> Result { + let args = self.args_with_config(engine_config, front_matter, extension_declarations)?; + self.invocation_with_args(engine_config, prompt_path, mcp_config_path, &args) + } + + fn invocation_with_args( + &self, + engine_config: &EngineConfig, + prompt_path: &str, + mcp_config_path: Option<&str>, + args: &str, + ) -> Result { match self { Engine::Copilot => { - let command_path = match front_matter.engine.command() { + let command_path = match engine_config.command() { Some(cmd) => { if !is_valid_command_path(cmd) { anyhow::bail!( @@ -431,7 +474,7 @@ impl Engine { &command_path, prompt_path, mcp_config_path, - &args, + args, )) } } @@ -577,6 +620,7 @@ fn validate_user_arg(arg: &str) -> Result<()> { } fn copilot_args( + engine_config: &EngineConfig, front_matter: &FrontMatter, extension_declarations: &[Declarations], ) -> Result { @@ -616,7 +660,7 @@ fn copilot_args( // Validate model name to prevent shell injection — copilot_params are embedded // inside a single-quoted bash string in the AWF command. - let model = front_matter.engine.model().unwrap_or(DEFAULT_COPILOT_MODEL); + let model = engine_config.model().unwrap_or(DEFAULT_COPILOT_MODEL); if model.is_empty() || !model .chars() @@ -629,7 +673,7 @@ fn copilot_args( ); } params.push(format!("--model {}", model)); - if let Some(0) = front_matter.engine.timeout_minutes() { + if let Some(0) = engine_config.timeout_minutes() { eprintln!( "Warning: Agent '{}' has timeout-minutes: 0, which means no time is allowed. \ The agent job will time out immediately. \ @@ -639,7 +683,7 @@ fn copilot_args( } // Wire engine.agent — selects a custom agent from .github/agents/ - if let Some(agent) = front_matter.engine.agent() { + if let Some(agent) = engine_config.agent() { if !is_valid_identifier(agent) { anyhow::bail!( "engine.agent '{}' contains invalid characters. \ @@ -651,7 +695,7 @@ fn copilot_args( } // Wire engine.api-target — sets the GHES/GHEC API endpoint hostname - if let Some(api_target) = front_matter.engine.api_target() { + if let Some(api_target) = engine_config.api_target() { if !is_valid_hostname(api_target) { anyhow::bail!( "engine.api-target '{}' contains invalid characters. \ @@ -688,7 +732,7 @@ fn copilot_args( // Wire engine.args — append user-provided CLI arguments after compiler-generated args. // User args are additive; they cannot remove compiler security flags but may override // non-security defaults via last-wins semantics (e.g., --model). - for arg in front_matter.engine.args() { + for arg in engine_config.args() { validate_user_arg(arg)?; params.push(arg.to_string()); } @@ -868,6 +912,29 @@ pub fn copilot_provider_env(engine_config: &EngineConfig) -> Result Result> { + let mut pairs = Vec::new(); + if let Some(env_map) = engine_config.env() { + for (key, value) in env_map + .iter() + .filter(|(key, _)| !key.starts_with(COPILOT_PROVIDER_PREFIX)) + { + validate_engine_env_entry(key, value)?; + pairs.push((key.clone(), value.clone())); + } + } + pairs.extend(copilot_provider_env(engine_config)?); + pairs.sort(); + Ok(pairs) +} + /// Validate a single `engine.env` entry: enforces env-var-name rules, blocks /// compiler-controlled keys, rejects pipeline-command injection / newlines, and /// applies the BYOM provider expression allowlist (allowlisted keys may carry an diff --git a/src/inspect/cli.rs b/src/inspect/cli.rs index e0ab936f..8b3ef52a 100644 --- a/src/inspect/cli.rs +++ b/src/inspect/cli.rs @@ -293,7 +293,7 @@ pub async fn build_lint(source: &Path) -> Result { let summary = PipelineSummary::from_pipeline(&pipeline)?; let mut findings = lint::lint(&summary); - findings.extend(lint::lint_front_matter_tasks(&front_matter)); + findings.extend(lint::lint_front_matter_tasks(&front_matter)?); Ok(lint::report_from_findings(findings)) } diff --git a/src/inspect/lint.rs b/src/inspect/lint.rs index d395fc8e..aa3cfe28 100644 --- a/src/inspect/lint.rs +++ b/src/inspect/lint.rs @@ -323,8 +323,8 @@ fn location_for(job: &JobSummary, step: Option<&str>) -> LintLocation { } } -/// Lint rule: authored task steps (`setup` / `steps` / `post-steps` / -/// `teardown`) whose inputs are invalid for the ADO task they invoke. +/// Lint rule: authored task steps whose inputs are invalid for the ADO task +/// they invoke, including nested threat-detection pre/post steps. /// /// This is the agent-facing surface of the typed-builder task validation (see /// `crate::compile::ir::tasks::parse`). It runs over the **front matter** rather @@ -339,18 +339,20 @@ fn location_for(job: &JobSummary, step: Option<&str>) -> LintLocation { /// in front matter, before jobs/stages exist, so `location` does **not** carry /// real graph identifiers like the structural rules do. Instead it encodes the /// authored source position: `stage` is always `None`, `job` holds the -/// front-matter step *list* name (`"setup"` / `"steps"` / `"post-steps"` / -/// `"teardown"`), and `step` holds the offending ADO task id (e.g. -/// `"CopyFiles@2"`). Consumers correlating findings with the pipeline graph -/// should branch on `code == "task-input-invalid"` and treat `job` as a list -/// name rather than a job id for this class. -pub fn lint_front_matter_tasks(front_matter: &FrontMatter) -> Vec { - crate::compile::ir::tasks::parse::validate_front_matter_task_steps( - &front_matter.setup, - &front_matter.steps, - &front_matter.post_steps, - &front_matter.teardown, - ) +/// front-matter step *list* name, and `step` holds the offending ADO task id +/// (e.g. `"CopyFiles@2"`). Consumers correlating findings with the pipeline +/// graph should branch on `code == "task-input-invalid"` and treat `job` as a +/// list name rather than a job id for this class. +pub fn lint_front_matter_tasks(front_matter: &FrontMatter) -> anyhow::Result> { + let threat_detection = front_matter.threat_detection_config()?; + Ok(crate::compile::ir::tasks::parse::validate_named_task_step_lists(&[ + ("setup", &front_matter.setup), + ("steps", &front_matter.steps), + ("post-steps", &front_matter.post_steps), + ("teardown", &front_matter.teardown), + ("threat-detection.steps", &threat_detection.steps), + ("threat-detection.post-steps", &threat_detection.post_steps), + ]) .into_iter() .map(|f| LintFinding { severity: LintSeverity::Warning, @@ -362,7 +364,7 @@ pub fn lint_front_matter_tasks(front_matter: &FrontMatter) -> Vec { step: Some(f.task), }), }) - .collect() + .collect()) } #[cfg(test)] @@ -463,7 +465,7 @@ mod tests { "---\nname: t\ndescription: d\nsteps:\n- task: CopyFiles@2\n inputs:\n Contents: \"**\"\n Bogus: nope\n---\n", ) .unwrap(); - let findings = lint_front_matter_tasks(&front_matter); + let findings = lint_front_matter_tasks(&front_matter).unwrap(); assert_eq!(findings.len(), 1, "got: {findings:?}"); let f = &findings[0]; assert_eq!(f.code, "task-input-invalid"); @@ -473,13 +475,41 @@ mod tests { assert_eq!(loc.step.as_deref(), Some("CopyFiles@2")); } + #[test] + fn invalid_threat_detection_task_step_uses_nested_location() { + let (front_matter, _) = crate::compile::parse_markdown( + "---\nname: t\ndescription: d\nsafe-outputs:\n threat-detection:\n \ + post-steps:\n - task: CopyFiles@2\n inputs:\n \ + Contents: \"**\"\n Bogus: nope\n---\n", + ) + .unwrap(); + let findings = lint_front_matter_tasks(&front_matter).unwrap(); + assert_eq!(findings.len(), 1, "got: {findings:?}"); + let loc = findings[0].location.as_ref().expect("location present"); + assert_eq!(loc.job.as_deref(), Some("threat-detection.post-steps")); + assert_eq!(loc.step.as_deref(), Some("CopyFiles@2")); + } + #[test] fn valid_front_matter_task_steps_emit_no_findings() { let (front_matter, _) = crate::compile::parse_markdown( "---\nname: t\ndescription: d\nsteps:\n- task: CopyFiles@2\n inputs:\n Contents: \"**\"\n TargetFolder: out\n- bash: echo hi\n---\n", ) .unwrap(); - assert!(lint_front_matter_tasks(&front_matter).is_empty()); + assert!(lint_front_matter_tasks(&front_matter).unwrap().is_empty()); + } + + #[test] + fn malformed_threat_detection_returns_error_instead_of_panicking() { + let (front_matter, _) = crate::compile::parse_markdown( + "---\nname: t\ndescription: d\nsafe-outputs:\n threat-detection:\n \ + promtp: typo\n---\n", + ) + .unwrap(); + let error = lint_front_matter_tasks(&front_matter) + .unwrap_err() + .to_string(); + assert!(error.contains("invalid configuration"), "{error}"); } #[test] diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index 28dfbd15..c6f2de9a 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -7151,6 +7151,306 @@ fn job_block<'a>(compiled: &'a str, job: &str) -> &'a str { } } +#[test] +fn threat_detection_custom_prompt_engine_and_steps_are_detection_scoped() { + let source = r#"--- +name: "Threat Detection Config" +description: "Exercises detector configuration" +engine: + id: copilot + model: agent-model + version: "1.0.1" + args: + - --reasoning-effort=high + env: + INHERITED_ENV: "agent" +safe-outputs: + noop: {} + threat-detection: + prompt: | + Focus on unsafe deserialization. + engine: + model: detection-model + version: "2.0.2" + args: + - --reasoning-effort=low + env: + DETECTION_ENV: "enabled" + steps: + - bash: echo pre + displayName: Detection custom pre + post-steps: + - bash: echo post + displayName: Detection custom post +--- + +## Agent +"#; + let (ok, compiled, stderr) = compile_inline_source("threat-detection-config", source); + assert!(ok, "pipeline should compile: {stderr}"); + + let agent = job_block(&compiled, "Agent"); + let detection = job_block(&compiled, "Detection"); + assert!(agent.contains("--model agent-model"), "{agent}"); + assert!(agent.contains("--reasoning-effort=high"), "{agent}"); + assert!(!agent.contains("--model detection-model"), "{agent}"); + assert!(!agent.contains("DETECTION_ENV"), "{agent}"); + + assert!(detection.contains("--model detection-model"), "{detection}"); + assert!(detection.contains("--reasoning-effort=low"), "{detection}"); + assert!(!detection.contains("--reasoning-effort=high"), "{detection}"); + assert!(detection.contains("INHERITED_ENV: agent"), "{detection}"); + assert!(detection.contains("DETECTION_ENV: enabled"), "{detection}"); + assert!( + detection.contains("## Additional Instructions") + && detection.contains("Focus on unsafe deserialization.") + && detection.contains("THREAT_DETECTION_RESULT:"), + "{detection}" + ); + + let pre = detection.find("Detection custom pre").unwrap(); + let run = detection + .find("Run threat analysis (AWF network isolated)") + .unwrap(); + let post = detection.find("Detection custom post").unwrap(); + assert!(pre < run && run < post, "{detection}"); + assert!(detection.contains("2.0.2"), "{detection}"); +} + +#[test] +fn threat_detection_disabled_preserves_outputs_artifacts_and_manual_review() { + let source = r#"--- +name: "Threat Detection Disabled" +description: "Pass-through Detection with approval" +safe-outputs: + require-approval: true + create-pull-request: {} + threat-detection: false +--- + +## Agent +"#; + let (ok, compiled, stderr) = compile_inline_source("threat-detection-disabled", source); + assert!(ok, "pipeline should compile: {stderr}"); + + let detection = job_block(&compiled, "Detection"); + assert!( + detection.contains("AI threat detection was explicitly disabled"), + "{detection}" + ); + assert!(!detection.contains("Run threat analysis (AWF network isolated)")); + assert!(!detection.contains("Prepare threat analysis prompt")); + assert!(detection.contains("name: threatAnalysis"), "{detection}"); + assert!( + detection.contains( + "##vso[task.setvariable variable=SafeToProcess;isOutput=true]true" + ), + "{detection}" + ); + assert!(detection.contains("name: reviewedProposals"), "{detection}"); + assert!( + detection.contains("artifact: analyzed_outputs_$(Build.BuildId)"), + "{detection}" + ); + + let review = job_block(&compiled, "ManualReview"); + assert!( + review.contains("dependencies.Detection.outputs['threatAnalysis.SafeToProcess']") + && review.contains( + "dependencies.Detection.outputs['reviewedProposals.HasReviewedProposals']" + ), + "{review}" + ); + let safe_outputs = job_block(&compiled, "SafeOutputs"); + assert!( + safe_outputs + .contains("dependencies.Detection.outputs['threatAnalysis.SafeToProcess']"), + "{safe_outputs}" + ); +} + +#[test] +fn threat_detection_disabled_skips_unused_engine_install_resolution() { + let source = r#"--- +name: "Threat Detection Disabled Invalid Version" +description: "Disabled Detection does not install its engine" +safe-outputs: + noop: {} + threat-detection: + enabled: false + engine: + version: "bad version" +--- + +## Agent +"#; + let (ok, compiled, stderr) = + compile_inline_source("threat-detection-disabled-invalid-version", source); + assert!(ok, "disabled Detection should not resolve install steps: {stderr}"); + let detection = job_block(&compiled, "Detection"); + assert!(detection.contains("Bypass AI threat analysis"), "{detection}"); + assert!(!detection.contains("bad version"), "{detection}"); +} + +#[test] +fn threat_detection_timeout_inherits_and_can_override_agent_timeout() { + let inherited = r#"--- +name: "Threat Detection Timeout" +description: "Detection inherits top-level timeout" +engine: + id: copilot + timeout-minutes: 17 + env: + INHERITED_DETECTION_ENV: "true" +safe-outputs: + noop: {} +--- + +## Agent +"#; + let (ok, compiled, stderr) = + compile_inline_source("threat-detection-timeout-inherited", inherited); + assert!(ok, "pipeline should compile: {stderr}"); + assert!( + job_block(&compiled, "Agent").contains("timeoutInMinutes: 17"), + "{}", + job_block(&compiled, "Agent") + ); + assert!( + job_block(&compiled, "Detection").contains("timeoutInMinutes: 17"), + "{}", + job_block(&compiled, "Detection") + ); + assert!( + job_block(&compiled, "Detection").contains("INHERITED_DETECTION_ENV: 'true'"), + "{}", + job_block(&compiled, "Detection") + ); + + let overridden = inherited.replace( + " noop: {}", + " noop: {}\n threat-detection:\n engine:\n timeout-minutes: 9", + ); + let (ok, compiled, stderr) = + compile_inline_source("threat-detection-timeout-overridden", &overridden); + assert!(ok, "pipeline should compile: {stderr}"); + assert!( + job_block(&compiled, "Agent").contains("timeoutInMinutes: 17"), + "{}", + job_block(&compiled, "Agent") + ); + assert!( + job_block(&compiled, "Detection").contains("timeoutInMinutes: 9"), + "{}", + job_block(&compiled, "Detection") + ); +} + +#[test] +fn threat_detection_provider_override_updates_detection_firewall_only() { + let source = r#"--- +name: "Threat Detection Provider" +description: "Detection-only provider" +engine: + id: copilot + model: agent-model +safe-outputs: + noop: {} + threat-detection: + engine: + model: detection-model + provider: + base-url: https://detector.example.com/v1 + api-key: $(DETECTION_API_KEY) +--- + +## Agent +"#; + let (ok, compiled, stderr) = compile_inline_source("threat-detection-provider", source); + assert!(ok, "pipeline should compile: {stderr}"); + + let agent = job_block(&compiled, "Agent"); + let detection = job_block(&compiled, "Detection"); + assert!(!agent.contains("detector.example.com"), "{agent}"); + assert!(detection.contains("detector.example.com"), "{detection}"); + assert!(detection.contains("COPILOT_PROVIDER_BASE_URL"), "{detection}"); + assert!(detection.contains("DETECTION_API_KEY"), "{detection}"); + assert!( + detection.contains("--exclude-env COPILOT_PROVIDER_API_KEY"), + "{detection}" + ); +} + +#[test] +fn threat_detection_config_compiles_for_every_target() { + for target in ["standalone", "1es", "job", "stage"] { + let source = format!( + r#"--- +name: "Threat Detection {target}" +description: "Cross-target threat detection" +target: {target} +safe-outputs: + noop: {{}} + threat-detection: + prompt: "Cross-target detector instruction" + steps: + - bash: echo pre + displayName: Cross-target detection pre + post-steps: + - bash: echo post + displayName: Cross-target detection post +--- + +## Agent +"# + ); + let (ok, compiled, stderr) = + compile_inline_source(&format!("threat-detection-{target}"), &source); + assert!(ok, "{target} should compile: {stderr}"); + assert!( + compiled.contains("Cross-target detector instruction") + && compiled.contains("Cross-target detection pre") + && compiled.contains("Cross-target detection post"), + "{target} output missing threat-detection config:\n{compiled}" + ); + } +} + +#[test] +fn workflow_source_path_rejects_ado_expression_before_prompt_embedding() { + let temp_dir = std::env::temp_dir().join(format!( + "ado-aw-source-path-injection-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).unwrap(); + let input = temp_dir.join("$(System.AccessToken).md"); + let output_path = temp_dir.join("out.yml"); + fs::write( + &input, + "---\nname: test\ndescription: test\nsafe-outputs:\n noop: {}\n---\n", + ) + .unwrap(); + + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(binary_path) + .args([ + "compile", + input.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + let _ = fs::remove_dir_all(&temp_dir); + + assert!(!output.status.success(), "source path expression must fail"); + assert!( + stderr.contains("workflow source path") && stderr.contains("ADO expression"), + "{stderr}" + ); +} + fn collect_pipeline_artifact_task_inputs( value: &serde_yaml::Value, inputs: &mut Vec,