Skip to content

Commit 6352f97

Browse files
jamesadevineCopilot
andcommitted
feat(secrets): embed ADO org+repo in marker JSON for disambiguation
When two repos in the same ADO project both define a file of the same name (e.g. each has its own `agents/foo.md`), today's `--source agents/foo.md` would silently match consumers of either template — the marker JSON carried only `source` and operators had no way to scope writes to "my" agents/foo.md. Add `org` and `repo` to the marker payload: * `src/detect.rs` — `MarkerMetadata` gains two `#[serde(default)]` `String` fields; struct now derives `Default` so test fixtures can use the `..Default::default()` spread. * `src/compile/extensions/ado_aw_marker.rs` — pull org from `ctx.ado_org()` and repo from `ctx.ado_context.repo_name`, lower-cased (ADO identifiers are case-insensitive). Empty when the compiler ran outside an ADO checkout; the non-GitHub-remote guard ensures this is rare in production. Echo line gains `org=... repo=...` for build-log readability. * `src/ado/mod.rs` — add `AdoContext::org_name()` accessor that mirrors `CompileContext::ado_org`; lives on the struct so non-compile callers (Preview-driven discovery) can reuse it. * `src/ado/discovery.rs` — when `source_filter` is active, `resolve_definitions_via_discovery` also requires the marker's `(org, repo)` to equal the current `ctx`'s. Markers with empty `org` and `repo` (legacy / non-ADO compiles) match leniently so already-deployed pipelines remain discoverable until they are recompiled. Helper `marker_origin_matches` extracted with four dedicated unit tests covering strict, case-insensitive, lenient, and half-populated cases. Schema version stays at 1 — this PR has not shipped, so there are no v1-without-org/repo markers in the wild that would need the bump for forward-compat detection. New fields parse fine on either side via `#[serde(default)]`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6f817cd commit 6352f97

4 files changed

Lines changed: 220 additions & 4 deletions

File tree

src/ado/discovery.rs

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,27 @@ fn is_direct_match(def: &DefinitionSummary, markers: &[MarkerMetadata]) -> bool
519519
yaml_normalized == stem
520520
}
521521

522+
/// Decide whether a marker's `(org, repo)` identifies the same
523+
/// repository as the discovery context. Empty marker fields (legacy
524+
/// markers produced before the org/repo embed landed, or markers from
525+
/// non-ADO compile environments) are treated as wildcards so existing
526+
/// deployments are not silently excluded. Once those lock files are
527+
/// recompiled, the match becomes strict.
528+
fn marker_origin_matches(
529+
marker: &MarkerMetadata,
530+
current_org_lc: &str,
531+
current_repo_lc: &str,
532+
) -> bool {
533+
if marker.org.is_empty() && marker.repo.is_empty() {
534+
return true;
535+
}
536+
// Marker fields are already lower-cased at emit time. Be defensive
537+
// anyway — round-tripping through serde_json doesn't change case
538+
// but a hand-edited fixture or future producer might.
539+
marker.org.eq_ignore_ascii_case(current_org_lc)
540+
&& marker.repo.eq_ignore_ascii_case(current_repo_lc)
541+
}
542+
522543
async fn parse_local_lock(path: &Path) -> Option<MarkerMetadata> {
523544
let content = tokio::fs::read_to_string(path).await.ok()?;
524545
// Two surfaces, in order of preference:
@@ -535,6 +556,8 @@ async fn parse_local_lock(path: &Path) -> Option<MarkerMetadata> {
535556
return Some(MarkerMetadata {
536557
schema: 0,
537558
source: h.source,
559+
org: String::new(),
560+
repo: String::new(),
538561
version: h.version,
539562
target: String::new(),
540563
});
@@ -614,6 +637,17 @@ pub fn discovered_to_matched(d: &DiscoveredPipeline) -> Option<MatchedDefinition
614637
/// **case-sensitive** even on Windows; pass the path in the same case
615638
/// it was compiled with.
616639
///
640+
/// Source-only matching is ambiguous when two repos in the same ADO
641+
/// project happen to define a file of the same name (e.g. both have
642+
/// `agents/foo.md`). To disambiguate, the marker carries the ADO
643+
/// `org` and `repo` of the compiling repository (lower-cased). When
644+
/// `source_filter` is active, the marker's `(org, repo)` must also
645+
/// equal `ctx`'s — i.e. the operator gets only consumers whose
646+
/// template originated in the **current repo**. Markers with empty
647+
/// `org` / `repo` (legacy or non-ADO compilers) match leniently so
648+
/// pre-existing deployments are not silently excluded; once everything
649+
/// is recompiled with this version, the match becomes strict.
650+
///
617651
/// Skip-summary warnings are emitted differently depending on whether
618652
/// `source_filter` is active:
619653
///
@@ -648,6 +682,20 @@ pub async fn resolve_definitions_via_discovery(
648682
let normalized_filter: Option<String> = source_filter
649683
.map(|s| crate::compile::normalize_source_path(Path::new(s)));
650684

685+
// Origin scoping: when filtering by `--source`, also require the
686+
// marker's (org, repo) to identify the current repository. This
687+
// disambiguates the source field when two repos in the same
688+
// project define files of the same name. Lower-cased to align with
689+
// the marker's lower-casing at emit time (ADO identifiers are
690+
// case-insensitive). Markers with empty fields (legacy / non-ADO
691+
// compiles) match leniently so already-deployed pipelines remain
692+
// discoverable until they are recompiled.
693+
let current_org = ctx
694+
.org_name()
695+
.map(|s| s.to_ascii_lowercase())
696+
.unwrap_or_default();
697+
let current_repo = ctx.repo_name.to_ascii_lowercase();
698+
651699
// Pass 1: classify each discovered definition into "keep / skip
652700
// silently / skip with reason". The previous shape stuffed all of
653701
// this into a side-effecting `.filter()` closure that mutated
@@ -662,7 +710,9 @@ pub async fn resolve_definitions_via_discovery(
662710

663711
for d in discovered {
664712
let matches_filter = match normalized_filter.as_deref() {
665-
Some(src) => d.markers.iter().any(|m| m.source == src),
713+
Some(src) => d.markers.iter().any(|m| {
714+
m.source == src && marker_origin_matches(m, &current_org, &current_repo)
715+
}),
666716
None => true,
667717
};
668718

@@ -844,6 +894,7 @@ mod tests {
844894
source: "agents/foo.md".to_string(),
845895
version: "0.30.0".to_string(),
846896
target: "standalone".to_string(),
897+
..Default::default()
847898
}];
848899
assert!(is_direct_match(&def, &markers));
849900
}
@@ -859,6 +910,7 @@ mod tests {
859910
source: "agents/foo.md".to_string(),
860911
version: "0.30.0".to_string(),
861912
target: "standalone".to_string(),
913+
..Default::default()
862914
}];
863915
assert!(is_direct_match(&def, &markers));
864916
}
@@ -877,6 +929,7 @@ mod tests {
877929
source: "agents/foo.md".to_string(),
878930
version: "0.30.0".to_string(),
879931
target: "standalone".to_string(),
932+
..Default::default()
880933
}];
881934
assert!(!is_direct_match(&def, &markers));
882935
}
@@ -889,6 +942,7 @@ mod tests {
889942
source: "agents/foo.md".to_string(),
890943
version: "0.30.0".to_string(),
891944
target: "stage".to_string(),
945+
..Default::default()
892946
}];
893947
assert!(!is_direct_match(&def, &markers));
894948
}
@@ -902,12 +956,14 @@ mod tests {
902956
source: "agents/foo.md".to_string(),
903957
version: "0.30.0".to_string(),
904958
target: "stage".to_string(),
959+
..Default::default()
905960
},
906961
MarkerMetadata {
907962
schema: 1,
908963
source: "agents/bar.md".to_string(),
909964
version: "0.30.0".to_string(),
910965
target: "job".to_string(),
966+
..Default::default()
911967
},
912968
];
913969
// Multiple markers = at least one template is being included
@@ -972,6 +1028,62 @@ mod tests {
9721028
);
9731029
}
9741030

1031+
// ── marker_origin_matches ────────────────────────────────────────
1032+
1033+
#[test]
1034+
fn origin_matches_strict_when_marker_has_org_and_repo() {
1035+
let marker = MarkerMetadata {
1036+
org: "myorg".to_string(),
1037+
repo: "templates-a".to_string(),
1038+
source: "agents/foo.md".to_string(),
1039+
..Default::default()
1040+
};
1041+
assert!(marker_origin_matches(&marker, "myorg", "templates-a"));
1042+
assert!(!marker_origin_matches(&marker, "myorg", "templates-b"));
1043+
assert!(!marker_origin_matches(&marker, "otherorg", "templates-a"));
1044+
}
1045+
1046+
#[test]
1047+
fn origin_matches_case_insensitively() {
1048+
// ADO identifiers are case-insensitive. Marker fields are
1049+
// lower-cased at emit time, but a fixture or hand-edited
1050+
// marker might carry uppercase — accept either.
1051+
let marker = MarkerMetadata {
1052+
org: "MyOrg".to_string(),
1053+
repo: "Templates-A".to_string(),
1054+
source: "agents/foo.md".to_string(),
1055+
..Default::default()
1056+
};
1057+
assert!(marker_origin_matches(&marker, "myorg", "templates-a"));
1058+
}
1059+
1060+
#[test]
1061+
fn origin_matches_leniently_when_marker_org_repo_empty() {
1062+
// Legacy markers (pre-org/repo embed) and markers compiled
1063+
// outside an ADO checkout carry empty org/repo. Match anything
1064+
// so existing deployments keep working until recompiled.
1065+
let marker = MarkerMetadata {
1066+
source: "agents/foo.md".to_string(),
1067+
..Default::default()
1068+
};
1069+
assert!(marker_origin_matches(&marker, "myorg", "templates-a"));
1070+
assert!(marker_origin_matches(&marker, "", ""));
1071+
}
1072+
1073+
#[test]
1074+
fn origin_matches_strictly_when_only_one_field_empty() {
1075+
// If only one half of (org, repo) is set, we treat the marker
1076+
// as non-legacy and require both to match. Pre-empts a
1077+
// malformed fixture passing through the lenient path.
1078+
let half_marker = MarkerMetadata {
1079+
org: "myorg".to_string(),
1080+
repo: String::new(),
1081+
source: "agents/foo.md".to_string(),
1082+
..Default::default()
1083+
};
1084+
assert!(!marker_origin_matches(&half_marker, "myorg", "templates-a"));
1085+
}
1086+
9751087
// ── discovered_to_matched ────────────────────────────────────────
9761088

9771089
fn discovered(status: DiscoveryStatus) -> DiscoveredPipeline {
@@ -1027,12 +1139,14 @@ mod tests {
10271139
source: "agents/a.md".to_string(),
10281140
version: "1.0".to_string(),
10291141
target: "job".to_string(),
1142+
..Default::default()
10301143
},
10311144
MarkerMetadata {
10321145
schema: 1,
10331146
source: "agents/b.md".to_string(),
10341147
version: "1.0".to_string(),
10351148
target: "stage".to_string(),
1149+
..Default::default()
10361150
},
10371151
];
10381152
let matched = discovered_to_matched(&d).expect("Consumer kept");
@@ -1063,6 +1177,7 @@ mod tests {
10631177
source: "agents/##vso[task.setvariable variable=X]value.md".to_string(),
10641178
version: "1.0".to_string(),
10651179
target: "job".to_string(),
1180+
..Default::default()
10661181
}];
10671182
let matched = discovered_to_matched(&d).expect("Consumer kept");
10681183
assert!(

src/ado/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,17 @@ pub struct AdoContext {
7979
pub repo_name: String,
8080
}
8181

82+
impl AdoContext {
83+
/// Extract just the org slug from `org_url` (e.g.
84+
/// `https://dev.azure.com/MyOrg/` → `Some("MyOrg")`). Mirrors the
85+
/// inline parse in `CompileContext::ado_org`; lives here so
86+
/// non-compile callers (Preview-driven discovery) can reuse it.
87+
pub fn org_name(&self) -> Option<&str> {
88+
let org = self.org_url.trim_end_matches('/').rsplit('/').next()?;
89+
if org.is_empty() { None } else { Some(org) }
90+
}
91+
}
92+
8293
/// Parse the ADO org, project, and repo from a git remote URL.
8394
///
8495
/// Supports:

src/compile/extensions/ado_aw_marker.rs

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,28 @@ impl CompilerExtension for AdoAwMarkerExtension {
5656
let version = env!("CARGO_PKG_VERSION");
5757
let target = ctx.front_matter.target.as_str();
5858

59+
// ADO origin of the source markdown — disambiguates the
60+
// `source` field when two repos in the same project happen to
61+
// have files of the same name (e.g. both define `agents/foo.md`).
62+
// Lower-cased so case-insensitive ADO identifiers compare cleanly.
63+
// Empty strings when no ADO context could be inferred — production
64+
// runs always have one thanks to the non-GitHub-remote guard, but
65+
// unit-test contexts via `CompileContext::for_test` will not.
66+
let org = ctx
67+
.ado_org()
68+
.map(|s| s.to_ascii_lowercase())
69+
.unwrap_or_default();
70+
let repo = ctx
71+
.ado_context
72+
.as_ref()
73+
.map(|c| c.repo_name.to_ascii_lowercase())
74+
.unwrap_or_default();
75+
5976
let metadata_json = serde_json::json!({
6077
"schema": 1,
6178
"source": source,
79+
"org": org,
80+
"repo": repo,
6281
"version": version,
6382
"target": target,
6483
})
@@ -69,7 +88,7 @@ impl CompilerExtension for AdoAwMarkerExtension {
6988
// the build log at runtime, which is a free human-discoverability
7089
// bonus and costs nothing because the step runs in milliseconds.
7190
//
72-
// The echo's source value goes through two sanitisations:
91+
// The echo's user-controlled values go through two sanitisations:
7392
//
7493
// 1. `crate::sanitize::neutralize_pipeline_commands` neutralises
7594
// `##vso[` and `##[` prefixes by wrapping them in backticks.
@@ -84,16 +103,28 @@ impl CompilerExtension for AdoAwMarkerExtension {
84103
// filename containing `'` (e.g. `agents/foo's.md`) doesn't
85104
// produce syntactically broken bash. `version` and `target`
86105
// are controlled inputs and can't contain either.
106+
//
107+
// `org` and `repo` are derived from ADO remote parsing, which
108+
// already restricts them to a safe character set, but we apply
109+
// the same defence-in-depth pattern for consistency.
87110
let echo_source = bash_single_quote_escape(
88111
&crate::sanitize::neutralize_pipeline_commands(&source),
89112
);
113+
let echo_org = bash_single_quote_escape(
114+
&crate::sanitize::neutralize_pipeline_commands(&org),
115+
);
116+
let echo_repo = bash_single_quote_escape(
117+
&crate::sanitize::neutralize_pipeline_commands(&repo),
118+
);
90119
let step = format!(
91120
"- bash: |\n \
92121
# ado-aw-metadata: {metadata}\n \
93-
echo 'ado-aw metadata: source={echo_source} version={version} target={target}'\n \
122+
echo 'ado-aw metadata: source={echo_source} org={echo_org} repo={echo_repo} version={version} target={target}'\n \
94123
displayName: \"ado-aw\"\n",
95124
metadata = metadata_json,
96125
echo_source = echo_source,
126+
echo_org = echo_org,
127+
echo_repo = echo_repo,
97128
version = version,
98129
target = target,
99130
);
@@ -150,6 +181,49 @@ mod tests {
150181
assert!(step.contains("\"source\":\"agents/foo.md\""), "step missing source field:\n{step}");
151182
assert!(step.contains("\"target\":\"standalone\""), "step missing target field:\n{step}");
152183
assert!(step.contains("\"schema\":1"), "step missing schema field:\n{step}");
184+
// No ado_context => org/repo emit as empty strings.
185+
assert!(step.contains("\"org\":\"\""), "step missing org field:\n{step}");
186+
assert!(step.contains("\"repo\":\"\""), "step missing repo field:\n{step}");
187+
}
188+
189+
#[test]
190+
fn org_and_repo_embed_from_ado_context_lowercased() {
191+
// When the compiler runs inside an ADO checkout (the production
192+
// path — the non-GitHub-remote guard enforces this), the JSON
193+
// marker carries `org` and `repo` so discovery can disambiguate
194+
// a same-named `source` across two repos in the same project.
195+
let fm = parse_fm("name: t\ndescription: x\n");
196+
let input_path = Path::new("agents/foo.md");
197+
let ctx = CompileContext {
198+
agent_name: &fm.name,
199+
front_matter: &fm,
200+
ado_context: Some(crate::ado::AdoContext {
201+
org_url: "https://dev.azure.com/MyOrg".to_string(),
202+
project: "MyProject".to_string(),
203+
repo_name: "Templates-A".to_string(),
204+
}),
205+
engine: crate::engine::Engine::Copilot,
206+
compile_dir: None,
207+
input_path: Some(input_path),
208+
};
209+
let steps = AdoAwMarkerExtension.setup_steps(&ctx).unwrap();
210+
assert_eq!(steps.len(), 1);
211+
let step = &steps[0];
212+
// ADO identifiers are case-insensitive; lowercase to make
213+
// comparisons in discovery deterministic.
214+
assert!(
215+
step.contains("\"org\":\"myorg\""),
216+
"expected lowercased org field:\n{step}"
217+
);
218+
assert!(
219+
step.contains("\"repo\":\"templates-a\""),
220+
"expected lowercased repo field:\n{step}"
221+
);
222+
// The echo line surfaces them too for build-log readability.
223+
assert!(
224+
step.contains("org=myorg repo=templates-a"),
225+
"expected echo to include org/repo:\n{step}"
226+
);
153227
}
154228

155229
#[test]

src/detect.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ pub const MARKER_STEP_PREFIX: &str = "# ado-aw-metadata:";
140140
/// The schema is forward-compatible: unknown JSON fields are ignored,
141141
/// and missing fields fall through to defaults (empty string / zero).
142142
/// Callers that need a specific field should check it explicitly.
143-
#[derive(Debug, Clone, Deserialize, PartialEq, Eq)]
143+
#[derive(Debug, Clone, Default, Deserialize, PartialEq, Eq)]
144144
pub struct MarkerMetadata {
145145
/// Schema version; `1` for the initial release.
146146
#[serde(default)]
@@ -149,6 +149,22 @@ pub struct MarkerMetadata {
149149
/// `agents/release-readiness.md`).
150150
#[serde(default)]
151151
pub source: String,
152+
/// ADO organisation name the source markdown was compiled in
153+
/// (e.g. `myorg`). Lowercased at emit time. Combined with
154+
/// [`MarkerMetadata::repo`] this disambiguates the marker's
155+
/// `source` field when two repos in the same ADO project happen
156+
/// to have files of the same name (e.g. both define
157+
/// `agents/foo.md`). Empty string when the compiler ran outside
158+
/// an ADO checkout (rare in production thanks to the
159+
/// non-GitHub-remote guard).
160+
#[serde(default)]
161+
pub org: String,
162+
/// ADO repository name the source markdown was compiled in
163+
/// (e.g. `templates-a`). Lowercased at emit time. See
164+
/// [`MarkerMetadata::org`] for rationale. Empty string when the
165+
/// compiler ran outside an ADO checkout.
166+
#[serde(default)]
167+
pub repo: String,
152168
/// Compiler version that produced this YAML (`CARGO_PKG_VERSION`).
153169
#[serde(default)]
154170
pub version: String,

0 commit comments

Comments
 (0)