Skip to content

Commit 4a3f999

Browse files
jamesadevineCopilot
andcommitted
fix(ado-script): address code-review findings (CVE bump, hardening, pagination, timeouts)
Addresses every blocker and should-fix item from the deep code review of the new scripts/ado-script/ TypeScript workspace. Blocking - vitest 2.1.9 -> 4.1.6: clears all 5 moderate-severity npm audit findings (vitest / vite / esbuild / @vitest/mocker / vite-node). No test API changes needed -- all 172 existing tests pass unmodified on Vitest 4. - GATE_SPEC DoS guard (src/gate/index.ts): cap base64-decoded spec at 256 KiB before JSON.parse. ADO env vars are ~32 KiB max so this is two orders of magnitude above any realistic spec. Hardening - Glob ReDoS in-place hardening (src/gate/predicates.ts): - cap pattern length at 1024 chars - cap '*' wildcards at 64 per pattern - pre-compile RegExp cache (Map) with 1024-entry FIFO eviction - rejected patterns log a warning + return false (fail-closed) - + 3 new vitest cases - Pre-flight predicate-tree validator (validatePredicateTree, called from index.ts main right after JSON.parse): walks the full tree and throws on any unknown 'type' discriminant *before* fact acquisition. Closes the gap where an unknown predicate was only surfaced when evaluatePredicate reached it -- if the required fact was unavailable, the unknown type was silently skipped. + 7 new vitest cases covering each known type, nested unknowns under and/or/not, missing operands/operand. ADO REST client (src/shared/ado-client.ts) - 30-second per-attempt timeout via Promise + setTimeout (configurable via ADO_API_TIMEOUT_MS env var); timeouts are treated as transient so the existing one-shot retry path catches them. + 2 new vitest cases. - getIterationChanges now paginates via 'top'/'skip' (page size 100, 100- page cap, 'short page' termination); + 2 new vitest cases. The SDK does not paginate getPullRequestIterations (ADO REST API returns all in one call) so that one is documented but left as-is. Operational - Lazy import of azure-devops-node-api in src/shared/auth.ts (dynamic import inside getWebApi). Entry-point bundle dropped from ~1.1 MB to 78 KB; the SDK is now in a separate 2.7 MB ncc chunk loaded only when an ADO API call is made. Bypass-only / pipeline-var-only runs save the cold-start tax. 'zip -r ado-script/dist' in release.yml continues to package both chunks. - ado-script CI workflow now also runs on push to main, not only PR, so any drift in types.gen.ts that slips through (e.g. from a force-push or bypassed CI) is caught loudly the moment it lands on main. - New ADO_API_TIMEOUT_MS env var documented in docs/ado-script.md. Test counts: 186 vitest (was 172), 1519 cargo unit. cargo clippy clean. npm audit clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1534284 commit 4a3f999

11 files changed

Lines changed: 1113 additions & 880 deletions

File tree

.github/workflows/ado-script.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,20 @@ on:
99
- "Cargo.toml"
1010
- "Cargo.lock"
1111
- ".github/workflows/ado-script.yml"
12+
# Also run on pushes to main so any drift that slips through (e.g. a
13+
# merge that bypassed PR CI, or a force-push) is caught loudly the
14+
# moment it lands. If this fails on main, file a fix-drift issue and
15+
# land a PR to regenerate `src/shared/types.gen.ts` and re-bundle —
16+
# the workflow itself does not auto-PR.
17+
push:
18+
branches: [main]
19+
paths:
20+
- "scripts/ado-script/**"
21+
- "src/compile/filter_ir.rs"
22+
- "src/compile/extensions/trigger_filters.rs"
23+
- "Cargo.toml"
24+
- "Cargo.lock"
25+
- ".github/workflows/ado-script.yml"
1226

1327
env:
1428
CARGO_TERM_COLOR: always

docs/ado-script.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,19 @@ steps when any `filters:` block is active:
120120
The IR-to-bash codegen lives in `compile_gate_step_external`
121121
(`src/compile/filter_ir.rs:~1100`).
122122

123+
### Runtime env-var contract
124+
125+
The gate reads the following at runtime (in addition to the
126+
predicate-specific `ADO_*` facts emitted by `collect_ado_exports`):
127+
128+
| Env var | Source | Purpose |
129+
|---|---|---|
130+
| `GATE_SPEC` | compiled inline | Base64-encoded `GateSpec` JSON |
131+
| `SYSTEM_ACCESSTOKEN` | `$(System.AccessToken)` | ADO REST auth |
132+
| `ADO_COLLECTION_URI` | `$(System.CollectionUri)` | ADO org base URL |
133+
| `ADO_PROJECT` / `ADO_REPO_ID` / `ADO_PR_ID` | compiler-injected | PR-derived facts |
134+
| `ADO_API_TIMEOUT_MS` | optional override | Per-attempt timeout in ms for every ADO REST call. Defaults to 30 000 (30 s). On timeout, the call is retried once; if the retry also times out, the gate falls back to the per-fact `FailurePolicy`. |
135+
123136
## Adding a new internal use site
124137

125138
Suppose we want a `poll.js` bundle (e.g. for polling external systems):

0 commit comments

Comments
 (0)