Commit 6ad1816
feat(compile): runtime prompt loading via {{#runtime-import}} markers (#625)
* feat(compile): runtime prompt loading via {{#runtime-import}} markers
Adopts gh-aw's {{#runtime-import path}} marker model and the inlined-imports front-matter toggle. Agent prompt bodies (and the Stage-2 threat-analysis prompt) are now loaded at pipeline runtime by default; body edits no longer require ado-aw compile.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(compile): introduce System phase so ado-script installs before user runtimes
ADO's NodeTool@0 prepends to PATH, so when both AdoScriptExtension (gate+import bundle, requires Node 20.x) and NodeExtension (user-pinned version) emit NodeTool@0 into the same Agent job, the LAST install wins on PATH. Previously AdoScript ran in the Tool phase, AFTER Runtime — so its hardcoded 20.x silently overrode the user's pinned version.
Introduces a new ExtensionPhase::System variant that sorts before Runtime. AdoScript moves to System: its NodeTool@0 install runs first, the resolver step uses it during a brief 20.x window, then the user's NodeExtension's NodeTool@0 runs and the user's version wins on PATH for everything after.
Adds test_node_runtime_install_orders_after_ado_script_so_user_version_wins pinning this ordering. Updates extension-count tests for the new phase. Refreshes ExtensionPhase docs and the collect_extensions ordering policy.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): guard runtime-import marker paths against whitespace and ##vso injection
Addresses three findings from the 2026-05-19 Rust PR review on #625:
1. Reject whitespace in agent source paths at compile time when
inlined-imports is false. The runtime resolver
(scripts/ado-script/src/import/index.ts) matches marker bodies with
[^\s}]+, so a space silently truncated the path and produced either
a misleading runtime `file not found` or, for optional markers, an
unexpanded marker visible to the LLM. Mirrors the existing
resolve_imports_inline guard.
2. Aggregate all import errors instead of reporting only the first.
Previously hadError ??= captured one failure and silently swallowed
subsequent ones. Now every missing/unreadable required marker emits
its own ##vso[task.logissue type=error] line before exit(1).
3. Sanitize rawPath in ##vso error output. Strips `]`, CR, and LF
from path strings embedded in diagnostic lines so an unusual
path can no longer break the ##vso command framing.
Tests:
* tests/compiler_tests.rs::test_runtime_imports_default_rejects_source_path_with_whitespace
* scripts/ado-script/src/import/__tests__/error-reporting.test.ts (2 cases)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): block path-traversal in runtime-import resolver and guard fixture helper
Two follow-up findings from the 2026-05-19 review on #625:
1. Reject `..` path components in both resolvers.
`resolve_imports_inline` (compile-time, inlined-imports: true) and
`import.js` (runtime, inlined-imports: false) both accepted
`../`-style paths without restriction. A malicious markdown body
on an untrusted PR branch could therefore embed host files (e.g.
`{{#runtime-import ../../../../etc/passwd}}`) into the compiled
YAML or, at runtime, into the agent prompt. The new guard rejects
any path whose `/` or `\\`-split segments include `..`,
regardless of whether the path is absolute or relative. Literal
`..` characters inside a filename (e.g. `name..md`) are still
allowed because they are not segments.
2. `compile_fixture_with_inlined_imports` now refuses fixtures that
already declare `inlined-imports:`.
The helper used to inject `inlined-imports: true` by raw string
substitution before the closing `---`. If a future fixture
hard-coded `inlined-imports: false`, the rewritten front matter
would have two `inlined-imports:` keys; serde_yaml silently uses
the last one so the test would still pass, but the duplicate-key
fixture is confusing and the helper would silently flip the
author's intent. The guard panics with an actionable message.
Tests:
* src/compile/extensions/ado_script.rs: 5 new unit tests covering
relative/embedded/absolute/backslash `..` rejection and the literal
`name..md` allow case.
* scripts/ado-script/src/import/__tests__/path-traversal.test.ts:
4 new vitest cases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(compile): drop unused ADO_AW_IMPORT_BASE env var
`import.js` consults `ADO_AW_IMPORT_BASE` only when resolving a
relative marker path (`isAbsolute(rawPath) ? rawPath : resolve(base,
rawPath)`). In the pipeline the only marker `import.js` ever sees is
the compiler-generated top-level body marker, which embeds an absolute
`$(Build.SourcesDirectory)/...` path. The resolver is also single-pass
by design, so author-written nested relative markers inside the inlined
body are never re-expanded at runtime. The env var was therefore dead
code at runtime.
Changes:
- Remove the `env:` block from `resolver_step()` in
`src/compile/extensions/ado_script.rs` (and update its unit test to
assert the variable is absent).
- Drop the `process.env.ADO_AW_IMPORT_BASE ??` fallback in
`scripts/ado-script/src/import/index.ts`; `import.js` now always
uses `dirname(argv[2])` for relative-path resolution (irrelevant in
pipeline use, useful for local invocations).
- Replace the `ADO_AW_IMPORT_BASE`-override vitest with a
`dirname(target)` default-base test that pins the fallback for
standalone callers.
- Update `docs/ado-script.md` and `docs/runtime-imports.md` to drop
the env-var contract and explain why the runtime never sees a
relative marker.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): correct threat-prompt doc claim and gate required_hosts on actual download
Two findings from the latest PR review:
1. docs/template-markers.md falsely claimed that the
`{{ threat_analysis_prompt }}` marker is emitted as a
`{{#runtime-import ...}}` when `inlined-imports: false`. The
threat-analysis prompt is tooling-shipped (compiled into the
`ado-aw` binary via `include_str!`) and unconditionally inlined at
step 11 of `compile_shared`. The marker is for the agent body, not
the threat prompt. Rewrote the paragraph to reflect this and to
cross-reference the rationale in `src/compile/common.rs`.
2. `AdoScriptExtension::required_hosts()` always requested
`github.com`, even when `inlined-imports: true` AND no filters were
configured (so neither `setup_steps()` nor `prepare_steps()` emits
the NodeTool@0 + curl pair, and github.com is therefore unreachable
from the pipeline). For a security-sensitive project, the
allowlist should match the actual network reach of the compiled
pipeline. Now returns `vec![]` unless `has_gate()` or
`runtime_imports_active()`. Added three unit tests covering all
three branches (no-consumer, gate-active, imports-active).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): reject absolute paths in compile-time runtime-import resolver
Three findings from the latest PR review:
1. **Security**: `resolve_imports_inline` accepted absolute paths
without restriction. When `inlined-imports: true`, an untrusted PR
branch could embed arbitrary host files into the compiled YAML via
`{{#runtime-import /home/runner/.ssh/id_rsa}}`,
`{{#runtime-import C:\Users\runner\secret.txt}}`, or
`{{#runtime-import \\server\share\file}}`. The PR description
already called out the `..`-traversal threat for "untrusted PR
branches" — the same threat applies here. Compile-time resolution
now requires a **relative** path rooted under `base_dir` (the
source `.md` file's parent directory, which is inside the repo and
therefore part of the same trust domain). `std::path::Path::is_absolute`
is platform-dependent, so the guard also explicitly checks
`/`-prefixed and `\\`-prefixed shapes.
`import.js` (runtime resolver) keeps its absolute-path support
unchanged — the agent VM is a lower-risk environment and the
pipeline-generated body marker is always absolute.
2. **Cleanup**: `AdoScriptExtension::has_gate()` and `setup_steps()`
both called `lower_pr_filters()` / `lower_pipeline_filters()`,
doing the same lowering twice. Factored into a single
`lowered_checks()` helper that both call sites reuse.
3. **Comment**: `sanitizeForVsoMessage` in `import.js` now explains
why `[` is intentionally NOT stripped (without a closing `]` and a
fresh `##vso` prefix it cannot open a new logging command).
Tests:
* `rejects_absolute_posix_path_at_compile_time`,
`rejects_absolute_windows_drive_path_at_compile_time`,
`rejects_unc_path_at_compile_time` in
`src/compile/extensions/ado_script.rs`.
* `supports_relative_and_absolute_paths` test removed (no longer
reachable behaviour); replaced with `supports_relative_path_resolution`.
* `docs/runtime-imports.md` updated to document the new restriction
and call out the security rationale.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): reject `}` in runtime-import paths to match the runtime regex
`import.js`'s marker regex `[^\s}]+` excludes `}` from the path capture
so the regex terminates cleanly at the closing `}}`. The compile-time
resolver (`resolve_imports_inline`) terminated only at `}}` and would
therefore silently accept a path like `foo}bar.md` that the runtime
resolver would either truncate or leave unexpanded — a real
compile-vs-runtime divergence.
Reject `}` in paths up front at compile time so the failure mode is one
clear compile error rather than two different runtime behaviours
depending on `inlined-imports`. Added a comment to `import.js`'s
regex documenting that the compile-time side enforces the same
restriction.
Test: `rejects_path_containing_closing_brace` in
`src/compile/extensions/ado_script.rs`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): make absolute-path check platform-independent for runtime-import
`Path::is_absolute` is platform-dependent — on Linux it doesn't recognize
Windows drive-letter paths like `C:\Users\...` as absolute, so
`rejects_absolute_windows_drive_path_at_compile_time` failed on Linux CI
with `file not found` instead of the expected absolute-path rejection.
Added an explicit drive-letter detector that matches `[A-Za-z]:[/\\]`
via pure string inspection, so the guard fires on every host where
`ado-aw compile` may run regardless of the local platform's path
semantics.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): reject absolute paths in runtime resolver and pass --base from pipeline
The compile-time `resolve_imports_inline` already rejected absolute
paths, but `import.js` allowed them at runtime with a misleading
"Mirrors `resolve_imports_inline`" comment. Although the documented
single-pass design currently prevents author-written nested markers
from reaching `import.js`, the asymmetry was a defence-in-depth gap:
- the resolver explicitly says it handles "arbitrary author-written
markers in the agent body";
- if the design ever shifts to multi-pass, an author marker like
`{{#runtime-import /tmp/awf-tools/staging/mcpg-config.json}}` could
silently embed credentials into the agent's system prompt;
- the runtime and compile-time paths should enforce the same policy
so reviewers don't have to mentally track which guards apply where.
Changes:
* `scripts/ado-script/src/import/index.ts`
- Add `--base <path>` CLI arg (replaces the implicit `dirname(target)`
fallback for pipeline use; the fallback remains for standalone
invocation).
- Reject absolute paths the same way Rust does: POSIX `/foo`,
Windows drive-letter `C:\foo`/`C:/foo` (detected via a
platform-independent character scan because `path.isAbsolute` is
OS-dependent on this), and UNC `\\server\share`.
* `src/compile/common.rs`
- Emit a trigger-repo-relative marker (stripping
`$(Build.SourcesDirectory)/` from the resolved source path) so the
new absolute-path reject in `import.js` doesn't fire on the
compiler-generated body marker. The relative-form marker is the
only form `import.js` ever needs to resolve at runtime.
* `src/compile/extensions/ado_script.rs`
- Resolver step passes `--base "$(Build.SourcesDirectory)"` so the
relative marker resolves against the trigger-repo checkout root.
Tests:
* New vitest cases for the three absolute-path shapes (POSIX,
drive-letter, UNC) and for `--base` resolution.
* Replaced the "uses absolute snippet paths as-is" test with a
rejection assertion.
* Unit test updated to assert the resolver step now includes
`--base "$(Build.SourcesDirectory)"`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): reject `}` in runtime-import source path and fix stale extension refs
Two findings from the latest review:
1. **Bug**: `agent_marker_path` in `compile_shared` was only guarded
against whitespace, not `}`. An agent file at `agents/fo}o.md`
(valid on Linux/macOS/NTFS) would emit a malformed marker that the
runtime regex `[^\s}]+` truncates at the `}` and then fails to
match because the next two chars aren't `}}`. The marker would
survive as literal text in the agent's prompt with no error
surfaced. Added an explicit `}` check that mirrors the same guard
in `resolve_imports_inline`, with a clear error suggesting either
renaming the path or setting `inlined-imports: true`.
2. **Stale refs**: `TriggerFiltersExtension` and
`src/compile/extensions/trigger_filters.rs` were referenced in:
- `src/compile/pr_filters.rs` (3 comments)
- `.github/workflows/ado-script.yml` (path triggers)
- `tests/bash_lint_tests.rs` (annotation comment)
- `scripts/ado-script/README.md` (Bundles + Layout + See also)
- `site/src/content/docs/reference/filter-ir.mdx` (Integration
Points section + Gate Step Injection)
All updated to point at `AdoScriptExtension` /
`src/compile/extensions/ado_script.rs` (the consolidated extension
introduced earlier in this PR). The site doc was also extended to
note the additional `prepare_steps()` hook the extension owns for
the runtime-import resolver.
Tests:
* `test_runtime_imports_default_rejects_source_path_with_closing_brace`
in `tests/compiler_tests.rs` — exercises a source path with `}`
and asserts the compiler errors with the new guard.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>1 parent 1130ba4 commit 6ad1816
54 files changed
Lines changed: 3834 additions & 969 deletions
File tree
- .github/workflows
- docs
- scripts/ado-script
- src/import
- __tests__
- test
- fixtures/import
- site/src/content/docs/reference
- src/compile
- extensions
- tests
- fixtures
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
| 8 | + | |
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
| 72 | + | |
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
76 | | - | |
| 76 | + | |
77 | 77 | | |
78 | | - | |
| 78 | + | |
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
65 | | - | |
| 65 | + | |
66 | 66 | | |
67 | 67 | | |
68 | 68 | | |
| |||
159 | 159 | | |
160 | 160 | | |
161 | 161 | | |
162 | | - | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
163 | 165 | | |
164 | 166 | | |
165 | 167 | | |
| |||
172 | 174 | | |
173 | 175 | | |
174 | 176 | | |
175 | | - | |
| 177 | + | |
176 | 178 | | |
177 | 179 | | |
178 | 180 | | |
| |||
196 | 198 | | |
197 | 199 | | |
198 | 200 | | |
| 201 | + | |
| 202 | + | |
199 | 203 | | |
200 | 204 | | |
201 | 205 | | |
| |||
242 | 246 | | |
243 | 247 | | |
244 | 248 | | |
245 | | - | |
| 249 | + | |
246 | 250 | | |
247 | 251 | | |
248 | 252 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
7 | | - | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| |||
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
36 | 66 | | |
37 | 67 | | |
38 | 68 | | |
| |||
147 | 177 | | |
148 | 178 | | |
149 | 179 | | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
156 | 189 | | |
157 | | - | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
158 | 193 | | |
159 | 194 | | |
160 | 195 | | |
| |||
195 | 230 | | |
196 | 231 | | |
197 | 232 | | |
198 | | - | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
199 | 240 | | |
200 | | - | |
201 | | - | |
202 | | - | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
203 | 245 | | |
204 | 246 | | |
205 | 247 | | |
| |||
209 | 251 | | |
210 | 252 | | |
211 | 253 | | |
212 | | - | |
213 | | - | |
214 | | - | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
215 | 290 | | |
216 | 291 | | |
217 | 292 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
350 | 350 | | |
351 | 351 | | |
352 | 352 | | |
353 | | - | |
| 353 | + | |
354 | 354 | | |
355 | | - | |
356 | | - | |
357 | | - | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
358 | 362 | | |
359 | 363 | | |
360 | | - | |
| 364 | + | |
361 | 365 | | |
362 | 366 | | |
363 | | - | |
| 367 | + | |
364 | 368 | | |
365 | | - | |
| 369 | + | |
366 | 370 | | |
367 | | - | |
| 371 | + | |
368 | 372 | | |
369 | | - | |
370 | | - | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
371 | 377 | | |
372 | 378 | | |
373 | 379 | | |
| |||
379 | 385 | | |
380 | 386 | | |
381 | 387 | | |
382 | | - | |
| 388 | + | |
383 | 389 | | |
384 | 390 | | |
385 | 391 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
234 | 234 | | |
235 | 235 | | |
236 | 236 | | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
237 | 258 | | |
238 | 259 | | |
239 | 260 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
0 commit comments