From ec4e7e30d2c02bab1139d884c367a40a80001042 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:55:21 -0700 Subject: [PATCH 1/3] Restrict agent authority in TypeScript PR processing steps Compiler diagnostics can include third-party Azure command markers even inside JSON strings. Restrict custom task commands and variable setters so those diagnostics cannot acquire general agent authority. Allow only GH_TOKEN from token creation, while retaining trusted tool setup and artifact task capabilities and existing formatter escaping. --- .azure-pipelines/typescript-pr-dt.yml | 49 +++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/.azure-pipelines/typescript-pr-dt.yml b/.azure-pipelines/typescript-pr-dt.yml index 719351fbde..b4c8737d59 100644 --- a/.azure-pipelines/typescript-pr-dt.yml +++ b/.azure-pipelines/typescript-pr-dt.yml @@ -105,6 +105,9 @@ jobs: verify_sha base "$(git rev-parse HEAD^1)" "$EXPECTED_BASE_SHA" verify_sha head "$(git rev-parse HEAD^2)" "$EXPECTED_HEAD_SHA" displayName: Verify TypeScript checkout + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/TypeScript env: EXPECTED_BASE_SHA: ${{ parameters.expected_base_sha }} @@ -116,14 +119,23 @@ jobs: npm install --global "$(jq -r '.packageManager' package.json)" pnpm install --frozen-lockfile displayName: Install DefinitelyTyped tools dependencies + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools - bash: pnpm build displayName: Build DefinitelyTyped tools + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools - bash: npm ci displayName: Install PR TypeScript dependencies + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/TypeScript - bash: | @@ -131,6 +143,9 @@ jobs: npm run build npx hereby build:api displayName: Build PR TypeScript + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/TypeScript - bash: | @@ -138,10 +153,16 @@ jobs: npm install --global "$(jq -r '.packageManager' package.json)" pnpm install displayName: Install DefinitelyTyped dependencies + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped - bash: bash .azure-pipelines/scripts/run-typescript-pr-dt.sh PR displayName: Run dtslint with PR TypeScript + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools env: SHARD_COUNT: $(System.TotalJobsInPhase) @@ -152,12 +173,18 @@ jobs: git clean -xdf git switch --detach "$EXPECTED_BASE_SHA" displayName: Prepare base TypeScript checkout + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/TypeScript env: EXPECTED_BASE_SHA: ${{ parameters.expected_base_sha }} - bash: npm ci displayName: Install base TypeScript dependencies + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/TypeScript - bash: | @@ -165,10 +192,16 @@ jobs: npm run build npx hereby build:api displayName: Build base TypeScript + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/TypeScript - bash: bash .azure-pipelines/scripts/run-typescript-pr-dt.sh main displayName: Run dtslint with base TypeScript + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools env: SHARD_COUNT: $(System.TotalJobsInPhase) @@ -176,6 +209,9 @@ jobs: - bash: bash .azure-pipelines/scripts/compare-typescript-pr-dt.sh displayName: Compare dtslint logs + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools - publish: $(Pipeline.Workspace)/pr/prFailures$(System.JobPositionInPhase).json @@ -229,10 +265,17 @@ jobs: pnpm install --frozen-lockfile pnpm build displayName: Build DefinitelyTyped tools + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools - task: AzureCLI@2 displayName: Create GitHub token + target: + commands: restricted + settableVariables: + - GH_TOKEN inputs: azureSubscription: TypeScript Public CI scriptType: bash @@ -272,6 +315,9 @@ jobs: fi node packages/dtslint-runner/dist/post-results.js "${args[@]}" displayName: Post DT results + target: + commands: restricted + settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools env: DISTINCT_ID: ${{ parameters.distinct_id }} @@ -283,6 +329,9 @@ jobs: - bash: node /tmp/create-github-app-token.cjs displayName: Revoke GitHub token + target: + commands: restricted + settableVariables: none condition: always() continueOnError: true env: From 5f95e827bf90c4479cf32bfe38591903682093d4 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:59:00 -0700 Subject: [PATCH 2/3] Gate successful DT posting on complete validated shard artifacts Restricted Azure commands still permit successful early task completion. Validate every expected main and PR shard in an independent trusted task before creating posting credentials for successful runs. Share the shard count with the parallel job configuration and revalidate before posting. Require regular files containing failure arrays with string paths and errors, while accepting empty clean shards and preserving diagnostics. Keep validation errors fixed so malformed artifacts cannot inject agent commands through parser or filesystem errors. Preserve failure reports without requiring successful-run artifacts. --- .azure-pipelines/typescript-pr-dt.yml | 21 +++- packages/dtslint-runner/src/post-results.ts | 41 +++---- .../dtslint-runner/src/validate-results.ts | 66 +++++++++++ packages/dtslint-runner/test/results.test.ts | 110 ++++++++++++++++-- 4 files changed, 209 insertions(+), 29 deletions(-) create mode 100644 packages/dtslint-runner/src/validate-results.ts diff --git a/.azure-pipelines/typescript-pr-dt.yml b/.azure-pipelines/typescript-pr-dt.yml index b4c8737d59..01892495b1 100644 --- a/.azure-pipelines/typescript-pr-dt.yml +++ b/.azure-pipelines/typescript-pr-dt.yml @@ -31,6 +31,8 @@ resources: ref: refs/heads/master variables: + - name: dtslintShardCount + value: 16 - name: CI value: true - name: skipComponentGovernanceDetection @@ -51,7 +53,7 @@ jobs: workspace: clean: all strategy: - parallel: 16 + parallel: ${{ variables.dtslintShardCount }} steps: - checkout: self path: s/DefinitelyTyped-tools @@ -270,6 +272,21 @@ jobs: settableVariables: none workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools + # Restricted commands still allow task.complete; validate in a separate trusted task. + - bash: | + node packages/dtslint-runner/dist/validate-results.js \ + "$EXPECTED_SHARD_COUNT" \ + "$(Pipeline.Workspace)/mainFailures" \ + "$(Pipeline.Workspace)/prFailures" + displayName: Validate successful DT artifacts + condition: and(succeeded(), eq(variables['runResult'], 'Succeeded')) + target: + commands: restricted + settableVariables: none + workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools + env: + EXPECTED_SHARD_COUNT: ${{ variables.dtslintShardCount }} + - task: AzureCLI@2 displayName: Create GitHub token target: @@ -309,6 +326,7 @@ jobs: "$status" "$(Pipeline.Workspace)/mainFailures" "$(Pipeline.Workspace)/prFailures" + "$EXPECTED_SHARD_COUNT" ) else args+=("$status") @@ -321,6 +339,7 @@ jobs: workingDirectory: $(Pipeline.Workspace)/s/DefinitelyTyped-tools env: DISTINCT_ID: ${{ parameters.distinct_id }} + EXPECTED_SHARD_COUNT: ${{ variables.dtslintShardCount }} GH_TOKEN: $(GH_TOKEN) REQUESTING_USER: ${{ parameters.requesting_user }} RUN_RESULT: $(runResult) diff --git a/packages/dtslint-runner/src/post-results.ts b/packages/dtslint-runner/src/post-results.ts index 000dc00cf9..d4af35c2c1 100644 --- a/packages/dtslint-runner/src/post-results.ts +++ b/packages/dtslint-runner/src/post-results.ts @@ -1,21 +1,31 @@ import { Octokit } from "@octokit/rest"; -import { readFileSync } from "fs"; -import glob = require("glob"); import type { Failure } from "./add-github-links"; +import { readComparison } from "./validate-results"; type Errors = Failure[]; -// Args: [auth token] [buildId] [status comment] [user to tag] [issue] [distinct id] [job status] [?main errors file] [?branch errors file] +// Args: [auth token] [buildId] [status comment] [user to tag] [issue] [distinct id] [job status] +// Successful runs also require: [main errors directory] [branch errors directory] [expected shard count] export async function main() { - const [auth, buildId, statusCommentId, userToTag, issue, distinctId, status, mainErrorsPath, branchErrorsPath] = - process.argv.slice(2); + const [ + auth, + buildId, + statusCommentId, + userToTag, + issue, + distinctId, + status, + mainErrorsPath, + branchErrorsPath, + expectedShardCount, + ] = process.argv.slice(2); if (!auth) throw new Error("First argument must be a GitHub auth token."); if (!buildId) throw new Error("Second argument must be a build id."); if (!statusCommentId) throw new Error("Third argument must be a GitHub comment id."); if (!userToTag) throw new Error("Fourth argument must be a GitHub username."); if (!issue) throw new Error("Fifth argument must be a TypeScript issue/PR number."); if (!distinctId) throw new Error("Sixth argument must be a distinct ID."); - if (!status) throw new Error("Seventh argument must be a status ('ok' or 'fail')."); + if (status !== "ok" && status !== "fail") throw new Error("Seventh argument must be a status ('ok' or 'fail')."); const gh = new Octokit({ auth }); let checkLogsMessage = ""; @@ -38,20 +48,11 @@ export async function main() { ]; emoji = "❌"; } else { - const mainErrors: Errors = []; - if (mainErrorsPath) { - const mainFiles = glob.sync(`**/*.json`, { cwd: mainErrorsPath, absolute: true }); - for (const file of mainFiles) { - mainErrors.push(...(JSON.parse(readFileSync(file, "utf-8")) as Errors)); - } - } - const branchErrors: Errors = []; - if (branchErrorsPath) { - const branchFiles = glob.sync(`**/*.json`, { cwd: branchErrorsPath, absolute: true }); - for (const file of branchFiles) { - branchErrors.push(...(JSON.parse(readFileSync(file, "utf-8")) as Errors)); - } - } + const { main: mainErrors, branch: branchErrors } = readComparison( + mainErrorsPath, + branchErrorsPath, + expectedShardCount, + ); const reports = getDiffReports(mainErrors, branchErrors); if (reports.length) { diff --git a/packages/dtslint-runner/src/validate-results.ts b/packages/dtslint-runner/src/validate-results.ts new file mode 100644 index 0000000000..0262be615c --- /dev/null +++ b/packages/dtslint-runner/src/validate-results.ts @@ -0,0 +1,66 @@ +import { lstatSync, readFileSync } from "fs"; +import glob = require("glob"); +import type { Failure } from "./add-github-links"; + +export function readComparison( + mainErrorsPath: string | undefined, + branchErrorsPath: string | undefined, + expectedShardCount: string | undefined, +): { main: Failure[]; branch: Failure[] } { + try { + const shardCount = Number(expectedShardCount); + if ( + !mainErrorsPath || + !branchErrorsPath || + !Number.isSafeInteger(shardCount) || + shardCount < 1 || + String(shardCount) !== expectedShardCount + ) { + throw new Error(); + } + return { + main: readFailures(mainErrorsPath, "main", shardCount), + branch: readFailures(branchErrorsPath, "pr", shardCount), + }; + } catch { + // JSON parse and filesystem errors can contain artifact-controlled command markers. + throw new Error("Missing or invalid DT failure artifacts."); + } +} + +function readFailures(directory: string, prefix: "main" | "pr", shardCount: number): Failure[] { + const result: Failure[] = []; + for (let shard = 1; shard <= shardCount; shard++) { + const files = glob.sync(`**/${prefix}Failures${shard}.json`, { cwd: directory, absolute: true }); + if (files.length !== 1 || !lstatSync(files[0]).isFile()) { + throw new Error(); + } + const failures: unknown = JSON.parse(readFileSync(files[0], "utf8")); + if (!Array.isArray(failures) || !failures.every(isFailure)) { + throw new Error(); + } + result.push(...failures); + } + return result; +} + +function isFailure(value: unknown): value is Failure { + return ( + typeof value === "object" && + value !== null && + "path" in value && + typeof value.path === "string" && + "error" in value && + typeof value.error === "string" + ); +} + +if (require.main === module) { + const [shardCount, mainErrorsPath, branchErrorsPath] = process.argv.slice(2); + try { + readComparison(mainErrorsPath, branchErrorsPath, shardCount); + } catch { + console.error("Missing or invalid DT failure artifacts."); + process.exitCode = 1; + } +} diff --git a/packages/dtslint-runner/test/results.test.ts b/packages/dtslint-runner/test/results.test.ts index 2518e4d188..37123f40fd 100644 --- a/packages/dtslint-runner/test/results.test.ts +++ b/packages/dtslint-runner/test/results.test.ts @@ -1,9 +1,9 @@ import { execFileSync } from "child_process"; -import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "fs"; -import { tmpdir } from "os"; +import { mkdirSync, rmSync, symlinkSync, writeFileSync } from "fs"; import { dirname, join } from "path"; import { addGithubLinks, Failure } from "../src/add-github-links"; import { getDiffComment, getDiffLog, getResultComments, main } from "../src/post-results"; +import { readComparison } from "../src/validate-results"; const mockCreateComment = jest.fn(); const mockGetComment = jest.fn(); @@ -31,7 +31,8 @@ function writeFile(path: string, contents = ""): string { } beforeAll(() => { - checkout = mkdtempSync(join(tmpdir(), "dt-result-links-")); + checkout = join(__dirname, `.dt-result-links-${process.pid}`); + mkdirSync(checkout); git("init", "--quiet"); writeFile("types/example/package.json", "{}"); writeFile("types/example/tsconfig.json", "{}"); @@ -70,6 +71,79 @@ afterAll(() => { rmSync(checkout, { recursive: true, force: true }); }); +function writeResults(name: string, count: number, mainFailures: Failure[] = [], branchFailures: Failure[] = []) { + for (let shard = 1; shard <= count; shard++) { + writeFile( + `${name}/main/mainFailures${shard}/mainFailures${shard}.json`, + JSON.stringify(shard === 1 ? mainFailures : []), + ); + writeFile( + `${name}/pr/prFailures${shard}/prFailures${shard}.json`, + JSON.stringify(shard === 1 ? branchFailures : []), + ); + } + return { mainPath: join(checkout, name, "main"), branchPath: join(checkout, name, "pr") }; +} + +test("requires every configured main and PR shard, allowing complete clean runs", () => { + const { mainPath, branchPath } = writeResults("complete", 3); + expect(readComparison(mainPath, branchPath, "3")).toEqual({ main: [], branch: [] }); + for (const [directory, prefix] of [ + [mainPath, "main"], + [branchPath, "pr"], + ]) { + const file = join(directory, `${prefix}Failures3`, `${prefix}Failures3.json`); + rmSync(file); + expect(() => readComparison(mainPath, branchPath, "3")).toThrow("Missing or invalid DT failure artifacts."); + writeFileSync(file, "[]"); + } + expect(() => readComparison(mainPath, branchPath, "4")).toThrow("Missing or invalid DT failure artifacts."); + expect(() => readComparison(undefined, undefined, "3")).toThrow("Missing or invalid DT failure artifacts."); + expect(() => readComparison(mainPath, branchPath, undefined)).toThrow("Missing or invalid DT failure artifacts."); +}); + +test.each([ + "not JSON ##vso[task.complete result=Succeeded;done=true;]", + "{}", + "[null]", + '[{"path":"example"}]', + '[{"path":1,"error":"failure"}]', + '[{"path":"example","error":[]}]', +])("rejects malformed or wrong-shaped shards with a fixed error: %s", (contents) => { + const { mainPath, branchPath } = writeResults("invalid", 1); + writeFileSync(join(branchPath, "prFailures1/prFailures1.json"), contents); + expect(() => readComparison(mainPath, branchPath, "1")).toThrow( + new Error("Missing or invalid DT failure artifacts."), + ); +}); + +test("rejects directories, symlinks, and duplicate expected shard files", () => { + const { mainPath, branchPath } = writeResults("file-types", 1); + const file = join(branchPath, "prFailures1/prFailures1.json"); + rmSync(file); + mkdirSync(file); + expect(() => readComparison(mainPath, branchPath, "1")).toThrow("Missing or invalid DT failure artifacts."); + rmSync(file, { recursive: true }); + symlinkSync(join(mainPath, "mainFailures1/mainFailures1.json"), file); + expect(() => readComparison(mainPath, branchPath, "1")).toThrow("Missing or invalid DT failure artifacts."); + rmSync(file); + writeFileSync(file, "[]"); + writeFile("file-types/pr/duplicate/prFailures1.json", "[]"); + expect(() => readComparison(mainPath, branchPath, "1")).toThrow("Missing or invalid DT failure artifacts."); +}); + +test("preserves diagnostic command markers and optional failure metadata when reading shards", () => { + const failure: Failure = { + path: "example", + error: 'quoted "##vso[task.complete result=Succeeded;done=true;]" diagnostic', + packageUrl: `${repoUrl}/tree/main/types/example`, + projects: [{ path: "tsconfig.json", url: `${repoUrl}/blob/main/types/example/tsconfig.json` }], + errorLinks: [{ start: 0, end: 6, url: `${repoUrl}/blob/main/types/example/index.d.ts#L1` }], + }; + const { mainPath, branchPath } = writeResults("preserved", 2, [failure], [failure]); + expect(readComparison(mainPath, branchPath, "2")).toEqual({ main: [failure], branch: [failure] }); +}); + test("adds pinned package and Corsa diagnostic links without changing the raw errors", async () => { const file = join(checkout, "types/example/example-tests.ts"); const error = `${file}:12:3\nTypeScript@local compile error TS2322: Type '' is not assignable.\n\nindex.d.ts:2:1\nExpected type.`; @@ -344,6 +418,8 @@ test.each([ { scenario: "unchanged errors", kind: "same", emoji: "✅", count: 1 }, { scenario: "no errors", kind: "empty", emoji: "✅", count: 1 }, { scenario: "infrastructure failure", kind: "fail", emoji: "❌", count: 1 }, + { scenario: "missing successful-run shard", kind: "missing", emoji: "", count: 1 }, + { scenario: "invalid successful-run shard", kind: "invalid", emoji: "", count: 1 }, ])("posts results and updates the status for $scenario", async ({ kind, emoji, count }) => { const args = process.argv; const env = { ...process.env }; @@ -361,8 +437,14 @@ test.each([ : kind === "same" ? [oldError] : []; - writeFile("results/pr/failures.json", JSON.stringify(branchFailures)); - writeFile("results/main/failures.json", JSON.stringify(mainFailures)); + const { mainPath, branchPath } = writeResults("results", 2, mainFailures, branchFailures); + if (kind === "missing") { + rmSync(join(branchPath, "prFailures2/prFailures2.json")); + } else if (kind === "invalid") { + writeFileSync(join(branchPath, "prFailures2/prFailures2.json"), "{}"); + } else if (kind === "fail") { + rmSync(join(checkout, "results"), { recursive: true }); + } process.argv = [ "node", "post-results", @@ -373,8 +455,7 @@ test.each([ "789", "test-run", kind === "fail" ? "fail" : "ok", - join(checkout, "results/main"), - join(checkout, "results/pr"), + ...(kind === "fail" ? [] : [mainPath, branchPath, "2"]), ]; process.env.SYSTEM_COLLECTIONURI = "https://dev.azure.com/example/"; process.env.SYSTEM_TEAMPROJECT = "project"; @@ -394,9 +475,16 @@ test.each([ const result = main(); await jest.runAllTimersAsync(); await result; - expect(consoleError).not.toHaveBeenCalled(); expect(mockCreateComment).toHaveBeenCalledTimes(count); const bodies = mockCreateComment.mock.calls.map(([arg]) => arg.body as string); + if (kind === "missing" || kind === "invalid") { + expect(consoleError).toHaveBeenCalledWith(new Error("Missing or invalid DT failure artifacts.")); + expect(bodies[0]).toContain("something went wrong when publishing results"); + expect(bodies[0]).not.toContain("Everything looks the same!"); + expect(mockUpdateComment).not.toHaveBeenCalled(); + return; + } + expect(consoleError).not.toHaveBeenCalled(); expect(bodies.every((body) => body.length <= 65535)).toBe(true); const status = mockUpdateComment.mock.calls[0][0].body; expect(status).toContain(`[${emoji} Results]`); @@ -413,6 +501,12 @@ test.each([ expect(bodies[0]).toContain("Main only errors:"); expect(bodies[0]).toContain(oldError.error); } + if (kind === "empty") { + expect(bodies[0]).toContain("Everything looks the same!"); + } + if (kind === "fail") { + expect(bodies[0]).toContain("it looks like the DT test run failed"); + } } finally { process.argv = args; process.env = env; From 5a06bf6aba3da24cc6f2ed82e3e0bf7a2d80a15d Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:33:48 -0700 Subject: [PATCH 3/3] Add empty changeset --- .changeset/shaggy-rivers-look.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/shaggy-rivers-look.md diff --git a/.changeset/shaggy-rivers-look.md b/.changeset/shaggy-rivers-look.md new file mode 100644 index 0000000000..064435dc68 --- /dev/null +++ b/.changeset/shaggy-rivers-look.md @@ -0,0 +1,5 @@ +--- + +--- + +