From 9ac6441f4ff7880216cf6f5a05e06548d69d69cb Mon Sep 17 00:00:00 2001 From: James Devine Date: Mon, 3 Aug 2026 17:30:17 +0100 Subject: [PATCH] fix(smoke): tag builds via the request body so colon-bearing tags work Every child build in the first live candidate run logged: WARNING: could not tag build #629524: ADO PUT .../tags/smoke-case%3Anoop-target -> HTTP 400: A potentially dangerous Request.Path value was detected from the client (:). addBuildTags used the per-tag path form, PUT .../tags/{tag}. ADO's ASP.NET front end validates the DECODED request path, so a ':' is rejected even when correctly percent-encoded as %3A. Both tags this harness writes - smoke-case: and smoke-candidate: - carry one, so tagging failed for every case, every run. Switches to the body form, POST .../tags with a JSON array. That sidesteps path validation entirely, keeps the readable prefixed tag format, and tags all of them in one request instead of N. Verified live against build 629523: the PUT form returns 400, the POST form succeeds and the tags appear. Tagging is deliberately non-fatal, so this degraded silently into a warning: every case in a lane shares one definition, and tags are how a run is identified in that shared history, so losing them makes the lane history unnavigable without breaking anything loudly. Mutation-checked: restoring the PUT form fails the new test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 323b70ff-d193-4c6e-b4c7-9ec6c3dc6ebd --- .../__tests__/ado-rest.test.ts | 37 +++++++++++++++++++ .../src/compiler-smoke-e2e/ado-rest.ts | 18 ++++++--- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/scripts/ado-script/src/compiler-smoke-e2e/__tests__/ado-rest.test.ts b/scripts/ado-script/src/compiler-smoke-e2e/__tests__/ado-rest.test.ts index 644529d8..bce3a951 100644 --- a/scripts/ado-script/src/compiler-smoke-e2e/__tests__/ado-rest.test.ts +++ b/scripts/ado-script/src/compiler-smoke-e2e/__tests__/ado-rest.test.ts @@ -248,3 +248,40 @@ describe("redactToken", () => { ); }); }); + +describe("AdoRest.addBuildTags", () => { + // Regression: tags were sent one-per-request as PUT .../tags/, putting + // the tag in the URL PATH. ADO's ASP.NET front end validates the *decoded* + // path, so `smoke-case:canary` was rejected with HTTP 400 "A potentially + // dangerous Request.Path value was detected from the client (:)" even + // correctly encoded as %3A — which is every tag this harness writes. + // Observed live on build 629522's children. + it("sends tags in the request body, never in the URL path", async () => { + const calls: { url: string; method?: string; body?: unknown }[] = []; + const fetchImpl = vi.fn(async (url: string, init?: RequestInit) => { + calls.push({ url, method: init?.method, body: init?.body }); + return jsonResponse(200, { count: 2, value: ["smoke-case:canary"] }); + }); + const rest = makeRest(fetchImpl as unknown as typeof fetch); + + await rest.addBuildTags(4242, ["smoke-case:canary", "smoke-candidate:99"]); + + expect(calls).toHaveLength(1); + expect(calls[0]!.method).toBe("POST"); + expect(calls[0]!.url).toContain("/build/builds/4242/tags?api-version="); + // The colon-bearing values must not reach the path in any encoding. + expect(calls[0]!.url).not.toContain("smoke-case"); + expect(calls[0]!.url).not.toContain("%3A"); + expect(JSON.parse(String(calls[0]!.body))).toEqual([ + "smoke-case:canary", + "smoke-candidate:99", + ]); + }); + + it("makes no request when there are no tags", async () => { + const fetchImpl = vi.fn(async () => jsonResponse(200, {})); + const rest = makeRest(fetchImpl as unknown as typeof fetch); + await rest.addBuildTags(1, []); + expect(fetchImpl).not.toHaveBeenCalled(); + }); +}); diff --git a/scripts/ado-script/src/compiler-smoke-e2e/ado-rest.ts b/scripts/ado-script/src/compiler-smoke-e2e/ado-rest.ts index 041eee4d..3e953b32 100644 --- a/scripts/ado-script/src/compiler-smoke-e2e/ado-rest.ts +++ b/scripts/ado-script/src/compiler-smoke-e2e/ado-rest.ts @@ -225,14 +225,20 @@ export class AdoRest { * Every case in a lane shares one definition, so tags (alongside the * per-case `sourceBranch`) are how a run is identified in the lane's * history. Callers treat failures here as non-fatal. + * + * Uses the **body** form (`POST .../tags`) rather than the per-tag path form + * (`PUT .../tags/{tag}`). ADO's ASP.NET front end validates the *decoded* + * request path, so a tag containing `:` is rejected with HTTP 400 "A + * potentially dangerous Request.Path value was detected from the client (:)" + * even when correctly percent-encoded as `%3A`. Our tags are + * `smoke-case:` / `smoke-candidate:`, so every one of them hit + * that. Sending them in the body sidesteps path validation entirely, and + * tags all of them in a single request. */ async addBuildTags(buildId: number, tags: readonly string[]): Promise { - for (const tag of tags) { - const path = this.projPath( - `_apis/build/builds/${buildId}/tags/${AdoRest.seg(tag)}?api-version=7.1`, - ); - await this.request(path, { method: "PUT" }); - } + if (tags.length === 0) return; + const path = this.projPath(`_apis/build/builds/${buildId}/tags?api-version=7.1`); + await this.request(path, { method: "POST", body: [...tags] }); } /**