Skip to content

Commit 91d39a9

Browse files
jamesadevinegithub-actions[bot]CopilotCopilot
authored
fix(compile): tighten ADO org name validation to alphanumeric and hyphen only (#598)
* docs: fix documentation drift — enable command, engine install steps, command field - README.md: add missing `enable` command to CLI Reference section - docs/template-markers.md: update `{{ engine_install_steps }}` to describe target-aware install strategy (NuGet for 1ES, GitHub Releases for others) - docs/engine.md: clarify `command` field description — skips the default engine binary installation which is NuGet for 1ES targets but GitHub Releases for standalone/job/stage targets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(compile): use user's ADO org in 1ES NuGet feed URL instead of hardcoded msazuresphere Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/e1c74427-9ecf-4ff6-b925-4822facdd4de Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * fix(compile): tighten ADO org name validation to alphanumeric and hyphen only Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/e1c74427-9ecf-4ff6-b925-4822facdd4de Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * docs: fix documentation drift — engine_log_dir path and ado/mod.rs lifecycle commands (#601) - docs/template-markers.md: correct {{ engine_log_dir }} from ~/.copilot/logs to $HOME/.copilot/logs, matching src/engine.rs log_dir(). Add explanation that tilde does not expand inside double-quoted bash strings, which would cause the directory check to always fail and silently prevent log collection. - AGENTS.md: remove references to unimplemented lifecycle CLI commands (disable, remove, list, run, status, secrets) from the ado/mod.rs description; only the 'enable' command is currently implemented. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent a448ef0 commit 91d39a9

4 files changed

Lines changed: 118 additions & 17 deletions

File tree

docs/template-markers.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,10 @@ job- and stage-level templates don't emit a top-level pipeline name.
163163

164164
Should be replaced with engine-specific pipeline steps to install the engine binary. Generated by `Engine::install_steps()`. The install strategy is **target-aware**:
165165

166-
**For `target: 1es`** — authenticates with an internal Azure Artifacts NuGet feed and installs the package:
166+
**For `target: 1es`** — authenticates with the Azure Artifacts NuGet feed for the user's ADO organization and installs the package:
167+
- Optional bash step to resolve the ADO org at runtime (emitted only when the org cannot be inferred at compile time from the git remote): extracts the organization name from `$(System.CollectionUri)` and stores it in the `AW_ADO_ORG` pipeline variable.
167168
- `NuGetAuthenticate@1` task
168-
- `NuGetCommand@2` task to install `Microsoft.Copilot.CLI.linux-x64` from `pkgs.dev.azure.com/msazuresphere/_packaging/Guardian1ESPTUpstreamOrgFeed` (uses `engine.version` if set, otherwise `COPILOT_CLI_VERSION` constant; omits `-Version` flag when `"latest"`)
169+
- `NuGetCommand@2` task to install `Microsoft.Copilot.CLI.linux-x64` from `pkgs.dev.azure.com/{org}/_packaging/Guardian1ESPTUpstreamOrgFeed`, where `{org}` is the ADO organization inferred at compile time (e.g. `contoso`) or the runtime variable `$(AW_ADO_ORG)` when compile-time inference is unavailable. Uses `engine.version` if set, otherwise `COPILOT_CLI_VERSION` constant; omits `-Version` flag when `"latest"`.
169170
- Bash step to copy binary to `/tmp/awf-tools/copilot`
170171
- Bash step to verify installation
171172

site/src/content/docs/reference/template-markers.mdx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,19 @@ Should be replaced with the human-readable name from the front matter (e.g., "Da
102102

103103
## `{{ engine_install_steps }}`
104104

105-
Should be replaced with engine-specific pipeline steps to install the engine binary. Generated by `Engine::install_steps()`. For Copilot, this produces:
105+
Should be replaced with engine-specific pipeline steps to install the engine binary. Generated by `Engine::install_steps()`. For Copilot, the install strategy is **target-aware**:
106+
107+
**For `target: 1es`** — authenticates with the Azure Artifacts NuGet feed for the user's ADO organization:
108+
- Optional bash step "Resolve ADO organization": emitted only when the org cannot be inferred at compile time; extracts the organization name from `$(System.CollectionUri)` and stores it as the `AW_ADO_ORG` pipeline variable.
106109
- `NuGetAuthenticate@1` task
107-
- `NuGetCommand@2` task to install `Microsoft.Copilot.CLI.linux-x64` (uses `engine.version` if set, otherwise `COPILOT_CLI_VERSION` constant)
110+
- `NuGetCommand@2` task to install `Microsoft.Copilot.CLI.linux-x64` from `pkgs.dev.azure.com/{org}/_packaging/Guardian1ESPTUpstreamOrgFeed` (uses `engine.version` if set, otherwise `COPILOT_CLI_VERSION` constant; omits `-Version` flag when `"latest"`)
108111
- Bash step to copy binary to `/tmp/awf-tools/copilot`
109112
- Bash step to verify installation
110113

114+
**For all other targets** — downloads from GitHub Releases:
115+
- Bash step to download and verify the binary
116+
- Bash step to verify installation
117+
111118
Returns empty when `engine.command` is set (user provides own binary).
112119

113120
## `{{ engine_run }}`

src/compile/common.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2961,7 +2961,7 @@ pub async fn compile_shared(
29612961
"/tmp/awf-tools/threat-analysis-prompt.md",
29622962
None,
29632963
)?;
2964-
let engine_install_steps = ctx.engine.install_steps(&front_matter.engine, &front_matter.target)?;
2964+
let engine_install_steps = ctx.engine.install_steps(&front_matter.engine, &front_matter.target, ctx.ado_org())?;
29652965

29662966
// 5. Compute workspace, working directory, triggers
29672967
let effective_workspace = compute_effective_workspace(

src/engine.rs

Lines changed: 105 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,13 @@ impl Engine {
135135
/// Uses `engine_config.version()` if set in front matter, otherwise falls back
136136
/// to the pinned `COPILOT_CLI_VERSION` constant. Returns an empty string when
137137
/// `engine.command` is set (the user provides their own binary).
138-
pub fn install_steps(&self, engine_config: &EngineConfig, target: &CompileTarget) -> Result<String> {
138+
///
139+
/// `ado_org` is the ADO organization name inferred from the git remote at
140+
/// compile time. For 1ES targets it is embedded directly into the NuGet
141+
/// feed URL; when `None` a runtime extraction step is emitted instead.
142+
pub fn install_steps(&self, engine_config: &EngineConfig, target: &CompileTarget, ado_org: Option<&str>) -> Result<String> {
139143
match self {
140-
Engine::Copilot => copilot_install_steps(engine_config, target),
144+
Engine::Copilot => copilot_install_steps(engine_config, target, ado_org),
141145
}
142146
}
143147

@@ -504,7 +508,12 @@ fn copilot_env(engine_config: &EngineConfig) -> Result<String> {
504508
/// - Non-1ES: download Copilot CLI from GitHub Releases and verify SHA256.
505509
///
506510
/// Both paths stage the binary at `/tmp/awf-tools/copilot`.
507-
fn copilot_install_steps(engine_config: &EngineConfig, target: &CompileTarget) -> Result<String> {
511+
///
512+
/// `ado_org` is the ADO organization name inferred from the git remote at
513+
/// compile time. For 1ES it is used to construct the NuGet feed URL; when
514+
/// `None` a runtime extraction step is emitted that derives the org from
515+
/// `$(System.CollectionUri)`.
516+
fn copilot_install_steps(engine_config: &EngineConfig, target: &CompileTarget, ado_org: Option<&str>) -> Result<String> {
508517
// Custom binary path → skip NuGet install entirely
509518
if engine_config.command().is_some() {
510519
return Ok(String::new());
@@ -534,16 +543,63 @@ fn copilot_install_steps(engine_config: &EngineConfig, target: &CompileTarget) -
534543
format!("-Version {version} ")
535544
};
536545

546+
// Build the NuGet feed URL using the org name. When the org is known
547+
// at compile time (inferred from the git remote) it is embedded
548+
// directly. When it is not available a preceding bash step extracts
549+
// the org at runtime from the $(System.CollectionUri) ADO variable and
550+
// exposes it as $(AW_ADO_ORG) for use in the NuGetCommand arguments.
551+
let (org_resolve_step, nuget_org) = match ado_org {
552+
Some(org) => {
553+
// Validate the org name against ADO organization naming rules to
554+
// prevent injection. ADO org names are composed of ASCII
555+
// alphanumerics and hyphens only (no dots, no underscores).
556+
let org_valid = !org.is_empty()
557+
&& org.chars().all(|c| c.is_ascii_alphanumeric() || c == '-');
558+
if !org_valid {
559+
anyhow::bail!(
560+
"ADO organization '{}' contains invalid characters. \
561+
Only ASCII alphanumerics and '-' are allowed.",
562+
org
563+
);
564+
}
565+
(String::new(), org.to_string())
566+
}
567+
None => {
568+
// Emit a bash step that extracts the org name from the ADO
569+
// system variable $(System.CollectionUri) at runtime and
570+
// stores it as a pipeline variable.
571+
//
572+
// $(System.CollectionUri) is expanded by ADO before bash runs
573+
// (e.g. "https://dev.azure.com/myorg/"); the parameter
574+
// expansions strip the prefix and trailing slash to yield just
575+
// the org name ("myorg").
576+
let step = "\
577+
- bash: |
578+
set -eo pipefail
579+
# $(System.CollectionUri) is expanded by ADO before bash runs,
580+
# e.g. \"https://dev.azure.com/myorg/\".
581+
_COLLECTION_URI=\"$(System.CollectionUri)\"
582+
_ORG=\"${_COLLECTION_URI#https://dev.azure.com/}\"
583+
_ORG=\"${_ORG%/}\"
584+
echo \"##vso[task.setvariable variable=AW_ADO_ORG]$_ORG\"
585+
displayName: \"Resolve ADO organization\"
586+
587+
"
588+
.to_string();
589+
(step, "$(AW_ADO_ORG)".to_string())
590+
}
591+
};
592+
537593
return Ok(format!(
538594
"\
539-
- task: NuGetAuthenticate@1
595+
{org_resolve_step}- task: NuGetAuthenticate@1
540596
displayName: \"Authenticate NuGet Feed\"
541597
542598
- task: NuGetCommand@2
543599
displayName: \"Install Copilot CLI\"
544600
inputs:
545601
command: 'custom'
546-
arguments: 'install Microsoft.Copilot.CLI.linux-x64 -Source \"https://pkgs.dev.azure.com/msazuresphere/_packaging/Guardian1ESPTUpstreamOrgFeed/nuget/v3/index.json\" {version_arg}-OutputDirectory $(Agent.TempDirectory)/tools -ExcludeVersion -NonInteractive'
602+
arguments: 'install Microsoft.Copilot.CLI.linux-x64 -Source \"https://pkgs.dev.azure.com/{nuget_org}/_packaging/Guardian1ESPTUpstreamOrgFeed/nuget/v3/index.json\" {version_arg}-OutputDirectory $(Agent.TempDirectory)/tools -ExcludeVersion -NonInteractive'
547603
548604
- bash: |
549605
ls -la \"$(Agent.TempDirectory)/tools\"
@@ -1003,7 +1059,7 @@ mod tests {
10031059
let (fm, _) = parse_markdown(
10041060
"---\nname: test\ndescription: test\nengine:\n id: copilot\n version: '1.0.0 -Source https://evil.com'\n---\n",
10051061
).unwrap();
1006-
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target);
1062+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, None);
10071063
assert!(result.is_err());
10081064
assert!(result.unwrap_err().to_string().contains("invalid characters"));
10091065
}
@@ -1013,7 +1069,7 @@ mod tests {
10131069
let (fm, _) = parse_markdown(
10141070
"---\nname: test\ndescription: test\nengine:\n id: copilot\n version: \"1.0.0'\"\n---\n",
10151071
).unwrap();
1016-
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target);
1072+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, None);
10171073
assert!(result.is_err());
10181074
}
10191075

@@ -1022,7 +1078,7 @@ mod tests {
10221078
let (fm, _) = parse_markdown(
10231079
"---\nname: test\ndescription: test\nengine:\n id: copilot\n version: '1.0.34'\n---\n",
10241080
).unwrap();
1025-
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target).unwrap();
1081+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, None).unwrap();
10261082
assert!(result.contains("releases/download/v1.0.34"));
10271083
assert!(result.contains("Install Copilot CLI (v1.0.34)"));
10281084
}
@@ -1032,7 +1088,7 @@ mod tests {
10321088
let (fm, _) = parse_markdown(
10331089
"---\nname: test\ndescription: test\nengine:\n id: copilot\n version: 'v1.0.34'\n---\n",
10341090
).unwrap();
1035-
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target).unwrap();
1091+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, None).unwrap();
10361092
assert!(result.contains("releases/download/v1.0.34"));
10371093
assert!(result.contains("Install Copilot CLI (v1.0.34)"));
10381094
}
@@ -1042,7 +1098,7 @@ mod tests {
10421098
let (fm, _) = parse_markdown(
10431099
"---\nname: test\ndescription: test\nengine:\n id: copilot\n version: latest\n---\n",
10441100
).unwrap();
1045-
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target).unwrap();
1101+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, None).unwrap();
10461102
assert!(result.contains("releases/latest/download"), "latest should resolve via latest release URL");
10471103
assert!(result.contains("Install Copilot CLI (latest)"));
10481104
}
@@ -1052,9 +1108,10 @@ mod tests {
10521108
let (fm, _) = parse_markdown(
10531109
"---\nname: test\ndescription: test\ntarget: 1es\nengine:\n id: copilot\n version: latest\n---\n",
10541110
).unwrap();
1055-
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target).unwrap();
1111+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, Some("myorg")).unwrap();
10561112
assert!(result.contains("NuGetCommand@2"));
10571113
assert!(result.contains("Guardian1ESPTUpstreamOrgFeed"));
1114+
assert!(result.contains("pkgs.dev.azure.com/myorg/"));
10581115
assert!(!result.contains("-Version latest"));
10591116
}
10601117

@@ -1063,12 +1120,48 @@ mod tests {
10631120
let (fm, _) = parse_markdown(
10641121
"---\nname: test\ndescription: test\ntarget: 1es\nengine:\n id: copilot\n version: '1.0.34'\n---\n",
10651122
).unwrap();
1066-
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target).unwrap();
1123+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, Some("myorg")).unwrap();
10671124
assert!(result.contains("NuGetCommand@2"));
10681125
assert!(result.contains("Guardian1ESPTUpstreamOrgFeed"));
1126+
assert!(result.contains("pkgs.dev.azure.com/myorg/"));
10691127
assert!(result.contains("-Version 1.0.34"));
10701128
}
10711129

1130+
#[test]
1131+
fn engine_install_onees_uses_user_org_not_msazuresphere() {
1132+
let (fm, _) = parse_markdown(
1133+
"---\nname: test\ndescription: test\ntarget: 1es\nengine:\n id: copilot\n version: '1.0.34'\n---\n",
1134+
).unwrap();
1135+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, Some("contoso")).unwrap();
1136+
assert!(result.contains("pkgs.dev.azure.com/contoso/"));
1137+
assert!(!result.contains("msazuresphere"));
1138+
}
1139+
1140+
#[test]
1141+
fn engine_install_onees_runtime_fallback_when_org_unknown() {
1142+
let (fm, _) = parse_markdown(
1143+
"---\nname: test\ndescription: test\ntarget: 1es\nengine:\n id: copilot\n version: '1.0.34'\n---\n",
1144+
).unwrap();
1145+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, None).unwrap();
1146+
assert!(result.contains("NuGetCommand@2"));
1147+
assert!(result.contains("Guardian1ESPTUpstreamOrgFeed"));
1148+
// Runtime fallback: org extracted from $(System.CollectionUri)
1149+
assert!(result.contains("$(AW_ADO_ORG)"));
1150+
assert!(result.contains("$(System.CollectionUri)"));
1151+
assert!(result.contains("Resolve ADO organization"));
1152+
assert!(!result.contains("msazuresphere"));
1153+
}
1154+
1155+
#[test]
1156+
fn engine_install_onees_rejects_invalid_org() {
1157+
let (fm, _) = parse_markdown(
1158+
"---\nname: test\ndescription: test\ntarget: 1es\nengine:\n id: copilot\n version: '1.0.34'\n---\n",
1159+
).unwrap();
1160+
let result = Engine::Copilot.install_steps(&fm.engine, &fm.target, Some("evil; rm -rf /"));
1161+
assert!(result.is_err());
1162+
assert!(result.unwrap_err().to_string().contains("invalid characters"));
1163+
}
1164+
10721165
#[test]
10731166
fn normalize_version_tag_does_not_double_prefix_v() {
10741167
assert_eq!(normalize_version_tag("v1.0.34"), "v1.0.34");

0 commit comments

Comments
 (0)