Skip to content

Commit 17e372c

Browse files
fix(safeoutputs): support glob wildcards anywhere in allowed-tags patterns (#442)
* fix(safeoutputs): support glob wildcards anywhere in allowed-tags patterns tag_matches_pattern only handled a trailing '*' (prefix match). Patterns like 'copilot:repo=msazuresphere/4x4/*@main' silently fell through to exact-match comparison, rejecting valid tags such as 'copilot:repo=msazuresphere/4x4/VsCodeExtension@main'. Switch to glob_match::glob_match (already a dependency) so '*' works in any position. Also consolidate the inline matching in add_build_tag.rs to use the shared tag_matches_pattern helper, gaining case-insensitive matching it was previously missing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(safeoutputs): replace hand-rolled globs with shared wildcard_match Address PR review feedback: - Replace glob_match::glob_match with a purpose-built wildcard_match() that treats * as matching any character including /. Tags and artifact names are not file paths, so / should not act as a segment separator. - Add name_matches_pattern() (case-sensitive) alongside the existing tag_matches_pattern() (case-insensitive) for artifact name allow-lists. - Consolidate upload_build_attachment.rs and upload_pipeline_artifact.rs to use name_matches_pattern instead of inline strip_suffix + starts_with. - Leave queue_build.rs branch matching as-is — it has intentionally different semantics (case-sensitive, requires / separator). - Update docs/safe-outputs.md to describe * wildcard support instead of the old 'prefix wildcards' language. - Add comprehensive tests for wildcard_match, slash-crossing, and name_matches_pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4f2f188 commit 17e372c

5 files changed

Lines changed: 174 additions & 36 deletions

File tree

docs/safe-outputs.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ Creates an Azure DevOps work item.
7272
- `iteration-path` - Iteration path for the work item
7373
- `assignee` - User to assign (email or display name)
7474
- `tags` - Static list of tags always applied to the work item (regardless of agent input)
75-
- `allowed-tags` - Allowlist of tags the agent is permitted to use via the `tags` parameter. If empty, any agent-provided tags are accepted. Supports prefix wildcards: entries ending with `*` match by prefix (e.g., `"agent-*"` matches `"agent-created"`, `"agent-review"`, etc.).
75+
- `allowed-tags` - Allowlist of tags the agent is permitted to use via the `tags` parameter. If empty, any agent-provided tags are accepted. Supports `*` wildcards anywhere in the pattern (e.g., `"agent-*"` matches `"agent-created"`; `"copilot:repo=org/project/*@main"` matches any repo name).
7676
- `custom-fields` - Map of custom field reference names to values (e.g., `Custom.MyField: "value"`)
7777
- `max` - Maximum number of create-work-item outputs allowed per run (default: 1)
7878
- `include-stats` - Whether to append agent execution stats to the work item description (default: true)
@@ -112,7 +112,7 @@ safe-outputs:
112112
iteration-path: true # enable iteration path updates (default: false)
113113
assignee: true # enable assignee updates (default: false)
114114
tags: true # enable tag updates (default: false)
115-
allowed-tags: [] # Optional — restrict which tags the agent can set (empty = any; supports prefix wildcards like "agent-*")
115+
allowed-tags: [] # Optional — restrict which tags the agent can set (empty = any; supports * wildcards like "agent-*")
116116
```
117117
118118
**Security note:** Every field that can be modified requires explicit opt-in (`true`) in the front matter configuration. If the `max` limit is exceeded, additional entries are skipped rather than aborting the entire batch.
@@ -378,7 +378,7 @@ Adds a tag to an Azure DevOps build.
378378
```yaml
379379
safe-outputs:
380380
add-build-tag:
381-
allowed-tags: [] # Optional — restrict which tags can be applied (supports prefix wildcards)
381+
allowed-tags: [] # Optional — restrict which tags can be applied (supports * wildcards)
382382
tag-prefix: "agent-" # Optional — prefix prepended to all tags
383383
allow-any-build: false # When false, only the current pipeline build can be tagged (default: false)
384384
max: 1 # Maximum per run (default: 1)

src/safeoutputs/add_build_tag.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,10 @@ impl Executor for AddBuildTagResult {
161161

162162
// 4. Validate against allowed tags (if non-empty)
163163
if !config.allowed_tags.is_empty() {
164-
let allowed = config.allowed_tags.iter().any(|pattern| {
165-
if let Some(prefix) = pattern.strip_suffix('*') {
166-
final_tag.starts_with(prefix)
167-
} else {
168-
*pattern == final_tag
169-
}
170-
});
164+
let allowed = config
165+
.allowed_tags
166+
.iter()
167+
.any(|pattern| super::tag_matches_pattern(&final_tag, pattern));
171168
if !allowed {
172169
return Ok(ExecutionResult::failure(format!(
173170
"Tag '{}' is not in the allowed tags list",

src/safeoutputs/mod.rs

Lines changed: 159 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -209,22 +209,73 @@ pub(crate) fn resolve_repo_name(
209209
}
210210
}
211211

212+
/// Match a `value` against a `pattern` where `*` matches zero or more of **any**
213+
/// character (including `/`).
214+
///
215+
/// Unlike file-path glob matching, `/` is **not** treated as a segment separator —
216+
/// these patterns are used for tags, artifact names, and similar non-path strings.
217+
///
218+
/// Only the `*` wildcard is supported; there is no `?`, `[…]`, or `**` syntax.
219+
/// Literal `*` characters cannot be escaped — this is intentional since the values
220+
/// being matched (ADO tags, artifact names) cannot contain `*`.
221+
pub(crate) fn wildcard_match(pattern: &str, value: &str) -> bool {
222+
let p = pattern.as_bytes();
223+
let v = value.as_bytes();
224+
let (pn, vn) = (p.len(), v.len());
225+
226+
let mut pi = 0;
227+
let mut vi = 0;
228+
// Saved positions for backtracking on `*`
229+
let mut star_p = usize::MAX;
230+
let mut star_v: usize = 0;
231+
232+
while vi < vn {
233+
if pi < pn && p[pi] == b'*' {
234+
star_p = pi;
235+
star_v = vi;
236+
pi += 1;
237+
} else if pi < pn && p[pi] == v[vi] {
238+
pi += 1;
239+
vi += 1;
240+
} else if star_p != usize::MAX {
241+
// Backtrack: let the last `*` consume one more character
242+
pi = star_p + 1;
243+
star_v += 1;
244+
vi = star_v;
245+
} else {
246+
return false;
247+
}
248+
}
249+
250+
// Consume any trailing `*`s in the pattern
251+
while pi < pn && p[pi] == b'*' {
252+
pi += 1;
253+
}
254+
255+
pi == pn
256+
}
257+
212258
/// Return `true` if `tag` is matched by `pattern`.
213259
///
214-
/// Pattern matching rules (consistent with `add-build-tag` and `allowed-labels` in gh-aw):
215-
/// - Patterns ending with `*` are prefix wildcards: `"agent-*"` matches any tag whose
216-
/// prefix (before the `*`) case-insensitively equals the start of `tag`.
217-
/// - All other patterns are compared with case-insensitive exact equality.
260+
/// Uses [`wildcard_match`] with **case-insensitive** comparison. `*` in the
261+
/// pattern matches zero or more of any character (including `/`), so
262+
/// `copilot:repo=org/project/*@main` correctly matches
263+
/// `copilot:repo=org/project/MyRepo@main`.
218264
///
219-
/// Both comparisons are **case-insensitive** so that an operator who writes
220-
/// `allowed-tags: ["Agent-*"]` correctly matches an agent-provided tag `"agent-created"`.
265+
/// This is the shared matcher for `allowed-tags` in `create-work-item`,
266+
/// `update-work-item`, and `add-build-tag`.
221267
pub(crate) fn tag_matches_pattern(tag: &str, pattern: &str) -> bool {
222-
if let Some(prefix) = pattern.strip_suffix('*') {
223-
tag.to_ascii_lowercase()
224-
.starts_with(&prefix.to_ascii_lowercase())
225-
} else {
226-
pattern.eq_ignore_ascii_case(tag)
227-
}
268+
wildcard_match(
269+
&pattern.to_ascii_lowercase(),
270+
&tag.to_ascii_lowercase(),
271+
)
272+
}
273+
274+
/// Return `true` if `name` is matched by `pattern` (**case-sensitive**).
275+
///
276+
/// Uses [`wildcard_match`] for artifact-name allow-lists where case matters.
277+
pub(crate) fn name_matches_pattern(name: &str, pattern: &str) -> bool {
278+
wildcard_match(pattern, name)
228279
}
229280

230281
/// Validate a string against `git check-ref-format` rules.
@@ -481,6 +532,55 @@ mod tests {
481532
assert!(validate_git_ref_name("release/2026-04-17", "b").is_ok());
482533
}
483534

535+
// ─── wildcard_match ─────────────────────────────────────────────────
536+
537+
#[test]
538+
fn test_wildcard_match_exact() {
539+
assert!(wildcard_match("hello", "hello"));
540+
assert!(!wildcard_match("hello", "world"));
541+
}
542+
543+
#[test]
544+
fn test_wildcard_match_star_any() {
545+
assert!(wildcard_match("*", "anything"));
546+
assert!(wildcard_match("*", ""));
547+
assert!(wildcard_match("*", "a/b/c"));
548+
}
549+
550+
#[test]
551+
fn test_wildcard_match_trailing_star() {
552+
assert!(wildcard_match("agent-*", "agent-created"));
553+
assert!(wildcard_match("agent-*", "agent-"));
554+
assert!(!wildcard_match("agent-*", "bot-created"));
555+
}
556+
557+
#[test]
558+
fn test_wildcard_match_middle_star() {
559+
assert!(wildcard_match("a*z", "az"));
560+
assert!(wildcard_match("a*z", "abcz"));
561+
assert!(!wildcard_match("a*z", "abcy"));
562+
}
563+
564+
#[test]
565+
fn test_wildcard_match_star_crosses_slash() {
566+
// Unlike file-path globs, * matches across /
567+
assert!(wildcard_match("team/*", "team/sub/item"));
568+
assert!(wildcard_match("prefix/*@main", "prefix/a/b/c@main"));
569+
}
570+
571+
#[test]
572+
fn test_wildcard_match_multiple_stars() {
573+
assert!(wildcard_match("*-*", "a-b"));
574+
assert!(wildcard_match("*-*", "abc-def"));
575+
assert!(!wildcard_match("*-*", "abc"));
576+
}
577+
578+
#[test]
579+
fn test_wildcard_match_case_sensitive() {
580+
// wildcard_match itself is case-sensitive
581+
assert!(!wildcard_match("Hello", "hello"));
582+
}
583+
484584
// ─── tag_matches_pattern ───────────────────────────────────────────────
485585

486586
#[test]
@@ -515,4 +615,51 @@ mod tests {
515615
assert!(tag_matches_pattern("anything", "*"));
516616
assert!(tag_matches_pattern("", "*"));
517617
}
618+
619+
#[test]
620+
fn test_tag_matches_pattern_middle_wildcard() {
621+
// Glob wildcard in the middle of the pattern
622+
assert!(tag_matches_pattern(
623+
"copilot:repo=msazuresphere/4x4/VsCodeExtension@main",
624+
"copilot:repo=msazuresphere/4x4/*@main"
625+
));
626+
assert!(tag_matches_pattern(
627+
"copilot:repo=msazuresphere/4x4/DevTools@main",
628+
"copilot:repo=msazuresphere/4x4/*@main"
629+
));
630+
// Wrong suffix should not match
631+
assert!(!tag_matches_pattern(
632+
"copilot:repo=msazuresphere/4x4/DevTools@dev",
633+
"copilot:repo=msazuresphere/4x4/*@main"
634+
));
635+
}
636+
637+
#[test]
638+
fn test_tag_matches_pattern_middle_wildcard_case_insensitive() {
639+
assert!(tag_matches_pattern(
640+
"Copilot:Repo=MSAzureSphere/4x4/Tools@Main",
641+
"copilot:repo=msazuresphere/4x4/*@main"
642+
));
643+
}
644+
645+
#[test]
646+
fn test_tag_matches_pattern_star_crosses_slash() {
647+
// Hierarchical tags: * must match across /
648+
assert!(tag_matches_pattern("team/subgroup/item", "team/*"));
649+
}
650+
651+
// ─── name_matches_pattern ───────────────────────────────────────────────
652+
653+
#[test]
654+
fn test_name_matches_pattern_case_sensitive() {
655+
assert!(name_matches_pattern("report", "report"));
656+
assert!(!name_matches_pattern("Report", "report"));
657+
}
658+
659+
#[test]
660+
fn test_name_matches_pattern_wildcard() {
661+
assert!(name_matches_pattern("agent-report-123", "agent-*"));
662+
assert!(name_matches_pattern("agent-report", "agent-*"));
663+
assert!(!name_matches_pattern("bot-report", "agent-*"));
664+
}
518665
}

src/safeoutputs/upload_build_attachment.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -346,13 +346,10 @@ impl Executor for UploadBuildAttachmentResult {
346346

347347
// Check artifact-name allow-list (if configured).
348348
if !config.allowed_artifact_names.is_empty() {
349-
let allowed = config.allowed_artifact_names.iter().any(|pattern| {
350-
if let Some(prefix) = pattern.strip_suffix('*') {
351-
final_name.starts_with(prefix)
352-
} else {
353-
*pattern == final_name
354-
}
355-
});
349+
let allowed = config
350+
.allowed_artifact_names
351+
.iter()
352+
.any(|pattern| super::name_matches_pattern(&final_name, pattern));
356353
if !allowed {
357354
return Ok(ExecutionResult::failure(format!(
358355
"Artifact name '{}' is not in the allowed list",

src/safeoutputs/upload_pipeline_artifact.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,10 @@ impl Executor for UploadPipelineArtifactResult {
321321

322322
// ── Artifact-name allow-list ─────────────────────────────────────
323323
if !config.allowed_artifact_names.is_empty() {
324-
let allowed = config.allowed_artifact_names.iter().any(|pattern| {
325-
if let Some(prefix) = pattern.strip_suffix('*') {
326-
final_name.starts_with(prefix)
327-
} else {
328-
*pattern == final_name
329-
}
330-
});
324+
let allowed = config
325+
.allowed_artifact_names
326+
.iter()
327+
.any(|pattern| super::name_matches_pattern(&final_name, pattern));
331328
if !allowed {
332329
return Ok(ExecutionResult::failure(format!(
333330
"Artifact name '{}' is not in the allowed list",

0 commit comments

Comments
 (0)