Skip to content

Commit a771036

Browse files
fix(configure): support Azure CLI auth and fix YAML path matching (#98)
* fix(configure): support Azure CLI auth and fix YAML path matching - Add Azure CLI token acquisition as fallback when no PAT is provided (resolution order: --pat/env var > az account get-access-token > prompt) - Introduce AdoAuth enum to handle both Basic (PAT) and Bearer (CLI) auth - Fix list_definitions API call to include includeAllProperties=true, which is required for process.yamlFilename to be returned by ADO - Improve JSON parse error diagnostics to show response body snippet - Add debug logging for definition yaml paths and match comparisons Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(configure): address PR review feedback - Use .query() consistently for all URL parameters in list_definitions - Add tests for YAML path matching using make_def_with_yaml helper - AdoAuth is scoped to configure only; tools run in pipeline context where tokens are always PATs from SYSTEM_ACCESSTOKEN Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 03bcf61 commit a771036

1 file changed

Lines changed: 178 additions & 35 deletions

File tree

src/configure.rs

Lines changed: 178 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,60 @@
77
//! (reqwest + `.basic_auth("", Some(token))` for authentication).
88
99
use anyhow::{Context, Result};
10-
use log::{debug, info};
10+
use log::{debug, info, warn};
1111
use serde::Deserialize;
1212
use std::path::Path;
1313

1414
use crate::detect;
1515

16+
/// ADO resource ID for minting ADO-scoped tokens via Azure CLI.
17+
const ADO_RESOURCE_ID: &str = "499b84ac-1321-427f-aa17-267ca6975798";
18+
19+
/// Attempt to acquire an ADO-scoped access token via `az account get-access-token`.
20+
/// Returns `Ok(token)` if the Azure CLI is installed and the user is logged in,
21+
/// or an error if the CLI is missing or the command fails.
22+
async fn try_azure_cli_token() -> Result<String> {
23+
// On Windows, `az` is a .cmd batch script that must be invoked via cmd.exe.
24+
let output = if cfg!(windows) {
25+
tokio::process::Command::new("cmd")
26+
.args([
27+
"/C", "az", "account", "get-access-token",
28+
"--resource", ADO_RESOURCE_ID,
29+
"--query", "accessToken",
30+
"-o", "tsv",
31+
])
32+
.output()
33+
.await
34+
} else {
35+
tokio::process::Command::new("az")
36+
.args([
37+
"account", "get-access-token",
38+
"--resource", ADO_RESOURCE_ID,
39+
"--query", "accessToken",
40+
"-o", "tsv",
41+
])
42+
.output()
43+
.await
44+
}
45+
.context("Failed to run 'az account get-access-token'. Is the Azure CLI installed?")?;
46+
47+
if !output.status.success() {
48+
let stderr = String::from_utf8_lossy(&output.stderr);
49+
anyhow::bail!("Azure CLI token acquisition failed: {}", stderr.trim());
50+
}
51+
52+
let token = String::from_utf8(output.stdout)
53+
.context("Azure CLI returned non-UTF-8 token")?
54+
.trim()
55+
.to_string();
56+
57+
if token.is_empty() {
58+
anyhow::bail!("Azure CLI returned an empty token");
59+
}
60+
61+
Ok(token)
62+
}
63+
1664
// ==================== ADO context from git remote ====================
1765

1866
/// ADO context extracted from the git remote URL.
@@ -121,6 +169,23 @@ async fn get_git_remote_url(repo_path: &Path) -> Result<String> {
121169

122170
// ==================== ADO Build Definitions API ====================
123171

172+
/// Authentication method for ADO API calls.
173+
/// PATs use HTTP Basic auth; Azure CLI tokens use Bearer auth.
174+
#[derive(Clone)]
175+
enum AdoAuth {
176+
Pat(String),
177+
Bearer(String),
178+
}
179+
180+
impl AdoAuth {
181+
fn apply(&self, request: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
182+
match self {
183+
AdoAuth::Pat(pat) => request.basic_auth("", Some(pat)),
184+
AdoAuth::Bearer(token) => request.bearer_auth(token),
185+
}
186+
}
187+
}
188+
124189
/// Minimal subset of an ADO Build Definition for listing.
125190
#[derive(Debug, Deserialize)]
126191
struct DefinitionListResponse {
@@ -169,21 +234,22 @@ struct MatchedDefinition {
169234
async fn list_definitions(
170235
client: &reqwest::Client,
171236
ctx: &AdoContext,
172-
pat: &str,
237+
auth: &AdoAuth,
173238
) -> Result<Vec<DefinitionSummary>> {
174239
let mut all_definitions = Vec::new();
175240
let mut continuation_token: Option<String> = None;
176241

177242
loop {
178-
let url = format!(
179-
"{}/{}/_apis/build/definitions?api-version=7.1",
243+
let base_url = format!(
244+
"{}/{}/_apis/build/definitions",
180245
ctx.org_url.trim_end_matches('/'),
181246
ctx.project
182247
);
183248

184-
debug!("Listing definitions: {}", url);
249+
debug!("Listing definitions: {}", base_url);
185250

186-
let mut request = client.get(&url).basic_auth("", Some(pat));
251+
let mut request = auth.apply(client.get(&base_url))
252+
.query(&[("includeAllProperties", "true"), ("api-version", "7.1")]);
187253
if let Some(ref token) = continuation_token {
188254
request = request.query(&[("continuationToken", token)]);
189255
}
@@ -210,10 +276,16 @@ async fn list_definitions(
210276
.and_then(|v| v.to_str().ok())
211277
.map(|s| s.to_string());
212278

213-
let response: DefinitionListResponse = resp
214-
.json()
215-
.await
216-
.context("Failed to parse definitions response")?;
279+
let body = resp.text().await.context("Failed to read definitions response body")?;
280+
let response: DefinitionListResponse = serde_json::from_str(&body)
281+
.with_context(|| {
282+
let snippet: String = body.chars().take(500).collect();
283+
format!(
284+
"Failed to parse definitions response as JSON. \
285+
This usually means the PAT is invalid or expired. \
286+
Response body (first 500 chars):\n{snippet}"
287+
)
288+
})?;
217289

218290
all_definitions.extend(response.value);
219291

@@ -287,10 +359,10 @@ fn normalize_ado_yaml_path(path: &str) -> String {
287359
async fn match_definitions(
288360
client: &reqwest::Client,
289361
ctx: &AdoContext,
290-
pat: &str,
362+
auth: &AdoAuth,
291363
detected: &[detect::DetectedPipeline],
292364
) -> Result<Vec<MatchedDefinition>> {
293-
let definitions = list_definitions(client, ctx, pat).await?;
365+
let definitions = list_definitions(client, ctx, auth).await?;
294366
info!(
295367
"Found {} pipeline definitions in {}/{}",
296368
definitions.len(),
@@ -300,9 +372,28 @@ async fn match_definitions(
300372

301373
let mut matched = Vec::new();
302374

375+
// Log all definition yaml paths for debugging
376+
for def in &definitions {
377+
let yaml_path = def
378+
.process
379+
.as_ref()
380+
.and_then(|p| p.yaml_filename.as_ref())
381+
.map(|f| normalize_ado_yaml_path(f));
382+
debug!(
383+
"ADO definition: '{}' (id={}) yamlFilename={:?} normalized={:?}",
384+
def.name, def.id,
385+
def.process.as_ref().and_then(|p| p.yaml_filename.as_ref()),
386+
yaml_path
387+
);
388+
}
389+
303390
for pipeline in detected {
304391
let yaml_path_str = pipeline.yaml_path.to_string_lossy();
305392
let yaml_path_normalized = yaml_path_str.replace('\\', "/");
393+
debug!(
394+
"Matching local pipeline: raw={:?} normalized={:?} source={:?}",
395+
yaml_path_str, yaml_path_normalized, pipeline.source
396+
);
306397

307398
// Strategy 1: Match by YAML filename in the definition.
308399
// ADO stores yamlFilename with a leading '/' (e.g., "/.azdo/pipelines/agent.yml"),
@@ -373,16 +464,13 @@ async fn match_definitions(
373464

374465
/// Update the GITHUB_TOKEN pipeline variable on a definition.
375466
///
376-
/// Uses the same `.basic_auth("", Some(token))` pattern as the existing
377-
/// tools in `src/tools/` (e.g., create_work_item, create_pr).
378-
///
379467
/// Note: The GET→PUT cycle is not atomic. Concurrent `configure` runs against
380468
/// the same definition could overwrite each other's variables. This is acceptable
381469
/// for a CLI tool typically run by a single operator.
382470
async fn update_pipeline_variable(
383471
client: &reqwest::Client,
384472
ctx: &AdoContext,
385-
pat: &str,
473+
auth: &AdoAuth,
386474
definition_id: u64,
387475
variable_name: &str,
388476
variable_value: &str,
@@ -396,9 +484,8 @@ async fn update_pipeline_variable(
396484

397485
debug!("Fetching definition {}: {}", definition_id, get_url);
398486

399-
let resp = client
400-
.get(&get_url)
401-
.basic_auth("", Some(pat))
487+
let resp = auth
488+
.apply(client.get(&get_url))
402489
.send()
403490
.await
404491
.context("Failed to get pipeline definition")?;
@@ -414,10 +501,17 @@ async fn update_pipeline_variable(
414501
);
415502
}
416503

417-
let mut definition: serde_json::Value = resp
418-
.json()
419-
.await
420-
.context("Failed to parse definition JSON")?;
504+
let body = resp.text().await.context("Failed to read definition response body")?;
505+
let mut definition: serde_json::Value = serde_json::from_str(&body)
506+
.with_context(|| {
507+
let snippet: String = body.chars().take(500).collect();
508+
format!(
509+
"Failed to parse definition {} as JSON. \
510+
This usually means the PAT is invalid or expired. \
511+
Response body (first 500 chars):\n{snippet}",
512+
definition_id
513+
)
514+
})?;
421515

422516
// Ensure variables object exists
423517
if definition.get("variables").is_none() {
@@ -449,9 +543,8 @@ async fn update_pipeline_variable(
449543

450544
debug!("Updating definition {}: {}", definition_id, put_url);
451545

452-
let resp = client
453-
.put(&put_url)
454-
.basic_auth("", Some(pat))
546+
let resp = auth
547+
.apply(client.put(&put_url))
455548
.header("Content-Type", "application/json")
456549
.json(&definition)
457550
.send()
@@ -501,13 +594,29 @@ pub async fn run(
501594
.context("Failed to read token from interactive prompt")?,
502595
};
503596

504-
// Resolve PAT: CLI flag > env var (handled by clap) > interactive prompt
505-
let resolved_pat = match pat {
506-
Some(p) => p.to_string(),
507-
None => inquire::Password::new("Enter your Azure DevOps PAT:")
508-
.without_confirmation()
509-
.prompt()
510-
.context("Failed to read PAT from interactive prompt. Set AZURE_DEVOPS_EXT_PAT env var or use --pat flag.")?,
597+
// Resolve auth: CLI flag > env var (handled by clap) > Azure CLI > interactive prompt
598+
let auth = match pat {
599+
Some(p) => {
600+
info!("Using PAT from --pat flag or AZURE_DEVOPS_EXT_PAT env var");
601+
AdoAuth::Pat(p.to_string())
602+
}
603+
None => {
604+
info!("No PAT provided, trying Azure CLI authentication...");
605+
match try_azure_cli_token().await {
606+
Ok(token) => {
607+
println!("Using Azure CLI authentication (az account get-access-token)");
608+
AdoAuth::Bearer(token)
609+
}
610+
Err(e) => {
611+
warn!("Azure CLI auth failed: {:#}. Falling back to interactive prompt.", e);
612+
let pat = inquire::Password::new("Enter your Azure DevOps PAT:")
613+
.without_confirmation()
614+
.prompt()
615+
.context("Failed to read PAT from interactive prompt. Set AZURE_DEVOPS_EXT_PAT env var, log in with 'az login', or use --pat flag.")?;
616+
AdoAuth::Pat(pat)
617+
}
618+
}
619+
}
511620
};
512621

513622
// Step 1: Detect agentic pipelines
@@ -565,7 +674,7 @@ pub async fn run(
565674
.timeout(std::time::Duration::from_secs(30))
566675
.build()
567676
.context("Failed to create HTTP client")?;
568-
let matched = match_definitions(&client, &ado_ctx, &resolved_pat, &detected).await?;
677+
let matched = match_definitions(&client, &ado_ctx, &auth, &detected).await?;
569678

570679
if matched.is_empty() {
571680
println!("No matching ADO pipeline definitions found.");
@@ -600,7 +709,7 @@ pub async fn run(
600709
match update_pipeline_variable(
601710
&client,
602711
&ado_ctx,
603-
&resolved_pat,
712+
&auth,
604713
m.id,
605714
"GITHUB_TOKEN",
606715
&token,
@@ -729,6 +838,40 @@ mod tests {
729838
);
730839
}
731840

841+
#[test]
842+
fn test_yaml_path_match_finds_definition_by_yaml_filename() {
843+
let defs = vec![
844+
make_def(1, "Unrelated Pipeline"),
845+
make_def_with_yaml(2, "My Agent", "/.azdo/pipelines/agent.yml"),
846+
make_def(3, "Another Pipeline"),
847+
];
848+
let local_path = ".azdo/pipelines/agent.yml";
849+
let path_match = defs.iter().find(|d| {
850+
d.process
851+
.as_ref()
852+
.and_then(|p| p.yaml_filename.as_ref())
853+
.is_some_and(|f| normalize_ado_yaml_path(f) == local_path)
854+
});
855+
assert!(path_match.is_some());
856+
assert_eq!(path_match.unwrap().id, 2);
857+
}
858+
859+
#[test]
860+
fn test_yaml_path_match_no_match_when_process_is_none() {
861+
let defs = vec![
862+
make_def(1, "Classic Pipeline"),
863+
make_def(2, "Another Classic"),
864+
];
865+
let local_path = ".azdo/pipelines/agent.yml";
866+
let path_match = defs.iter().find(|d| {
867+
d.process
868+
.as_ref()
869+
.and_then(|p| p.yaml_filename.as_ref())
870+
.is_some_and(|f| normalize_ado_yaml_path(f) == local_path)
871+
});
872+
assert!(path_match.is_none());
873+
}
874+
732875
#[test]
733876
fn test_fuzzy_match_single_unambiguous() {
734877
let defs = vec![

0 commit comments

Comments
 (0)