diff --git a/AGENTS.md b/AGENTS.md index 49561a50..5e5a2f8a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,7 +75,8 @@ Every compiled pipeline runs as three sequential jobs: │ ├── fuzzy_schedule.rs # Fuzzy schedule parsing │ ├── logging.rs # File-based logging infrastructure │ ├── mcp.rs # SafeOutputs MCP server (stdio + HTTP) -│ ├── configure.rs # `configure` CLI command — orchestration shim atop `src/ado/` +│ ├── configure.rs # `configure` CLI command (deprecated) — hidden alias forwarding to `secrets set GITHUB_TOKEN` +│ ├── secrets.rs # `secrets set/list/delete` subcommand group — manages pipeline variables (never prints values from `list`) │ ├── enable.rs # `enable` CLI command — registers ADO build definitions for compiled pipelines and ensures they are enabled │ ├── ado/ # Shared Azure DevOps REST helpers (auth, list/match/PATCH/POST) │ │ └── mod.rs # Used by `configure` and the lifecycle commands (enable, disable, remove, list, run, status, secrets) diff --git a/docs/cli.md b/docs/cli.md index e4144da3..f9dea1dd 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -35,14 +35,23 @@ Global flags (apply to all subcommands): `--verbose, -v` (enable info-level logg - `--ado-project ` - Azure DevOps project name override - `--dry-run` - Validate inputs but skip ADO API calls (useful for local testing and QA review) -- `configure` - Detect agentic pipelines in a local repository and update the `GITHUB_TOKEN` pipeline variable on their Azure DevOps build definitions - - `--token ` / `GITHUB_TOKEN` env var - The new GITHUB_TOKEN value (prompted if omitted) - - `--org ` - Override: Azure DevOps organization URL (e.g. `https://dev.azure.com/myorg`) or just the org name (e.g. `myorg`, auto-prefixed to the canonical URL). Inferred from git remote by default. - - `--project ` - Override: Azure DevOps project name (inferred from git remote by default) - - `--pat ` / `AZURE_DEVOPS_EXT_PAT` env var - PAT for ADO API authentication (prompted if omitted) - - `--path ` - Path to the repository root (defaults to current directory) - - `--dry-run` - Preview changes without applying them - - `--definition-ids ` - Explicit pipeline definition IDs to update (comma-separated, skips auto-detection) +- `configure` *(deprecated; hidden in --help)* - Alias forwarding to `secrets set GITHUB_TOKEN`. Existing scripts keep working but get a stderr warning. The alias will be removed in the next minor release. + +- `secrets set [] [PATH]` - Set a pipeline variable (with `isSecret=true`) on every matched ADO definition. Value resolution: positional `` → `--value-stdin` (one line) → interactive tty prompt with echo off. + - `--allow-override` - Mark the variable as `allowOverride=true` (default: false). + - `--value-stdin` - Read the value from a single line on stdin. + - `--dry-run` - Print the planned set without calling the ADO API. + - `--org / --project / --pat` - ADO context overrides (same semantics as the other lifecycle commands). + - `--definition-ids ` - Explicit pipeline definition IDs (comma-separated; skips local-fixture auto-detection). + +- `secrets list [PATH]` - List variable names and their `isSecret` / `allowOverride` flags on every matched definition. **Never prints values.** + - `--json` - Emit machine-readable JSON. + - `--org / --project / --pat / --definition-ids` - As above. + +- `secrets delete [PATH]` - Delete the named variable from every matched definition. No-op when the variable is absent. + - `--dry-run` - Print the planned deletion plan without calling the ADO API. + - `--org / --project / --pat / --definition-ids` - As above. + - `enable [PATH]` - Register an ADO build definition for each compiled pipeline discovered under `PATH` (or the current directory) and ensure it is `enabled`. For each fixture, matches against the existing ADO definitions by `yamlFilename` first, then by sanitized display name; creates a new definition when neither matches, flips `queueStatus` to `enabled` when an existing definition is `disabled` / `paused`, and skips when it is already `enabled`. Fail-soft per fixture; exits non-zero if any fixture failed. - `--org ` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default. diff --git a/src/configure.rs b/src/configure.rs index d0eb678e..18eb4055 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -1,70 +1,16 @@ -//! The `configure` CLI command. +//! The `configure` CLI command (deprecated). //! -//! Detects agentic pipelines in a local repository and updates the -//! `GITHUB_TOKEN` pipeline variable on their corresponding Azure DevOps -//! build definitions. -//! -//! Note: this command is being renamed to `secrets set GITHUB_TOKEN` as -//! part of the Phase 1 CLI overhaul. The current entry point remains the -//! orchestration shim below; all shared ADO REST logic lives in -//! [`crate::ado`]. +//! Sets `GITHUB_TOKEN` on every matched ADO definition. This command +//! is retained as a hidden deprecation alias forwarding to +//! [`crate::secrets::run_set_github_token`]; new code should use +//! `ado-aw secrets set GITHUB_TOKEN ` instead. -use anyhow::{Context, Result}; +use anyhow::Result; use std::path::Path; -use crate::ado::{ - AdoAuth, AdoContext, MatchedDefinition, resolve_ado_context, resolve_auth, - resolve_definitions, update_pipeline_variable, -}; - -/// Resolves the GitHub token from the CLI flag or an interactive prompt. -fn resolve_token(token: Option<&str>) -> Result { - match token { - Some(t) => Ok(t.to_string()), - None => inquire::Password::new("Enter the new GITHUB_TOKEN:") - .without_confirmation() - .prompt() - .context("Failed to read token from interactive prompt"), - } -} - -/// Updates the `GITHUB_TOKEN` variable on every matched pipeline -/// definition and reports per-definition success/failure. -async fn apply_token_updates( - client: &reqwest::Client, - ado_ctx: &AdoContext, - auth: &AdoAuth, - matched: &[MatchedDefinition], - token: &str, -) -> Result<()> { - println!("Updating GITHUB_TOKEN on matched definitions..."); - let mut success_count = 0; - let mut failure_count = 0; - - for m in matched { - match update_pipeline_variable(client, ado_ctx, auth, m.id, "GITHUB_TOKEN", token).await { - Ok(()) => { - println!(" \u{2713} Updated '{}' (id={})", m.name, m.id); - success_count += 1; - } - Err(e) => { - eprintln!(" \u{2717} Failed to update '{}' (id={}): {}", m.name, m.id, e); - failure_count += 1; - } - } - } - - println!(); - println!("Done: {} updated, {} failed.", success_count, failure_count); - - if failure_count > 0 { - anyhow::bail!("{} definition(s) failed to update", failure_count); - } - - Ok(()) -} - -/// Run the configure command. +/// Forwarder for the legacy `configure --token` invocation. Emits a +/// deprecation warning to stderr and forwards to the unified +/// `secrets set GITHUB_TOKEN` code path. pub async fn run( token: Option<&str>, org: Option<&str>, @@ -74,69 +20,14 @@ pub async fn run( dry_run: bool, definition_ids: Option<&[u64]>, ) -> Result<()> { - let repo_path = match path { - Some(p) => tokio::fs::canonicalize(p) - .await - .with_context(|| format!("Could not resolve path: {}", p.display()))?, - None => tokio::fs::canonicalize(".") - .await - .context("Could not resolve current directory")?, - }; - - let token = resolve_token(token)?; - let auth = resolve_auth(pat).await?; - let ado_ctx = resolve_ado_context(&repo_path, org, project).await?; - - println!( - "ADO context: org={}, project={}{}", - ado_ctx.org_url, - ado_ctx.project, - if ado_ctx.repo_name.is_empty() { - String::new() - } else { - format!(", repo={}", ado_ctx.repo_name) - } - ); - println!(); - - let client = reqwest::Client::builder() - .timeout(std::time::Duration::from_secs(30)) - .build() - .context("Failed to create HTTP client")?; - - let Some(matched) = - resolve_definitions(&client, &ado_ctx, &auth, definition_ids, &repo_path).await? - else { - return Ok(()); - }; - - if matched.is_empty() { - println!("No matching ADO pipeline definitions found."); - println!("Make sure your pipelines are registered in Azure DevOps and point to the detected YAML files."); - return Ok(()); - } - - println!("{} definition(s) to update:", matched.len()); - for m in &matched { - if m.yaml_path.is_empty() { - println!(" [{}] '{}' (id={})", m.match_method, m.name, m.id); - } else { - println!( - " [{}] '{}' (id={}) \u{2190} {}", - m.match_method, m.name, m.id, m.yaml_path - ); - } - } - println!(); - - if dry_run { - println!("Dry run \u{2014} no changes applied."); - println!( - "Would update GITHUB_TOKEN on {} definition(s).", - matched.len() - ); - return Ok(()); - } - - apply_token_updates(&client, &ado_ctx, &auth, &matched, &token).await + crate::secrets::run_set_github_token( + token, + org, + project, + pat, + path, + dry_run, + definition_ids, + ) + .await } diff --git a/src/main.rs b/src/main.rs index 9d19f316..bc08df43 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,6 +17,7 @@ mod ndjson; pub mod runtimes; pub mod sanitize; mod safeoutputs; +mod secrets; mod tools; pub mod validate; @@ -24,6 +25,66 @@ use anyhow::{Context, Result}; use clap::{Parser, Subcommand}; use std::path::{Path, PathBuf}; +#[derive(Subcommand, Debug)] +enum SecretsCmd { + /// Set a pipeline variable on every matched definition (isSecret=true). + Set { + /// Variable name to set (e.g. `GITHUB_TOKEN`). + name: String, + /// Variable value. If omitted, falls back to `--value-stdin` or an + /// interactive tty prompt with echo off. + value: Option, + /// Path to the repository root (defaults to current directory). + path: Option, + #[arg(long)] + org: Option, + #[arg(long)] + project: Option, + #[arg(long, env = "AZURE_DEVOPS_EXT_PAT")] + pat: Option, + /// Mark the variable as `allowOverride=true` (default: false). + #[arg(long)] + allow_override: bool, + /// Read the value from a single line on stdin. + #[arg(long)] + value_stdin: bool, + #[arg(long)] + dry_run: bool, + /// Explicit definition IDs (skips local-fixture auto-detection). + #[arg(long, value_delimiter = ',')] + definition_ids: Option>, + }, + /// List variable names + flags on every matched definition. Never prints values. + List { + path: Option, + #[arg(long)] + org: Option, + #[arg(long)] + project: Option, + #[arg(long, env = "AZURE_DEVOPS_EXT_PAT")] + pat: Option, + #[arg(long)] + json: bool, + #[arg(long, value_delimiter = ',')] + definition_ids: Option>, + }, + /// Delete a named variable from every matched definition. + Delete { + name: String, + path: Option, + #[arg(long)] + org: Option, + #[arg(long)] + project: Option, + #[arg(long, env = "AZURE_DEVOPS_EXT_PAT")] + pat: Option, + #[arg(long)] + dry_run: bool, + #[arg(long, value_delimiter = ',')] + definition_ids: Option>, + }, +} + #[derive(Subcommand, Debug)] enum Commands { /// Compile markdown to pipeline definition (or recompile all detected pipelines) @@ -115,7 +176,9 @@ enum Commands { #[arg(long)] force: bool, }, - /// Detect agentic pipelines and update GITHUB_TOKEN on their ADO definitions + /// (Deprecated) Set GITHUB_TOKEN on every matched ADO definition. + /// Use `secrets set GITHUB_TOKEN ` instead. + #[command(hide = true)] Configure { /// The new GITHUB_TOKEN value (defaults to GITHUB_TOKEN env var; prompted if omitted) #[arg(long, env = "GITHUB_TOKEN")] @@ -140,6 +203,11 @@ enum Commands { #[arg(long, value_delimiter = ',')] definition_ids: Option>, }, + /// Manage pipeline-variable secrets on every matched ADO definition. + Secrets { + #[command(subcommand)] + action: SecretsCmd, + }, /// Register an ADO build definition for each compiled pipeline and ensure it's enabled. Enable { /// Path to the repository root (defaults to current directory). Used @@ -523,6 +591,7 @@ async fn main() -> Result<()> { Some(Commands::McpHttp { .. }) => "mcp-http", Some(Commands::Init { .. }) => "init", Some(Commands::Configure { .. }) => "configure", + Some(Commands::Secrets { .. }) => "secrets", Some(Commands::Enable { .. }) => "enable", None => "ado-aw", }; @@ -632,6 +701,72 @@ async fn main() -> Result<()> { ) .await?; } + Commands::Secrets { action } => match action { + SecretsCmd::Set { + name, + value, + path, + org, + project, + pat, + allow_override, + value_stdin, + dry_run, + definition_ids, + } => { + secrets::run_set(secrets::SetOptions { + name: &name, + value: value.as_deref(), + org: org.as_deref(), + project: project.as_deref(), + pat: pat.as_deref(), + path: path.as_deref(), + allow_override, + value_stdin, + dry_run, + definition_ids: definition_ids.as_deref(), + }) + .await?; + } + SecretsCmd::List { + path, + org, + project, + pat, + json, + definition_ids, + } => { + secrets::run_list(secrets::ListOptions { + org: org.as_deref(), + project: project.as_deref(), + pat: pat.as_deref(), + path: path.as_deref(), + json, + definition_ids: definition_ids.as_deref(), + }) + .await?; + } + SecretsCmd::Delete { + name, + path, + org, + project, + pat, + dry_run, + definition_ids, + } => { + secrets::run_delete(secrets::DeleteOptions { + name: &name, + org: org.as_deref(), + project: project.as_deref(), + pat: pat.as_deref(), + path: path.as_deref(), + dry_run, + definition_ids: definition_ids.as_deref(), + }) + .await?; + } + }, Commands::Enable { path, org, diff --git a/src/secrets.rs b/src/secrets.rs new file mode 100644 index 00000000..770904da --- /dev/null +++ b/src/secrets.rs @@ -0,0 +1,653 @@ +//! The `secrets` CLI command (subcommand group). +//! +//! Replaces `ado-aw configure` with a `secrets set / list / delete` +//! subcommand group. `secrets set GITHUB_TOKEN ` is the direct +//! replacement for `configure --token `; the legacy +//! `configure` invocation is still accepted (hidden in `--help`) and +//! prints a deprecation warning before forwarding to +//! [`run_set_github_token`]. +//! +//! Phase 1 of the pipeline-lifecycle CLI family — see `docs/cli.md`. +//! +//! ## Security +//! +//! - `secrets list` never prints variable values. It only emits names +//! and the `isSecret` / `allowOverride` flags. +//! - `secrets set` PUTs with `isSecret: true`. + +use anyhow::{Context, Result}; +use std::path::{Path, PathBuf}; + +use crate::ado::{ + AdoAuth, AdoContext, MatchedDefinition, get_definition_full, resolve_ado_context, + resolve_auth, resolve_definitions, +}; + +/// Description of one pipeline variable, for listing only. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VariableInfo { + pub name: String, + pub is_secret: bool, + pub allow_override: bool, +} + +/// Validate a variable name. ADO permits arbitrary names but the CLI +/// rejects empty/whitespace-only/whitespace-containing inputs since +/// those almost always indicate a quoting bug. +pub fn validate_variable_name(name: &str) -> Result<()> { + if name.is_empty() { + anyhow::bail!("Variable name must not be empty."); + } + if name.chars().any(char::is_whitespace) { + anyhow::bail!( + "Variable name '{}' contains whitespace; check shell quoting.", + name + ); + } + Ok(()) +} + +/// Pure: produce a copy of `definition` with the named variable set +/// to `(value, isSecret=true, allow_override)`. Preserves all other +/// keys. +pub fn apply_variable_set( + mut definition: serde_json::Value, + name: &str, + value: &str, + allow_override: bool, +) -> serde_json::Value { + if definition.get("variables").is_none() + || !definition["variables"].is_object() + { + definition["variables"] = serde_json::json!({}); + } + definition["variables"][name] = serde_json::json!({ + "value": value, + "isSecret": true, + "allowOverride": allow_override, + }); + definition +} + +/// Pure: produce a copy of `definition` with the named variable +/// removed. No-op if it wasn't present. +pub fn apply_variable_delete( + mut definition: serde_json::Value, + name: &str, +) -> serde_json::Value { + if let Some(vars) = definition.get_mut("variables").and_then(|v| v.as_object_mut()) { + vars.remove(name); + } + definition +} + +/// Pure: project a definition's `variables` object to a sorted +/// list of [`VariableInfo`]. Never reads or surfaces the `value` +/// field — listings must be safe to dump to stdout. +pub fn list_variables(definition: &serde_json::Value) -> Vec { + let Some(vars) = definition.get("variables").and_then(|v| v.as_object()) else { + return Vec::new(); + }; + let mut out: Vec = vars + .iter() + .map(|(k, v)| VariableInfo { + name: k.clone(), + is_secret: v.get("isSecret").and_then(|b| b.as_bool()).unwrap_or(false), + allow_override: v + .get("allowOverride") + .and_then(|b| b.as_bool()) + .unwrap_or(false), + }) + .collect(); + out.sort_by(|a, b| a.name.cmp(&b.name)); + out +} + +// ==================== Shared HTTP helpers ==================== + +async fn put_definition( + client: &reqwest::Client, + ctx: &AdoContext, + auth: &AdoAuth, + id: u64, + body: &serde_json::Value, +) -> Result<()> { + let url = format!( + "{}/{}/_apis/build/definitions/{}?api-version=7.1", + ctx.org_url.trim_end_matches('/'), + ctx.project, + id + ); + + let resp = auth + .apply(client.put(&url)) + .header("Content-Type", "application/json") + .json(body) + .send() + .await + .with_context(|| format!("Failed to PUT definition {}", id))?; + + let status = resp.status(); + if !status.is_success() { + let resp_body = resp.text().await.unwrap_or_default(); + anyhow::bail!( + "ADO API returned {} when PUTting definition {}: {}", + status, + id, + resp_body + ); + } + Ok(()) +} + +// ==================== `secrets set` ==================== + +pub struct SetOptions<'a> { + pub name: &'a str, + pub value: Option<&'a str>, + pub org: Option<&'a str>, + pub project: Option<&'a str>, + pub pat: Option<&'a str>, + pub path: Option<&'a Path>, + pub allow_override: bool, + pub value_stdin: bool, + pub dry_run: bool, + pub definition_ids: Option<&'a [u64]>, +} + +pub async fn run_set(opts: SetOptions<'_>) -> Result<()> { + validate_variable_name(opts.name)?; + + let repo_path: PathBuf = match opts.path { + Some(p) => tokio::fs::canonicalize(p) + .await + .with_context(|| format!("Could not resolve path: {}", p.display()))?, + None => tokio::fs::canonicalize(".") + .await + .context("Could not resolve current directory")?, + }; + + let value = resolve_value(opts.name, opts.value, opts.value_stdin)?; + let auth = resolve_auth(opts.pat).await?; + let ado_ctx = resolve_ado_context(&repo_path, opts.org, opts.project).await?; + + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + .build() + .context("Failed to create HTTP client")?; + + let Some(matched) = resolve_definitions( + &client, + &ado_ctx, + &auth, + opts.definition_ids, + &repo_path, + ) + .await? + else { + return Ok(()); + }; + + if matched.is_empty() { + anyhow::bail!( + "No ADO definitions matched any local fixture. Run `ado-aw list` to \ + diagnose." + ); + } + + print_matched_summary(&matched); + + if opts.dry_run { + println!( + "[dry-run] Would set '{}' (isSecret=true, allowOverride={}) on {} definition(s).", + opts.name, + opts.allow_override, + matched.len() + ); + return Ok(()); + } + + let mut success = 0usize; + let mut failure = 0usize; + for m in &matched { + match apply_set_one( + &client, + &ado_ctx, + &auth, + m.id, + opts.name, + &value, + opts.allow_override, + ) + .await + { + Ok(()) => { + println!(" ✓ '{}' set on '{}' (id={})", opts.name, m.name, m.id); + success += 1; + } + Err(e) => { + eprintln!(" ✗ '{}' on '{}' (id={}): {:#}", opts.name, m.name, m.id, e); + failure += 1; + } + } + } + + println!(); + println!("Done: {} succeeded, {} failed.", success, failure); + if failure > 0 { + anyhow::bail!("{} definition(s) failed", failure); + } + Ok(()) +} + +async fn apply_set_one( + client: &reqwest::Client, + ctx: &AdoContext, + auth: &AdoAuth, + id: u64, + name: &str, + value: &str, + allow_override: bool, +) -> Result<()> { + let definition = get_definition_full(client, ctx, auth, id).await?; + let updated = apply_variable_set(definition, name, value, allow_override); + put_definition(client, ctx, auth, id, &updated).await +} + +/// Resolve the variable value from the CLI inputs: explicit positional +/// `value` first, then `--value-stdin` (reads exactly one line), then +/// an interactive tty prompt with echo off. +fn resolve_value( + name: &str, + explicit: Option<&str>, + value_stdin: bool, +) -> Result { + if let Some(v) = explicit { + return Ok(v.to_string()); + } + if value_stdin { + use std::io::BufRead; + let mut line = String::new(); + let stdin = std::io::stdin(); + stdin.lock().read_line(&mut line).context("Failed to read value from stdin")?; + let trimmed = line.trim_end_matches(['\r', '\n']).to_string(); + if trimmed.is_empty() { + anyhow::bail!("--value-stdin read an empty value"); + } + return Ok(trimmed); + } + inquire::Password::new(&format!("Enter value for {}:", name)) + .without_confirmation() + .prompt() + .context("Failed to read value from interactive prompt") +} + +// ==================== `secrets list` ==================== + +pub struct ListOptions<'a> { + pub org: Option<&'a str>, + pub project: Option<&'a str>, + pub pat: Option<&'a str>, + pub path: Option<&'a Path>, + pub json: bool, + pub definition_ids: Option<&'a [u64]>, +} + +pub async fn run_list(opts: ListOptions<'_>) -> Result<()> { + let repo_path: PathBuf = match opts.path { + Some(p) => tokio::fs::canonicalize(p) + .await + .with_context(|| format!("Could not resolve path: {}", p.display()))?, + None => tokio::fs::canonicalize(".") + .await + .context("Could not resolve current directory")?, + }; + + let auth = resolve_auth(opts.pat).await?; + let ado_ctx = resolve_ado_context(&repo_path, opts.org, opts.project).await?; + + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + .build() + .context("Failed to create HTTP client")?; + + let Some(matched) = resolve_definitions( + &client, + &ado_ctx, + &auth, + opts.definition_ids, + &repo_path, + ) + .await? + else { + return Ok(()); + }; + + if matched.is_empty() { + anyhow::bail!( + "No ADO definitions matched any local fixture. Run `ado-aw list` to \ + diagnose." + ); + } + + let mut payload = serde_json::json!({}); + for m in &matched { + let definition = get_definition_full(&client, &ado_ctx, &auth, m.id).await?; + let vars = list_variables(&definition); + + if opts.json { + payload[m.id.to_string()] = serde_json::json!({ + "name": m.name, + "variables": vars.iter().map(|v| serde_json::json!({ + "name": v.name, + "isSecret": v.is_secret, + "allowOverride": v.allow_override, + })).collect::>(), + }); + } else { + println!("● {} (id={})", m.name, m.id); + if vars.is_empty() { + println!(" (no variables)"); + } else { + for v in &vars { + println!( + " - {} isSecret={} allowOverride={}", + v.name, v.is_secret, v.allow_override + ); + } + } + println!(); + } + } + + if opts.json { + println!("{}", serde_json::to_string_pretty(&payload)?); + } + Ok(()) +} + +// ==================== `secrets delete` ==================== + +pub struct DeleteOptions<'a> { + pub name: &'a str, + pub org: Option<&'a str>, + pub project: Option<&'a str>, + pub pat: Option<&'a str>, + pub path: Option<&'a Path>, + pub dry_run: bool, + pub definition_ids: Option<&'a [u64]>, +} + +pub async fn run_delete(opts: DeleteOptions<'_>) -> Result<()> { + validate_variable_name(opts.name)?; + + let repo_path: PathBuf = match opts.path { + Some(p) => tokio::fs::canonicalize(p) + .await + .with_context(|| format!("Could not resolve path: {}", p.display()))?, + None => tokio::fs::canonicalize(".") + .await + .context("Could not resolve current directory")?, + }; + + let auth = resolve_auth(opts.pat).await?; + let ado_ctx = resolve_ado_context(&repo_path, opts.org, opts.project).await?; + + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + .build() + .context("Failed to create HTTP client")?; + + let Some(matched) = resolve_definitions( + &client, + &ado_ctx, + &auth, + opts.definition_ids, + &repo_path, + ) + .await? + else { + return Ok(()); + }; + + if matched.is_empty() { + anyhow::bail!( + "No ADO definitions matched any local fixture. Run `ado-aw list` to \ + diagnose." + ); + } + + print_matched_summary(&matched); + + if opts.dry_run { + println!( + "[dry-run] Would delete variable '{}' from {} definition(s) (no-op when absent).", + opts.name, + matched.len() + ); + return Ok(()); + } + + let mut success = 0usize; + let mut failure = 0usize; + for m in &matched { + match apply_delete_one(&client, &ado_ctx, &auth, m.id, opts.name).await { + Ok(()) => { + println!(" ✓ '{}' removed from '{}' (id={})", opts.name, m.name, m.id); + success += 1; + } + Err(e) => { + eprintln!( + " ✗ removing '{}' from '{}' (id={}): {:#}", + opts.name, m.name, m.id, e + ); + failure += 1; + } + } + } + + println!(); + println!("Done: {} succeeded, {} failed.", success, failure); + if failure > 0 { + anyhow::bail!("{} definition(s) failed", failure); + } + Ok(()) +} + +async fn apply_delete_one( + client: &reqwest::Client, + ctx: &AdoContext, + auth: &AdoAuth, + id: u64, + name: &str, +) -> Result<()> { + let definition = get_definition_full(client, ctx, auth, id).await?; + let updated = apply_variable_delete(definition, name); + put_definition(client, ctx, auth, id, &updated).await +} + +// ==================== Deprecation alias ==================== + +/// Shim for the legacy `configure --token` invocation. Sets +/// `GITHUB_TOKEN` (isSecret=true, allowOverride preserved) on every +/// matched definition. Same fail-soft + accumulated-counts pattern as +/// the old `configure` body, lifted here verbatim so the deprecation +/// alias is byte-equivalent. +pub async fn run_set_github_token( + token: Option<&str>, + org: Option<&str>, + project: Option<&str>, + pat: Option<&str>, + path: Option<&Path>, + dry_run: bool, + definition_ids: Option<&[u64]>, +) -> Result<()> { + eprintln!( + "warning: 'ado-aw configure' is deprecated; use 'ado-aw secrets set GITHUB_TOKEN' \ + instead. The alias will be removed in the next minor release." + ); + run_set(SetOptions { + name: "GITHUB_TOKEN", + value: token, + org, + project, + pat, + path, + allow_override: false, + value_stdin: false, + dry_run, + definition_ids, + }) + .await +} + +// ==================== Shared display helpers ==================== + +fn print_matched_summary(matched: &[MatchedDefinition]) { + println!("{} definition(s) matched:", matched.len()); + for m in matched { + if m.yaml_path.is_empty() { + println!(" [{}] '{}' (id={})", m.match_method, m.name, m.id); + } else { + println!( + " [{}] '{}' (id={}) ← {}", + m.match_method, m.name, m.id, m.yaml_path + ); + } + } + println!(); +} + +#[cfg(test)] +mod tests { + use super::*; + + // ============ validate_variable_name ============ + + #[test] + fn validate_rejects_empty() { + assert!(validate_variable_name("").is_err()); + } + + #[test] + fn validate_rejects_whitespace() { + assert!(validate_variable_name(" ").is_err()); + assert!(validate_variable_name("FOO BAR").is_err()); + assert!(validate_variable_name("FOO\tBAR").is_err()); + } + + #[test] + fn validate_accepts_typical_names() { + assert!(validate_variable_name("GITHUB_TOKEN").is_ok()); + assert!(validate_variable_name("MY-VAR").is_ok()); + assert!(validate_variable_name("a.b.c").is_ok()); + } + + // ============ apply_variable_set ============ + + #[test] + fn set_creates_variables_object_when_missing() { + let def = serde_json::json!({ "name": "x" }); + let out = apply_variable_set(def, "FOO", "bar", false); + assert_eq!(out["variables"]["FOO"]["value"], "bar"); + assert_eq!(out["variables"]["FOO"]["isSecret"], true); + assert_eq!(out["variables"]["FOO"]["allowOverride"], false); + } + + #[test] + fn set_preserves_other_variables() { + let def = serde_json::json!({ + "variables": { "OTHER": { "value": "x", "isSecret": true, "allowOverride": false } } + }); + let out = apply_variable_set(def, "FOO", "bar", true); + assert_eq!(out["variables"]["OTHER"]["value"], "x"); + assert_eq!(out["variables"]["FOO"]["value"], "bar"); + assert_eq!(out["variables"]["FOO"]["allowOverride"], true); + } + + #[test] + fn set_overwrites_existing_variable() { + let def = serde_json::json!({ + "variables": { "FOO": { "value": "old", "isSecret": true, "allowOverride": false } } + }); + let out = apply_variable_set(def, "FOO", "new", true); + assert_eq!(out["variables"]["FOO"]["value"], "new"); + assert_eq!(out["variables"]["FOO"]["allowOverride"], true); + } + + // ============ apply_variable_delete ============ + + #[test] + fn delete_removes_existing_variable() { + let def = serde_json::json!({ + "variables": { + "FOO": { "value": "v" }, + "BAR": { "value": "w" } + } + }); + let out = apply_variable_delete(def, "FOO"); + assert!(out["variables"].get("FOO").is_none()); + assert_eq!(out["variables"]["BAR"]["value"], "w"); + } + + #[test] + fn delete_is_noop_when_variable_absent() { + let def = serde_json::json!({ "variables": { "FOO": { "value": "v" } } }); + let out = apply_variable_delete(def, "MISSING"); + assert_eq!(out["variables"]["FOO"]["value"], "v"); + } + + #[test] + fn delete_is_noop_when_variables_object_missing() { + let def = serde_json::json!({ "name": "x" }); + let out = apply_variable_delete(def, "MISSING"); + assert!(out.get("variables").is_none() || out["variables"].is_null()); + } + + // ============ list_variables (no values surfaced) ============ + + #[test] + fn list_emits_names_and_flags_only() { + let def = serde_json::json!({ + "variables": { + "TOKEN": { "value": "super-secret-leak-me", "isSecret": true, "allowOverride": false }, + "DEBUG": { "value": "true", "isSecret": false, "allowOverride": true } + } + }); + let out = list_variables(&def); + assert_eq!(out.len(), 2); + // Sorted by name. + assert_eq!(out[0].name, "DEBUG"); + assert!(!out[0].is_secret); + assert!(out[0].allow_override); + assert_eq!(out[1].name, "TOKEN"); + assert!(out[1].is_secret); + assert!(!out[1].allow_override); + } + + #[test] + fn list_returns_empty_when_no_variables_object() { + let def = serde_json::json!({ "name": "x" }); + assert!(list_variables(&def).is_empty()); + } + + /// Guardrail: the VariableInfo struct has no `value` field. If you + /// ever feel tempted to add one, you'll need to change the + /// printer — and ideally have a different review reason than + /// "convenience". + #[test] + fn variable_info_has_no_value_field_in_debug_repr() { + let def = serde_json::json!({ + "variables": { + "TOKEN": { "value": "super-secret", "isSecret": true, "allowOverride": false } + } + }); + let out = list_variables(&def); + let dbg = format!("{:?}", out[0]); + assert!( + !dbg.contains("super-secret"), + "VariableInfo Debug must not leak values, got: {}", + dbg + ); + } +} diff --git a/tests/secrets_integration.rs b/tests/secrets_integration.rs new file mode 100644 index 00000000..30895332 --- /dev/null +++ b/tests/secrets_integration.rs @@ -0,0 +1,119 @@ +//! Integration tests for `ado-aw secrets` (and the deprecation alias). + +use std::path::PathBuf; + +fn binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")) +} + +#[test] +fn secrets_help_advertises_subcommands() { + let output = std::process::Command::new(binary()) + .args(["secrets", "--help"]) + .output() + .expect("Failed to run ado-aw secrets --help"); + assert!(output.status.success(), "--help should exit 0"); + let stdout = String::from_utf8_lossy(&output.stdout); + for sub in ["set", "list", "delete"] { + assert!( + stdout.contains(sub), + "secrets --help should advertise the {sub} subcommand, got:\n{stdout}" + ); + } +} + +#[test] +fn secrets_set_help_advertises_flags() { + let output = std::process::Command::new(binary()) + .args(["secrets", "set", "--help"]) + .output() + .expect("Failed to run ado-aw secrets set --help"); + assert!(output.status.success(), "--help should exit 0"); + let stdout = String::from_utf8_lossy(&output.stdout); + for flag in ["--allow-override", "--value-stdin", "--dry-run", "--pat"] { + assert!( + stdout.contains(flag), + "secrets set --help should advertise {flag}, got:\n{stdout}" + ); + } +} + +#[test] +fn secrets_list_help_warns_no_values() { + let output = std::process::Command::new(binary()) + .args(["secrets", "list", "--help"]) + .output() + .expect("Failed to run ado-aw secrets list --help"); + assert!(output.status.success(), "--help should exit 0"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("--json"), + "secrets list --help should advertise --json, got:\n{stdout}" + ); +} + +#[test] +fn configure_is_hidden_in_top_level_help() { + let output = std::process::Command::new(binary()) + .arg("--help") + .output() + .expect("Failed to run ado-aw --help"); + assert!(output.status.success(), "--help should exit 0"); + let stdout = String::from_utf8_lossy(&output.stdout); + // `secrets` is the documented replacement. + assert!( + stdout.contains("secrets"), + "Top-level --help should advertise the secrets subcommand, got:\n{stdout}" + ); + // The legacy `configure` line must not appear (it's hidden). + // We allow any line that mentions "configure" elsewhere but the + // top-level commands list must not include the literal subcommand. + // We check the "Commands:" section, which is everything between + // `Commands:` and `Options:`. + let lower = stdout.to_lowercase(); + if let (Some(c), Some(o)) = (lower.find("commands:"), lower.find("options:")) { + let block = &lower[c..o]; + assert!( + !block.contains("configure"), + "configure should be hidden in top-level --help; commands block was:\n{block}" + ); + } +} + +#[test] +fn configure_invocation_still_works_and_warns() { + // We can't drive a real ADO call from a unit test, but the + // deprecation warning is emitted by the very first line of + // `run_set_github_token`, so we trigger it by passing arguments + // that will fail early after the warning prints. + // + // Use a path that doesn't exist; the warning is emitted before + // the path canonicalization step. The command will still + // ultimately fail, which is fine — we only assert on stderr. + let output = std::process::Command::new(binary()) + .args([ + "configure", + "--org", + "fake", + "--project", + "fake", + "--pat", + "dummy", + "--token", + "x", + "--path", + "/definitely-does-not-exist-9c4f0", + "--dry-run", + ]) + .output() + .expect("Failed to run ado-aw configure"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("deprecated"), + "configure should emit a deprecation warning, got stderr:\n{stderr}" + ); + assert!( + stderr.contains("secrets set GITHUB_TOKEN") || stderr.contains("secrets set"), + "deprecation warning should mention the replacement, got stderr:\n{stderr}" + ); +}