From cf059ec8cbfd0214e024312a6876bc890ce2dfcc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 05:57:17 +0000 Subject: [PATCH 1/2] Initial plan From 95c044a28ec519194680cc107de04d958256e266 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 06:20:38 +0000 Subject: [PATCH 2/2] fix(safe-outputs): reject cross-organization repos in create-pull-request Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --- docs/front-matter.md | 7 ++ docs/safe-outputs.md | 10 ++ src/compile/mod.rs | 27 +++++ src/compile/types.rs | 75 ++++++++++++ src/main.rs | 28 +++++ src/safe_outputs/create_pull_request.rs | 122 ++++++++++++++++++++ src/safe_outputs/result.rs | 9 ++ src/safe_outputs/upload_build_attachment.rs | 1 + 8 files changed, 279 insertions(+) diff --git a/docs/front-matter.md b/docs/front-matter.md index bddcfd9f..4fa2c417 100644 --- a/docs/front-matter.md +++ b/docs/front-matter.md @@ -554,6 +554,13 @@ Aliases must be unique case-insensitively because they become checkout directory names on Windows agents. `root`, `repo`, and `self` are reserved in every casing; `self` is the compiler-owned path for the pipeline repository. +> **Cross-organization `type: git` repositories.** A `type: git` entry with an +> `endpoint:` set (used for a repository outside the pipeline's own Azure +> DevOps organization) checks out correctly, but `create-pull-request` cannot +> yet target it: Stage 3 composes every ADO Git REST call from the pipeline's +> own organization/project. See the limitation note under +> [`create-pull-request`](safe-outputs.md#create-pull-request). + ### Tuning checkout fetch behavior (`fetch-depth` / `fetch-tags`) On large monorepos the checkout step can dominate the run. Azure DevOps can diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index 65377845..681ff5af 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -955,6 +955,16 @@ Creates a pull request with code changes made by the agent. When invoked: During Stage 3 execution, the repository is validated against the allowed list (from `checkout:` + "self"), then the patch is applied and a PR is created in Azure DevOps. +> **Cross-organization repositories are not yet supported.** Every ADO Git +> REST call the executor makes is composed from the pipeline's own +> organization/project. A `repos:` alias checked out from a **different** +> Azure DevOps organization (a `type: git` entry with an `endpoint:` service +> connection — see [`docs/front-matter.md`](front-matter.md#repositories-repos)) +> cannot be targeted: the compiler warns when `create-pull-request` and such an +> alias are both configured, and Stage 3 rejects the alias with a clear error +> (including under `--dry-run`) instead of silently composing a request against +> the wrong organization. + **Shallow-clone agent pools (automatic):** The diff base is computed at agent time from the checked-out repository. For same-organization Azure Repos, `prepare-pr-base.js` asks the ADO Diffs API for the exact `commonCommit`, diff --git a/src/compile/mod.rs b/src/compile/mod.rs index a8ab6d70..85b0158d 100644 --- a/src/compile/mod.rs +++ b/src/compile/mod.rs @@ -224,6 +224,33 @@ async fn compile_pipeline_inner( // Validate checkout list against repositories common::validate_checkout_list(&front_matter.repositories, &front_matter.checkout)?; + // Cross-organization `create-pull-request` advisory (warning-only): a + // `repos:` alias whose `type: git` entry sets `endpoint:` lives in a + // different Azure DevOps organization than the pipeline. Stage 3 composes + // every ADO Git REST call from the pipeline's own organization/project, so + // `create-pull-request` cannot yet target such an alias — surface this at + // compile time rather than as a confusing runtime 404 (see issue #1934). + if front_matter.safe_outputs.contains_key("create-pull-request") { + let cross_org_aliases = front_matter.checkout_cross_organization_repo_aliases(); + if !cross_org_aliases.is_empty() { + let mut aliases: Vec<&String> = cross_org_aliases.iter().collect(); + aliases.sort(); + let aliases = aliases + .iter() + .map(|a| a.as_str()) + .collect::>() + .join(", "); + eprintln!( + "Warning: create-pull-request is enabled and repos: checks out {aliases} from \ + another Azure DevOps organization (a `type: git` entry with `endpoint:` set). \ + create-pull-request cannot yet target a cross-organization repository — it \ + composes every Git API call against this pipeline's own organization and \ + project, so a call against {aliases} will fail at runtime even though checkout \ + succeeds." + ); + } + } + // Checkout-aware path-layout advisories (warning-only): surface // hand-written paths that won't exist under the resolved checkout // layout, plus deprecated directory markers left in the agent body. diff --git a/src/compile/types.rs b/src/compile/types.rs index 7f107f5b..00b81388 100644 --- a/src/compile/types.rs +++ b/src/compile/types.rs @@ -2668,6 +2668,26 @@ impl FrontMatter { }) } + /// Checked-out repo aliases whose `repos:` entry is `type: git` with an + /// `endpoint:` set — the documented signal that the repository lives in a + /// **different** Azure DevOps organization than the pipeline (same-org + /// Azure Repos `git` entries never need an `endpoint:`, per the `repos:` + /// field reference in `docs/front-matter.md`). Stage 3 composes every ADO + /// Git REST call from the pipeline's own organization/project, so these + /// aliases cannot yet be targeted by repo-write safe outputs like + /// `create-pull-request`. + pub fn checkout_cross_organization_repo_aliases(&self) -> std::collections::HashSet { + self.repositories + .iter() + .filter(|r| { + r.repo_type == "git" + && r.endpoint.is_some() + && self.checkout.iter().any(|a| a == &r.repository) + }) + .map(|r| r.repository.clone()) + .collect() + } + /// Map each checked-out repo alias to its `repos: ref`, for resolving a /// per-repo create-pull-request target branch. `self` is intentionally /// absent (its ref is the runtime trigger branch, not a static `repos:` ref). @@ -7325,6 +7345,61 @@ Body assert!(fm.create_pr_config().is_none()); } + #[test] + fn test_checkout_cross_organization_repo_aliases_flags_endpoint_git_repos() { + let content = r#"--- +name: "Cross-org Agent" +description: "x" +repos: + - One/azlocal-overlay + - name: AzureForOperatorsIndustry/nc-api-testing + ref: refs/heads/main + endpoint: afoi-x-org-pipeline + - name: AzureForOperatorsIndustry/nc-resource-testing + ref: refs/heads/main + endpoint: afoi-x-org-pipeline + checkout: false +safe-outputs: + create-pull-request: +--- + +Body +"#; + let (mut fm, _) = super::super::common::parse_markdown(content).unwrap(); + let (repos, checkout, checkout_fetch) = super::super::common::resolve_repos(&fm).unwrap(); + fm.repositories = repos; + fm.checkout = checkout; + fm.checkout_fetch = checkout_fetch; + + let cross_org = fm.checkout_cross_organization_repo_aliases(); + // Only checked-out aliases participate; `nc-resource-testing` opts out + // of checkout, so it must not appear even though it has an `endpoint:`. + assert_eq!( + cross_org, + std::collections::HashSet::from(["nc-api-testing".to_string()]) + ); + } + + #[test] + fn test_checkout_cross_organization_repo_aliases_empty_for_same_org_repos() { + let content = r#"--- +name: "Same-org Agent" +description: "x" +repos: + - One/azlocal-overlay +--- + +Body +"#; + let (mut fm, _) = super::super::common::parse_markdown(content).unwrap(); + let (repos, checkout, checkout_fetch) = super::super::common::resolve_repos(&fm).unwrap(); + fm.repositories = repos; + fm.checkout = checkout; + fm.checkout_fetch = checkout_fetch; + + assert!(fm.checkout_cross_organization_repo_aliases().is_empty()); + } + #[test] fn test_front_matter_safe_outputs_noop_object_form() { // `noop: {}` must parse as an empty mapping — distinct from diff --git a/src/main.rs b/src/main.rs index fbd1fd25..f5606b28 100644 --- a/src/main.rs +++ b/src/main.rs @@ -819,6 +819,19 @@ struct ResolvedExecutionConfig { struct ResolvedExecutionRepository { repository: String, name: String, + /// ADO repository resource type (`"git"`, `"github"`, …). Defaults to + /// `"git"` for resolved-config JSON emitted before this field existed. + #[serde(default = "default_resolved_repo_type", rename = "type")] + repo_type: String, + /// Service connection name, when set. Present alongside `type: git` only + /// for a repository in a different Azure DevOps organization — see + /// `FrontMatter::checkout_cross_organization_repo_aliases`. + #[serde(default)] + endpoint: Option, +} + +fn default_resolved_repo_type() -> String { + "git".to_string() } #[derive(Debug, serde::Deserialize)] @@ -868,6 +881,18 @@ async fn build_execution_context_from_resolved( .map(|repository| (alias.clone(), repository.name.clone())) }) .collect(); + let cross_organization_repositories = config + .checkout + .iter() + .filter(|alias| { + config.repositories.iter().any(|repository| { + &repository.repository == *alias + && repository.repo_type == "git" + && repository.endpoint.is_some() + }) + }) + .cloned() + .collect(); let mut ctx = crate::safe_outputs::ExecutionContext::default(); if let Some(url) = ado_org_url { @@ -881,6 +906,7 @@ async fn build_execution_context_from_resolved( ctx.tool_configs = config.tool_configs.clone(); ctx.allowed_repositories = allowed_repositories; ctx.repo_refs = config.repo_refs.clone(); + ctx.cross_organization_repositories = cross_organization_repositories; ctx.dry_run = dry_run; let otel_path = safe_output_dir.join(agent_stats::OTEL_FILENAME); @@ -1082,6 +1108,8 @@ async fn build_execution_context( // the same helper the compiler uses at build time, so the two paths cannot // diverge. ctx.repo_refs = front_matter.checkout_repo_refs(); + ctx.cross_organization_repositories = + front_matter.checkout_cross_organization_repo_aliases(); ctx.dry_run = dry_run; // Load agent stats from OTel JSONL if available diff --git a/src/safe_outputs/create_pull_request.rs b/src/safe_outputs/create_pull_request.rs index 47430e88..5836d411 100644 --- a/src/safe_outputs/create_pull_request.rs +++ b/src/safe_outputs/create_pull_request.rs @@ -493,6 +493,33 @@ pub(crate) fn short_branch(git_ref: &str) -> &str { .unwrap_or(git_ref) } +/// Returns a clear failure result when `repository` resolves to a checkout +/// alias flagged as cross-organization (`ctx.cross_organization_repositories`). +/// +/// `create-pull-request` composes every Git REST call from the pipeline's own +/// `ado_org_url`/`ado_project`, so a `repos:` alias checked out from another +/// Azure DevOps organization via an `endpoint:` service connection cannot be +/// targeted correctly today — the request would silently resolve against the +/// wrong organization (a 404, not a permissions error). Rejecting this +/// up front, before any network call, replaces that confusing failure with an +/// actionable one and lets `--dry-run` catch it too (issue #1934). +fn reject_cross_organization_repository( + repository: &str, + ctx: &ExecutionContext, +) -> Option { + let alias = crate::safe_outputs::canonical_repository_alias(repository, ctx)?; + if !ctx.cross_organization_repositories.contains(&alias) { + return None; + } + Some(ExecutionResult::failure(format!( + "Repository '{repository}' (checkout alias '{alias}') is checked out from another \ + Azure DevOps organization via a `repos:` `endpoint:` service connection. \ + create-pull-request cannot yet target a cross-organization repository — it composes \ + every Git API call against this pipeline's own organization and project. See \ + docs/safe-outputs.md for details." + ))) +} + impl CreatePrConfig { /// Resolve the target (base) branch for a PR against `repo_alias` /// (`"self"` or a `checkout:` alias). Shared by the compiler (to deepen the @@ -594,6 +621,27 @@ impl Executor for CreatePrResult { format!("create PR: '{}' in repo '{}'", self.title, self.repository) } + /// Rejects a cross-organization repository alias before the default + /// dry-run short-circuit, so `--dry-run` surfaces the same failure a real + /// run would hit instead of reporting a false "would execute" success + /// (issue #1934). + async fn execute_sanitized( + &mut self, + ctx: &ExecutionContext, + ) -> anyhow::Result { + self.sanitize_content_fields(); + if let Some(failure) = reject_cross_organization_repository(&self.repository, ctx) { + return Ok(failure); + } + if ctx.dry_run { + return Ok(ExecutionResult::success(format!( + "[DRY-RUN] Would execute: {}", + self.dry_run_summary() + ))); + } + self.execute_impl(ctx).await + } + async fn execute_impl(&self, ctx: &ExecutionContext) -> anyhow::Result { info!( "Creating PR: '{}' in repository '{}'", @@ -2599,6 +2647,79 @@ mod tests { assert_eq!(short_branch("refs/heads/"), "refs/heads/"); } + fn cross_org_ctx() -> ExecutionContext { + ExecutionContext { + allowed_repositories: std::collections::HashMap::from([ + ("cross-org-repo".to_string(), "OtherProj/cross-org-repo".to_string()), + ("same-org-repo".to_string(), "Proj/same-org-repo".to_string()), + ]), + cross_organization_repositories: std::collections::HashSet::from([ + "cross-org-repo".to_string(), + ]), + ..Default::default() + } + } + + #[test] + fn test_reject_cross_organization_repository_flags_cross_org_alias() { + let ctx = cross_org_ctx(); + let result = reject_cross_organization_repository("cross-org-repo", &ctx); + assert!(result.is_some()); + let result = result.unwrap(); + assert!(!result.success); + assert!( + result.message.contains("cross-organization"), + "message should explain the cross-organization limitation: {}", + result.message + ); + } + + #[test] + fn test_reject_cross_organization_repository_allows_same_org_alias() { + let ctx = cross_org_ctx(); + assert!(reject_cross_organization_repository("same-org-repo", &ctx).is_none()); + assert!(reject_cross_organization_repository("self", &ctx).is_none()); + } + + #[test] + fn test_reject_cross_organization_repository_matches_by_trailing_name() { + let ctx = cross_org_ctx(); + // Matches through `lookup_allowed_repository_alias`'s trailing-name fallback. + assert!(reject_cross_organization_repository("cross-org-repo", &ctx).is_some()); + assert!( + reject_cross_organization_repository("OtherProj/cross-org-repo", &ctx).is_some() + ); + } + + #[tokio::test] + async fn test_dry_run_surfaces_cross_organization_rejection() { + let mut ctx = cross_org_ctx(); + ctx.dry_run = true; + + let mut result = CreatePrResult { + name: CreatePrResult::NAME.to_string(), + title: "Fix bug in parser".to_string(), + description: "This PR fixes a critical bug in the parser module.".to_string(), + source_branch: "agent/fix".to_string(), + patch_file: "patch.diff".to_string(), + repository: "cross-org-repo".to_string(), + agent_labels: vec![], + base_commit: None, + patch_sha256: "deadbeef".to_string(), + }; + + let execution = result.execute_sanitized(&ctx).await.unwrap(); + assert!( + !execution.success, + "dry-run must not report success for a cross-organization repository" + ); + assert!( + execution.message.contains("cross-organization"), + "dry-run message should explain the limitation: {}", + execution.message + ); + } + #[test] fn test_target_branches_sanitizes_both_keys_and_values() { use crate::sanitize::SanitizeConfig; @@ -3192,6 +3313,7 @@ index 0000000..abcdefg github_api_url: "https://api.github.com".to_string(), allowed_repositories: std::collections::HashMap::new(), repo_refs: std::collections::HashMap::new(), + cross_organization_repositories: std::collections::HashSet::new(), agent_stats: None, dry_run: false, build_id: None, diff --git a/src/safe_outputs/result.rs b/src/safe_outputs/result.rs index c596f90f..14dee953 100644 --- a/src/safe_outputs/result.rs +++ b/src/safe_outputs/result.rs @@ -131,6 +131,14 @@ pub struct ExecutionContext { /// ref (full `refs/heads/…` or short). `self` is absent (its ref is the /// runtime trigger branch, not a static `repos:` ref). pub repo_refs: HashMap, + /// Checkout aliases (keys of [`Self::allowed_repositories`]) whose + /// `repos:` entry is `type: git` with an `endpoint:` set — the documented + /// signal that the repository lives in a **different** Azure DevOps + /// organization than the pipeline. Every ADO Git REST call this executor + /// makes is composed from [`Self::ado_org_url`]/[`Self::ado_project`], so + /// a repository-write safe output must reject these aliases rather than + /// silently target the wrong organization (see issue #1934). + pub cross_organization_repositories: HashSet, /// Agent execution statistics parsed from OTel JSONL pub agent_stats: Option, /// When true, executors validate inputs but skip network calls @@ -409,6 +417,7 @@ impl ExecutionContext { repository_provider: env("BUILD_REPOSITORY_PROVIDER"), allowed_repositories: HashMap::new(), repo_refs: HashMap::new(), + cross_organization_repositories: HashSet::new(), agent_stats: None, dry_run: false, diff --git a/src/safe_outputs/upload_build_attachment.rs b/src/safe_outputs/upload_build_attachment.rs index 606cc77d..7da41b0f 100644 --- a/src/safe_outputs/upload_build_attachment.rs +++ b/src/safe_outputs/upload_build_attachment.rs @@ -925,6 +925,7 @@ attachment-type: "agent-artifact" github_api_url: "https://api.github.com".to_string(), allowed_repositories: std::collections::HashMap::new(), repo_refs: std::collections::HashMap::new(), + cross_organization_repositories: std::collections::HashSet::new(), agent_stats: None, dry_run, build_id: Some(1234),