Skip to content

Commit 6877def

Browse files
authored
Allow workspace: to target a checked-out repository alias (#332)
1 parent 13c0af1 commit 6877def

3 files changed

Lines changed: 261 additions & 25 deletions

File tree

AGENTS.md

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ schedule: daily around 14:00 # Fuzzy schedule syntax - see Schedule Syntax secti
144144
# branches:
145145
# - main
146146
# - release/*
147-
workspace: repo # Optional: "root" or "repo". If not specified, defaults based on checkout configuration (see below).
147+
workspace: repo # Optional: "root", "repo" (alias: "self"), or a checked-out repository alias. If not specified, defaults based on checkout configuration (see below).
148148
pool: AZS-1ES-L-MMS-ubuntu-22.04 # Agent pool name (string format). Defaults to AZS-1ES-L-MMS-ubuntu-22.04.
149149
# pool: # Alternative object format (required for 1ES if specifying os)
150150
# name: AZS-1ES-L-MMS-ubuntu-22.04
@@ -762,15 +762,27 @@ If `timeout-minutes` is not configured, this is replaced with an empty string.
762762
Should be replaced with the appropriate working directory based on the effective workspace setting.
763763

764764
**Workspace Resolution Logic:**
765-
1. If `workspace` is explicitly set in front matter, that value is used
765+
1. If `workspace` is explicitly set in front matter, that value is used (after validation)
766766
2. If `workspace` is not set and `checkout:` contains additional repositories, defaults to `repo`
767767
3. If `workspace` is not set and only `self` is checked out, defaults to `root`
768768

769-
**Warning:** If `workspace: repo` is explicitly set but no additional repositories are in `checkout:`, a warning is emitted because when only `self` is checked out, `$(Build.SourcesDirectory)` already contains the repository content directly.
769+
**Warning:** If `workspace: repo` (or `self`) is explicitly set but no additional repositories are in `checkout:`, a warning is emitted because when only `self` is checked out, `$(Build.SourcesDirectory)` already contains the repository content directly.
770770

771-
**Values:**
772-
- `root`: `$(Build.SourcesDirectory)` - the checkout root directory
773-
- `repo`: `$(Build.SourcesDirectory)/$(Build.Repository.Name)` - the repository's subfolder
771+
**Accepted values:**
772+
- `root` → `$(Build.SourcesDirectory)` — the checkout root directory
773+
- `repo` (or `self`) → `$(Build.SourcesDirectory)/$(Build.Repository.Name)` — the trigger repository's subfolder
774+
- `<alias>` → `$(Build.SourcesDirectory)/<alias>` — a specific checked-out repository's subfolder. The alias must appear in the `checkout:` list (which itself must be a subset of `repositories:`). This form is only valid when at least one additional repository is checked out; otherwise compilation fails.
775+
776+
**Example — pointing the agent's workspace at a checked-out repository:**
777+
```yaml
778+
repositories:
779+
- repository: exp23-a7-nw
780+
type: git
781+
name: msazuresphere/exp23-a7-nw
782+
checkout:
783+
- exp23-a7-nw
784+
workspace: exp23-a7-nw # Resolves to $(Build.SourcesDirectory)/exp23-a7-nw
785+
```
774786

775787
This is used for the `workingDirectory` property of the copilot task.
776788

src/compile/common.rs

Lines changed: 217 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,13 @@ pub fn generate_checkout_self() -> String {
304304
"- checkout: self".to_string()
305305
}
306306

307+
/// Names that are reserved by the `workspace:` resolver and therefore cannot
308+
/// be used as repository aliases / `checkout:` entries. If a user defines a
309+
/// repository named `repo` and writes `workspace: repo`, the special-cased
310+
/// reserved arm would silently win over the alias resolution, producing the
311+
/// wrong working directory. We reject this at compile time instead.
312+
const RESERVED_WORKSPACE_NAMES: &[&str] = &["root", "repo", "self"];
313+
307314
/// Validate that all entries in checkout list exist in repositories
308315
pub fn validate_checkout_list(repositories: &[Repository], checkout: &[String]) -> Result<()> {
309316
if checkout.is_empty() {
@@ -321,40 +328,119 @@ pub fn validate_checkout_list(repositories: &[Repository], checkout: &[String])
321328
repo_names
322329
);
323330
}
331+
if RESERVED_WORKSPACE_NAMES.contains(&name.as_str()) {
332+
anyhow::bail!(
333+
"Checkout entry '{}' uses a name reserved by the 'workspace:' resolver \
334+
({:?}). Rename the repository alias to avoid ambiguity with \
335+
'workspace: {}'.",
336+
name,
337+
RESERVED_WORKSPACE_NAMES,
338+
name
339+
);
340+
}
324341
}
325342

326343
Ok(())
327344
}
328345

346+
/// Sentinel prefix used to encode a repository-alias workspace selection
347+
/// in the string returned by [`compute_effective_workspace`]. The prefix is
348+
/// only ever produced internally by `compute_effective_workspace` from a
349+
/// user-supplied alias that has just been checked against the `checkout:`
350+
/// list, so the encoded value never round-trips back through user input.
351+
const WORKSPACE_ALIAS_PREFIX: &str = "alias:";
352+
329353
/// Compute the effective workspace based on explicit setting and checkout configuration.
354+
///
355+
/// Accepted values for `explicit_workspace`:
356+
/// - `"root"` — `$(Build.SourcesDirectory)` (the checkout root)
357+
/// - `"repo"` or `"self"` — the trigger repository's subfolder
358+
/// - any repository alias listed in `checkout` — that repository's subfolder
359+
///
360+
/// Returns an encoded string that [`generate_working_directory`] resolves to
361+
/// the actual ADO path expression.
330362
pub fn compute_effective_workspace(
331363
explicit_workspace: &Option<String>,
332364
checkout: &[String],
333365
agent_name: &str,
334-
) -> String {
366+
) -> Result<String> {
335367
let has_additional_checkouts = !checkout.is_empty();
336368

337369
match explicit_workspace {
338-
Some(ws) if ws == "repo" && !has_additional_checkouts => {
339-
eprintln!(
340-
"Warning: Agent '{}' has workspace: repo but no additional repositories in checkout. \
341-
When only 'self' is checked out, $(Build.SourcesDirectory) already contains the repository content. \
342-
The workspace setting has no effect in this case.",
343-
agent_name
344-
);
345-
"repo".to_string()
370+
Some(ws) => {
371+
let ws = ws.as_str();
372+
match ws {
373+
"root" => Ok("root".to_string()),
374+
"repo" | "self" => {
375+
if !has_additional_checkouts {
376+
eprintln!(
377+
"Warning: Agent '{}' has workspace: {} but no additional repositories in checkout. \
378+
When only 'self' is checked out, $(Build.SourcesDirectory) already contains the repository content. \
379+
The workspace setting has no effect in this case.",
380+
agent_name, ws
381+
);
382+
}
383+
Ok("repo".to_string())
384+
}
385+
alias => {
386+
// Defense in depth: even though aliases are constrained
387+
// by `validate_checkout_list` to match a `repository:`
388+
// name, refuse anything that could escape the workspace
389+
// root once embedded into the working directory path.
390+
if !validate::is_safe_path_segment(alias) {
391+
anyhow::bail!(
392+
"Agent '{}' has workspace: '{}' which is not a safe path \
393+
segment. Repository aliases must not be empty, contain '..', \
394+
'/', '\\\\' or start with '.'.",
395+
agent_name,
396+
alias
397+
);
398+
}
399+
// A single contains() check covers both "alias not in
400+
// checkout" and "checkout is empty" — produce one error
401+
// message that clearly lists what would have been valid.
402+
if !checkout.iter().any(|c| c == alias) {
403+
if checkout.is_empty() {
404+
anyhow::bail!(
405+
"Agent '{}' has workspace: '{}' but no additional repositories are checked out. \
406+
A repository alias for workspace is only valid when at least one repository appears in 'checkout:'. \
407+
Use 'root', 'repo' (or 'self'), or add the repository to the 'checkout:' list.",
408+
agent_name,
409+
alias
410+
);
411+
}
412+
anyhow::bail!(
413+
"Agent '{}' has workspace: '{}' which does not match any checked-out repository. \
414+
Valid values: 'root', 'repo' (or 'self'), or one of {:?}",
415+
agent_name,
416+
alias,
417+
checkout
418+
);
419+
}
420+
Ok(format!("{}{}", WORKSPACE_ALIAS_PREFIX, alias))
421+
}
422+
}
346423
}
347-
Some(ws) => ws.clone(),
348-
None if has_additional_checkouts => "repo".to_string(),
349-
None => "root".to_string(),
424+
None if has_additional_checkouts => Ok("repo".to_string()),
425+
None => Ok("root".to_string()),
350426
}
351427
}
352428

353429
/// Generate working directory based on workspace setting
354430
pub fn generate_working_directory(effective_workspace: &str) -> String {
431+
if let Some(alias) = effective_workspace.strip_prefix(WORKSPACE_ALIAS_PREFIX) {
432+
return format!("$(Build.SourcesDirectory)/{}", alias);
433+
}
355434
match effective_workspace {
356435
"repo" => "$(Build.SourcesDirectory)/$(Build.Repository.Name)".to_string(),
357-
"root" | _ => "$(Build.SourcesDirectory)".to_string(),
436+
"root" => "$(Build.SourcesDirectory)".to_string(),
437+
// compute_effective_workspace only ever returns "root", "repo", or an
438+
// "alias:<name>" sentinel; any other value indicates a programming
439+
// error rather than user input. Fall back to the safest path.
440+
other => {
441+
debug_assert!(false, "unexpected effective workspace value: {other}");
442+
"$(Build.SourcesDirectory)".to_string()
443+
}
358444
}
359445
}
360446

@@ -1638,7 +1724,7 @@ pub async fn compile_shared(
16381724
&front_matter.workspace,
16391725
&front_matter.checkout,
16401726
&front_matter.name,
1641-
);
1727+
)?;
16421728
let working_directory = generate_working_directory(&effective_workspace);
16431729
let pipeline_resources = generate_pipeline_resources(&front_matter.triggers)?;
16441730
let has_schedule = front_matter.schedule.is_some();
@@ -1796,37 +1882,132 @@ mod tests {
17961882

17971883
#[test]
17981884
fn test_workspace_explicit_root() {
1799-
let ws = compute_effective_workspace(&Some("root".to_string()), &[], "agent");
1885+
let ws = compute_effective_workspace(&Some("root".to_string()), &[], "agent").unwrap();
18001886
assert_eq!(ws, "root");
1887+
assert_eq!(generate_working_directory(&ws), "$(Build.SourcesDirectory)");
18011888
}
18021889

18031890
#[test]
18041891
fn test_workspace_explicit_repo_with_checkouts() {
18051892
let checkouts = vec!["other-repo".to_string()];
1806-
let ws = compute_effective_workspace(&Some("repo".to_string()), &checkouts, "agent");
1893+
let ws =
1894+
compute_effective_workspace(&Some("repo".to_string()), &checkouts, "agent").unwrap();
1895+
assert_eq!(ws, "repo");
1896+
assert_eq!(
1897+
generate_working_directory(&ws),
1898+
"$(Build.SourcesDirectory)/$(Build.Repository.Name)"
1899+
);
1900+
}
1901+
1902+
#[test]
1903+
fn test_workspace_explicit_self_alias_for_repo() {
1904+
let checkouts = vec!["other-repo".to_string()];
1905+
let ws =
1906+
compute_effective_workspace(&Some("self".to_string()), &checkouts, "agent").unwrap();
1907+
// 'self' is a synonym for 'repo' (the trigger repository).
18071908
assert_eq!(ws, "repo");
1909+
assert_eq!(
1910+
generate_working_directory(&ws),
1911+
"$(Build.SourcesDirectory)/$(Build.Repository.Name)"
1912+
);
18081913
}
18091914

18101915
#[test]
18111916
fn test_workspace_implicit_root_no_checkouts() {
1812-
let ws = compute_effective_workspace(&None, &[], "agent");
1917+
let ws = compute_effective_workspace(&None, &[], "agent").unwrap();
18131918
assert_eq!(ws, "root");
18141919
}
18151920

18161921
#[test]
18171922
fn test_workspace_implicit_repo_with_checkouts() {
18181923
let checkouts = vec!["other-repo".to_string()];
1819-
let ws = compute_effective_workspace(&None, &checkouts, "agent");
1924+
let ws = compute_effective_workspace(&None, &checkouts, "agent").unwrap();
18201925
assert_eq!(ws, "repo");
18211926
}
18221927

18231928
#[test]
18241929
fn test_workspace_explicit_repo_no_checkouts_still_returns_repo() {
18251930
// Emits a warning but still returns "repo"
1826-
let ws = compute_effective_workspace(&Some("repo".to_string()), &[], "agent");
1931+
let ws = compute_effective_workspace(&Some("repo".to_string()), &[], "agent").unwrap();
1932+
assert_eq!(ws, "repo");
1933+
}
1934+
1935+
#[test]
1936+
fn test_workspace_explicit_self_no_checkouts_still_returns_repo() {
1937+
// 'self' takes the same code path as 'repo'; it should also warn
1938+
// and still resolve to the repo subfolder.
1939+
let ws = compute_effective_workspace(&Some("self".to_string()), &[], "agent").unwrap();
18271940
assert_eq!(ws, "repo");
18281941
}
18291942

1943+
#[test]
1944+
fn test_workspace_explicit_alias_with_traversal_fails() {
1945+
let checkouts = vec!["../sibling".to_string()];
1946+
let err = compute_effective_workspace(
1947+
&Some("../sibling".to_string()),
1948+
&checkouts,
1949+
"agent",
1950+
)
1951+
.unwrap_err();
1952+
let msg = err.to_string();
1953+
assert!(msg.contains("not a safe path"), "msg: {msg}");
1954+
}
1955+
1956+
#[test]
1957+
fn test_workspace_explicit_alias_with_slash_fails() {
1958+
let checkouts = vec!["foo/bar".to_string()];
1959+
let err = compute_effective_workspace(
1960+
&Some("foo/bar".to_string()),
1961+
&checkouts,
1962+
"agent",
1963+
)
1964+
.unwrap_err();
1965+
let msg = err.to_string();
1966+
assert!(msg.contains("not a safe path"), "msg: {msg}");
1967+
}
1968+
1969+
#[test]
1970+
fn test_workspace_explicit_alias_resolves_to_repo_subdir() {
1971+
let checkouts = vec!["exp23-a7-nw".to_string(), "another-repo".to_string()];
1972+
let ws = compute_effective_workspace(
1973+
&Some("exp23-a7-nw".to_string()),
1974+
&checkouts,
1975+
"agent",
1976+
)
1977+
.unwrap();
1978+
assert_eq!(
1979+
generate_working_directory(&ws),
1980+
"$(Build.SourcesDirectory)/exp23-a7-nw"
1981+
);
1982+
}
1983+
1984+
#[test]
1985+
fn test_workspace_explicit_alias_not_in_checkout_fails() {
1986+
let checkouts = vec!["other-repo".to_string()];
1987+
let err = compute_effective_workspace(
1988+
&Some("exp23-a7-nw".to_string()),
1989+
&checkouts,
1990+
"agent",
1991+
)
1992+
.unwrap_err();
1993+
let msg = err.to_string();
1994+
assert!(msg.contains("exp23-a7-nw"), "msg: {msg}");
1995+
assert!(msg.contains("does not match"), "msg: {msg}");
1996+
}
1997+
1998+
#[test]
1999+
fn test_workspace_explicit_alias_no_checkouts_fails() {
2000+
let err =
2001+
compute_effective_workspace(&Some("exp23-a7-nw".to_string()), &[], "agent")
2002+
.unwrap_err();
2003+
let msg = err.to_string();
2004+
assert!(msg.contains("exp23-a7-nw"), "msg: {msg}");
2005+
assert!(
2006+
msg.contains("no additional repositories are checked out"),
2007+
"msg: {msg}"
2008+
);
2009+
}
2010+
18302011
// ─── validate_checkout_list ───────────────────────────────────────────────
18312012

18322013
#[test]
@@ -1874,6 +2055,23 @@ mod tests {
18742055
assert!(result.is_ok());
18752056
}
18762057

2058+
#[test]
2059+
fn test_validate_checkout_list_reserved_name_fails() {
2060+
// A repo aliased "repo" would silently shadow `workspace: repo`, so
2061+
// reject it at compile time.
2062+
let repos = vec![Repository {
2063+
repository: "repo".to_string(),
2064+
repo_type: "git".to_string(),
2065+
name: "org/repo".to_string(),
2066+
repo_ref: "refs/heads/main".to_string(),
2067+
}];
2068+
let checkout = vec!["repo".to_string()];
2069+
let err = validate_checkout_list(&repos, &checkout).unwrap_err();
2070+
let msg = err.to_string();
2071+
assert!(msg.contains("reserved"), "msg: {msg}");
2072+
assert!(msg.contains("'repo'"), "msg: {msg}");
2073+
}
2074+
18772075
// ─── Engine::args (copilot params) ──────────────────────────────────────
18782076

18792077
#[test]

src/validate.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@ use std::collections::HashMap;
1515

1616
// ── Character allowlist validators ──────────────────────────────────────────
1717

18+
/// Validate that a string is safe to embed as a single path segment (e.g. a
19+
/// repository alias appended to `$(Build.SourcesDirectory)`). Rejects empty
20+
/// strings, anything containing `..`, path separators (`/`, `\`), or leading
21+
/// `.` to prevent path traversal / hidden-directory escapes.
22+
pub fn is_safe_path_segment(s: &str) -> bool {
23+
!s.is_empty()
24+
&& !s.contains("..")
25+
&& !s.contains('/')
26+
&& !s.contains('\\')
27+
&& !s.starts_with('.')
28+
}
29+
1830
/// Characters allowed in engine.command paths (absolute path chars only).
1931
/// Prevents shell injection when the path is embedded in AWF single-quoted commands.
2032
pub fn is_valid_command_path(s: &str) -> bool {
@@ -388,6 +400,20 @@ mod tests {
388400

389401
// ── Character allowlist validators ──────────────────────────────────
390402

403+
#[test]
404+
fn test_is_safe_path_segment() {
405+
assert!(is_safe_path_segment("my-repo"));
406+
assert!(is_safe_path_segment("exp23-a7-nw"));
407+
assert!(is_safe_path_segment("repo_v2"));
408+
assert!(!is_safe_path_segment(""));
409+
assert!(!is_safe_path_segment(".."));
410+
assert!(!is_safe_path_segment("../sibling"));
411+
assert!(!is_safe_path_segment("foo/bar"));
412+
assert!(!is_safe_path_segment("foo\\bar"));
413+
assert!(!is_safe_path_segment(".hidden"));
414+
assert!(!is_safe_path_segment("foo..bar"));
415+
}
416+
391417
#[test]
392418
fn test_is_valid_command_path() {
393419
assert!(is_valid_command_path("/tmp/awf-tools/copilot"));

0 commit comments

Comments
 (0)