Skip to content

Commit 162f5ad

Browse files
jamesadevineCopilot
andcommitted
fix(compile): docs syntax, ADO expression injection, refs/heads stripping
Documentation fixes: - front-matter.md: all PatternFilter examples updated to bare string glob syntax (was using removed {match: ...} object form + regex) - filter-ir.md: all RegexMatch/regex_match references updated to GlobMatch/glob_match Security: - expression escape hatch now validated against ADO expressions via contains_ado_expression() — blocks macro injection ADO branch prefix handling: - gate-eval.py strips refs/heads/, refs/tags/, refs/pull/ from branch fact values so patterns like 'feature/*' match naturally - Pattern side also stripped so 'refs/heads/feature/*' matches too - 5 new Python tests for ref prefix stripping Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f8a5013 commit 162f5ad

5 files changed

Lines changed: 65 additions & 21 deletions

File tree

docs/filter-ir.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ supports these predicate types:
113113

114114
| Predicate | Bash Shape | Example |
115115
|-----------|-----------|---------|
116-
| `RegexMatch { fact, pattern }` | `echo "$VAR" \| grep -qE 'pattern'` | Title matches `\[review\]` |
116+
| `GlobMatch { fact, pattern }` | `fnmatch(value, pattern)` | Title matches `*[review]*` |
117117
| `Equality { fact, value }` | `[ "$VAR" = "value" ]` | Draft is `false` |
118118
| `ValueInSet { fact, values, case_insensitive }` | `echo "$VAR" \| grep -q[i]E '^(a\|b)$'` | Author in allow-list |
119119
| `ValueNotInSet { fact, values, case_insensitive }` | Inverse of `ValueInSet` | Author not in block-list |
@@ -169,12 +169,12 @@ Maps each field of `PrFilters` to a `FilterCheck`:
169169

170170
| Field | Predicate | Fact(s) | Tag Suffix |
171171
|-------|-----------|---------|------------|
172-
| `title` | `RegexMatch` | `PrTitle` | `title-mismatch` |
172+
| `title` | `GlobMatch` | `PrTitle` | `title-mismatch` |
173173
| `author.include` | `ValueInSet` (case-insensitive) | `AuthorEmail` | `author-mismatch` |
174174
| `author.exclude` | `ValueNotInSet` (case-insensitive) | `AuthorEmail` | `author-excluded` |
175-
| `source_branch` | `RegexMatch` | `SourceBranch` | `source-branch-mismatch` |
176-
| `target_branch` | `RegexMatch` | `TargetBranch` | `target-branch-mismatch` |
177-
| `commit_message` | `RegexMatch` | `CommitMessage` | `commit-message-mismatch` |
175+
| `source_branch` | `GlobMatch` | `SourceBranch` | `source-branch-mismatch` |
176+
| `target_branch` | `GlobMatch` | `TargetBranch` | `target-branch-mismatch` |
177+
| `commit_message` | `GlobMatch` | `CommitMessage` | `commit-message-mismatch` |
178178
| `labels` | `LabelSetMatch` | `PrLabels` (→ `PrMetadata`) | `labels-mismatch` |
179179
| `draft` | `Equality` | `PrIsDraft` (→ `PrMetadata`) | `draft-mismatch` |
180180
| `changed_files` | `FileGlobMatch` | `ChangedFiles` | `changed-files-mismatch` |
@@ -187,8 +187,8 @@ Maps each field of `PrFilters` to a `FilterCheck`:
187187

188188
| Field | Predicate | Fact(s) | Tag Suffix |
189189
|-------|-----------|---------|------------|
190-
| `source_pipeline` | `RegexMatch` | `TriggeredByPipeline` | `source-pipeline-mismatch` |
191-
| `branch` | `RegexMatch` | `TriggeringBranch` | `branch-mismatch` |
190+
| `source_pipeline` | `GlobMatch` | `TriggeredByPipeline` | `source-pipeline-mismatch` |
191+
| `branch` | `GlobMatch` | `TriggeringBranch` | `branch-mismatch` |
192192
| `time_window` | `TimeWindow` | `CurrentUtcMinutes` | `time-window-mismatch` |
193193
| `build_reason.include` | `ValueInSet` | `BuildReason` | `build-reason-mismatch` |
194194
| `build_reason.exclude` | `ValueNotInSet` | `BuildReason` | `build-reason-excluded` |
@@ -286,7 +286,7 @@ quoting issues. Decoded, it contains:
286286
"checks": [
287287
{
288288
"name": "title",
289-
"predicate": {"type": "regex_match", "fact": "pr_title", "pattern": "\\[review\\]"},
289+
"predicate": {"type": "glob_match", "fact": "pr_title", "pattern": "*[review]*"},
290290
"tag_suffix": "title-mismatch"
291291
},
292292
{
@@ -335,7 +335,7 @@ The bash shim exports only the ADO macros needed by the spec's facts:
335335

336336
| `type` | Fields | Description |
337337
|--------|--------|-------------|
338-
| `regex_match` | `fact`, `pattern` | Python `re.search()` |
338+
| `glob_match` | `fact`, `pattern` | Glob match (`*` any chars, `?` single char) |
339339
| `equals` | `fact`, `value` | Exact string equality |
340340
| `value_in_set` | `fact`, `values`, `case_insensitive` | Value membership |
341341
| `value_not_in_set` | `fact`, `values`, `case_insensitive` | Inverse membership |
@@ -424,3 +424,4 @@ step-by-step guide. In summary:
424424
`lower_pipeline_filters`)
425425
6. Add validation rules if the new filter can conflict with existing ones
426426
7. Write tests: lowering, validation, spec serialization, and evaluator
427+

docs/front-matter.md

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,7 @@ on: # trigger configuration (unified under on: key)
8080
- main
8181
- release/*
8282
filters: # optional runtime filters (compiled to gate step)
83-
source-pipeline:
84-
match: "Build.*"
83+
source-pipeline: "Build*"
8584
time-window:
8685
start: "09:00"
8786
end: "17:00"
@@ -91,19 +90,15 @@ on: # trigger configuration (unified under on: key)
9190
paths:
9291
include: [src/*]
9392
filters: # runtime PR filters (compiled to gate step)
94-
title:
95-
match: "\\[review\\]"
93+
title: "*[review]*"
9694
author:
9795
include: ["alice@corp.com"]
9896
draft: false
9997
labels:
10098
any-of: ["run-agent"]
101-
source-branch:
102-
match: "^feature/.*"
103-
target-branch:
104-
match: "^main$"
105-
commit-message:
106-
match: "^(?!.*\\[skip-agent\\])"
99+
source-branch: "feature/*"
100+
target-branch: "main"
101+
commit-message: "*[skip-agent]*"
107102
changed-files:
108103
include: ["src/**/*.rs"]
109104
min-changes: 5

scripts/gate-eval.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@
1919
"changed_file_count": ["changed_files"],
2020
}
2121

22+
# ADO branch variables return refs/heads/... or refs/pull/... prefixed values.
23+
# Strip the prefix so user patterns like "feature/*" match naturally.
24+
_REF_PREFIXES = ("refs/heads/", "refs/tags/", "refs/pull/")
25+
26+
def _strip_ref_prefix(value):
27+
"""Strip refs/heads/ (or similar) prefix from a branch/ref value."""
28+
for prefix in _REF_PREFIXES:
29+
if value.startswith(prefix):
30+
return value[len(prefix):]
31+
return value
32+
2233
# ─── Fact acquisition ────────────────────────────────────────────────────────
2334

2435
def acquire_fact(kind, acquired):
@@ -35,7 +46,13 @@ def acquire_fact(kind, acquired):
3546
"triggering_branch": "ADO_TRIGGERING_BRANCH",
3647
}
3748
if kind in env_facts:
38-
return os.environ.get(env_facts[kind], "")
49+
value = os.environ.get(env_facts[kind], "")
50+
# ADO branch variables include refs/heads/ prefix — strip it
51+
# so user patterns like "feature/*" match without the prefix.
52+
# Also strip from the pattern side in glob_match (below).
53+
if kind in ("source_branch", "target_branch", "triggering_branch"):
54+
value = _strip_ref_prefix(value)
55+
return value
3956

4057
if kind == "pr_metadata":
4158
return _fetch_pr_metadata()
@@ -125,7 +142,7 @@ def evaluate(pred, facts):
125142
# Simple glob: * matches anything, ? matches single char.
126143
# Brackets are NOT character classes (treated literally).
127144
import re as _re
128-
pattern = pred["pattern"]
145+
pattern = _strip_ref_prefix(pred["pattern"])
129146
# Escape everything except * and ?, then convert * → .* and ? → .
130147
regex = _re.escape(pattern).replace(r"\*", ".*").replace(r"\?", ".")
131148
return bool(_re.fullmatch(regex, value))

src/compile/common.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2019,6 +2019,13 @@ pub async fn compile_shared(
20192019

20202020
// Validate expression escape hatches against injection
20212021
for expr in &expressions {
2022+
if crate::validate::contains_ado_expression(expr) {
2023+
anyhow::bail!(
2024+
"Filter expression contains ADO expression ('${{{{', '$(', or '$[') which could \
2025+
exfiltrate secrets or escalate permissions. Found: '{}'",
2026+
expr
2027+
);
2028+
}
20222029
if crate::validate::contains_template_marker(expr) {
20232030
anyhow::bail!(
20242031
"Filter expression contains template marker '{{{{' which could cause injection. Found: '{}'",

tests/gate_eval_tests.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,30 @@
2121

2222
evaluate = gate_eval.evaluate
2323
predicate_facts = gate_eval.predicate_facts
24+
_strip_ref_prefix = gate_eval._strip_ref_prefix
25+
26+
27+
# ─── Ref prefix stripping tests ─────────────────────────────────────────────
28+
29+
30+
class TestStripRefPrefix:
31+
def test_refs_heads(self):
32+
assert _strip_ref_prefix("refs/heads/feature/my-branch") == "feature/my-branch"
33+
34+
def test_refs_tags(self):
35+
assert _strip_ref_prefix("refs/tags/v1.0.0") == "v1.0.0"
36+
37+
def test_refs_pull(self):
38+
assert _strip_ref_prefix("refs/pull/42/merge") == "42/merge"
39+
40+
def test_no_prefix(self):
41+
assert _strip_ref_prefix("main") == "main"
42+
43+
def test_pattern_stripping_in_glob(self):
44+
"""User patterns like refs/heads/feature/* should match feature/my-branch"""
45+
pred = {"type": "glob_match", "fact": "source_branch", "pattern": "refs/heads/feature/*"}
46+
facts = {"source_branch": "feature/my-branch"}
47+
assert evaluate(pred, facts) is True
2448

2549

2650
# ─── Predicate evaluation tests ─────────────────────────────────────────────

0 commit comments

Comments
 (0)