diff --git a/.github/workflows/ado-script.yml b/.github/workflows/ado-script.yml index 51208bbf4..c70041197 100644 --- a/.github/workflows/ado-script.yml +++ b/.github/workflows/ado-script.yml @@ -6,6 +6,9 @@ on: - "scripts/ado-script/**" - "src/compile/filter_ir.rs" - "src/compile/extensions/ado_script.rs" + - "src/compile/agentic_pipeline.rs" + - "src/compile/container_invocation.rs" + - "src/compile/mcpg.rs" - "src/ado_proxy/**" - "src/main.rs" - "Cargo.toml" @@ -22,6 +25,9 @@ on: - "scripts/ado-script/**" - "src/compile/filter_ir.rs" - "src/compile/extensions/ado_script.rs" + - "src/compile/agentic_pipeline.rs" + - "src/compile/container_invocation.rs" + - "src/compile/mcpg.rs" - "src/ado_proxy/**" - "src/main.rs" - "Cargo.toml" @@ -79,6 +85,9 @@ jobs: - name: Smoke-test bundles working-directory: scripts/ado-script + env: + ADO_AW_TEST_DOCKER: "1" + ADO_AW_TEST_AWF: "1" run: npm run test:smoke - name: E2E gate test diff --git a/AGENTS.md b/AGENTS.md index d4fee67a6..9f231c073 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,6 +75,7 @@ fail-closed and only pauses when the agent actually proposed a reviewed output. │ │ ├── stage.rs # Stage-level ADO template compiler (target: stage) │ │ ├── stage_ir.rs # Stage target typed-IR builder │ │ ├── az_wrapper.rs # Renders the `az` CLI redirect wrapper installed into the agent sandbox (env-based `HTTPS_PROXY` redirect, not argument rewriting) +│ │ ├── container_invocation.rs # Typed compiler-owned `docker run` IR (validated shell words, lifecycle, hardening, mounts, entrypoint, command) lowered to a ShellScript fragment; no raw-argument escape hatch │ │ ├── source_path_guard.rs # Validation guard for untrusted workflow source-path inputs used by audit + mcp_author │ │ ├── shell/ # Typed generation of every shell script the compiler emits (see docs/extending.md "Generated shell scripts") │ │ │ ├── mod.rs # ShellScript: raw-string bodies + a typed shell-quoted binding prelude; `# ado-aw:fragment` splicing; into_step() @@ -320,6 +321,7 @@ fail-closed and only pauses when the agent actually proposed a reviewed output. │ ├── executor-e2e/ # Stage 3 safe-output E2E test harness (not a bundle; runs deterministic scenarios against a real ADO project and files a GitHub issue on failure) │ ├── compiler-smoke-e2e/ # Smoke E2E orchestrator (not a bundle): stages each case in `tests/smoke/cases.json` to the fixed `.smoke/pipeline.yml` path on its own per-case `ado-aw-mirror` ref, queues it against its credential *lane* definition, and asserts they go green. Two modes via `SMOKE_COMPILER_SOURCE`: `candidate` (compiler built from this commit, pinned pipeline-artifact) and `released` (latest release asset, release URLs required). Built to `test-bin/` by `build:compiler-smoke-e2e`, listed in `NON_BUNDLE_DIRS`. │ ├── prepare-pr-base/ # create-pull-request preparer (bundled to prepare-pr-base.js): Agent mode uses ADO diff metadata + bounded fallback; SafeOutputs fetches the target tip; cross-org targets use isolated credentials + exact remote matching +│ ├── azure-wif-refresh/ # Renewable Azure Pipelines workload-identity assertion writer for user-defined stdio MCP servers; trusted sidecar receives request credentials on stdin and rotates a private token file │ ├── ado-proxy/ # Credential-isolated ADO policy proxy (bundled to ado-proxy.js). The pipeline mounts it into node:20-slim and starts it before AWF; AWF attaches the trusted container via --topology-attach. scope.ts builds the organization-relative current/additional scope index; catalog.gen.json + ../shared/ado-proxy-catalog.types.gen.ts are generated from Rust by export-ado-proxy-catalog{,-schema} and drift-guarded; a catalog_version mismatch fails closed at startup. │ ├── trigger-e2e/ # Test-only gate-spec / trigger-evaluation harness (not a bundle): mirrors Rust `Fact::ALL` in `gate-spec.ts`; `fact-catalog.gen.json` is generated by `export-fact-catalog` and drift-guarded by CI │ └── shared/ # Shared modules across bundles (auth, ado-client, env-facts, types.gen.ts) @@ -459,7 +461,8 @@ index to jump to the right page. (`scripts/ado-script/`): the bundled TypeScript runtime helpers (`gate.js`, `import.js`, the execution-context `exec-context-*.js` bundles, `conclusion.js`, `approval-summary.js`, - `github-app-token.js`, and `prepare-pr-base.js`), schemars-driven + `github-app-token.js`, `prepare-pr-base.js`, and + `azure-wif-refresh.js`), schemars-driven type codegen, the A2 design decision, the bundle env contract modelled in `src/compile/ado_bundle.rs`, and the `trigger-e2e/` gate-spec drift guard (kept in sync via `export-fact-catalog`). diff --git a/docs/ado-script.md b/docs/ado-script.md index 1fd93bff3..6d9522038 100644 --- a/docs/ado-script.md +++ b/docs/ado-script.md @@ -3,7 +3,7 @@ `ado-script` is the umbrella name for the TypeScript workspace at [`scripts/ado-script/`](../scripts/ado-script/). It produces small, ncc-bundled Node programs that the **compiler injects into every emitted -pipeline** as runtime helpers. Today it produces thirteen bundles: +pipeline** as runtime helpers. Today it produces the following shipped bundles: - `gate.js` — trigger-filter gate evaluator (Setup job). - `import.js` — runtime prompt resolver described in @@ -88,6 +88,16 @@ pipeline** as runtime helpers. Today it produces thirteen bundles: shell-local or in masked `SYSTEM_ACCESSTOKEN` env and spawned-git `GIT_CONFIG_*`, never argv or `.git/config`. Runs outside AWF. See [`safe-outputs.md`](safe-outputs.md#create-pull-request). +- `azure-wif-refresh.js` — long-lived trusted sidecar for + `mcp-servers..azure-auth`. It receives the initial Azure Pipelines + workload-identity assertion and `System.AccessToken` in a one-shot stdin + document, requests replacement assertions from `System.OidcRequestUri` using + the runtime service-connection GUID, and atomically rotates a mode-0644 file + inside a private mode-0700 host directory mounted read-only into + the target MCP container. Request credentials remain in sidecar memory and + never enter the agent, MCP environment, Docker arguments, logs, status + documents, or artifacts. See + [`mcp.md`](mcp.md#renewable-azure-workload-identity). > **Internal-only.** `ado-script` is not a user-facing front-matter > feature. Authors never write an `ado-script:` block in their agent diff --git a/docs/extending.md b/docs/extending.md index dfe51275e..7a84e95d5 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -411,6 +411,49 @@ fragment defines must be declared in the consumer's `externals:`, which forces the inter-phase contract somewhere a reviewer can see it. Declaring a fragment without marking it (or vice versa) is a test failure, not a silent no-op. +A fragment generated from typed IR declares the variables it consumes through +`fragment_uses:`. This keeps registry-wide shellcheck coverage accurate without +adding fake no-op reads to the emitted script: + +```rust +fragments: [run_container], +fragment_uses: [ + run_container => [CONTAINER, IMAGE], +], +``` + +### Compiler-owned container invocations + +Use `compile::container_invocation::DockerRun` for a compiler-owned +`docker run` command, especially when the container handles credentials. It +models the image, lifecycle, name, network, user, hardening flags, mounts, +entrypoint, and command arguments as typed values, then lowers to a shell +fragment at the final boundary. + +`DockerRun` intentionally has no raw argument escape hatch. Add a typed field +and validation when a compiler-owned container needs a new Docker capability; +do not insert a hand-authored flag into a `shell_script!` body. `Docker@2` +remains the separate typed ADO task for image build/push/login/logout actions. + +Keep orchestration and control flow in registered shell. Only the security- +sensitive command construction belongs in the container invocation IR: + +```rust +let run = DockerRun::new(ShellWord::variable("IMAGE")?) + .detached() + .name(ShellWord::variable("CONTAINER")?) + .read_only() + .mount(DockerMount::read_only( + ShellWord::variable("BUNDLE")?, + "/app/bundle.js", + )?) + .render_bash()?; + +ShellScript::new(&START_CONTAINER) + .fragment("run_container", run) + .into_step("Start container") +``` + ### Reviewing the scripts as files ```bash diff --git a/docs/front-matter.md b/docs/front-matter.md index 0d9379c10..681160a47 100644 --- a/docs/front-matter.md +++ b/docs/front-matter.md @@ -95,6 +95,9 @@ mcp-servers: CUSTOM_TOKEN: pipeline-variable: CUSTOM_TOKEN # ADO pipeline/variable-group/same-job source STATIC_CONFIG: "value" # literal value embedded in MCPG config + azure-auth: # optional renewable Azure workload identity + service-connection: my-arm-service-connection + mount-path: /var/run/ado-aw/azure # optional; token is written below this path allowed: - custom_function_1 - custom_function_2 diff --git a/docs/mcp.md b/docs/mcp.md index 651c5ee1c..2edbec1f1 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -52,6 +52,13 @@ mcp-servers: - `env:` - Environment variables for the MCP server process. Use a string for a static value or `{ pipeline-variable: NAME }` to read an ADO pipeline, variable-group, queue-time, or earlier-same-job variable at runtime. +- `azure-auth:` - Renewable Azure workload identity from an ARM service + connection. The compiler supplies the Azure Identity environment contract + and rotates the federated assertion for the lifetime of the Agent job. + Supported only for containerized stdio servers. + - `service-connection:` - Required ARM workload-identity service connection. + - `mount-path:` - Optional container directory for the assertion; defaults + to `/var/run/ado-aw/azure`. **HTTP servers:** - `url:` - HTTP endpoint URL for the remote MCP server @@ -83,6 +90,56 @@ variable-group, and queue-time variables exist from job start; a `task.setvariable` source must be published by an earlier step in the same job. Cross-job/stage output expressions are not accepted by `pipeline-variable`. +## Renewable Azure workload identity + +Use `azure-auth` when a containerized MCP server uses an Azure Identity SDK and +may need to acquire an Azure token late in a long-running Agent job: + +```yaml +mcp-servers: + kusto: + container: "node:22-slim" + entrypoint: "sh" + entrypoint-args: + - "-c" + - "exec npx -y @azure/mcp@latest server start --namespace kusto" + azure-auth: + service-connection: my-arm-service-connection + # Optional; defaults to /var/run/ado-aw/azure + mount-path: /var/run/ado-aw/azure +``` + +The compiler injects these values into the MCP container: + +```text +AZURE_CLIENT_ID= +AZURE_TENANT_ID= +AZURE_FEDERATED_TOKEN_FILE=/var/run/ado-aw/azure/token +``` + +An AzureCLI@3 setup task obtains the initial workload-identity assertion and +starts a trusted refresh sidecar. The sidecar uses the job's +`System.AccessToken` and `System.OidcRequestUri` to request replacement +assertions before their JWT expiry and atomically rotates the private token +file. The MCP container receives the token directory through a read-only +mount, so inode-replacing rotation remains visible; the agent and MCP server never receive +`System.AccessToken`. + +`azure-auth` fails closed when: + +- the server is HTTP or has no `container`; +- the service connection does not use workload identity federation; +- the author also sets `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, or + `AZURE_FEDERATED_TOKEN_FILE`; +- a user mount overlaps the compiler-owned destination; +- the initial assertion or refresher readiness check fails. + +The assertion is not an Azure access token. Azure Identity inside the MCP +container exchanges it for the resource-specific access token requested by the +server. AzureCLI@3's experimental `keepAzSessionActive` option does not replace +this feature: it refreshes only while that AzureCLI task remains running, but +the MCP server is used later during the separate Agent step. + The first-party `tools.azure-devops` integration is deliberately different: it gives the MCP a non-secret sentinel in `ADO_MCP_AUTH_TOKEN`. The real `SC_READ_TOKEN` is delivered only to `ado-proxy` over stdin and is injected @@ -119,3 +176,8 @@ network: 4. **MCPG Gateway**: All MCP traffic flows through the MCP Gateway which enforces tool-level filtering 5. **Trusted egress**: MCPG and the stdio/HTTP backends it spawns from `mcp-servers:` front matter are trusted infrastructure that runs outside the agent's Squid-enforced allowlist — they have direct network egress and are not subject to `network.allowed`/`network.blocked`. Only the Copilot agent process itself is confined to the AWF sandbox and its domain allowlist; see [`docs/mcpg.md`](mcpg.md) and [`docs/network.md`](network.md) for the topology. 6. **SafeOutputs is further hardened**: unlike arbitrary `mcp-servers:` entries, the compiler-owned `safeoutputs` MCPG backend is not a user-configurable trusted-egress container — it is a dedicated stdio child spawned by MCPG from the pinned AWF `agent` image with `--network none`, `--cap-drop ALL`, a read-only rootfs, and the host ADO runner's non-root UID/GID. It has no network access at all, trusted or otherwise; see [`docs/mcpg.md`](mcpg.md). +7. **Azure credential custody**: `azure-auth` keeps `System.AccessToken` in the + trusted AzureCLI@3/refresher path. Only the short-lived federated assertion + is mounted into the target MCP, read-only. Credential files are created + beneath `$(Agent.TempDirectory)`, never runner `/tmp`, because AWF exposes + runner `/tmp` inside the agent sandbox. diff --git a/docs/mcpg.md b/docs/mcpg.md index e2f26c9f8..a389671e7 100644 --- a/docs/mcpg.md +++ b/docs/mcpg.md @@ -93,7 +93,12 @@ no bridge-gateway resolution, and no `host.docker.internal` mapping. internal request through Squid. 5. MCPG routes tool calls to the appropriate upstream (SafeOutputs or custom MCPs). Detection is unaffected — it never attaches to MCPG. -6. After the agent completes, MCPG (and any stdio children it spawned, +6. For a custom stdio MCP with `azure-auth`, a separate trusted + `azure-wif-refresh.js` sidecar rotates a federated assertion beneath + `$(Agent.TempDirectory)`. MCPG mounts only its token directory read-only into + the target MCP container and forwards non-secret client/tenant IDs through + its typed launch environment. +7. After the agent completes, MCPG (and any stdio children it spawned, including SafeOutputs) are stopped. ## MCPG Configuration Format @@ -163,6 +168,9 @@ The MCPG is automatically configured in generated standalone pipelines: 1. **Config Generation**: The compiler generates `mcpg-config.json` from the agent's `mcp-servers:` front matter, including the compiler-owned `safeoutputs` stdio entry above. 2. **MCPG Start**: The MCPG Docker container (`awmg-mcpg`) starts on Docker's bridge network, published to the host at `127.0.0.1:8080`, with config via stdin and the Docker socket mounted so it can spawn stdio children (including SafeOutputs) on demand. 3. **Agent Execution**: AWF runs the Agent rootlessly with `--network-isolation --topology-attach awmg-mcpg`, attaching the MCPG container to `awf-net`; copilot connects to MCPG at `awmg-mcpg:8080` over HTTP, and reaches SafeOutputs tools transparently through MCPG's stdio routing. -4. **Cleanup**: MCPG and any stdio children it spawned (including SafeOutputs) are stopped after the agent completes (condition: always). +4. **Cleanup**: MCPG and any stdio children it spawned (including SafeOutputs) + are stopped after the agent completes (condition: always). Renewable Azure + assertion sidecars are then stopped and their private + `$(Agent.TempDirectory)/ado-aw-azure-auth/` directories removed. The MCPG config is written to `$(Agent.TempDirectory)/staging/mcpg-config.json` in its own pipeline step, making it easy to inspect and debug. SafeOutputs is always run with the `ado-aw mcp` stdio subcommand through MCPG. diff --git a/docs/network.md b/docs/network.md index 8f8959056..6d8ff7417 100644 --- a/docs/network.md +++ b/docs/network.md @@ -134,6 +134,39 @@ not found" failure mode. See [`docs/tools.md`](tools.md#built-in-clis) for the agent-facing contract (auth scope, available subcommands). +## Renewable Azure authentication for MCP servers + +`mcp-servers..azure-auth` is a trusted-infrastructure credential path for +containerized stdio MCP servers. It is separate from the agent-facing Azure CLI +wrapper described above: + +- AzureCLI@3 receives an ARM workload-identity service connection and exposes + the initial federated assertion only to its trusted setup script. +- `System.AccessToken` and the initial assertion are streamed to a dedicated + refresh sidecar over a one-shot FIFO; neither is stored in Docker + environment, command arguments, generated YAML, or a host credential file. +- The sidecar keeps the ADO request credential in memory and writes only the + renewable federated assertion beneath `$(Agent.TempDirectory)`. +- MCPG mounts the assertion's token-only directory read-only into the configured + MCP container, so atomic file replacement is visible without exposing + sidecar status or material channels. +- The AWF agent receives no credential mount, no identity environment + variables from `azure-auth`, and no route to the refresher container. The + compiler excludes its internal client/tenant ID variables from both Agent + and Detection environment passthrough. + +The host auth root and per-server directory use mode `0700`. Only the +token-only directory is `0755`, with assertion files `0644`, so an MCP +container running under a different UID can read its read-only mount. +Unrelated unprivileged host users cannot traverse the private parent +directories. These permissions do not isolate processes sharing the runner +UID or host root; agent isolation depends on keeping the auth directory out +of AWF's filesystem mounts, including its workspace and `/host` aliases. + +The credential directory must not move to runner `/tmp`: AWF mounts runner +`/tmp` into the agent chroot, making files there agent-readable. See +[`docs/mcp.md`](mcp.md#renewable-azure-workload-identity) for configuration. + ## Adding Additional Hosts Agents can specify additional allowed hosts in their front matter using either ecosystem identifiers or raw domain patterns: diff --git a/scripts/ado-script/.gitignore b/scripts/ado-script/.gitignore index 85b8cf336..019aca5c2 100644 --- a/scripts/ado-script/.gitignore +++ b/scripts/ado-script/.gitignore @@ -16,6 +16,7 @@ conclusion.js github-app-token.js prepare-pr-base.js ado-proxy.js +azure-wif-refresh.js schema *.tsbuildinfo test-bin diff --git a/scripts/ado-script/package.json b/scripts/ado-script/package.json index 1261d0ef2..40a170f58 100644 --- a/scripts/ado-script/package.json +++ b/scripts/ado-script/package.json @@ -7,8 +7,8 @@ "node": ">=20.0.0" }, "scripts": { - "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:conclusion && npm run build:approval-summary && npm run build:github-app-token && npm run build:prepare-pr-base && npm run build:ado-proxy", - "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); for (const n of ['gate','import','exec-context-pr','exec-context-pr-synth','exec-context-manual','exec-context-pipeline','exec-context-ci-push','exec-context-workitem','exec-context-schedule','exec-context-pr-checks','exec-context-repo','conclusion','approval-summary','github-app-token','prepare-pr-base','ado-proxy']) fs.rmSync(n+'.js',{force:true});\"", + "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:conclusion && npm run build:approval-summary && npm run build:github-app-token && npm run build:prepare-pr-base && npm run build:ado-proxy && npm run build:azure-wif-refresh", + "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); for (const n of ['gate','import','exec-context-pr','exec-context-pr-synth','exec-context-manual','exec-context-pipeline','exec-context-ci-push','exec-context-workitem','exec-context-schedule','exec-context-pr-checks','exec-context-repo','conclusion','approval-summary','github-app-token','prepare-pr-base','ado-proxy','azure-wif-refresh']) fs.rmSync(n+'.js',{force:true});\"", "build:gate": "ncc build src/gate/index.ts -o .ado-build/gate -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/gate/index.js','gate.js'); fs.rmSync('.ado-build/gate',{recursive:true,force:true});\"", "build:import": "ncc build src/import/index.ts -o .ado-build/import -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/import/index.js','import.js'); fs.rmSync('.ado-build/import',{recursive:true,force:true});\"", "build:exec-context-pr": "ncc build src/exec-context-pr/index.ts -o .ado-build/exec-context-pr -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-pr/index.js','exec-context-pr.js'); fs.rmSync('.ado-build/exec-context-pr',{recursive:true,force:true});\"", @@ -25,13 +25,14 @@ "build:github-app-token": "ncc build src/github-app-token/index.ts -o .ado-build/github-app-token -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/github-app-token/index.js','github-app-token.js'); fs.rmSync('.ado-build/github-app-token',{recursive:true,force:true});\"", "build:prepare-pr-base": "ncc build src/prepare-pr-base/index.ts -o .ado-build/prepare-pr-base -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/prepare-pr-base/index.js','prepare-pr-base.js'); fs.rmSync('.ado-build/prepare-pr-base',{recursive:true,force:true});\"", "build:ado-proxy": "ncc build src/ado-proxy/index.ts -o .ado-build/ado-proxy -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/ado-proxy/index.js','ado-proxy.js'); fs.rmSync('.ado-build/ado-proxy',{recursive:true,force:true});\"", + "build:azure-wif-refresh": "ncc build src/azure-wif-refresh/index.ts -o .ado-build/azure-wif-refresh -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/azure-wif-refresh/index.js','azure-wif-refresh.js'); fs.rmSync('.ado-build/azure-wif-refresh',{recursive:true,force:true});\"", "build:executor-e2e": "ncc build src/executor-e2e/index.ts -o .ado-build/executor-e2e -m -t && node -e \"const fs=require('node:fs'); fs.mkdirSync('test-bin',{recursive:true}); fs.copyFileSync('.ado-build/executor-e2e/index.js','test-bin/executor-e2e.js'); fs.rmSync('.ado-build/executor-e2e',{recursive:true,force:true});\"", "build:trigger-e2e": "ncc build src/trigger-e2e/index.ts -o .ado-build/trigger-e2e -m -t && node -e \"const fs=require('node:fs'); fs.mkdirSync('test-bin',{recursive:true}); fs.copyFileSync('.ado-build/trigger-e2e/index.js','test-bin/trigger-e2e.js'); fs.rmSync('.ado-build/trigger-e2e',{recursive:true,force:true});\"", "build:compiler-smoke-e2e": "ncc build src/compiler-smoke-e2e/index.ts -o .ado-build/compiler-smoke-e2e -m -t && node -e \"const fs=require('node:fs'); fs.mkdirSync('test-bin',{recursive:true}); fs.copyFileSync('.ado-build/compiler-smoke-e2e/index.js','test-bin/compiler-smoke-e2e.js'); fs.rmSync('.ado-build/compiler-smoke-e2e',{recursive:true,force:true});\"", "build:check": "ls -lh gate.js && wc -c gate.js", "codegen": "node -e \"require('node:fs').mkdirSync('schema', { recursive: true })\" && cargo run --quiet --manifest-path ../../Cargo.toml -- export-gate-schema --output schema/gate-spec.schema.json && npx json2ts schema/gate-spec.schema.json -o src/shared/types.gen.ts --bannerComment \"// AUTO-GENERATED from Rust IR via cargo run -- export-gate-schema. Do not edit; run npm run codegen.\" && cargo run --quiet --manifest-path ../../Cargo.toml -- export-fact-catalog --output src/trigger-e2e/fact-catalog.gen.json && cargo run --quiet --manifest-path ../../Cargo.toml -- export-ado-proxy-catalog-schema --output schema/ado-proxy-catalog.schema.json && npx json2ts schema/ado-proxy-catalog.schema.json -o src/shared/ado-proxy-catalog.types.gen.ts --bannerComment \"// AUTO-GENERATED from Rust via cargo run -- export-ado-proxy-catalog-schema. Do not edit; run npm run codegen.\" && cargo run --quiet --manifest-path ../../Cargo.toml -- export-ado-proxy-catalog --output src/ado-proxy/catalog.gen.json", "test": "vitest run", - "test:smoke": "npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:conclusion && npm run build:approval-summary && npm run build:github-app-token && npm run build:prepare-pr-base && npm run build:ado-proxy && vitest run -c vitest.config.smoke.ts", + "test:smoke": "npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:conclusion && npm run build:approval-summary && npm run build:github-app-token && npm run build:prepare-pr-base && npm run build:ado-proxy && npm run build:azure-wif-refresh && vitest run -c vitest.config.smoke.ts", "lint": "echo TODO", "typecheck": "tsc --noEmit" }, diff --git a/scripts/ado-script/src/azure-wif-refresh/__tests__/index.test.ts b/scripts/ado-script/src/azure-wif-refresh/__tests__/index.test.ts new file mode 100644 index 000000000..42b7eda1e --- /dev/null +++ b/scripts/ado-script/src/azure-wif-refresh/__tests__/index.test.ts @@ -0,0 +1,531 @@ +import { + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + assertionTiming, + parseJwtExpiryMs, + parseMaterial, + requestOidcToken, + runRefresher, + writeAtomic, + type AtomicWriter, + type FetchLike, + type RefreshMaterial, + type StatusDocument, +} from "../index.js"; + +const INITIAL_TOKEN = "initial.secret.token"; +const SYSTEM_TOKEN = "system.secret.token"; + +function jwt(expSeconds: number): string { + const header = Buffer.from('{"alg":"none"}').toString("base64url"); + const payload = Buffer.from(JSON.stringify({ exp: expSeconds })).toString( + "base64url", + ); + return `${header}.${payload}.signature`; +} + +function material(overrides: Partial = {}): RefreshMaterial { + return { + initialIdToken: INITIAL_TOKEN, + systemAccessToken: SYSTEM_TOKEN, + oidcRequestUri: + "https://dev.azure.com/example/_apis/distributedtask/hubs/build/plans/plan/jobs/job/oidctoken", + serviceConnectionId: "11111111-2222-3333-4444-555555555555", + tokenPath: "/state/token", + readyPath: "/state/ready.json", + statusPath: "/state/status.json", + ...overrides, + }; +} + +function recordingWriter() { + const files = new Map(); + const writes: Array<{ path: string; content: string; mode: number }> = []; + const writer: AtomicWriter = async (path, content, mode) => { + writes.push({ path, content, mode }); + files.set(path, { content, mode }); + }; + return { files, writes, writer }; +} + +function statusDocuments( + writes: Array<{ path: string; content: string }>, + path = "/state/status.json", +): StatusDocument[] { + return writes + .filter((write) => write.path === path) + .map((write) => JSON.parse(write.content) as StatusDocument); +} + +const tempDirs: string[] = []; + +afterEach(() => { + vi.restoreAllMocks(); + for (const directory of tempDirs.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +describe("material and expiry parsing", () => { + it("accepts the closed material schema and rejects unknown or empty fields", () => { + expect(parseMaterial(JSON.stringify(material()))).toEqual(material()); + expect(() => + parseMaterial(JSON.stringify({ ...material(), extra: "nope" })), + ).toThrow(/unknown fields/); + expect(() => + parseMaterial(JSON.stringify({ ...material(), serviceConnectionId: "" })), + ).toThrow(/serviceConnectionId/); + expect(() => + parseMaterial( + JSON.stringify({ ...material(), serviceConnectionId: "not-a-guid" }), + ), + ).toThrow(/GUID/); + }); + + it("parses a JWT exp without verifying the signature", () => { + expect(parseJwtExpiryMs(jwt(1_700_000_123))).toBe(1_700_000_123_000); + expect(parseJwtExpiryMs("not-a-jwt")).toBeUndefined(); + expect(parseJwtExpiryMs("a.e30.c")).toBeUndefined(); + expect(parseJwtExpiryMs("a.WyJub3QiLCJhbiIsIm9iamVjdCJd.c")).toBeUndefined(); + }); + + it("refreshes 60 seconds before exp and falls back to four minutes", () => { + const now = 1_700_000_000_000; + expect(assertionTiming(jwt(now / 1000 + 300), now)).toEqual({ + expiresAt: now + 300_000, + refreshAt: now + 240_000, + fallback: false, + }); + expect(assertionTiming("malformed", now)).toEqual({ + expiresAt: now + 300_000, + refreshAt: now + 240_000, + fallback: true, + }); + }); +}); + +describe("atomic publication", () => { + it("atomically replaces the assertion with mode 0644", async () => { + const directory = mkdtempSync(join(tmpdir(), "ado-aw-wif-")); + tempDirs.push(directory); + const tokenPath = join(directory, "token"); + writeFileSync(tokenPath, "old", "utf8"); + + await writeAtomic(tokenPath, INITIAL_TOKEN, 0o644); + + expect(readFileSync(tokenPath, "utf8")).toBe(INITIAL_TOKEN); + if (process.platform !== "win32") { + expect(statSync(tokenPath).mode & 0o777).toBe(0o644); + } + expect(readdirSync(directory)).toEqual(["token"]); + }); +}); + +describe("OIDC refresh request", () => { + it("posts to the supplied endpoint with the bearer and service connection GUID", async () => { + const signal = new AbortController().signal; + const fetchFn = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ oidcToken: "refreshed.assertion.value" }), + }); + const value = material({ + oidcRequestUri: "https://example.test/oidc", + serviceConnectionId: "id with spaces", + }); + + await expect(requestOidcToken(value, signal, fetchFn)).resolves.toBe( + "refreshed.assertion.value", + ); + expect(fetchFn).toHaveBeenCalledWith( + "https://example.test/oidc?api-version=7.1&serviceConnectionId=id%20with%20spaces", + { + method: "POST", + headers: { + Authorization: `Bearer ${SYSTEM_TOKEN}`, + "Content-Type": "application/json", + "X-TFS-FedAuthRedirect": "Suppress", + }, + body: "{}", + signal, + }, + ); + }); + + it("rejects a response without a non-empty oidcToken", async () => { + const fetchFn = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ oidcToken: "" }), + }); + + await expect( + requestOidcToken(material(), new AbortController().signal, fetchFn), + ).rejects.toThrow(); + }); +}); + +describe("refresh state machine", () => { + it("publishes the initial assertion before readiness, then stops cleanly", async () => { + let now = 1_700_000_000_000; + const token = jwt(now / 1000 + 300); + const controller = new AbortController(); + const { writes, files, writer } = recordingWriter(); + + const rc = await runRefresher( + material({ initialIdToken: token }), + controller.signal, + { + now: () => now, + writeAtomic: writer, + sleep: async (ms) => { + now += ms; + controller.abort(); + }, + provider: { createOidcToken: vi.fn() }, + }, + ); + + expect(rc).toBe(0); + expect(writes[0]!.path).toBe("/state/status.json"); + expect(JSON.parse(writes[0]!.content)).toMatchObject({ + state: "starting", + }); + expect(writes[1]).toEqual({ + path: "/state/token", + content: token, + mode: 0o644, + }); + expect(writes[2]!.path).toBe("/state/status.json"); + expect(JSON.parse(writes[2]!.content)).toMatchObject({ + state: "ready", + }); + expect(writes[3]!.path).toBe("/state/ready.json"); + expect(JSON.parse(writes[3]!.content)).toMatchObject({ + state: "ready", + }); + expect(JSON.parse(files.get("/state/status.json")!.content)).toMatchObject({ + state: "stopped", + }); + }); + + it("requests and publishes a replacement at exp minus 60 seconds", async () => { + let now = 1_700_000_000_000; + const initial = jwt(now / 1000 + 300); + const replacement = jwt(now / 1000 + 600); + const controller = new AbortController(); + const { writes, writer } = recordingWriter(); + const provider = vi.fn().mockResolvedValue(replacement); + let sleepCount = 0; + + const rc = await runRefresher( + material({ initialIdToken: initial }), + controller.signal, + { + now: () => now, + writeAtomic: writer, + provider: { createOidcToken: provider }, + sleep: async (ms) => { + now += ms; + sleepCount += 1; + if (sleepCount === 2) controller.abort(); + }, + }, + ); + + expect(rc).toBe(0); + expect(provider).toHaveBeenCalledTimes(1); + expect(provider.mock.invocationCallOrder[0]).toBeDefined(); + const tokenWrites = writes.filter((write) => write.path === "/state/token"); + expect(tokenWrites.map((write) => write.content)).toEqual([ + initial, + replacement, + ]); + expect( + statusDocuments(writes).some( + (status) => + status.state === "refreshing" && + status.updatedAt === new Date(1_700_000_240_000).toISOString(), + ), + ).toBe(true); + }); + + it("uses the malformed-exp fallback and emits no token material in warnings", async () => { + let now = 1_700_000_000_000; + const controller = new AbortController(); + const { writer } = recordingWriter(); + const report = vi.fn(); + const provider = vi.fn().mockResolvedValue(jwt(now / 1000 + 600)); + let sleepCount = 0; + + const rc = await runRefresher(material(), controller.signal, { + now: () => now, + writeAtomic: writer, + report, + provider: { createOidcToken: provider }, + sleep: async (ms) => { + if (sleepCount === 0) expect(ms).toBe(240_000); + now += ms; + sleepCount += 1; + if (sleepCount === 2) controller.abort(); + }, + }); + + expect(rc).toBe(0); + expect(provider).toHaveBeenCalledTimes(1); + const output = report.mock.calls.flat().join("\n"); + expect(output).toContain("conservative timing"); + expect(output).not.toContain(INITIAL_TOKEN); + expect(output).not.toContain(SYSTEM_TOKEN); + }); + + it("retries transient failures with capped exponential backoff", async () => { + let now = 1_700_000_000_000; + const initial = jwt(now / 1000 + 180); + const replacement = jwt(now / 1000 + 600); + const controller = new AbortController(); + const { writes, writer } = recordingWriter(); + const error = Object.assign(new Error("throttled"), { statusCode: 429 }); + const provider = vi + .fn() + .mockRejectedValueOnce(error) + .mockResolvedValueOnce(replacement); + const sleeps: number[] = []; + + const rc = await runRefresher( + material({ initialIdToken: initial }), + controller.signal, + { + now: () => now, + writeAtomic: writer, + provider: { createOidcToken: provider }, + sleep: async (ms) => { + sleeps.push(ms); + now += ms; + if (provider.mock.calls.length === 2) controller.abort(); + }, + }, + ); + + expect(rc).toBe(0); + expect(provider).toHaveBeenCalledTimes(2); + expect(sleeps.slice(0, 2)).toEqual([120_000, 1_000]); + expect(statusDocuments(writes)).toContainEqual( + expect.objectContaining({ + state: "refreshing", + errorCategory: "throttled", + }), + ); + }); + + it("aborts the OIDC fetch when a request times out", async () => { + let now = 1_700_000_000_000; + const controller = new AbortController(); + const { writes, writer } = recordingWriter(); + let fetchSignal: AbortSignal | undefined; + const fetchFn: FetchLike = async (_url, init) => { + fetchSignal = init.signal; + return await new Promise((_, reject) => { + init.signal.addEventListener( + "abort", + () => reject(new Error("fetch aborted")), + { once: true }, + ); + }); + }; + let sleepCount = 0; + + const rc = await runRefresher( + material({ initialIdToken: jwt(now / 1000 + 61) }), + controller.signal, + { + now: () => now, + writeAtomic: writer, + requestTimeoutMs: 1, + provider: { + createOidcToken: async (value, signal) => + await requestOidcToken(value, signal, fetchFn), + }, + sleep: async (ms) => { + now += ms; + sleepCount += 1; + if (sleepCount === 2) controller.abort(); + }, + }, + ); + + expect(rc).toBe(0); + expect(fetchSignal?.aborted).toBe(true); + expect(statusDocuments(writes)).toContainEqual( + expect.objectContaining({ + state: "refreshing", + errorCategory: "timeout", + }), + ); + }); + + it("aborts the OIDC fetch during shutdown", async () => { + let now = 1_700_000_000_000; + const controller = new AbortController(); + const { writes, writer } = recordingWriter(); + let fetchSignal: AbortSignal | undefined; + const fetchFn: FetchLike = async (_url, init) => { + fetchSignal = init.signal; + queueMicrotask(() => controller.abort()); + return await new Promise((_, reject) => { + init.signal.addEventListener( + "abort", + () => reject(new Error("fetch aborted")), + { once: true }, + ); + }); + }; + + const rc = await runRefresher( + material({ initialIdToken: jwt(now / 1000 + 61) }), + controller.signal, + { + now: () => now, + writeAtomic: writer, + provider: { + createOidcToken: async (value, signal) => + await requestOidcToken(value, signal, fetchFn), + }, + sleep: async (ms) => { + now += ms; + }, + }, + ); + + expect(rc).toBe(0); + expect(fetchSignal?.aborted).toBe(true); + expect(statusDocuments(writes).at(-1)).toMatchObject({ + state: "stopped", + }); + }); + + it("rejects an empty refresh without overwriting the current assertion", async () => { + let now = 1_700_000_000_000; + const initial = jwt(now / 1000 + 61); + const { writes, writer } = recordingWriter(); + + const rc = await runRefresher( + material({ initialIdToken: initial }), + new AbortController().signal, + { + now: () => now, + writeAtomic: writer, + provider: { createOidcToken: vi.fn().mockResolvedValue("") }, + sleep: async (ms) => { + now += ms; + }, + }, + ); + + expect(rc).toBe(1); + expect( + writes.filter((write) => write.path === "/state/token"), + ).toHaveLength(1); + expect(statusDocuments(writes).at(-1)).toMatchObject({ + state: "unhealthy", + errorCategory: "invalid-response", + }); + }); + + it("becomes unhealthy only after refresh failures outlive the assertion", async () => { + let now = 1_700_000_000_000; + const initial = jwt(now / 1000 + 62); + const { writes, writer } = recordingWriter(); + const provider = vi + .fn() + .mockRejectedValue(Object.assign(new Error("server body"), { + statusCode: 503, + })); + + const rc = await runRefresher( + material({ initialIdToken: initial }), + new AbortController().signal, + { + now: () => now, + writeAtomic: writer, + provider: { createOidcToken: provider }, + sleep: async (ms) => { + now += ms; + }, + }, + ); + + expect(rc).toBe(1); + expect(provider.mock.calls.length).toBeGreaterThan(1); + expect(statusDocuments(writes).at(-1)).toMatchObject({ + state: "unhealthy", + errorCategory: "server", + }); + }); + + it("redacts both credentials from diagnostics and persisted status", async () => { + let now = 1_700_000_000_000; + const initial = jwt(now / 1000 + 61); + const { writes, writer } = recordingWriter(); + const report = vi.fn(); + const credentialError = new Error( + `request failed with ${INITIAL_TOKEN} and ${SYSTEM_TOKEN}`, + ); + + const rc = await runRefresher( + material({ initialIdToken: initial }), + new AbortController().signal, + { + now: () => now, + writeAtomic: writer, + report, + provider: { + createOidcToken: vi.fn().mockRejectedValue(credentialError), + }, + sleep: async (ms) => { + now += ms; + }, + }, + ); + + expect(rc).toBe(1); + const observable = [ + ...report.mock.calls.flat().map(String), + ...writes + .filter((write) => write.path !== "/state/token") + .map((write) => write.content), + ].join("\n"); + expect(observable).not.toContain(INITIAL_TOKEN); + expect(observable).not.toContain(SYSTEM_TOKEN); + }); + + it("preserves network categories for coded startup failures", async () => { + const report = vi.fn(); + const networkError = Object.assign(new Error("connection reset"), { + code: "ECONNRESET", + }); + + const rc = await runRefresher( + material(), + new AbortController().signal, + { + report, + writeAtomic: vi.fn().mockRejectedValue(networkError), + }, + ); + + expect(rc).toBe(1); + expect(report).toHaveBeenCalledWith("sidecar failed (network)"); + }); +}); diff --git a/scripts/ado-script/src/azure-wif-refresh/index.ts b/scripts/ado-script/src/azure-wif-refresh/index.ts new file mode 100644 index 000000000..2094e6478 --- /dev/null +++ b/scripts/ado-script/src/azure-wif-refresh/index.ts @@ -0,0 +1,793 @@ +/** + * azure-wif-refresh — maintain a rotating Azure federated assertion file. + * + * The trusted host writes one JSON material document to stdin. This sidecar + * keeps the Azure DevOps bearer in memory, publishes only the federated + * assertion, and refreshes it before expiry for an MCP container that mounts + * the token path read-only. + */ +import { randomUUID } from "node:crypto"; +import { + chmod, + mkdir, + open, + rename, + unlink, +} from "node:fs/promises"; +import { basename, dirname, join } from "node:path"; +import type { Readable } from "node:stream"; + +const REFRESH_SKEW_MS = 60_000; +const FALLBACK_REFRESH_MS = 4 * 60_000; +const FALLBACK_VALIDITY_MS = 5 * 60_000; +const INITIAL_RETRY_MS = 1_000; +const MAX_RETRY_MS = 30_000; +const REQUEST_TIMEOUT_MS = 30_000; +const MAX_MATERIAL_BYTES = 1024 * 1024; + +const MATERIAL_FIELDS = [ + "initialIdToken", + "systemAccessToken", + "oidcRequestUri", + "serviceConnectionId", + "tokenPath", + "readyPath", + "statusPath", +] as const; + +type MaterialField = (typeof MATERIAL_FIELDS)[number]; + +export interface RefreshMaterial { + readonly initialIdToken: string; + readonly systemAccessToken: string; + readonly oidcRequestUri: string; + readonly serviceConnectionId: string; + readonly tokenPath: string; + readonly readyPath: string; + readonly statusPath: string; +} + +export type ErrorCategory = + | "timeout" + | "network" + | "throttled" + | "server" + | "client" + | "invalid-response" + | "filesystem" + | "unknown"; + +export type SidecarState = + | "starting" + | "ready" + | "refreshing" + | "unhealthy" + | "stopped"; + +export interface StatusDocument { + readonly state: SidecarState; + readonly updatedAt: string; + readonly assertionExpiresAt?: string; + readonly nextRefreshAt?: string; + readonly lastRefreshAt?: string; + readonly stoppedAt?: string; + readonly errorCategory?: ErrorCategory; +} + +export interface ReadyDocument { + readonly state: "ready"; + readonly readyAt: string; + readonly assertionExpiresAt: string; +} + +interface AssertionTiming { + readonly expiresAt: number; + readonly refreshAt: number; + readonly fallback: boolean; +} + +export interface OidcProvider { + createOidcToken( + material: RefreshMaterial, + signal: AbortSignal, + ): Promise; +} + +export type AtomicWriter = ( + path: string, + content: string, + mode: number, +) => Promise; + +export interface RuntimeDependencies { + readonly now?: () => number; + readonly sleep?: (ms: number, signal: AbortSignal) => Promise; + readonly provider?: OidcProvider; + readonly writeAtomic?: AtomicWriter; + readonly report?: (message: string) => void; + readonly requestTimeoutMs?: number; +} + +export class MaterialError extends Error {} +export class ShutdownError extends Error {} +class RequestTimeoutError extends Error {} +class InvalidResponseError extends Error {} + +function asRecord(value: unknown): Record { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new MaterialError("material must be a JSON object"); + } + return value as Record; +} + +function requireNonemptyString( + source: Record, + field: MaterialField, +): string { + const value = source[field]; + if (typeof value !== "string" || value.trim() === "") { + throw new MaterialError(`${field} must be a non-empty string`); + } + return value; +} + +function requireGuid( + source: Record, + field: "serviceConnectionId", +): string { + const value = requireNonemptyString(source, field); + if ( + !/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test( + value, + ) + ) { + throw new MaterialError(`${field} must be a GUID`); + } + return value; +} + +/** Parse and strictly validate the one-shot stdin material document. */ +export function parseMaterial(raw: string): RefreshMaterial { + if (raw.trim() === "") { + throw new MaterialError("no material on stdin"); + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new MaterialError("material is not valid JSON"); + } + + const source = asRecord(parsed); + const allowed = new Set(MATERIAL_FIELDS); + if (Object.keys(source).some((key) => !allowed.has(key))) { + throw new MaterialError("material contains unknown fields"); + } + + return { + initialIdToken: requireNonemptyString(source, "initialIdToken"), + systemAccessToken: requireNonemptyString(source, "systemAccessToken"), + oidcRequestUri: requireNonemptyString(source, "oidcRequestUri"), + serviceConnectionId: requireGuid(source, "serviceConnectionId"), + tokenPath: requireNonemptyString(source, "tokenPath"), + readyPath: requireNonemptyString(source, "readyPath"), + statusPath: requireNonemptyString(source, "statusPath"), + }; +} + +/** + * Read exactly one top-level JSON object, then detach from stdin without + * waiting for the producer to close the pipe. + */ +export function readOneJsonDocument( + input: Readable = process.stdin, +): Promise { + return new Promise((resolve, reject) => { + let buffer = ""; + let started = false; + let depth = 0; + let inString = false; + let escaped = false; + + const cleanup = (): void => { + input.off("data", onData); + input.off("end", onEnd); + input.off("error", onError); + input.pause(); + }; + + const fail = (message: string): void => { + cleanup(); + reject(new MaterialError(message)); + }; + + const onData = (chunk: Buffer | string): void => { + const text = chunk.toString(); + if (Buffer.byteLength(buffer) + Buffer.byteLength(text) > MAX_MATERIAL_BYTES) { + fail("material exceeds the size limit"); + return; + } + const previousLength = buffer.length; + buffer += text; + + for (let i = previousLength; i < buffer.length; i += 1) { + const char = buffer[i]!; + if (!started) { + if (/\s/.test(char)) continue; + if (char !== "{") { + fail("material must be a JSON object"); + return; + } + started = true; + depth = 1; + continue; + } + if (inString) { + if (escaped) { + escaped = false; + } else if (char === "\\") { + escaped = true; + } else if (char === '"') { + inString = false; + } + continue; + } + if (char === '"') { + inString = true; + } else if (char === "{" || char === "[") { + depth += 1; + } else if (char === "}" || char === "]") { + depth -= 1; + if (depth === 0) { + const document = buffer.slice(0, i + 1); + if (buffer.slice(i + 1).trim() !== "") { + fail("material contains trailing data"); + return; + } + cleanup(); + resolve(document); + return; + } + if (depth < 0) { + fail("material is not valid JSON"); + return; + } + } + } + }; + + const onEnd = (): void => { + fail("stdin ended before a complete material document was received"); + }; + const onError = (): void => { + fail("cannot read material from stdin"); + }; + + input.setEncoding("utf8"); + input.on("data", onData); + input.once("end", onEnd); + input.once("error", onError); + input.resume(); + }); +} + +/** Decode a JWT expiry without verifying its signature. */ +export function parseJwtExpiryMs(token: string): number | undefined { + const segments = token.split("."); + if (segments.length !== 3 || !segments[1]) return undefined; + try { + const payload: unknown = JSON.parse( + Buffer.from(segments[1], "base64url").toString("utf8"), + ); + if (typeof payload !== "object" || payload === null || Array.isArray(payload)) { + return undefined; + } + const exp = (payload as Record).exp; + if ( + typeof exp !== "number" || + !Number.isSafeInteger(exp) || + exp <= 0 || + exp > Math.floor(Number.MAX_SAFE_INTEGER / 1000) + ) { + return undefined; + } + return exp * 1000; + } catch { + return undefined; + } +} + +export function assertionTiming(token: string, now: number): AssertionTiming { + const expiresAt = parseJwtExpiryMs(token); + if (expiresAt === undefined) { + return { + expiresAt: now + FALLBACK_VALIDITY_MS, + refreshAt: now + FALLBACK_REFRESH_MS, + fallback: true, + }; + } + return { + expiresAt, + refreshAt: Math.max(now, expiresAt - REFRESH_SKEW_MS), + fallback: false, + }; +} + +/** Replace a file atomically using a private same-directory temporary file. */ +export async function writeAtomic( + path: string, + content: string, + mode: number, +): Promise { + const directory = dirname(path); + await mkdir(directory, { recursive: true }); + const temporaryPath = join( + directory, + `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`, + ); + let handle; + try { + handle = await open(temporaryPath, "wx", mode); + await handle.writeFile(content, "utf8"); + await handle.sync(); + await handle.chmod(mode); + await handle.close(); + handle = undefined; + await rename(temporaryPath, path); + await chmod(path, mode); + } catch (error) { + await handle?.close().catch(() => undefined); + await unlink(temporaryPath).catch(() => undefined); + throw error; + } +} + +function defaultReport(message: string): void { + process.stderr.write(`[azure-wif-refresh] ${message}\n`); +} + +function defaultSleep(ms: number, signal: AbortSignal): Promise { + if (signal.aborted) return Promise.reject(new ShutdownError()); + return new Promise((resolve, reject) => { + const timer = setTimeout(done, ms); + + function done(): void { + signal.removeEventListener("abort", aborted); + resolve(); + } + function aborted(): void { + clearTimeout(timer); + signal.removeEventListener("abort", aborted); + reject(new ShutdownError()); + } + + signal.addEventListener("abort", aborted, { once: true }); + }); +} + +export interface FetchLike { + ( + url: string, + init: { + method: "POST"; + headers: Record; + body: string; + signal: AbortSignal; + }, + ): Promise<{ + ok: boolean; + status: number; + json(): Promise; + }>; +} + +class HttpResponseError extends Error { + readonly statusCode: number; + + constructor(statusCode: number) { + super("OIDC endpoint returned a non-success status"); + this.statusCode = statusCode; + } +} + +/** Request a fresh assertion from the job-scoped Azure DevOps OIDC endpoint. */ +export async function requestOidcToken( + material: RefreshMaterial, + signal: AbortSignal, + fetchFn: FetchLike = fetch as unknown as FetchLike, +): Promise { + const url = + `${material.oidcRequestUri}?api-version=7.1&serviceConnectionId=` + + encodeURIComponent(material.serviceConnectionId); + const response = await fetchFn(url, { + method: "POST", + headers: { + Authorization: `Bearer ${material.systemAccessToken}`, + "Content-Type": "application/json", + "X-TFS-FedAuthRedirect": "Suppress", + }, + body: "{}", + signal, + }); + if (!response.ok) { + throw new HttpResponseError(response.status); + } + const body: unknown = await response.json(); + if (typeof body !== "object" || body === null || Array.isArray(body)) { + throw new InvalidResponseError(); + } + return extractOidcToken((body as Record).oidcToken); +} + +class AzureDevOpsOidcProvider implements OidcProvider { + async createOidcToken( + material: RefreshMaterial, + signal: AbortSignal, + ): Promise { + return await requestOidcToken(material, signal); + } +} + +function httpStatusCode(error: unknown): number | undefined { + if (!error || typeof error !== "object") return undefined; + const value = error as { + statusCode?: unknown; + response?: { status?: unknown; statusCode?: unknown }; + }; + if (typeof value.statusCode === "number") return value.statusCode; + if (typeof value.response?.status === "number") return value.response.status; + if (typeof value.response?.statusCode === "number") { + return value.response.statusCode; + } + return undefined; +} + +export function errorCategory(error: unknown): ErrorCategory { + if (error instanceof RequestTimeoutError) return "timeout"; + if (error instanceof InvalidResponseError) return "invalid-response"; + + const status = httpStatusCode(error); + if (status === 429) return "throttled"; + if (status !== undefined && status >= 500 && status < 600) return "server"; + if (status !== undefined && status >= 400 && status < 500) return "client"; + + if (error && typeof error === "object") { + const code = (error as { code?: unknown }).code; + if ( + typeof code === "string" && + new Set([ + "ECONNABORTED", + "ECONNREFUSED", + "ECONNRESET", + "EHOSTUNREACH", + "ENETDOWN", + "ENETUNREACH", + "ENOTFOUND", + "EPIPE", + "ETIMEDOUT", + "EAI_AGAIN", + "UND_ERR_CONNECT_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", + ]).has(code) + ) { + return "network"; + } + } + return "unknown"; +} + +function iso(timestamp: number): string { + return new Date(timestamp).toISOString(); +} + +async function writeJson( + writer: AtomicWriter, + path: string, + document: StatusDocument | ReadyDocument, +): Promise { + await writer(path, `${JSON.stringify(document)}\n`, 0o644); +} + +function extractOidcToken(value: unknown): string { + if (typeof value !== "string" || value.trim() === "") { + throw new InvalidResponseError(); + } + return value; +} + +async function requestWithTimeout( + provider: OidcProvider, + material: RefreshMaterial, + signal: AbortSignal, + timeoutMs: number, +): Promise { + if (signal.aborted) throw new ShutdownError(); + const requestController = new AbortController(); + let timeout: NodeJS.Timeout | undefined; + let abort: (() => void) | undefined; + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + requestController.abort(); + reject(new RequestTimeoutError()); + }, timeoutMs); + }); + const abortPromise = new Promise((_, reject) => { + abort = () => { + requestController.abort(); + reject(new ShutdownError()); + }; + signal.addEventListener("abort", abort, { once: true }); + }); + try { + return await Promise.race([ + provider.createOidcToken(material, requestController.signal), + timeoutPromise, + abortPromise, + ]); + } finally { + if (timeout !== undefined) clearTimeout(timeout); + if (abort !== undefined) signal.removeEventListener("abort", abort); + } +} + +async function stopped( + material: RefreshMaterial, + writer: AtomicWriter, + now: () => number, +): Promise { + const timestamp = now(); + await writeJson(writer, material.statusPath, { + state: "stopped", + updatedAt: iso(timestamp), + stoppedAt: iso(timestamp), + }); + return 0; +} + +async function unhealthy( + material: RefreshMaterial, + writer: AtomicWriter, + now: () => number, + timing: AssertionTiming, + category: ErrorCategory, +): Promise { + await writeJson(writer, material.statusPath, { + state: "unhealthy", + updatedAt: iso(now()), + assertionExpiresAt: iso(timing.expiresAt), + errorCategory: category, + }); + return 1; +} + +/** + * Run the refresh state machine until shutdown or until no valid assertion + * remains. All diagnostics and status fields are fixed, sanitized values. + */ +export async function runRefresher( + material: RefreshMaterial, + signal: AbortSignal, + dependencies: RuntimeDependencies = {}, +): Promise { + const now = dependencies.now ?? Date.now; + const sleep = dependencies.sleep ?? defaultSleep; + const provider = dependencies.provider ?? new AzureDevOpsOidcProvider(); + const writer = dependencies.writeAtomic ?? writeAtomic; + const report = dependencies.report ?? defaultReport; + const requestTimeoutMs = + dependencies.requestTimeoutMs ?? REQUEST_TIMEOUT_MS; + + let timing = assertionTiming(material.initialIdToken, now()); + let lastRefreshAt: number | undefined; + let lastFailureCategory: ErrorCategory | undefined; + + try { + await writeJson(writer, material.statusPath, { + state: "starting", + updatedAt: iso(now()), + }); + await writer(material.tokenPath, material.initialIdToken, 0o644); + if (timing.fallback) { + report("assertion expiry is unavailable; using conservative timing"); + } + if (timing.expiresAt <= now()) { + return await unhealthy( + material, + writer, + now, + timing, + "invalid-response", + ); + } + + const readyAt = now(); + await writeJson(writer, material.statusPath, { + state: "ready", + updatedAt: iso(readyAt), + assertionExpiresAt: iso(timing.expiresAt), + nextRefreshAt: iso(timing.refreshAt), + }); + await writeJson(writer, material.readyPath, { + state: "ready", + readyAt: iso(readyAt), + assertionExpiresAt: iso(timing.expiresAt), + }); + + for (;;) { + if (signal.aborted) return await stopped(material, writer, now); + const waitMs = Math.max(0, timing.refreshAt - now()); + try { + await sleep(waitMs, signal); + } catch (error) { + if (error instanceof ShutdownError || signal.aborted) { + return await stopped(material, writer, now); + } + throw error; + } + if (signal.aborted) return await stopped(material, writer, now); + + let retryMs = INITIAL_RETRY_MS; + for (;;) { + if (signal.aborted) return await stopped(material, writer, now); + const attemptAt = now(); + if (attemptAt >= timing.expiresAt) { + return await unhealthy( + material, + writer, + now, + timing, + lastFailureCategory ?? "timeout", + ); + } + + await writeJson(writer, material.statusPath, { + state: "refreshing", + updatedAt: iso(attemptAt), + assertionExpiresAt: iso(timing.expiresAt), + lastRefreshAt: + lastRefreshAt === undefined ? undefined : iso(lastRefreshAt), + }); + + try { + const remainingMs = Math.max(1, timing.expiresAt - now()); + const response = await requestWithTimeout( + provider, + material, + signal, + Math.min(requestTimeoutMs, remainingMs), + ); + const token = extractOidcToken(response); + const refreshedAt = now(); + const nextTiming = assertionTiming(token, refreshedAt); + if (!nextTiming.fallback && nextTiming.expiresAt <= refreshedAt) { + throw new InvalidResponseError(); + } + if (nextTiming.fallback) { + report("refreshed assertion expiry is unavailable; using conservative timing"); + } + if (signal.aborted) return await stopped(material, writer, now); + + await writer(material.tokenPath, token, 0o644); + timing = nextTiming; + lastRefreshAt = refreshedAt; + lastFailureCategory = undefined; + await writeJson(writer, material.statusPath, { + state: "ready", + updatedAt: iso(refreshedAt), + assertionExpiresAt: iso(timing.expiresAt), + nextRefreshAt: iso(timing.refreshAt), + lastRefreshAt: iso(lastRefreshAt), + }); + break; + } catch (error) { + if (error instanceof ShutdownError || signal.aborted) { + return await stopped(material, writer, now); + } + const category = errorCategory(error); + lastFailureCategory = category; + const currentTime = now(); + if (currentTime >= timing.expiresAt) { + return await unhealthy( + material, + writer, + now, + timing, + category, + ); + } + + const delay = Math.min( + retryMs, + MAX_RETRY_MS, + timing.expiresAt - currentTime, + ); + report(`refresh failed (${category}); retrying while assertion is valid`); + await writeJson(writer, material.statusPath, { + state: "refreshing", + updatedAt: iso(currentTime), + assertionExpiresAt: iso(timing.expiresAt), + nextRefreshAt: iso(currentTime + delay), + lastRefreshAt: + lastRefreshAt === undefined ? undefined : iso(lastRefreshAt), + errorCategory: category, + }); + try { + await sleep(delay, signal); + } catch (sleepError) { + if (sleepError instanceof ShutdownError || signal.aborted) { + return await stopped(material, writer, now); + } + throw sleepError; + } + retryMs = Math.min(retryMs * 2, MAX_RETRY_MS); + } + } + } + } catch (error) { + if (error instanceof ShutdownError || signal.aborted) { + try { + return await stopped(material, writer, now); + } catch { + report("failed to write stopped status (filesystem)"); + return 1; + } + } + const classified = errorCategory(error); + const category = + classified === "unknown" && + error && + typeof error === "object" && + "code" in error + ? "filesystem" + : classified; + report(`sidecar failed (${category})`); + try { + return await unhealthy(material, writer, now, timing, category); + } catch { + report("failed to write unhealthy status (filesystem)"); + return 1; + } + } +} + +/** Parse stdin, install signal handlers, and run the long-lived sidecar. */ +export async function main(): Promise { + let material: RefreshMaterial; + try { + material = parseMaterial(await readOneJsonDocument()); + } catch (error) { + defaultReport( + error instanceof MaterialError + ? `configuration error: ${error.message}` + : "configuration error: cannot read material", + ); + return 1; + } + + const controller = new AbortController(); + const shutdown = (): void => controller.abort(); + process.once("SIGTERM", shutdown); + process.once("SIGINT", shutdown); + try { + return await runRefresher(material, controller.signal); + } finally { + process.removeListener("SIGTERM", shutdown); + process.removeListener("SIGINT", shutdown); + } +} + +if ( + typeof process !== "undefined" && + process.argv[1]?.endsWith("azure-wif-refresh.js") +) { + void main().then( + (code) => { + process.exitCode = code; + }, + () => { + defaultReport("sidecar failed (unknown)"); + process.exitCode = 1; + }, + ); +} diff --git a/scripts/ado-script/test/azure-wif-isolation.test.ts b/scripts/ado-script/test/azure-wif-isolation.test.ts new file mode 100644 index 000000000..607a23443 --- /dev/null +++ b/scripts/ado-script/test/azure-wif-isolation.test.ts @@ -0,0 +1,300 @@ +import { spawnSync } from "node:child_process"; +import { createHash, randomUUID } from "node:crypto"; +import { + chmodSync, + copyFileSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { homedir, tmpdir } from "node:os"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { parse } from "yaml"; + +const testDir = dirname(fileURLToPath(import.meta.url)); +const repo = resolve(testDir, "../../.."); +const bundle = resolve(testDir, "../azure-wif-refresh.js"); +const dockerEnabled = process.env.ADO_AW_TEST_DOCKER === "1"; +const awfEnabled = process.env.ADO_AW_TEST_AWF === "1"; +const image = "node:20-slim"; + +interface PipelineStep { + displayName?: string; + bash?: string; + inputs?: { inlineScript?: string }; +} + +interface Pipeline { + jobs: Array<{ job: string; steps: PipelineStep[] }>; +} + +interface McpgConfig { + mcpServers: Record; +} + +function run(command: string, args: string[], env?: NodeJS.ProcessEnv): string { + const result = spawnSync(command, args, { + encoding: "utf8", + timeout: 180_000, + maxBuffer: 4 * 1024 * 1024, + env, + }); + if (result.error) throw result.error; + if (result.status !== 0) { + throw new Error(`${command} exited ${result.status}\n${result.stdout}\n${result.stderr}`); + } + return result.stdout.trim(); +} + +function docker(...args: string[]): string { + return run("docker", args); +} + +function compileFixture(directory: string) { + const source = join(directory, "workflow.md"); + const output = join(directory, "workflow.yml"); + writeFileSync(source, `--- +name: WIF isolation +description: Credential-free isolation regression +mcp-servers: + fixture: + container: node:20-slim + args: [--user, "20001:20001"] + azure-auth: + service-connection: unused-test-connection +--- +Do not invoke an agent. This workflow is only compiled by the test. +`); + const binary = resolve(repo, `target/debug/ado-aw${process.platform === "win32" ? ".exe" : ""}`); + run(binary, ["compile", "--force", source, "-o", output]); + const pipeline: Pipeline = parse(readFileSync(output, "utf8")); + const start = pipeline.jobs.flatMap((job) => job.steps) + .find((step) => step.displayName === "Start Azure auth refresher (fixture)"); + const script = start?.inputs?.inlineScript; + if (!script) throw new Error("compiled Azure WIF startup is missing"); + const launch = script.indexOf("docker run \\\n"); + if (launch < 0) throw new Error("compiled Azure WIF container launch is missing"); + const identities = [...script.matchAll(/^(?:CLIENT|TENANT)_VARIABLE='([^']+)'$/gm)] + .map((match) => match[1]!); + expect(identities).toHaveLength(2); + const configStep = pipeline.jobs.flatMap((job) => job.steps) + .find((step) => step.displayName === "Prepare MCPG config"); + const configMatch = configStep?.bash?.match( + /cat > "\$AGENT_TEMP\/staging\/mcpg-config.json" << '([^']+)'\n([\s\S]*?)\n\1/, + ); + if (!configMatch) throw new Error("compiled MCPG configuration is missing"); + const config: McpgConfig = JSON.parse(configMatch[2]!); + const mounts = config.mcpServers.fixture?.mounts; + if (!mounts || mounts.length !== 1) throw new Error("expected one compiler-owned assertion mount"); + return { pipeline, setup: script.slice(0, launch), identities, tokenMount: mounts[0]! }; +} + +async function until(message: string, predicate: () => boolean): Promise { + const deadline = Date.now() + 30_000; + while (!predicate()) { + if (Date.now() >= deadline) throw new Error(message); + await new Promise((resolve) => setTimeout(resolve, 100)); + } +} + +describe.skipIf(!dockerEnabled)("Azure WIF Linux filesystem contract", () => { + it("rotates through a read-only different-UID mount without exposing private siblings", async () => { + expect(docker("info", "--format", "{{.OSType}}")).toBe("linux"); + expect(existsSync(bundle), "build:azure-wif-refresh must run first").toBe(true); + const directory = mkdtempSync(join(tmpdir(), "ado-aw-wif-docker-")); + const volume = `ado-aw-wif-${randomUUID()}`; + const producer = `${volume}-producer`; + const consumer = `${volume}-consumer`; + let containerCreated = false; + let consumerCreated = false; + let volumeCreated = false; + try { + const { setup, tokenMount } = compileFixture(directory); + // Only Docker cleanup is stubbed. Run the compiler's actual credential + // metadata checks, umask, directory permissions and FIFO creation. + writeFileSync(join(directory, "setup.sh"), ` +docker() { [ "$1" = rm ] || return 1; } +${setup.replaceAll("$(Agent.TempDirectory)", "/state")} +`); + copyFileSync(bundle, join(directory, "refresher.mjs")); + copyFileSync(join(testDir, "fixtures/azure-wif-isolation.mjs"), join(directory, "worker.mjs")); + docker("volume", "create", volume); + volumeCreated = true; + docker("create", "--name", producer, "--network", "none", + "--mount", `type=volume,source=${volume},target=/state`, + image, "node", "/inputs/worker.mjs"); + containerCreated = true; + docker("cp", `${directory}${process.platform === "win32" ? "\\." : "/."}`, `${producer}:/inputs`); + docker("start", producer); + const exec = (script: string) => docker("exec", producer, "node", "-e", script); + await until("refresher did not publish readiness", () => { + const ready = exec(`const f=require("fs"); + if (!f.existsSync("/state/auth-path")) { console.log("pending"); } + else { const p=f.readFileSync("/state/auth-path","utf8"); + console.log(f.existsSync(p+"/ready.json") ? "ready" : "pending"); }`); + if (ready === "ready") return true; + expect(docker("inspect", "-f", "{{.State.Running}}", producer), + docker("logs", producer)).toBe("true"); + return false; + }); + const auth = exec('process.stdout.write(require("fs").readFileSync("/state/auth-path","utf8"))'); + const hostRoot = docker("volume", "inspect", "--format", "{{.Mountpoint}}", volume); + const [mountSource, mountDestination, mountMode] = tokenMount + .replace("$(Agent.TempDirectory)", hostRoot).split(":"); + if (!mountSource || !mountDestination || !mountMode) { + throw new Error("malformed compiler-generated assertion mount"); + } + expect(mountSource).toBe(`${hostRoot}${auth.slice("/state".length)}/token.d`); + expect(mountDestination).toBe("/var/run/ado-aw/azure"); + expect(mountMode).toBe("ro"); + docker("create", "--name", consumer, "--network", "none", + "--user", "20001:20001", "--cap-drop", "ALL", + "-v", `${mountSource}:${mountDestination}:${mountMode}`, + image, "node", "-e", "setInterval(()=>{},1000)"); + consumerCreated = true; + docker("start", consumer); + const readAsConsumer = (script: string) => docker( + "exec", consumer, "node", "-e", script.replaceAll("/identity", mountDestination), + ); + const initial = readAsConsumer('process.stdout.write(require("fs").readFileSync("/identity/token","utf8"))'); + exec('require("fs").writeFileSync("/state/advance-1","")'); + await until("refresher did not rotate the assertion", () => + exec(`const f=require("fs"); const s=JSON.parse(f.readFileSync(${JSON.stringify(auth + "/status.json")},"utf8")); console.log(s.lastRefreshAt ? "rotated" : "pending")`) === "rotated"); + const replacement = readAsConsumer('process.stdout.write(require("fs").readFileSync("/identity/token","utf8"))'); + expect(replacement).not.toBe(initial); + expect(JSON.parse(Buffer.from(replacement.split(".")[1]!, "base64url").toString()).exp) + .toBeGreaterThan(JSON.parse(Buffer.from(initial.split(".")[1]!, "base64url").toString()).exp); + readAsConsumer(` + const f=require("fs"),a=require("assert/strict"); + a.equal(f.statSync("/identity/token").mode & 511, 420); + a.throws(()=>f.writeFileSync("/identity/token","tampered"),{code:"EROFS"}); + for (const p of ["/identity/material","/identity/status.json","/identity/ready.json", + "/identity/../material","/identity/../status.json"]) { + a.throws(()=>f.readFileSync(p),{code:"ENOENT"}); + }`); + docker("run", "--rm", "--network", "none", "--user", "20001:20001", + "--cap-drop", "ALL", "--mount", `type=bind,source=${hostRoot},target=/host-view,readonly`, + image, "node", "-e", ` + const f=require("fs"),a=require("assert/strict"); + a.equal(f.readFileSync("/host-view/public","utf8"),"host-view-control"); + a.throws(()=>f.readFileSync(${JSON.stringify(`/host-view${auth.slice("/state".length)}/token.d/token`)}),{code:"EACCES"});`); + exec('require("fs").writeFileSync("/state/stop","")'); + expect(docker("wait", producer)).toBe("0"); + } finally { + if (consumerCreated) docker("rm", "-f", consumer); + if (containerCreated) docker("rm", "-f", producer); + if (volumeCreated) docker("volume", "rm", volume); + rmSync(directory, { recursive: true, force: true }); + } + }, 180_000); +}); + +describe.skipIf(!awfEnabled)("Azure WIF real AWF boundary", () => { + it("hides the host assertion and internal IDs from normal and chroot paths", async () => { + expect(process.platform, "the real AWF regression requires Linux").toBe("linux"); + if (!process.getuid || !process.getgid) throw new Error("Linux process identity is unavailable"); + const owner = `${process.getuid()}:${process.getgid()}`; + expect(docker("info", "--format", "{{.OSType}}")).toBe("linux"); + // /tmp is intentionally agent-readable in AWF; keep private material in + // a sibling of the workspace, outside both /tmp and mounted home subdirs. + const directory = mkdtempSync(join(homedir(), "ado-aw-wif-awf-")); + try { + const { pipeline, identities } = compileFixture(directory); + const workspace = join(directory, "workspace"); + const temp = join(directory, "runner-temp"); + const tools = join(directory, "tools"); + const home = join(directory, "home"); + const auth = join(temp, "ado-aw-azure-auth", "fixture"); + mkdirSync(workspace, { recursive: true }); + mkdirSync(home); + mkdirSync(join(auth, "token.d"), { recursive: true }); + chmodSync(join(temp, "ado-aw-azure-auth"), 0o700); + chmodSync(auth, 0o700); + chmodSync(join(auth, "token.d"), 0o755); + writeFileSync(join(auth, "token.d/token"), "synthetic-assertion", { mode: 0o644 }); + symlinkSync(join(auth, "token.d/token"), join(workspace, "token-link")); + mkdirSync(join(tools, "awf"), { recursive: true }); + + const runStep = pipeline.jobs.find((job) => job.job === "Agent")?.steps + .find((step) => step.bash?.includes("AWF_ARGS+=(--skip-pull --env-all)")); + if (!runStep?.bash) throw new Error("compiled AWF invocation is missing"); + const capture = join(directory, "awf-args"); + writeFileSync(join(tools, "awf/awf"), `#!/bin/sh +if [ "$1" = logs ]; then exit 0; fi +printf '%s\\0' "$@" > '${capture}' +`, { mode: 0o755 }); + const script = runStep.bash + .replaceAll("$(Agent.TempDirectory)", temp) + .replaceAll("$(Pipeline.Workspace)", tools) + .replaceAll("$(Build.SourcesDirectory)", workspace); + const env: NodeJS.ProcessEnv = { + PATH: process.env.PATH, + HOME: home, + WORKING_DIRECTORY: workspace, + WIF_TEST_AUTH: auth, + WIF_TEST_WORKSPACE: workspace, + }; + for (const name of identities) env[name] = "synthetic-identity"; + run("bash", ["-c", script], env); + const captured = readFileSync(capture, "utf8").split("\0").filter(Boolean); + const version = captured[captured.indexOf("--image-tag") + 1]; + expect(version).toMatch(/^\d+\.\d+\.\d+$/); + const executable = join(directory, "awf"); + const release = `https://github.com/github/gh-aw-firewall/releases/download/v${version}`; + const binaryResponse = await fetch(`${release}/awf-linux-x64`); + if (!binaryResponse.ok) throw new Error(`AWF download: HTTP ${binaryResponse.status}`); + const bytes = Buffer.from(await binaryResponse.arrayBuffer()); + const checksumsResponse = await fetch(`${release}/checksums.txt`); + if (!checksumsResponse.ok) throw new Error(`AWF checksums: HTTP ${checksumsResponse.status}`); + const checksums = await checksumsResponse.text(); + const checksum = checksums.split("\n").find((line) => /\s+\*?awf-linux-x64$/.test(line.trim())); + expect(checksum, "missing AWF checksum").toBeDefined(); + expect(createHash("sha256").update(bytes).digest("hex")).toBe(checksum!.split(/\s+/)[0]); + writeFileSync(executable, bytes, { mode: 0o755 }); + + const commandIndex = captured.indexOf("--"); + expect(commandIndex).toBeGreaterThan(0); + const args: string[] = []; + for (let i = 0; i < commandIndex; i++) { + // This probe exercises filesystem/env isolation, not MCP networking: + // omit the absent MCPG peer and allow AWF to pull its pinned images. + if (captured[i] === "--topology-attach") { i++; continue; } + if (captured[i] === "--skip-pull") continue; + args.push(captured[i]!); + } + const probe = `set -eu +test "$(cat "$WIF_TEST_WORKSPACE/control")" = workspace-visible +for p in "$WIF_TEST_AUTH/token.d/token" "/host$WIF_TEST_AUTH/token.d/token" "$WIF_TEST_WORKSPACE/token-link"; do + if cat "$p" >/dev/null 2>&1; then echo "Assertion exposed: $p" >&2; exit 1; fi +done +${identities.map((key) => `if printenv '${key}' >/dev/null; then echo "Internal identity exposed" >&2; exit 1; fi`).join("\n")} +echo wif-isolation-passed +`; + writeFileSync(join(workspace, "control"), "workspace-visible"); + writeFileSync(join(workspace, "probe.sh"), probe); + // AWF owns fixed container names. Never let a test replace an unrelated + // local AWF session; CI runs this probe once on its dedicated runner. + expect(docker("ps", "-aq", "--filter", "name=awf-"), + "an existing AWF session must be stopped by its owner first").toBe(""); + const output = run(executable, [ + ...args, "--work-dir", join(directory, "awf-state"), + "--agent-timeout", "1", + "--", `bash '${join(workspace, "probe.sh")}'`, + ], { ...env, GITHUB_WORKSPACE: workspace }); + expect(output).toContain("wif-isolation-passed"); + } finally { + // AWF's container setup creates root-owned files in its synthetic home. + // Restore only this disposable tree, without following symlinks. + docker("run", "--rm", "--network", "none", + "--mount", `type=bind,source=${directory},target=/fixture`, + image, "chown", "-Rh", owner, "/fixture"); + rmSync(directory, { recursive: true, force: true }); + } + }, 240_000); +}); diff --git a/scripts/ado-script/test/fixtures/azure-wif-isolation.mjs b/scripts/ado-script/test/fixtures/azure-wif-isolation.mjs new file mode 100644 index 000000000..16a464fb8 --- /dev/null +++ b/scripts/ado-script/test/fixtures/azure-wif-isolation.mjs @@ -0,0 +1,75 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { runRefresher } from "./refresher.mjs"; + +async function main() { + const state = "/state"; + const owner = 10001; + fs.chmodSync("/inputs", 0o755); + fs.chownSync(state, owner, owner); + fs.writeFileSync(path.join(state, "public"), "host-view-control"); + process.setgid(owner); + process.setuid(owner); + + const setup = spawnSync("bash", ["/inputs/setup.sh"], { + encoding: "utf8", + env: { + PATH: process.env.PATH, + idToken: "synthetic-initial-assertion", + servicePrincipalId: "11111111-2222-3333-4444-555555555555", + tenantId: "11111111-2222-3333-4444-555555555555", + AZURESUBSCRIPTION_SERVICE_CONNECTION_ID: "11111111-2222-3333-4444-555555555555", + SYSTEM_ACCESSTOKEN: "synthetic-job-token", + SYSTEM_OIDCREQUESTURI: "https://example.invalid/oidc", + }, + }); + assert.equal(setup.status, 0, setup.stderr); + const authRoot = path.join(state, "ado-aw-azure-auth"); + const serverDirs = fs.readdirSync(authRoot); + assert.equal(serverDirs.length, 1); + const auth = path.join(authRoot, serverDirs[0]); + fs.writeFileSync(path.join(state, "auth-path"), auth); + assert.equal(fs.statSync(authRoot).mode & 0o777, 0o700); + assert.equal(fs.statSync(auth).mode & 0o777, 0o700); + assert.equal(fs.statSync(path.join(auth, "token.d")).mode & 0o777, 0o755); + assert.equal(fs.statSync(path.join(auth, "material")).mode & 0o777, 0o600); + assert.ok(fs.statSync(path.join(auth, "material")).isFIFO()); + + let now = 1_700_000_000_000; + const jwt = (exp) => `eyJhbGciOiJub25lIn0.${Buffer.from(JSON.stringify({ exp })).toString("base64url")}.fake`; + const controller = new AbortController(); + let sleepCount = 0; + const code = await runRefresher({ + initialIdToken: jwt(now / 1000 + 300), + systemAccessToken: "synthetic-job-token", + oidcRequestUri: "https://example.invalid/oidc", + serviceConnectionId: "11111111-2222-3333-4444-555555555555", + tokenPath: path.join(auth, "token.d/token"), + readyPath: path.join(auth, "ready.json"), + statusPath: path.join(auth, "status.json"), + }, controller.signal, { + now: () => now, + provider: { createOidcToken: async () => jwt(now / 1000 + 300) }, + sleep: async (ms) => { + const marker = path.join(state, `advance-${++sleepCount}`); + const deadline = Date.now() + 60_000; + while (!fs.existsSync(marker)) { + if (fs.existsSync(path.join(state, "stop"))) { + controller.abort(); + return; + } + assert.ok(Date.now() < deadline, "test controller did not advance the refresher"); + await new Promise((resolve) => setTimeout(resolve, 25)); + } + now += ms; + }, + }); + assert.equal(code, 0); +} + +main().catch((error) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/src/compile/ado_bundle.rs b/src/compile/ado_bundle.rs index 831c55bcc..6ec933a5a 100644 --- a/src/compile/ado_bundle.rs +++ b/src/compile/ado_bundle.rs @@ -63,6 +63,10 @@ pub enum Bundle { /// containerized SafeOutputs MCP server can compute a diff base on /// shallow-default pools. PreparePrBase, + /// Renewable Azure Pipelines workload-identity assertion writer for + /// user-defined stdio MCP servers. Runs in a trusted sidecar for the + /// lifetime of the Agent job and receives credentials on stdin. + AzureWifRefresh, /// Credential-isolated Azure DevOps policy engine. Unlike every other /// bundle it is not invoked by a pipeline step: it is bind-mounted into /// the `ado-proxy` container and run there, for the whole lifetime of the @@ -153,6 +157,7 @@ impl Bundle { Bundle::Conclusion, Bundle::GithubAppToken, Bundle::PreparePrBase, + Bundle::AzureWifRefresh, Bundle::AdoProxy, ]; @@ -180,6 +185,7 @@ impl Bundle { Bundle::Conclusion => paths::CONCLUSION_PATH, Bundle::GithubAppToken => paths::GITHUB_APP_TOKEN_PATH, Bundle::PreparePrBase => paths::PREPARE_PR_BASE_PATH, + Bundle::AzureWifRefresh => paths::AZURE_WIF_REFRESH_PATH, Bundle::AdoProxy => paths::ADO_PROXY_PATH, } } @@ -209,6 +215,7 @@ impl Bundle { // Authenticates to the GitHub API with its own App JWT / minted // token, not the ADO bearer. | Bundle::GithubAppToken + | Bundle::AzureWifRefresh // Receives its ADO bearer inside the stdin material document, not // from the environment — deliberately, so the credential is not // visible in the container's `Env` or the process table. diff --git a/src/compile/agentic_pipeline.rs b/src/compile/agentic_pipeline.rs index f5e9c9411..ede1bb5d1 100644 --- a/src/compile/agentic_pipeline.rs +++ b/src/compile/agentic_pipeline.rs @@ -72,6 +72,9 @@ use super::common::{ HEADER_MARKER, MCPG_CONTAINER_NAME, MCPG_DOMAIN, MCPG_IMAGE, MCPG_PORT, MCPG_VERSION, image_ref, }; +use super::container_invocation::{ + DockerMount, DockerRun, DockerTmpfs, ShellWord, +}; use super::custom_tools::{CustomToolDefinition, collect_custom_tool_definitions}; use super::extensions::ado_script as paths; use super::extensions::{CompileContext, CompilerExtension, Declarations, Extension, McpgConfig}; @@ -83,7 +86,9 @@ use super::ir::output::{OutputDecl, OutputRef}; use super::ir::step::{ BashStep, CheckoutRepo, CheckoutStep, DownloadStep, PublishStep, Step, SubmodulesOpt, TaskStep, }; -use super::ir::tasks::azure_cli::{AzureCli, ScriptLocation, ScriptType}; +use super::ir::tasks::azure_cli::{ + AzureCli, AzureCliV3, AzureCliV3Connection, ScriptLocation, ScriptType, +}; use super::ir::tasks::docker_installer::DockerInstaller; use super::ir::tasks::download_package::DownloadPackage; use super::ir::tasks::download_pipeline_artifact::{ @@ -153,6 +158,21 @@ fn copilot_byom_exclude_keys(is_copilot: bool, engine_config: &EngineConfig) -> keys } +fn awf_exclude_keys( + front_matter: &FrontMatter, + is_copilot: bool, + engine_config: &EngineConfig, +) -> Result> { + let mut keys = copilot_byom_exclude_keys(is_copilot, engine_config); + for (server_name, _, _) in front_matter.azure_authenticated_mcp_servers() { + keys.push(super::mcpg::azure_auth_client_variable(server_name)?.into_inner()); + keys.push(super::mcpg::azure_auth_tenant_variable(server_name)?.into_inner()); + } + keys.sort(); + keys.dedup(); + Ok(keys) +} + /// Shared back-end for the three IR-driven target compilers /// (standalone / stage / job). Performs all the heavy lifting: /// validates the front matter, computes every scalar, fans out @@ -243,7 +263,7 @@ fn fanout_extension_declarations( /// Bundle of engine-derived values computed once per pipeline compile: /// prompt invocations, install steps, composed env blocks, and the -/// Copilot BYOM/BYOK exclusion keys for both the Agent and Detection +/// provider/MCP identity exclusion keys for both the Agent and Detection /// engines. Split out of [`build_pipeline_context`] purely to keep that /// function's cognitive complexity manageable — behaviour is unchanged. struct EngineSetup { @@ -304,10 +324,10 @@ fn build_engine_setup( // future non-Copilot engine whose env happens to contain a COPILOT_PROVIDER_* // key is never treated as a Copilot provider credential. let is_copilot = matches!(ctx.engine, crate::engine::Engine::Copilot); - let byom_exclude_keys = copilot_byom_exclude_keys(is_copilot, &front_matter.engine); + let byom_exclude_keys = awf_exclude_keys(front_matter, is_copilot, &front_matter.engine)?; let detection_is_copilot = matches!(detection_engine, crate::engine::Engine::Copilot); let detection_byom_exclude_keys = - copilot_byom_exclude_keys(detection_is_copilot, detection_engine_config); + awf_exclude_keys(front_matter, detection_is_copilot, detection_engine_config)?; let detection_engine_env = if detection_is_copilot { crate::engine::copilot_detection_env(detection_engine_config)? } else { @@ -845,8 +865,8 @@ pub(crate) struct StandaloneCtx { /// `{{#runtime-import ...}}` marker). pub(crate) agent_content_value: String, pub(crate) debug_pipeline: bool, - /// Actual provider credential env keys present to pass to AWF `--exclude-env`; - /// empty for non-BYOM. AWF's API proxy itself is always enabled. + /// Provider credential and internal MCP identity env keys excluded from AWF. + /// AWF's API proxy itself is always enabled. pub(crate) byom_exclude_keys: Vec, pub(crate) detection_byom_exclude_keys: Vec, /// Validated inherited/overridden custom env for Detection. @@ -1253,7 +1273,12 @@ fn build_agent_job( // 14. AWF path step (when extensions declare path prepends) push_raw_yaml_if_nonempty(&mut steps, &cfg.awf_path_step_yaml)?; - // 14a. Credential-isolated Azure DevOps policy engine. + // 14a. Renewable Azure workload-identity assertions for user-defined + // stdio MCP servers. The ado-script bundle was delivered by the + // always-on extension above when this feature is active. + steps.extend(start_azure_wif_refresh_steps(front_matter)?); + + // 14b. Credential-isolated Azure DevOps policy engine. // // Must precede MCPG: the Azure DevOps MCP is redirected at the // engine's container address, and that address does not exist until @@ -1393,7 +1418,11 @@ fn build_agent_job( // 20. Stop MCPG and SafeOutputs steps.push(Step::Bash(stop_mcpg_step())); - // 20a. Stop the policy engine, then remove its network. `--rm` only fires + // 20a. Stop renewable Azure assertion sidecars after MCPG has stopped its + // stdio children and released their read-only token mounts. + steps.extend(stop_azure_wif_refresh_steps(front_matter)); + + // 20b. Stop the policy engine, then remove its network. `--rm` only fires // on a clean exit, so an OOM or SIGKILL would otherwise leave the // container — and the credential it holds in memory — running past // the job. @@ -4162,7 +4191,10 @@ shell_script! { START_MCPG { interpreter: Bash, bindings: [MCPG_CONTAINER, MCPG_IMAGE, MCPG_PORT, MCPG_DOMAIN], - externals: [MCP_GATEWAY_API_KEY, ADO_PROXY_IP, MCPG_ENV_NAMES], + externals: [ + MCP_GATEWAY_API_KEY, ADO_PROXY_IP, + MCPG_ENV_NAMES, MCPG_REQUIRED_ENV_NAMES + ], fragments: [], body: r###" # Substitute runtime values into MCPG config @@ -4175,6 +4207,43 @@ MCPG_CONFIG=$(sed \ -e "s|\${ADO_PROXY_IP}|${ADO_PROXY_IP:-}|g" \ /tmp/awf-tools/staging/mcpg-config.json) +: "${MCPG_REQUIRED_ENV_NAMES:=}" +# Required internal bindings are produced by earlier authenticated setup tasks. +# Refuse to launch MCPG with empty identity metadata, then replace only exact +# JSON string placeholders so values remain correctly escaped. +# shellcheck disable=SC2086 +for MCPG_ENV_NAME in $MCPG_REQUIRED_ENV_NAMES; do + MCPG_ENV_VALUE="${!MCPG_ENV_NAME:-}" + # shellcheck disable=SC2016 # '$(' is a literal unresolved ADO macro prefix. + if [ -z "$MCPG_ENV_VALUE" ] || [[ "$MCPG_ENV_VALUE" == '$('* ]]; then + echo "##vso[task.complete result=Failed]required MCPG environment variable '$MCPG_ENV_NAME' is empty" + exit 1 + fi +done +if [ -n "$MCPG_REQUIRED_ENV_NAMES" ]; then + MCPG_CONFIG=$(printf '%s' "$MCPG_CONFIG" | python3 -c ' +import json +import os +import sys + +replacements = { + "$" + "{" + name + "}": os.environ[name] + for name in os.environ["MCPG_REQUIRED_ENV_NAMES"].split() +} + +def replace(value): + if isinstance(value, str): + return replacements.get(value, value) + if isinstance(value, list): + return [replace(item) for item in value] + if isinstance(value, dict): + return {key: replace(item) for key, item in value.items()} + return value + +json.dump(replace(json.load(sys.stdin)), sys.stdout, separators=(",", ":")) +') +fi + # A client redirected at an empty address would resolve the real # Azure DevOps instead of the policy engine, quietly restoring the # direct path this design removes. Fail loudly rather than start. @@ -4306,6 +4375,15 @@ fn start_mcpg_step( .with_env( "MCPG_ENV_NAMES", EnvValue::literal(mcpg_launch_env.names().collect::>().join(" ")), + ) + .with_env( + "MCPG_REQUIRED_ENV_NAMES", + EnvValue::literal( + mcpg_launch_env + .required_names() + .collect::>() + .join(" "), + ), ); for (name, value) in mcpg_launch_env.iter() { step = step.with_env(name, value.clone()); @@ -4326,11 +4404,13 @@ fn awf_image_flags(supply_chain: Option<&SupplyChainConfig>) -> String { block } -/// Build AWF environment-exclusion flag lines for a Copilot BYOM/BYOK run. +/// Build AWF environment-exclusion flag lines for provider credentials and +/// compiler-owned MCP identity variables. /// -/// `exclude_keys` are the provider credential env keys present in `engine.env` -/// (canonical uppercase `COPILOT_PROVIDER_*` names). AWF 0.27.32+ always enables -/// its API proxy, so only one `--exclude-env ` line is needed per key. +/// `exclude_keys` includes provider credential env keys present in `engine.env` +/// and the generated client/tenant ID keys for Azure-authenticated MCPs. +/// AWF 0.27.32+ always enables its API proxy, so only one `--exclude-env ` +/// line is needed per key. /// /// How the credential reaches the provider without reaching the agent: AWF's /// api-proxy sidecar reads the *real* @@ -4366,8 +4446,7 @@ shell_script! { /// - `topology_attach` — one `--topology-attach` line per trusted peer /// (MCPG always, ado-proxy when the policy engine is enabled) /// - `image_flags` — `--image-tag` plus optional `--image-registry` - /// - `exclude_env` — one `--exclude-env ` line per BYOM/BYOK secret - /// AWF's api-proxy sidecar strips out of the agent env + /// - `exclude_env` — provider credentials and internal MCP identity keys /// - `awf_mounts` — the compiler-supplied chain of `--mount "…"` args /// - `routed_engine_run` — the single-quoted `NO_PROXY` prefix + engine /// command that AWF invokes inside the sandbox @@ -4922,6 +5001,254 @@ fn stop_mcpg_step() -> BashStep { .with_condition(Condition::Always) } +shell_script! { + /// Start one trusted Azure workload-identity refresh sidecar. + /// + /// AzureCLI@3 supplies the initial `idToken`, client ID and tenant ID. + /// `System.AccessToken` is explicitly mapped onto the task and reaches the + /// sidecar only through a one-shot FIFO material document. The sidecar + /// retains the request credential in memory and writes only rotating + /// federated assertions to the private Agent.TempDirectory mount. + /// + /// This deliberately remains one authenticated IR task: AzureCLI scopes + /// `idToken` and the service-principal metadata to its script process. + /// Splitting validation, FIFO creation, container startup, material + /// transfer, and readiness checking across pipeline steps would require + /// persisting or exporting those credentials across the task boundary. + START_AZURE_WIF_REFRESH { + interpreter: Bash, + bindings: [ + AGENT_TEMP, RUNTIME_ID, REFRESH_CONTAINER, REFRESH_IMAGE, + REFRESH_BUNDLE, CLIENT_VARIABLE, TENANT_VARIABLE + ], + externals: [ + SYSTEM_ACCESSTOKEN, SYSTEM_OIDCREQUESTURI + ], + fragments: [run_container], + fragment_uses: [ + run_container => [ + REFRESH_CONTAINER, REFRESH_IMAGE, REFRESH_BUNDLE + ], + ], + body: r###" +set -euo pipefail + +AZURE_WIF_ID_TOKEN=$(printenv idToken || true) +AZURE_WIF_CLIENT_ID=$(printenv servicePrincipalId || true) +AZURE_WIF_TENANT_ID=$(printenv tenantId || true) +AZURE_WIF_SERVICE_CONNECTION_ID=$(printenv AZURESUBSCRIPTION_SERVICE_CONNECTION_ID || true) +GUID_RE='^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$' +if [ -z "$AZURE_WIF_ID_TOKEN" ] \ + || ! [[ "$AZURE_WIF_CLIENT_ID" =~ $GUID_RE ]] \ + || ! [[ "$AZURE_WIF_TENANT_ID" =~ $GUID_RE ]] \ + || ! [[ "$AZURE_WIF_SERVICE_CONNECTION_ID" =~ $GUID_RE ]]; then + echo "##vso[task.complete result=Failed]azure-auth requires an ARM workload-identity service connection that exposes idToken, servicePrincipalId and tenantId" + exit 1 +fi +if [ -z "${SYSTEM_ACCESSTOKEN:-}" ]; then + echo "##vso[task.complete result=Failed]System.AccessToken is unavailable for Azure workload-identity refresh" + exit 1 +fi +if [ -z "${SYSTEM_OIDCREQUESTURI:-}" ]; then + echo "##vso[task.complete result=Failed]System.OidcRequestUri is unavailable for Azure workload-identity refresh" + exit 1 +fi + +umask 077 +AUTH_ROOT="$AGENT_TEMP/ado-aw-azure-auth" +AUTH_DIR="$AUTH_ROOT/$RUNTIME_ID" +docker rm -f "$REFRESH_CONTAINER" >/dev/null 2>&1 || true +rm -rf "$AUTH_DIR" +mkdir -p "$AUTH_DIR/token.d" +chmod 700 "$AUTH_ROOT" "$AUTH_DIR" +# Only token.d is mounted into the MCP container, whose UID may differ from +# the runner's. Private parents protect the host path, not the mounted view. +chmod 755 "$AUTH_DIR/token.d" +MATERIAL_FIFO="$AUTH_DIR/material" +mkfifo -m 600 "$MATERIAL_FIFO" + +# ado-aw:fragment run_container + +# The short-lived encoder inherits the AzureCLI task environment and writes +# directly to the FIFO. Credentials never become process arguments or files. +MATERIAL_STATUS=0 +MATERIAL_FIFO="$MATERIAL_FIFO" \ +AZURE_WIF_ID_TOKEN="$AZURE_WIF_ID_TOKEN" \ +AZURE_WIF_SERVICE_CONNECTION_ID="$AZURE_WIF_SERVICE_CONNECTION_ID" \ +SYSTEM_ACCESSTOKEN="$SYSTEM_ACCESSTOKEN" \ +SYSTEM_OIDCREQUESTURI="$SYSTEM_OIDCREQUESTURI" \ +timeout 60s node -e ' +const fs = require("node:fs"); +const env = process.env; +const required = [ + "AZURE_WIF_ID_TOKEN", "AZURE_WIF_SERVICE_CONNECTION_ID", + "SYSTEM_ACCESSTOKEN", "SYSTEM_OIDCREQUESTURI", "MATERIAL_FIFO" +]; +for (const name of required) { + if (!env[name]) throw new Error(`missing ${name}`); +} +const material = { + initialIdToken: env.AZURE_WIF_ID_TOKEN, + systemAccessToken: env.SYSTEM_ACCESSTOKEN, + oidcRequestUri: env.SYSTEM_OIDCREQUESTURI, + serviceConnectionId: env.AZURE_WIF_SERVICE_CONNECTION_ID, + tokenPath: "/var/lib/ado-aw-azure-auth/token.d/token", + readyPath: "/var/lib/ado-aw-azure-auth/ready.json", + statusPath: "/var/lib/ado-aw-azure-auth/status.json" +}; +fs.writeFileSync(env.MATERIAL_FIFO, JSON.stringify(material)); +' || MATERIAL_STATUS=$? +rm -f "$MATERIAL_FIFO" +if [ "$MATERIAL_STATUS" -ne 0 ]; then + echo "##vso[task.logissue type=error]Failed to hand Azure workload-identity material to refresher" + docker logs "$REFRESH_CONTAINER" 2>&1 || true + exit 1 +fi + +printf '##vso[task.setvariable variable=%s]%s\n' "$CLIENT_VARIABLE" "$AZURE_WIF_CLIENT_ID" +printf '##vso[task.setvariable variable=%s]%s\n' "$TENANT_VARIABLE" "$AZURE_WIF_TENANT_ID" + +READY=false +for _i in $(seq 1 30); do + if [ -s "$AUTH_DIR/token.d/token" ] \ + && [ -s "$AUTH_DIR/ready.json" ] \ + && jq -e '.state == "ready"' "$AUTH_DIR/ready.json" >/dev/null 2>&1; then + READY=true + break + fi + if [ "$(docker inspect -f '{{.State.Running}}' "$REFRESH_CONTAINER" 2>/dev/null || true)" != "true" ]; then + break + fi + sleep 1 +done +if [ "$READY" != "true" ]; then + echo "##vso[task.logissue type=error]Azure workload-identity refresher failed to become ready" + docker logs "$REFRESH_CONTAINER" 2>&1 || true + exit 1 +fi +"###, + } +} + +fn azure_wif_refresh_container_invocation() -> Result { + DockerRun::new(ShellWord::variable("REFRESH_IMAGE")?) + .detached() + .name(ShellWord::variable("REFRESH_CONTAINER")?) + .network(ShellWord::literal("bridge")?) + .user(ShellWord::current_user()) + .cap_drop_all() + .no_new_privileges() + .read_only() + .tmpfs(DockerTmpfs::new("/tmp", "rw,nosuid,nodev,noexec")?) + .pids_limit(64) + .entrypoint(ShellWord::literal("sh")?) + .mount(DockerMount::read_only( + ShellWord::variable("REFRESH_BUNDLE")?, + "/app/azure-wif-refresh.js", + )?) + .mount(DockerMount::read_write( + ShellWord::variable("AUTH_DIR")?, + "/var/lib/ado-aw-azure-auth", + )?) + .command_arg(ShellWord::literal("-c")?) + .command_arg(ShellWord::literal( + "exec node /app/azure-wif-refresh.js < /var/lib/ado-aw-azure-auth/material", + )?) + .discard_stdout() + .render_bash() +} + +fn start_azure_wif_refresh_steps(front_matter: &FrontMatter) -> Result> { + let mut steps = Vec::new(); + for (server_name, _, auth) in front_matter.azure_authenticated_mcp_servers() { + let runtime_id = super::mcpg::azure_auth_runtime_id(server_name).to_ascii_lowercase(); + let client_variable = super::mcpg::azure_auth_client_variable(server_name)?; + let tenant_variable = super::mcpg::azure_auth_tenant_variable(server_name)?; + let script = ShellScript::new(&START_AZURE_WIF_REFRESH) + .bind("AGENT_TEMP", Binding::ado_macro("Agent.TempDirectory")) + .bind_text("RUNTIME_ID", &runtime_id) + .bind_text( + "REFRESH_CONTAINER", + super::mcpg::azure_auth_container_name(server_name), + ) + .bind_text("REFRESH_IMAGE", ADO_PROXY_IMAGE) + .bind_text("REFRESH_BUNDLE", paths::AZURE_WIF_REFRESH_PATH) + .bind_text("CLIENT_VARIABLE", client_variable.as_str()) + .bind_text("TENANT_VARIABLE", tenant_variable.as_str()) + .fragment( + "run_container", + azure_wif_refresh_container_invocation()?, + ) + .render(); + let task = AzureCliV3::new( + AzureCliV3Connection::AzureRm(auth.service_connection.as_str().to_string()), + ScriptType::Bash, + ScriptLocation::Inline(script), + ) + .add_spn_to_environment(true) + .visible_az_login(false) + .with_display_name(format!("Start Azure auth refresher ({server_name})")) + .into_step() + .with_env("SYSTEM_ACCESSTOKEN", EnvValue::secret("System.AccessToken")); + steps.push(Step::Task(task)); + } + Ok(steps) +} + +shell_script! { + /// Stop one Azure workload-identity refresh sidecar and delete its private + /// assertion directory. The step is idempotent for partial startup paths. + STOP_AZURE_WIF_REFRESH { + interpreter: Bash, + bindings: [AGENT_TEMP, RUNTIME_ID, REFRESH_CONTAINER], + externals: [], + fragments: [], + body: r###" +REFRESH_FAILED=false +STATUS_PATH="$AGENT_TEMP/ado-aw-azure-auth/$RUNTIME_ID/status.json" +if [ -s "$STATUS_PATH" ] && jq -e '.state == "unhealthy"' "$STATUS_PATH" >/dev/null 2>&1; then + echo "##vso[task.logissue type=error]Azure workload-identity refresher reported an unhealthy state" + REFRESH_FAILED=true +fi +if docker inspect "$REFRESH_CONTAINER" >/dev/null 2>&1 \ + && [ "$(docker inspect -f '{{.State.Running}}' "$REFRESH_CONTAINER")" != "true" ]; then + echo "##vso[task.logissue type=error]Azure workload-identity refresher exited before cleanup" + REFRESH_FAILED=true +fi +if [ "$REFRESH_FAILED" = "true" ]; then + docker logs "$REFRESH_CONTAINER" 2>&1 || true +fi +docker stop --time 10 "$REFRESH_CONTAINER" >/dev/null 2>&1 || true +docker rm -f "$REFRESH_CONTAINER" >/dev/null 2>&1 || true +rm -rf "$AGENT_TEMP/ado-aw-azure-auth/$RUNTIME_ID" +if [ "$REFRESH_FAILED" = "true" ]; then + exit 1 +fi +"###, + } +} + +fn stop_azure_wif_refresh_steps(front_matter: &FrontMatter) -> Vec { + front_matter + .azure_authenticated_mcp_servers() + .into_iter() + .map(|(server_name, _, _)| { + let runtime_id = super::mcpg::azure_auth_runtime_id(server_name).to_ascii_lowercase(); + Step::Bash( + ShellScript::new(&STOP_AZURE_WIF_REFRESH) + .bind("AGENT_TEMP", Binding::ado_macro("Agent.TempDirectory")) + .bind_text("RUNTIME_ID", &runtime_id) + .bind_text( + "REFRESH_CONTAINER", + super::mcpg::azure_auth_container_name(server_name), + ) + .into_step(format!("Stop Azure auth refresher ({server_name})")) + .with_condition(Condition::Always), + ) + }) + .collect() +} + /// Start the `ado-proxy` policy engine as a host container. /// /// Mirrors [`start_mcpg_step`]: an ordinary bridge-networked container started @@ -5001,7 +5328,7 @@ fn start_ado_proxy_step(front_matter: &FrontMatter) -> BashStep { ) .fragment( "run_container", - phase_body(&START_ADO_PROXY_RUN_CONTAINER), + ado_proxy_run_container_phase(), ) .fragment( "handover_material", @@ -5029,6 +5356,76 @@ fn phase_body(def: &crate::compile::shell::ShellScriptDef) -> String { crate::compile::shell::dedent(body).trim().to_string() } +fn ado_proxy_run_container_phase() -> String { + ShellScript::new(&START_ADO_PROXY_RUN_CONTAINER) + .fragment( + "container_invocation", + ado_proxy_container_invocation() + .render_bash() + .expect("compiler-owned ado-proxy container invocation must be valid"), + ) + .render() + .trim() + .to_string() +} + +fn ado_proxy_container_invocation() -> DockerRun { + DockerRun::new( + ShellWord::variable("PROXY_IMAGE") + .expect("compiler-owned shell variable must be valid"), + ) + .detached() + .name( + ShellWord::variable("PROXY_CONTAINER") + .expect("compiler-owned shell variable must be valid"), + ) + .network( + ShellWord::variable("PROXY_NETWORK") + .expect("compiler-owned shell variable must be valid"), + ) + .entrypoint(ShellWord::literal("sh").expect("static entrypoint must be valid")) + .mount( + DockerMount::read_only( + ShellWord::variable("PROXY_SCRIPT_PATH") + .expect("compiler-owned shell variable must be valid"), + "/app/ado-proxy.js", + ) + .expect("static ado-proxy bundle mount must be valid"), + ) + .mount( + DockerMount::read_only( + ShellWord::variable("PROXY_DIR") + .expect("compiler-owned shell variable must be valid") + .with_literal("/policy") + .expect("static policy suffix must be valid"), + "/etc/ado-proxy", + ) + .expect("static ado-proxy policy mount must be valid"), + ) + .mount( + DockerMount::read_write( + ShellWord::variable("AZ_WRAPPER_DIR") + .expect("compiler-owned shell variable must be valid"), + "/var/lib/ado-proxy", + ) + .expect("static ado-proxy CA mount must be valid"), + ) + .mount( + DockerMount::read_write( + ShellWord::literal("/tmp/gh-aw/ado-proxy-logs") + .expect("static log path must be valid"), + "/var/log/ado-proxy", + ) + .expect("static ado-proxy log mount must be valid"), + ) + .command_arg(ShellWord::literal("-c").expect("static command flag must be valid")) + .command_arg( + ShellWord::variable("CONTAINER_ENTRYPOINT") + .expect("compiler-owned shell variable must be valid"), + ) + .discard_stdout() +} + /// The one-liner passed to the container's `sh -c`. Kept in sync with the /// registered [`START_ADO_PROXY_CONTAINER_ENTRYPOINT_SH`] script via /// `container_entrypoint_matches_registered_body`. @@ -5220,7 +5617,13 @@ shell_script! { PROXY_CONTAINER, PROXY_NETWORK, PROXY_SCRIPT_PATH, PROXY_DIR, PROXY_IMAGE, CONTAINER_ENTRYPOINT, AZ_WRAPPER_DIR ], - fragments: [], + fragments: [container_invocation], + fragment_uses: [ + container_invocation => [ + PROXY_CONTAINER, PROXY_NETWORK, PROXY_SCRIPT_PATH, + PROXY_DIR, PROXY_IMAGE, CONTAINER_ENTRYPOINT, AZ_WRAPPER_DIR + ], + ], body: r###" # Remove any container left behind by an interrupted run. docker rm -f "$PROXY_CONTAINER" 2>/dev/null || true @@ -5234,17 +5637,7 @@ mkdir -p /tmp/gh-aw/ado-proxy-logs # A container-local FIFO preserves the stdin-only custody contract: # material is streamed through `docker exec -i`, never written to a # runner path, container layer, argv, or environment. -docker run -d \ - --name "$PROXY_CONTAINER" \ - --network "$PROXY_NETWORK" \ - --entrypoint sh \ - -v "$PROXY_SCRIPT_PATH:/app/ado-proxy.js:ro" \ - -v "$PROXY_DIR/policy:/etc/ado-proxy:ro" \ - -v "$AZ_WRAPPER_DIR:/var/lib/ado-proxy" \ - -v /tmp/gh-aw/ado-proxy-logs:/var/log/ado-proxy \ - "$PROXY_IMAGE" \ - -c "$CONTAINER_ENTRYPOINT" \ - >/dev/null +# ado-aw:fragment container_invocation "###, } } @@ -7336,6 +7729,120 @@ safe-outputs: .0 } + fn azure_auth_fm() -> FrontMatter { + crate::compile::parse_markdown( + "---\nname: t\ndescription: x\nmcp-servers:\n kusto:\n container: node:22-slim\n azure-auth:\n service-connection: my-arm-sc\n---\n", + ) + .unwrap() + .0 + } + + #[test] + fn azure_auth_identity_exclusions_are_independent_of_the_engine() { + let fm = azure_auth_fm(); + let client = super::super::mcpg::azure_auth_client_variable("kusto").unwrap(); + let tenant = super::super::mcpg::azure_auth_tenant_variable("kusto").unwrap(); + for is_copilot in [true, false] { + let keys = awf_exclude_keys(&fm, is_copilot, &fm.engine).unwrap(); + assert_eq!(keys, vec![client.as_str(), tenant.as_str()]); + } + let plain = test_front_matter("name: t\ndescription: d\n"); + assert!(awf_exclude_keys(&plain, true, &plain.engine).unwrap().is_empty()); + } + + #[test] + fn azure_auth_identity_exclusions_preserve_provider_credentials() { + let fm = azure_auth_fm(); + let provider = test_front_matter( + "name: t\ndescription: d\nengine:\n id: copilot\n env:\n COPILOT_PROVIDER_API_KEY: fake-key\n", + ); + let keys = awf_exclude_keys(&fm, true, &provider.engine).unwrap(); + assert_eq!(keys.len(), 3); + assert!(keys.contains(&"COPILOT_PROVIDER_API_KEY".to_string())); + assert!(keys.contains( + &super::super::mcpg::azure_auth_client_variable("kusto").unwrap().into_inner() + )); + assert!(keys.contains( + &super::super::mcpg::azure_auth_tenant_variable("kusto").unwrap().into_inner() + )); + } + + #[test] + fn azure_auth_refresher_uses_typed_azure_cli_v3_and_stdin_custody() { + let steps = start_azure_wif_refresh_steps(&azure_auth_fm()).unwrap(); + let [Step::Task(task)] = steps.as_slice() else { + panic!("expected one AzureCLI@3 task"); + }; + assert_eq!(task.task, "AzureCLI@3"); + assert_eq!( + task.inputs.get("connectionType").map(String::as_str), + Some("azureRM") + ); + assert_eq!( + task.inputs.get("azureSubscription").map(String::as_str), + Some("my-arm-sc") + ); + assert_eq!( + task.inputs.get("addSpnToEnvironment").map(String::as_str), + Some("true") + ); + assert!(matches!( + task.env.get("SYSTEM_ACCESSTOKEN"), + Some(EnvValue::Secret(name)) if name == "System.AccessToken" + )); + let script = task.inputs.get("inlineScript").unwrap(); + assert!(script.contains("mkfifo -m 600")); + assert!(script.contains("docker run \\\n -d \\")); + assert!(!script.contains(" --rm \\")); + assert!(script.contains("azure-wif-refresh.js")); + assert!(script.contains("fs.writeFileSync(env.MATERIAL_FIFO")); + assert!(script.contains("SYSTEM_OIDCREQUESTURI")); + assert!(script.contains("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID")); + assert!(script.contains("$AUTH_DIR/token.d/token")); + assert!(!script.contains("-e SYSTEM_ACCESSTOKEN")); + assert!(!script.contains("--token")); + assert!(script.contains("AGENT_TEMP='$(Agent.TempDirectory)'")); + assert!(script.contains("chmod 700 \"$AUTH_ROOT\" \"$AUTH_DIR\"")); + assert!(script.contains("chmod 755 \"$AUTH_DIR/token.d\"")); + } + + #[test] + fn credential_container_launches_lower_from_typed_invocations() { + let has_raw_docker_run = |body: &str| { + body.lines() + .map(str::trim_start) + .any(|line| line.starts_with("docker run")) + }; + assert!(!has_raw_docker_run(START_AZURE_WIF_REFRESH.body)); + assert!( + START_AZURE_WIF_REFRESH + .fragment_uses + .iter() + .any(|(name, _)| *name == "run_container") + ); + assert!(!has_raw_docker_run(START_ADO_PROXY_RUN_CONTAINER.body)); + assert!( + START_ADO_PROXY_RUN_CONTAINER + .fragment_uses + .iter() + .any(|(name, _)| *name == "container_invocation") + ); + } + + #[test] + fn azure_auth_refresher_cleanup_is_always_and_scoped() { + let steps = stop_azure_wif_refresh_steps(&azure_auth_fm()); + let [Step::Bash(step)] = steps.as_slice() else { + panic!("expected one cleanup bash step"); + }; + assert_eq!(step.condition, Some(Condition::Always)); + assert!(step.script.contains("docker rm -f \"$REFRESH_CONTAINER\"")); + assert!( + step.script + .contains("rm -rf \"$AGENT_TEMP/ado-aw-azure-auth/$RUNTIME_ID\"") + ); + } + // ── start_ado_proxy_step / stop_ado_proxy_step ────────────────────────── #[test] @@ -7388,7 +7895,7 @@ safe-outputs: step.script ); assert!( - step.script.contains("docker run -d") + step.script.contains("docker run \\\n -d \\") && step.script.contains("mkfifo \"$MATERIAL_FIFO\""), "the container must be detached from the Bash task before material handover" ); @@ -7408,9 +7915,9 @@ safe-outputs: #[test] fn ado_proxy_container_lifecycle_is_independent_of_the_start_task() { let script = start_ado_proxy_step(&proxy_fm()).script; - assert!(script.contains("docker run -d")); + assert!(script.contains("docker run \\\n -d \\")); assert!( - !script.contains("docker run -i --rm"), + !script.contains(" --rm \\"), "attached --rm containers disappear when Azure Pipelines cleans up task STDIO" ); assert!(script.contains("docker logs --tail 200")); @@ -7475,7 +7982,7 @@ safe-outputs: assert!(script.contains("--public-ca-file /var/lib/ado-proxy/ado-proxy-ca.pem")); assert!( script.contains(&format!("AZ_WRAPPER_DIR='{AZ_WRAPPER_DIR}'")) - && script.contains("-v \"$AZ_WRAPPER_DIR:/var/lib/ado-proxy\""), + && script.contains("-v \"${AZ_WRAPPER_DIR}:/var/lib/ado-proxy:rw\""), "the wrapper directory must be bound and mounted at /var/lib/ado-proxy: {script}" ); assert!( @@ -7527,18 +8034,18 @@ safe-outputs: let script = start_ado_proxy_step(&proxy_fm()).script; assert_eq!(ADO_PROXY_IMAGE, common::ADO_MCP_IMAGE); // The image and the bundle path both reach the body through bindings, - // so the docker invocation references them as `$PROXY_IMAGE` and - // `$PROXY_SCRIPT_PATH` while the concrete values live in the prelude. + // so the docker invocation references them as `${PROXY_IMAGE}` and + // `${PROXY_SCRIPT_PATH}` while the concrete values live in the prelude. assert!( script.contains(&format!("PROXY_IMAGE='{ADO_PROXY_IMAGE}'")) - && script.contains("\"$PROXY_IMAGE\" \\"), + && script.contains("\"${PROXY_IMAGE}\" \\"), "docker run must reuse the bound $PROXY_IMAGE: {script}" ); assert!( script.contains(&format!( "PROXY_SCRIPT_PATH='{}'", paths::ADO_PROXY_PATH - )) && script.contains("\"$PROXY_SCRIPT_PATH:/app/ado-proxy.js:ro\""), + )) && script.contains("\"${PROXY_SCRIPT_PATH}:/app/ado-proxy.js:ro\""), "docker run must mount the bound ado-proxy bundle: {script}" ); } @@ -7627,6 +8134,10 @@ safe-outputs: step.env.get("MCPG_ENV_NAMES"), Some(EnvValue::Literal(value)) if value == "DEBUG DEST_TOKEN" )); + assert!( + step.script + .contains(r#""$" + "{" + name + "}": os.environ[name]"#) + ); assert!(step.script.contains("MCPG_DOCKER_ENV_ARGS+=(-e")); assert!(step.script.contains("\"${MCPG_DOCKER_ENV_ARGS[@]}\"")); assert!(!step.script.contains("DEST_TOKEN=\"$SOURCE_TOKEN\"")); diff --git a/src/compile/common.rs b/src/compile/common.rs index be9fec4b1..b24239e47 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -3265,6 +3265,54 @@ fn validate_stdio_mcp( ); } } + if let Some(auth) = &opts.azure_auth { + if opts.args.iter().any(|arg| { + matches!( + arg.as_str(), + "-e" + | "--env" + | "--env-file" + | "-v" + | "--volume" + | "--mount" + | "--volumes-from" + ) || arg.starts_with("--env=") + || arg.starts_with("--env-file=") + || arg.starts_with("--volume=") + || arg.starts_with("--mount=") + || (arg.starts_with("-e") && arg.len() > 2) + || (arg.starts_with("-v") && arg.len() > 2) + }) { + anyhow::bail!( + "mcp-servers.{name}.args cannot contain Docker env or mount flags when azure-auth is configured; use the structured env and mounts fields" + ); + } + for reserved in [ + "AZURE_CLIENT_ID", + "AZURE_TENANT_ID", + "AZURE_FEDERATED_TOKEN_FILE", + ] { + if opts.env.contains_key(reserved) { + anyhow::bail!( + "mcp-servers.{name}.env.{reserved} conflicts with compiler-owned azure-auth" + ); + } + } + let auth_destination = auth.mount_path.as_str(); + for mount in &opts.mounts { + let parsed = Mount::try_from(mount.as_str()) + .with_context(|| format!("invalid container mount `{mount}`"))?; + let destination = parsed.destination(); + if destination == auth_destination + || destination.starts_with(&format!("{auth_destination}/")) + || auth_destination.starts_with(&format!("{destination}/")) + { + anyhow::bail!( + "mcp-servers.{name}.mounts destination '{destination}' conflicts with azure-auth.mount-path '{auth_destination}'" + ); + } + } + } let literal_env: HashMap = opts .env .iter() @@ -3282,8 +3330,10 @@ fn validate_stdio_mcp( /// Build a stdio `McpgServerConfig` from a container-based MCP options block. fn build_stdio_mcpg_server( + name: &str, container: &str, opts: &crate::compile::types::McpOptions, + launch_env: &mut super::mcpg::McpgLaunchEnvironment, ) -> Result { let mut runtime = ContainerRuntimeConfig::builder().extra_args(&opts.args); for mount in &opts.mounts { @@ -3292,6 +3342,36 @@ fn build_stdio_mcpg_server( .with_context(|| format!("invalid container mount `{mount}`"))?, ); } + let mut env: std::collections::BTreeMap = opts + .env + .iter() + .map(|(name, value)| (name.clone(), value.mcpg_value())) + .collect(); + if let Some(auth) = &opts.azure_auth { + let client_variable = super::mcpg::azure_auth_client_variable(name)?; + let tenant_variable = super::mcpg::azure_auth_tenant_variable(name)?; + launch_env.bind_internal_pipeline_variable( + client_variable.as_str(), + &client_variable, + format!("mcp-servers.{name}.azure-auth client id"), + )?; + launch_env.bind_internal_pipeline_variable( + tenant_variable.as_str(), + &tenant_variable, + format!("mcp-servers.{name}.azure-auth tenant id"), + )?; + let host_token_dir = format!("{}/token.d", super::mcpg::azure_auth_host_directory(name)); + runtime = runtime.mount(Mount::read_only(host_token_dir, auth.mount_path.as_str())?); + env.insert( + "AZURE_CLIENT_ID".to_string(), + format!("${{{}}}", client_variable.as_str()), + ); + env.insert( + "AZURE_TENANT_ID".to_string(), + format!("${{{}}}", tenant_variable.as_str()), + ); + env.insert("AZURE_FEDERATED_TOKEN_FILE".to_string(), auth.token_path()); + } Ok(McpgServerConfig { server_type: "stdio".to_string(), container: Some(container.to_string()), @@ -3300,15 +3380,10 @@ fn build_stdio_mcpg_server( runtime: runtime.build()?, url: None, headers: None, - env: if opts.env.is_empty() { + env: if env.is_empty() { None } else { - Some( - opts.env - .iter() - .map(|(name, value)| (name.clone(), value.mcpg_value())) - .collect(), - ) + Some(env) }, tools: nonempty_vec(&opts.allowed), }) @@ -3341,6 +3416,14 @@ fn try_add_user_mcp( ) -> Result<()> { // Prevent user-defined MCPs from overwriting the reserved safeoutputs backend if name.eq_ignore_ascii_case("safeoutputs") { + if matches!( + config, + McpConfig::WithOptions(options) if options.azure_auth.is_some() + ) { + anyhow::bail!( + "mcp-servers.{name}.azure-auth cannot target the compiler-owned safeoutputs server" + ); + } log::warn!( "MCP name 'safeoutputs' is reserved for the compiler-owned safe outputs backend — skipping" ); @@ -3364,6 +3447,14 @@ fn try_add_user_mcp( // Skip if already auto-configured by an extension (e.g., tools.azure-devops) if servers.contains_key(name) { + if matches!( + config, + McpConfig::WithOptions(options) if options.azure_auth.is_some() + ) { + anyhow::bail!( + "mcp-servers.{name}.azure-auth cannot target a server owned by a compiler extension" + ); + } return Ok(()); } @@ -3391,7 +3482,7 @@ fn try_add_user_mcp( if let Some(container) = &opts.container { validate_stdio_mcp(name, container, opts)?; - let server = build_stdio_mcpg_server(container, opts) + let server = build_stdio_mcpg_server(name, container, opts, launch_env) .with_context(|| format!("invalid runtime configuration for MCP `{name}`"))?; for (destination, value) in &opts.env { if let Some(source) = value.pipeline_variable() { @@ -3404,6 +3495,11 @@ fn try_add_user_mcp( } servers.insert(name.to_string(), server); } else if let Some(url) = &opts.url { + if opts.azure_auth.is_some() { + anyhow::bail!( + "mcp-servers.{name}.azure-auth is only supported for containerized stdio MCP servers" + ); + } // HTTP-based MCP (remote server) for w in validate::validate_mcp_url(url, name) { eprintln!("{}", w); @@ -3429,6 +3525,11 @@ fn try_add_user_mcp( } servers.insert(name.to_string(), build_http_mcpg_server(url, opts)); } else { + if opts.azure_auth.is_some() { + anyhow::bail!( + "mcp-servers.{name}.azure-auth requires a containerized stdio MCP server" + ); + } log::warn!("MCP '{}' has no container or url — skipping", name); } @@ -7852,6 +7953,109 @@ safe-outputs: assert_eq!(env["STATIC"], "value"); } + #[test] + fn test_compile_mcpg_injects_renewable_azure_auth() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nmcp-servers:\n kusto:\n container: node:22-slim\n azure-auth:\n service-connection: my-arm-sc\n---\n", + ) + .unwrap(); + let compilation = compile_mcpg(&fm, &collect_exts_and_decls(&fm).1, false).unwrap(); + let server = &compilation.config.mcp_servers["kusto"]; + let auth = fm.azure_authenticated_mcp_servers()[0].2; + let env = server.env.as_ref().unwrap(); + let client = super::super::mcpg::azure_auth_client_variable("kusto").unwrap(); + let tenant = super::super::mcpg::azure_auth_tenant_variable("kusto").unwrap(); + assert_eq!(env["AZURE_CLIENT_ID"], format!("${{{}}}", client.as_str())); + assert_eq!(env["AZURE_TENANT_ID"], format!("${{{}}}", tenant.as_str())); + assert_eq!(env["AZURE_FEDERATED_TOKEN_FILE"], auth.token_path()); + assert!(matches!( + compilation.launch_env.get(client.as_str()), + Some(crate::compile::ir::env::EnvValue::PipelineVar(source)) + if source == client.as_str() + )); + assert!( + compilation + .launch_env + .required_names() + .any(|name| name == client.as_str()) + ); + let mounts = server.runtime.mounts(); + assert!(mounts.iter().any(|mount| { + mount.source() + == format!( + "{}/token.d", + super::super::mcpg::azure_auth_host_directory("kusto") + ) + && mount.destination() == auth.mount_path.as_str() + && mount.is_read_only() + })); + } + + #[test] + fn test_compile_mcpg_rejects_azure_auth_on_http_server() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nmcp-servers:\n remote:\n url: https://mcp.example.com\n azure-auth:\n service-connection: my-arm-sc\n---\n", + ) + .unwrap(); + let error = compile_mcpg(&fm, &collect_exts_and_decls(&fm).1, false) + .unwrap_err() + .to_string(); + assert!(error.contains("only supported for containerized stdio")); + } + + #[test] + fn test_compile_mcpg_rejects_azure_auth_env_override() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nmcp-servers:\n tool:\n container: img:latest\n azure-auth:\n service-connection: my-arm-sc\n env:\n AZURE_CLIENT_ID: override\n---\n", + ) + .unwrap(); + let error = compile_mcpg(&fm, &collect_exts_and_decls(&fm).1, false) + .unwrap_err() + .to_string(); + assert!(error.contains("AZURE_CLIENT_ID")); + assert!(error.contains("compiler-owned azure-auth")); + } + + #[test] + fn test_compile_mcpg_rejects_azure_auth_mount_collision() { + let (fm, _) = parse_markdown( + "---\nname: test\ndescription: test\nmcp-servers:\n tool:\n container: img:latest\n azure-auth:\n service-connection: my-arm-sc\n mounts:\n - /host:/var/run/ado-aw:ro\n---\n", + ) + .unwrap(); + let error = compile_mcpg(&fm, &collect_exts_and_decls(&fm).1, false) + .unwrap_err() + .to_string(); + assert!(error.contains("conflicts with azure-auth.mount-path")); + } + + #[test] + fn test_compile_mcpg_rejects_azure_auth_runtime_env_or_mount_flags() { + for args in [ + "[-e, AZURE_CLIENT_ID=override]", + "[--env-file, /tmp/override.env]", + "[--env-file=/tmp/override.env]", + "[-v, /host:/var/run/ado-aw/azure]", + "[--mount=type=bind,source=/host,target=/var/run/ado-aw/azure]", + ] { + let source = format!( + "---\nname: test\ndescription: test\nmcp-servers:\n tool:\n container: img:latest\n azure-auth:\n service-connection: my-arm-sc\n args: {args}\n---\n" + ); + let (fm, _) = parse_markdown(&source).unwrap(); + let error = compile_mcpg(&fm, &collect_exts_and_decls(&fm).1, false) + .unwrap_err() + .to_string(); + assert!(error.contains("Docker env or mount flags"), "{error}"); + } + } + + #[test] + fn test_azure_auth_rejects_unsafe_container_mount_path() { + let result = parse_markdown( + "---\nname: test\ndescription: test\nmcp-servers:\n tool:\n container: img:latest\n azure-auth:\n service-connection: my-arm-sc\n mount-path: /var/run/../secret\n---\n", + ); + assert!(result.is_err()); + } + #[test] fn test_compile_mcpg_rejects_invalid_destination_env_name() { let (fm, _) = parse_markdown( diff --git a/src/compile/container_invocation.rs b/src/compile/container_invocation.rs new file mode 100644 index 000000000..57635dbc2 --- /dev/null +++ b/src/compile/container_invocation.rs @@ -0,0 +1,531 @@ +//! Typed compiler-owned container invocations. +//! +//! `Docker@2` models Azure Pipelines build/push/login tasks; it cannot start a +//! long-lived runtime container. This module models `docker run` independently +//! of the pipeline transport, then lowers it to a shell fragment at the final +//! boundary. Compiler-owned credential containers deliberately get no raw +//! argument escape hatch. + +use std::collections::BTreeMap; + +use anyhow::{Result, bail}; + +use crate::secure::ContainerAbsolutePath; + +use super::shell::bindings::{contains_secret_name, is_shell_var_name, single_quote}; + +#[derive(Debug, Clone, PartialEq, Eq)] +enum ShellPart { + Literal(String), + Variable(String), + CurrentUid, + CurrentGid, +} + +/// One shell argument assembled from literals and validated variable +/// references. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ShellWord { + parts: Vec, +} + +impl ShellWord { + pub fn literal(value: impl Into) -> Result { + let value = value.into(); + validate_literal(&value)?; + Ok(Self { + parts: vec![ShellPart::Literal(value)], + }) + } + + pub fn variable(name: impl Into) -> Result { + let name = name.into(); + if !is_shell_var_name(&name) { + bail!( + "container invocation variable '{name}' is invalid; expected SCREAMING_SNAKE_CASE" + ); + } + if contains_secret_name(&name) { + bail!("credential '{name}' must not be passed through a container command argument"); + } + Ok(Self { + parts: vec![ShellPart::Variable(name)], + }) + } + + pub fn current_user() -> Self { + Self { + parts: vec![ + ShellPart::CurrentUid, + ShellPart::Literal(":".to_string()), + ShellPart::CurrentGid, + ], + } + } + + pub fn with_literal(mut self, value: impl Into) -> Result { + let value = value.into(); + validate_literal(&value)?; + self.parts.push(ShellPart::Literal(value)); + Ok(self) + } + + fn is_empty(&self) -> bool { + self.parts + .iter() + .all(|part| matches!(part, ShellPart::Literal(value) if value.is_empty())) + } + + fn render(&self) -> String { + if self + .parts + .iter() + .all(|part| matches!(part, ShellPart::Literal(_))) + { + let literal = self + .parts + .iter() + .filter_map(|part| match part { + ShellPart::Literal(value) => Some(value.as_str()), + _ => None, + }) + .collect::(); + return single_quote(&literal); + } + + let mut rendered = String::from("\""); + for part in &self.parts { + match part { + ShellPart::Literal(value) => { + for character in value.chars() { + if matches!(character, '\\' | '"' | '$' | '`') { + rendered.push('\\'); + } + rendered.push(character); + } + } + ShellPart::Variable(name) => { + rendered.push_str("${"); + rendered.push_str(name); + rendered.push('}'); + } + ShellPart::CurrentUid => rendered.push_str("$(id -u)"), + ShellPart::CurrentGid => rendered.push_str("$(id -g)"), + } + } + rendered.push('"'); + rendered + } +} + +fn validate_literal(value: &str) -> Result<()> { + if value.contains(['\0', '\n', '\r']) { + bail!("container invocation arguments must be single-line and contain no NUL bytes"); + } + Ok(()) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DockerMountMode { + ReadOnly, + ReadWrite, +} + +impl DockerMountMode { + fn as_str(self) -> &'static str { + match self { + Self::ReadOnly => "ro", + Self::ReadWrite => "rw", + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DockerMount { + source: ShellWord, + destination: ContainerAbsolutePath, + mode: DockerMountMode, +} + +impl DockerMount { + pub fn read_only(source: ShellWord, destination: impl Into) -> Result { + Self::new(source, destination, DockerMountMode::ReadOnly) + } + + pub fn read_write(source: ShellWord, destination: impl Into) -> Result { + Self::new(source, destination, DockerMountMode::ReadWrite) + } + + fn new( + source: ShellWord, + destination: impl Into, + mode: DockerMountMode, + ) -> Result { + let destination = ContainerAbsolutePath::parse(destination)?; + if source.is_empty() { + bail!("container mounts require a non-empty source"); + } + Ok(Self { + source, + destination, + mode, + }) + } + + fn render(&self) -> Result { + self.source + .clone() + .with_literal(format!(":{}:{}", self.destination, self.mode.as_str())) + .map(|word| word.render()) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DockerTmpfs { + destination: ContainerAbsolutePath, + options: String, +} + +impl DockerTmpfs { + pub fn new(destination: impl Into, options: impl Into) -> Result { + let destination = ContainerAbsolutePath::parse(destination)?; + let options = options.into(); + if options.is_empty() || options.contains(':') { + bail!("container tmpfs options must be non-empty and must not contain `:`"); + } + validate_literal(&options)?; + Ok(Self { + destination, + options, + }) + } + + fn render(&self) -> String { + ShellWord { + parts: vec![ShellPart::Literal(format!( + "{}:{}", + self.destination, self.options + ))], + } + .render() + } +} + +/// A validated `docker run` invocation for a compiler-owned container. +#[derive(Debug, Clone)] +pub struct DockerRun { + image: ShellWord, + names: Vec, + networks: Vec, + users: Vec, + detached: bool, + remove_on_exit: bool, + cap_drop_all: bool, + no_new_privileges: bool, + read_only: bool, + tmpfs: Vec, + pids_limits: Vec, + entrypoints: Vec, + mounts: Vec, + command: Vec, + discard_stdout: bool, +} + +impl DockerRun { + pub fn new(image: ShellWord) -> Self { + Self { + image, + names: Vec::new(), + networks: Vec::new(), + users: Vec::new(), + detached: false, + remove_on_exit: false, + cap_drop_all: false, + no_new_privileges: false, + read_only: false, + tmpfs: Vec::new(), + pids_limits: Vec::new(), + entrypoints: Vec::new(), + mounts: Vec::new(), + command: Vec::new(), + discard_stdout: false, + } + } + + pub fn name(mut self, value: ShellWord) -> Self { + self.names.push(value); + self + } + + pub fn network(mut self, value: ShellWord) -> Self { + self.networks.push(value); + self + } + + pub fn user(mut self, value: ShellWord) -> Self { + self.users.push(value); + self + } + + pub fn detached(mut self) -> Self { + self.detached = true; + self + } + + #[allow(dead_code)] + pub fn remove_on_exit(mut self) -> Self { + self.remove_on_exit = true; + self + } + + pub fn cap_drop_all(mut self) -> Self { + self.cap_drop_all = true; + self + } + + pub fn no_new_privileges(mut self) -> Self { + self.no_new_privileges = true; + self + } + + pub fn read_only(mut self) -> Self { + self.read_only = true; + self + } + + pub fn tmpfs(mut self, value: DockerTmpfs) -> Self { + self.tmpfs.push(value); + self + } + + pub fn pids_limit(mut self, value: u32) -> Self { + self.pids_limits.push(value); + self + } + + pub fn entrypoint(mut self, value: ShellWord) -> Self { + self.entrypoints.push(value); + self + } + + pub fn mount(mut self, value: DockerMount) -> Self { + self.mounts.push(value); + self + } + + pub fn command_arg(mut self, value: ShellWord) -> Self { + self.command.push(value); + self + } + + pub fn discard_stdout(mut self) -> Self { + self.discard_stdout = true; + self + } + + pub fn render_bash(&self) -> Result { + if self.image.is_empty() { + bail!("container image must not be empty"); + } + validate_singleton(&self.names, "name")?; + validate_singleton(&self.networks, "network")?; + validate_singleton(&self.users, "user")?; + validate_singleton(&self.pids_limits, "PID limit")?; + validate_singleton(&self.entrypoints, "entrypoint")?; + if self.pids_limits.first() == Some(&0) { + bail!("container PID limit must be greater than zero"); + } + validate_mount_destinations(&self.mounts)?; + + let mut segments = vec!["docker run".to_string()]; + if self.detached { + segments.push("-d".to_string()); + } + if self.remove_on_exit { + segments.push("--rm".to_string()); + } + if let Some(name) = self.names.first() { + segments.push(format!("--name {}", name.render())); + } + if let Some(network) = self.networks.first() { + segments.push(format!("--network {}", network.render())); + } + if let Some(user) = self.users.first() { + segments.push(format!("--user {}", user.render())); + } + if self.cap_drop_all { + segments.push("--cap-drop ALL".to_string()); + } + if self.no_new_privileges { + segments.push("--security-opt no-new-privileges".to_string()); + } + if self.read_only { + segments.push("--read-only".to_string()); + } + for tmpfs in &self.tmpfs { + segments.push(format!("--tmpfs {}", tmpfs.render())); + } + if let Some(limit) = self.pids_limits.first() { + segments.push(format!("--pids-limit {limit}")); + } + if let Some(entrypoint) = self.entrypoints.first() { + segments.push(format!("--entrypoint {}", entrypoint.render())); + } + for mount in &self.mounts { + segments.push(format!("-v {}", mount.render()?)); + } + segments.push(self.image.render()); + segments.extend(self.command.iter().map(ShellWord::render)); + + let mut rendered = String::new(); + for (index, segment) in segments.iter().enumerate() { + if index == 0 { + rendered.push_str(segment); + } else { + rendered.push_str(" "); + rendered.push_str(segment); + } + if index + 1 != segments.len() || self.discard_stdout { + rendered.push_str(" \\\n"); + } else { + rendered.push('\n'); + } + } + if self.discard_stdout { + rendered.push_str(" >/dev/null\n"); + } + Ok(rendered) + } +} + +fn validate_singleton(values: &[T], setting: &str) -> Result<()> { + if values.len() > 1 { + bail!("container invocation must not configure {setting} more than once"); + } + Ok(()) +} + +fn validate_mount_destinations(mounts: &[DockerMount]) -> Result<()> { + let mut destinations = BTreeMap::new(); + for mount in mounts { + if destinations + .insert(mount.destination.as_str(), mount.source.render()) + .is_some() + { + bail!( + "container mount destination '{}' is configured more than once", + mount.destination + ); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn renders_hardened_detached_container_without_raw_arguments() { + let invocation = DockerRun::new(ShellWord::variable("IMAGE").unwrap()) + .detached() + .name(ShellWord::variable("CONTAINER").unwrap()) + .network(ShellWord::literal("bridge").unwrap()) + .user(ShellWord::current_user()) + .cap_drop_all() + .no_new_privileges() + .read_only() + .tmpfs(DockerTmpfs::new("/tmp", "rw,nosuid,nodev,noexec").unwrap()) + .pids_limit(64) + .entrypoint(ShellWord::literal("sh").unwrap()) + .mount( + DockerMount::read_only(ShellWord::variable("BUNDLE").unwrap(), "/app/bundle.js") + .unwrap(), + ) + .command_arg(ShellWord::literal("-c").unwrap()) + .command_arg(ShellWord::literal("exec node /app/bundle.js").unwrap()) + .discard_stdout(); + + assert_eq!( + invocation.render_bash().unwrap(), + concat!( + "docker run \\\n", + " -d \\\n", + " --name \"${CONTAINER}\" \\\n", + " --network 'bridge' \\\n", + " --user \"$(id -u):$(id -g)\" \\\n", + " --cap-drop ALL \\\n", + " --security-opt no-new-privileges \\\n", + " --read-only \\\n", + " --tmpfs '/tmp:rw,nosuid,nodev,noexec' \\\n", + " --pids-limit 64 \\\n", + " --entrypoint 'sh' \\\n", + " -v \"${BUNDLE}:/app/bundle.js:ro\" \\\n", + " \"${IMAGE}\" \\\n", + " '-c' \\\n", + " 'exec node /app/bundle.js' \\\n", + " >/dev/null\n", + ) + ); + } + + #[test] + fn rejects_duplicate_singletons_and_mount_destinations() { + let duplicate_name = DockerRun::new(ShellWord::literal("image").unwrap()) + .name(ShellWord::literal("one").unwrap()) + .name(ShellWord::literal("two").unwrap()); + assert!(duplicate_name.render_bash().is_err()); + + let duplicate_mount = DockerRun::new(ShellWord::literal("image").unwrap()) + .mount(DockerMount::read_only(ShellWord::literal("/one").unwrap(), "/target").unwrap()) + .mount( + DockerMount::read_write(ShellWord::literal("/two").unwrap(), "/target").unwrap(), + ); + assert!(duplicate_mount.render_bash().is_err()); + } + + #[test] + fn mount_destinations_use_the_validated_container_path_contract() { + for path in [ + "/", + "relative", + "/token:rw", + "/token:ro:/other", + "/var/../token", + "/var//token", + "/token/", + "/token\tfile", + "/token;command", + ] { + assert!( + DockerMount::read_only(ShellWord::literal("/source").unwrap(), path).is_err(), + "mount accepted {path:?}" + ); + assert!( + DockerTmpfs::new(path, "rw,nosuid").is_err(), + "tmpfs accepted {path:?}" + ); + } + for options in ["", "rw:ro", "rw\nnoexec", "rw\0"] { + assert!( + DockerTmpfs::new("/tmp", options).is_err(), + "tmpfs accepted options {options:?}" + ); + } + } + + #[test] + fn shell_words_quote_literals_and_validate_variables() { + assert_eq!(ShellWord::literal("a'b").unwrap().render(), "'a'\\''b'"); + assert_eq!( + ShellWord::variable("ROOT") + .unwrap() + .with_literal("/child") + .unwrap() + .render(), + "\"${ROOT}/child\"" + ); + assert!(ShellWord::variable("bad-name").is_err()); + assert!(ShellWord::variable("ADO_PROXY_BEARER").is_err()); + } +} diff --git a/src/compile/extensions/ado_script.rs b/src/compile/extensions/ado_script.rs index 7cd1a52e8..26bd43c07 100644 --- a/src/compile/extensions/ado_script.rs +++ b/src/compile/extensions/ado_script.rs @@ -298,6 +298,9 @@ pub(crate) const GITHUB_APP_TOKEN_PATH: &str = "/tmp/ado-aw-scripts/ado-script/g /// the containerized SafeOutputs MCP server can compute a diff base on /// shallow-default agent pools. pub(crate) const PREPARE_PR_BASE_PATH: &str = "/tmp/ado-aw-scripts/ado-script/prepare-pr-base.js"; +/// Path to the renewable Azure workload-identity assertion sidecar bundle. +pub(crate) const AZURE_WIF_REFRESH_PATH: &str = + "/tmp/ado-aw-scripts/ado-script/azure-wif-refresh.js"; const RELEASE_BASE_URL: &str = "https://github.com/githubnext/ado-aw/releases/download"; /// Single always-on extension that owns all `ado-script` bundle wiring. @@ -379,6 +382,9 @@ pub struct AdoScriptExtension { /// emitted by `build_agent_job`, not this extension, so the flag drives the /// shared bundle download. pub prepare_pr_base_active: bool, + /// Whether any user-defined stdio MCP server configures `azure-auth`. + /// Drives Agent-job bundle delivery for `azure-wif-refresh.js`. + pub azure_mcp_auth_active: bool, /// PR trigger config required to build `PR_SYNTH_SPEC`. `Some(_)` /// is the single source of truth for "synthetic-from-ci path is /// active for this agent" — `is_some()` replaces what used to be a @@ -1162,6 +1168,7 @@ impl CompilerExtension for AdoScriptExtension { || self.safe_outputs_summary_active || self.github_app_token_active || self.prepare_pr_base_active + || self.azure_mcp_auth_active { agent_prepare_steps .extend(install_and_download_steps_typed(self.supply_chain.as_ref())); @@ -1369,6 +1376,7 @@ mod tests { safe_outputs_summary_active: false, github_app_token_active: false, prepare_pr_base_active: false, + azure_mcp_auth_active: false, pr_trigger_for_synth: None, supply_chain: None, } @@ -1448,6 +1456,7 @@ mod tests { safe_outputs_summary_active: false, github_app_token_active: false, prepare_pr_base_active: false, + azure_mcp_auth_active: false, pr_trigger_for_synth: Some(PrTriggerConfig { branches: Some(BranchFilter { include: vec!["main".into()], @@ -1507,6 +1516,7 @@ mod tests { safe_outputs_summary_active: false, github_app_token_active: false, prepare_pr_base_active: false, + azure_mcp_auth_active: false, pr_trigger_for_synth: Some(PrTriggerConfig { branches: Some(BranchFilter { include: vec!["main".into()], @@ -2267,6 +2277,7 @@ mod tests { safe_outputs_summary_active: false, github_app_token_active: false, prepare_pr_base_active: false, + azure_mcp_auth_active: false, pr_trigger_for_synth: Some(PrTriggerConfig { branches: Some(BranchFilter { include: vec!["main".into()], @@ -2742,6 +2753,20 @@ mod tests { assert!(decl.agent_prepare_steps.is_empty()); } + #[test] + fn declarations_agent_prepare_download_fires_for_azure_mcp_auth() { + let mut ext = ext_with(None, None, true); + ext.azure_mcp_auth_active = true; + let fm: FrontMatter = serde_yaml::from_str("name: t\ndescription: t").unwrap(); + let ctx = CompileContext::for_test(&fm); + let steps = ext.declarations(&ctx).unwrap().agent_prepare_steps; + assert_eq!(steps.len(), 2, "install + download only"); + assert!(matches!(&steps[0], Step::Task(t) if t.task == "UseNode@1")); + assert!( + matches!(&steps[1], Step::Bash(b) if b.display_name.contains("Download ado-aw scripts")) + ); + } + /// `declarations()` setup_steps must surface a typed /// `Step::Task(UseNode@1)` followed by `Step::Bash` (download) /// followed by the typed gate `Step::Bash` when a PR gate is @@ -2802,6 +2827,7 @@ mod tests { safe_outputs_summary_active: false, github_app_token_active: false, prepare_pr_base_active: false, + azure_mcp_auth_active: false, pr_trigger_for_synth: Some(PrTriggerConfig { branches: Some(BranchFilter { include: vec!["main".into()], diff --git a/src/compile/extensions/mod.rs b/src/compile/extensions/mod.rs index e8147c0af..26a7f6736 100644 --- a/src/compile/extensions/mod.rs +++ b/src/compile/extensions/mod.rs @@ -825,6 +825,7 @@ pub fn collect_extensions(front_matter: &FrontMatter) -> Vec { // emits before the Copilot run. Same loose-coupling pattern as // `github_app_token_active`. prepare_pr_base_active: front_matter.create_pr_config().is_some(), + azure_mcp_auth_active: front_matter.has_azure_authenticated_mcp_servers(), pr_trigger_for_synth, supply_chain: front_matter.supply_chain().cloned(), } diff --git a/src/compile/ir/step.rs b/src/compile/ir/step.rs index 057772623..391a716e2 100644 --- a/src/compile/ir/step.rs +++ b/src/compile/ir/step.rs @@ -182,6 +182,12 @@ impl TaskStep { self.inputs.insert(key.into(), value.into()); self } + + /// Add (or replace) an env-var binding. + pub fn with_env(mut self, key: impl Into, value: EnvValue) -> Self { + self.env.insert(key.into(), value); + self + } } /// A `- checkout: …` step. @@ -259,6 +265,19 @@ mod tests { assert_eq!(s.outputs[0].name, "AW_OUT", "output name should be AW_OUT"); } + #[test] + fn task_step_builder_composes_inputs_and_env() { + let s = TaskStep::new("AzureCLI@3", "Azure CLI") + .with_input("scriptType", "bash") + .with_env("SYSTEM_ACCESSTOKEN", EnvValue::secret("System.AccessToken")); + + assert_eq!(s.inputs.get("scriptType").map(String::as_str), Some("bash")); + assert_eq!( + s.env.get("SYSTEM_ACCESSTOKEN"), + Some(&EnvValue::secret("System.AccessToken")) + ); + } + #[test] fn step_id_returns_none_for_anchorless_kinds() { let chk = Step::Checkout(CheckoutStep { diff --git a/src/compile/ir/tasks/azure_cli.rs b/src/compile/ir/tasks/azure_cli.rs index bf6db18a3..74cf239aa 100644 --- a/src/compile/ir/tasks/azure_cli.rs +++ b/src/compile/ir/tasks/azure_cli.rs @@ -242,6 +242,7 @@ pub struct AzureCliV3 { connection: AzureCliV3Connection, script_type: ScriptType, location: ScriptLocation, + add_spn_to_environment: Option, visible_az_login: Option, display_name: Option, } @@ -256,11 +257,19 @@ impl AzureCliV3 { connection, script_type, location, + add_spn_to_environment: None, visible_az_login: None, display_name: None, } } + /// `addSpnToEnvironment` — expose service-principal details and, for a + /// workload-identity connection, the short-lived `idToken`. + pub fn add_spn_to_environment(mut self, value: bool) -> Self { + self.add_spn_to_environment = Some(value); + self + } + pub fn visible_az_login(mut self, value: bool) -> Self { self.visible_az_login = Some(value); self @@ -293,6 +302,11 @@ impl AzureCliV3 { .with_input("scriptPath", path); } } + push_bool( + &mut task, + "addSpnToEnvironment", + self.add_spn_to_environment, + ); push_bool(&mut task, "visibleAzLogin", self.visible_az_login); task } @@ -490,6 +504,7 @@ mod tests { ScriptType::Bash, ScriptLocation::Inline("echo token\n".to_string()), ) + .add_spn_to_environment(true) .visible_az_login(false) .into_step(); @@ -507,6 +522,10 @@ mod tests { task.inputs.get("visibleAzLogin").map(String::as_str), Some("false") ); + assert_eq!( + task.inputs.get("addSpnToEnvironment").map(String::as_str), + Some("true") + ); } #[test] diff --git a/src/compile/mcpg.rs b/src/compile/mcpg.rs index 4a6c20028..dd8c87a65 100644 --- a/src/compile/mcpg.rs +++ b/src/compile/mcpg.rs @@ -12,13 +12,25 @@ pub struct McpgEnvName(String); impl McpgEnvName { pub fn parse(value: impl Into, origin: &str) -> Result { + Self::parse_with_internal(value, origin, false) + } + + fn parse_internal(value: impl Into, origin: &str) -> Result { + Self::parse_with_internal(value, origin, true) + } + + fn parse_with_internal( + value: impl Into, + origin: &str, + allow_internal: bool, + ) -> Result { let value = value.into(); if !crate::validate::is_valid_env_var_name(&value) { bail!( "{origin} environment variable name '{value}' is invalid; expected [A-Za-z_][A-Za-z0-9_]*" ); } - if value.starts_with("ADO_AW_MCPG_INTERNAL_") + if (!allow_internal && value.starts_with("ADO_AW_MCPG_INTERNAL_")) || matches!( value.as_str(), "MCP_GATEWAY_API_KEY" @@ -34,6 +46,7 @@ impl McpgEnvName { | "MCPG_CONFIG" | "GATEWAY_OUTPUT" | "MCPG_ENV_NAMES" + | "MCPG_REQUIRED_ENV_NAMES" | "MCPG_DOCKER_ENV_ARGS" | "MCPG_ENV_NAME" ) @@ -54,6 +67,7 @@ impl McpgEnvName { struct McpgLaunchBinding { value: EnvValue, origin: String, + required: bool, } #[derive(Debug, Clone, Default)] @@ -68,7 +82,28 @@ impl McpgLaunchEnvironment { source: &AdoVariableName, origin: impl Into, ) -> Result<()> { - self.bind(destination, EnvValue::pipeline_var(source.as_str()), origin) + self.bind( + destination, + EnvValue::pipeline_var(source.as_str()), + origin, + false, + false, + ) + } + + pub fn bind_internal_pipeline_variable( + &mut self, + destination: impl Into, + source: &AdoVariableName, + origin: impl Into, + ) -> Result<()> { + self.bind( + destination, + EnvValue::pipeline_var(source.as_str()), + origin, + true, + true, + ) } pub fn bind_literal( @@ -77,7 +112,7 @@ impl McpgLaunchEnvironment { value: impl Into, origin: impl Into, ) -> Result<()> { - self.bind(destination, EnvValue::literal(value), origin) + self.bind(destination, EnvValue::literal(value), origin, false, false) } fn bind( @@ -85,11 +120,17 @@ impl McpgLaunchEnvironment { destination: impl Into, value: EnvValue, origin: impl Into, + allow_internal: bool, + required: bool, ) -> Result<()> { let origin = origin.into(); - let destination = McpgEnvName::parse(destination, &origin)?; + let destination = if allow_internal { + McpgEnvName::parse_internal(destination, &origin)? + } else { + McpgEnvName::parse(destination, &origin)? + }; if let Some(existing) = self.bindings.get(&destination) { - if existing.value == value { + if existing.value == value && existing.required == required { return Ok(()); } bail!( @@ -101,8 +142,14 @@ impl McpgLaunchEnvironment { value ); } - self.bindings - .insert(destination, McpgLaunchBinding { value, origin }); + self.bindings.insert( + destination, + McpgLaunchBinding { + value, + origin, + required, + }, + ); Ok(()) } @@ -116,6 +163,12 @@ impl McpgLaunchEnvironment { self.bindings.keys().map(McpgEnvName::as_str) } + pub fn required_names(&self) -> impl Iterator { + self.bindings + .iter() + .filter_map(|(name, binding)| binding.required.then_some(name.as_str())) + } + #[cfg(test)] pub fn get(&self, name: &str) -> Option<&EnvValue> { self.bindings @@ -124,6 +177,38 @@ impl McpgLaunchEnvironment { } } +pub fn azure_auth_runtime_id(server_name: &str) -> String { + crate::hash::sha256_hex(server_name.as_bytes())[..16].to_ascii_uppercase() +} + +pub fn azure_auth_client_variable(server_name: &str) -> Result { + AdoVariableName::parse(format!( + "ADO_AW_MCPG_INTERNAL_AZURE_{}_CLIENT_ID", + azure_auth_runtime_id(server_name) + )) +} + +pub fn azure_auth_tenant_variable(server_name: &str) -> Result { + AdoVariableName::parse(format!( + "ADO_AW_MCPG_INTERNAL_AZURE_{}_TENANT_ID", + azure_auth_runtime_id(server_name) + )) +} + +pub fn azure_auth_host_directory(server_name: &str) -> String { + format!( + "$(Agent.TempDirectory)/ado-aw-azure-auth/{}", + azure_auth_runtime_id(server_name).to_ascii_lowercase() + ) +} + +pub fn azure_auth_container_name(server_name: &str) -> String { + format!( + "ado-aw-azure-auth-{}", + azure_auth_runtime_id(server_name).to_ascii_lowercase() + ) +} + #[derive(Debug, Clone)] pub struct McpgCompilation { pub config: McpgConfig, @@ -189,4 +274,21 @@ mod tests { assert!(error.contains("reserved")); } } + + #[test] + fn compiler_internal_binding_is_required_and_user_inaccessible() { + let source = AdoVariableName::parse("ADO_AW_MCPG_INTERNAL_AZURE_TEST_CLIENT_ID").unwrap(); + let mut env = McpgLaunchEnvironment::default(); + env.bind_internal_pipeline_variable(source.as_str(), &source, "compiler azure-auth") + .unwrap(); + assert_eq!( + env.required_names().collect::>(), + vec!["ADO_AW_MCPG_INTERNAL_AZURE_TEST_CLIENT_ID"] + ); + let error = env + .bind_pipeline_variable(source.as_str(), &source, "user") + .unwrap_err() + .to_string(); + assert!(error.contains("reserved")); + } } diff --git a/src/compile/mod.rs b/src/compile/mod.rs index 2f9492aac..3de257c19 100644 --- a/src/compile/mod.rs +++ b/src/compile/mod.rs @@ -11,6 +11,7 @@ pub mod az_wrapper; pub(crate) use common::resolve_repos; pub(crate) mod ado_bundle; pub(crate) mod agentic_pipeline; +pub(crate) mod container_invocation; #[cfg(test)] mod codemod_integration_test; pub(crate) mod codemods; diff --git a/src/compile/shell/bindings.rs b/src/compile/shell/bindings.rs index 8c772405c..fb8874a15 100644 --- a/src/compile/shell/bindings.rs +++ b/src/compile/shell/bindings.rs @@ -249,14 +249,15 @@ const DOCUMENT_DELIMITER: &str = "ADO_AW_SHELL_DOC_EOF"; /// Refuse a value that names a credential. See [`SECRET_NAMES`]. #[track_caller] fn assert_not_secret(value: &str) { - for secret in SECRET_NAMES { - assert!( - !value.contains(secret), - "a credential must not reach the generated prelude: {value:?} \ - mentions {secret}. Pass it through `with_env` / `EnvValue::secret` \ - so Azure DevOps masks it." - ); - } + assert!( + !contains_secret_name(value), + "a credential must not reach the generated prelude: {value:?}. \ + Pass it through `with_env` / `EnvValue::secret` so Azure DevOps masks it." + ); +} + +pub(crate) fn contains_secret_name(value: &str) -> bool { + SECRET_NAMES.iter().any(|secret| value.contains(secret)) } /// POSIX single-quoting: the only escape available inside `'…'` is to close diff --git a/src/compile/shell/lint.rs b/src/compile/shell/lint.rs index de66feca8..77d5554e2 100644 --- a/src/compile/shell/lint.rs +++ b/src/compile/shell/lint.rs @@ -21,9 +21,9 @@ use std::process::{Command, Stdio}; use serde::Deserialize; -use super::registry::{ShellScriptDef, all_scripts}; use super::FRAGMENT_MARKER; use super::bindings::is_shell_var_name; +use super::registry::{ShellScriptDef, all_scripts}; /// One shellcheck JSON finding. #[derive(Debug, Deserialize)] @@ -127,7 +127,10 @@ fn every_declared_variable_name_is_a_valid_shell_name() { } } } - assert!(problems.is_empty(), "invalid shell variable names:\n{problems}"); + assert!( + problems.is_empty(), + "invalid shell variable names:\n{problems}" + ); } #[test] @@ -148,6 +151,33 @@ fn every_phase_is_also_a_declared_fragment() { assert!(problems.is_empty(), "phase declaration drift:\n{problems}"); } +#[test] +fn every_dynamic_fragment_use_is_declared() { + let mut problems = String::new(); + for def in all_scripts() { + for (fragment, variables) in def.fragment_uses { + if !def.fragments.contains(fragment) { + problems.push_str(&format!( + " {} declares uses for unknown fragment `{fragment}` ({}:{})\n", + def.name, def.file, def.line + )); + } + for variable in *variables { + if !def.bindings.contains(variable) && !def.externals.contains(variable) { + problems.push_str(&format!( + " {} fragment `{fragment}` uses undeclared variable `{variable}` ({}:{})\n", + def.name, def.file, def.line + )); + } + } + } + } + assert!( + problems.is_empty(), + "dynamic fragment variable declaration drift:\n{problems}" + ); +} + #[test] fn a_composed_script_is_linted_with_its_phases_spliced() { // Guards the mechanism the SC2034-on-every-binding failure exposed: an @@ -214,7 +244,10 @@ fn every_declared_fragment_has_a_marker_and_vice_versa() { } } } - assert!(problems.is_empty(), "fragment declaration drift:\n{problems}"); + assert!( + problems.is_empty(), + "fragment declaration drift:\n{problems}" + ); } #[test] @@ -324,9 +357,7 @@ mod tests { // `$NF` belongs to awk, not to the shell — nothing expands inside // '…'. Treating it as a shell variable would force the author to // rename another language's variable to satisfy this checker. - let vars = referenced_vars( - r#"awk -F/ '{ if (NF>1) print $NF }' <<< "$COLLECTION""#, - ); + let vars = referenced_vars(r#"awk -F/ '{ if (NF>1) print $NF }' <<< "$COLLECTION""#); assert_eq!(vars, vec!["COLLECTION"]); } @@ -340,7 +371,10 @@ mod tests { fn assigned_in_body_recognises_the_common_forms() { assert!(assigned_in_body("PROXY_DIR=$(mktemp -d)", "PROXY_DIR")); assert!(assigned_in_body("export PROXY_DIR=/tmp", "PROXY_DIR")); - assert!(assigned_in_body("for PROXY_HOST in $HOSTS; do", "PROXY_HOST")); + assert!(assigned_in_body( + "for PROXY_HOST in $HOSTS; do", + "PROXY_HOST" + )); assert!(assigned_in_body("set -eu; UMASK=1", "UMASK")); assert!(!assigned_in_body("echo \"$PROXY_DIR\"", "PROXY_DIR")); } diff --git a/src/compile/shell/mod.rs b/src/compile/shell/mod.rs index 83923a992..d5290b83f 100644 --- a/src/compile/shell/mod.rs +++ b/src/compile/shell/mod.rs @@ -477,6 +477,7 @@ echo hello externals, fragments, phases: &[], + fragment_uses: &[], body, file: file!(), line: line!(), diff --git a/src/compile/shell/registry.rs b/src/compile/shell/registry.rs index f7fff8572..b94eb3e76 100644 --- a/src/compile/shell/registry.rs +++ b/src/compile/shell/registry.rs @@ -76,6 +76,11 @@ pub struct ShellScriptDef { /// Every entry must also appear in [`fragments`](Self::fragments); a test /// enforces it. pub phases: &'static [(&'static str, &'static ShellScriptDef)], + /// Variables consumed by dynamically generated fragments. + /// + /// The lint source marks these as used without adding fake reads to the + /// emitted runtime script. + pub fragment_uses: &'static [(&'static str, &'static [&'static str])], /// The script itself, verbatim, exactly as it will run. pub body: &'static str, /// Source file, for the export provenance header. @@ -117,6 +122,13 @@ impl ShellScriptDef { out.push_str(name); out.push_str("='ado-aw-lint-stub'\n"); } + let mut fragment_uses = Vec::new(); + self.collect_fragment_uses(&mut fragment_uses); + for name in fragment_uses { + out.push_str(": \"${"); + out.push_str(name); + out.push_str("}\"\n"); + } out.push_str("# --- end lint stubs ---\n"); let body = super::dedent(body.trim_start_matches('\n')); @@ -136,6 +148,19 @@ impl ShellScriptDef { pub fn export_file_name(&self) -> String { format!("{}.sh", self.name.replace("::", "__")) } + + fn collect_fragment_uses(&self, out: &mut Vec<&'static str>) { + for (_, variables) in self.fragment_uses { + for variable in *variables { + if !out.contains(variable) { + out.push(variable); + } + } + } + for (_, phase) in self.phases { + phase.collect_fragment_uses(out); + } + } } inventory::collect!(ShellScriptDef); @@ -146,7 +171,9 @@ inventory::collect!(ShellScriptDef); /// document that it reads it — but the lint must not stub-assign it. /// Shellcheck already treats them as set, and assigning some of them is /// itself a finding (`PATH=…` trips SC2123). -const SHELL_PROVIDED: &[&str] = &["PATH", "HOME", "TMPDIR", "PWD", "IFS", "SHELL", "USER", "TERM"]; +const SHELL_PROVIDED: &[&str] = &[ + "PATH", "HOME", "TMPDIR", "PWD", "IFS", "SHELL", "USER", "TERM", +]; /// Every registered script, in a stable order (sorted by [`ShellScriptDef::name`]). /// @@ -219,6 +246,35 @@ macro_rules! shell_script { externals: [$($external),*], fragments: [$($fragment),*], phases: [], + fragment_uses: [], + body: $body, + } + } + }; + ( + $(#[$meta:meta])* + $ident:ident { + interpreter: $interpreter:ident, + bindings: [$($binding:ident),* $(,)?], + externals: [$($external:ident),* $(,)?], + fragments: [$($fragment:ident),* $(,)?], + fragment_uses: [ + $($used_fragment:ident => [$($used_variable:ident),* $(,)?]),* $(,)? + ], + body: $body:expr $(,)? + } + ) => { + $crate::shell_script! { + $(#[$meta])* + $ident { + interpreter: $interpreter, + bindings: [$($binding),*], + externals: [$($external),*], + fragments: [$($fragment),*], + phases: [], + fragment_uses: [ + $($used_fragment => [$($used_variable),*]),* + ], body: $body, } } @@ -233,6 +289,33 @@ macro_rules! shell_script { phases: [$($phase:ident = $phase_def:path),* $(,)?], body: $body:expr $(,)? } + ) => { + $crate::shell_script! { + $(#[$meta])* + $ident { + interpreter: $interpreter, + bindings: [$($binding),*], + externals: [$($external),*], + fragments: [$($fragment),*], + phases: [$($phase = $phase_def),*], + fragment_uses: [], + body: $body, + } + } + }; + ( + $(#[$meta:meta])* + $ident:ident { + interpreter: $interpreter:ident, + bindings: [$($binding:ident),* $(,)?], + externals: [$($external:ident),* $(,)?], + fragments: [$($fragment:ident),* $(,)?], + phases: [$($phase:ident = $phase_def:path),* $(,)?], + fragment_uses: [ + $($used_fragment:ident => [$($used_variable:ident),* $(,)?]),* $(,)? + ], + body: $body:expr $(,)? + } ) => { $(#[$meta])* #[allow(dead_code)] @@ -244,6 +327,14 @@ macro_rules! shell_script { externals: &[$(stringify!($external)),*], fragments: &[$(stringify!($fragment)),*], phases: &[$((stringify!($phase), &$phase_def)),*], + fragment_uses: &[ + $( + ( + stringify!($used_fragment), + &[$(stringify!($used_variable)),*], + ) + ),* + ], body: $body, file: file!(), line: line!(), @@ -311,7 +402,10 @@ echo "$TARGET $FROM_ENV $ORG" #[test] fn lint_source_stubs_every_declared_variable() { let source = REGISTRY_FIXTURE.lint_source(); - assert!(source.starts_with("#!/bin/sh\n"), "shebang stays first: {source}"); + assert!( + source.starts_with("#!/bin/sh\n"), + "shebang stays first: {source}" + ); assert!(source.contains("TARGET='ado-aw-lint-stub'")); // An external is stubbed too: it genuinely arrives from outside, so // SC2154 on it would be noise. @@ -324,12 +418,20 @@ echo "$TARGET $FROM_ENV $ORG" assert!(source.contains("# ado-aw:fragment resolve_org")); // Nothing undeclared is invented. assert!(!source.contains("UNDECLARED='ado-aw-lint-stub'")); - assert!(source.trim_end().ends_with("echo \"$TARGET $FROM_ENV $ORG\"")); + assert!( + source + .trim_end() + .ends_with("echo \"$TARGET $FROM_ENV $ORG\"") + ); } #[test] fn export_file_names_are_path_safe() { - assert!(REGISTRY_FIXTURE.export_file_name().ends_with("__REGISTRY_FIXTURE.sh")); + assert!( + REGISTRY_FIXTURE + .export_file_name() + .ends_with("__REGISTRY_FIXTURE.sh") + ); assert!(!REGISTRY_FIXTURE.export_file_name().contains(':')); } diff --git a/src/compile/types.rs b/src/compile/types.rs index 79de7262e..69a83d9f8 100644 --- a/src/compile/types.rs +++ b/src/compile/types.rs @@ -1563,6 +1563,40 @@ impl FrontMatter { .map(|p| p.overrides()) .unwrap_or_else(|| EMPTY_OVERRIDES.get_or_init(HashMap::new)) } + + pub fn azure_authenticated_mcp_servers(&self) -> Vec<(&str, &McpOptions, &AzureMcpAuthConfig)> { + let mut servers = self + .mcp_servers + .iter() + .filter_map(|(name, config)| match config { + McpConfig::WithOptions(options) + if options.enabled.unwrap_or(true) && options.azure_auth.is_some() => + { + Some(( + name.as_str(), + options.as_ref(), + options + .azure_auth + .as_ref() + .expect("azure-auth presence checked"), + )) + } + _ => None, + }) + .collect::>(); + servers.sort_by_key(|(name, _, _)| *name); + servers + } + + pub fn has_azure_authenticated_mcp_servers(&self) -> bool { + self.mcp_servers.values().any(|config| { + matches!( + config, + McpConfig::WithOptions(options) + if options.enabled.unwrap_or(true) && options.azure_auth.is_some() + ) + }) + } } /// Compile-time source for a remote reusable import. @@ -3843,6 +3877,30 @@ pub struct McpPipelineVariable { pub pipeline_variable: crate::secure::AdoVariableName, } +fn default_azure_mcp_mount_path() -> crate::secure::ContainerAbsolutePath { + crate::secure::ContainerAbsolutePath::parse("/var/run/ado-aw/azure") + .expect("compiler-owned Azure MCP mount path is valid") +} + +/// Renewable workload-identity authentication for a containerized MCP server. +#[derive(Debug, Deserialize, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct AzureMcpAuthConfig { + /// ARM workload-identity service connection used to mint ADO ID tokens. + #[serde(rename = "service-connection")] + pub service_connection: crate::secure::ServiceConnection, + /// Directory mounted into the MCP container. The assertion is written to + /// `/token`. + #[serde(default = "default_azure_mcp_mount_path", rename = "mount-path")] + pub mount_path: crate::secure::ContainerAbsolutePath, +} + +impl AzureMcpAuthConfig { + pub fn token_path(&self) -> String { + format!("{}/token", self.mount_path.as_str()) + } +} + /// Detailed MCP options #[derive(Debug, Deserialize, Clone, Default, SanitizeConfig)] pub struct McpOptions { @@ -3878,6 +3936,11 @@ pub struct McpOptions { /// the typed MCPG launch-step environment. #[serde(default)] pub env: HashMap, + /// Renewable Azure workload-identity authentication for containerized + /// stdio MCP servers. + #[serde(default, rename = "azure-auth")] + #[sanitize_config(skip)] + pub azure_auth: Option, } /// Unified trigger configuration — `on:` front matter key. diff --git a/src/secure.rs b/src/secure.rs index 41e95c774..cf6819f0e 100644 --- a/src/secure.rs +++ b/src/secure.rs @@ -187,6 +187,34 @@ validated_string! { StrictRelativePath, "path", validate::validate_relative_segment_path } +validated_string! { + /// An absolute POSIX path inside a container. + /// + /// Used for compiler-owned mount destinations. Rejects root, traversal, + /// empty/dot components, control characters, shell metacharacters, and + /// Docker's `:` mount separator. + ContainerAbsolutePath, "container path", |value: &str, label: &str| { + if !value.starts_with('/') || value == "/" { + anyhow::bail!("{label} must be an absolute POSIX path below `/`"); + } + if value.ends_with('/') + || !value + .chars() + .all(|character| character.is_ascii_alphanumeric() || "/._-".contains(character)) + { + anyhow::bail!("{label} contains characters that are unsafe in a container mount"); + } + if value + .split('/') + .skip(1) + .any(|segment| segment.is_empty() || segment == "." || segment == "..") + { + anyhow::bail!("{label} must not contain empty, `.` or `..` path components"); + } + Ok(()) + } +} + validated_string! { /// A single safe path segment / alias (e.g. a repository checkout alias). PathSegment, "segment", |value: &str, label: &str| { @@ -604,6 +632,31 @@ mod tests { assert!(StrictRelativePath::parse("a:b").is_err()); } + #[test] + fn container_absolute_path_rules() { + assert!(ContainerAbsolutePath::parse("/var/run/ado-aw/azure").is_ok()); + assert!(ContainerAbsolutePath::parse("/").is_err()); + assert!(ContainerAbsolutePath::parse("relative/path").is_err()); + assert!(ContainerAbsolutePath::parse("/var/run/../secret").is_err()); + assert!(ContainerAbsolutePath::parse("/var//run").is_err()); + assert!(ContainerAbsolutePath::parse("/var/run:rw").is_err()); + assert!(ContainerAbsolutePath::parse("/var/$(TOKEN)").is_err()); + for unsafe_path in [ + "/var/run/token dir", + "/var/run/token\tdir", + "/var/run/'token'", + "/var/run/\"token\"", + "/var/run/token;command", + "/var/run/token|command", + "/var/run/token&command", + ] { + assert!( + ContainerAbsolutePath::parse(unsafe_path).is_err(), + "{unsafe_path:?} must be rejected" + ); + } + } + #[test] fn path_segment_rejects_separators() { assert!(PathSegment::parse("my-repo").is_ok()); diff --git a/tests/azure-wif-refresh-e2e/README.md b/tests/azure-wif-refresh-e2e/README.md new file mode 100644 index 000000000..91d945da3 --- /dev/null +++ b/tests/azure-wif-refresh-e2e/README.md @@ -0,0 +1,54 @@ +# Azure WIF refresh E2E + +This manual Azure Pipelines test proves the runtime boundary that local tests +cannot model: an ARM workload-identity service connection can obtain a fresh +Azure DevOps ID token after the assertion exposed by AzureCLI@3 has expired. + +Queue `azure-pipelines.yml` and set the `serviceConnection` parameter to an +authorized ARM workload-identity service connection. The test: + +1. builds the candidate `azure-wif-refresh.js` bundle; +2. starts it with the job's `System.AccessToken`, `System.OidcRequestUri`, and + AzureCLI@3 service-connection metadata; +3. waits until the original assertion has expired; +4. verifies that the projected token changed and has a later expiry; and +5. exchanges the refreshed assertion directly with Entra for an Azure access + token, without relying on an Azure CLI token cache. + +The test logs expiry timestamps and assertion hashes only. It never prints or +publishes token values. + +## Credential-free regressions + +The existing `ado-script` GitHub Actions job also runs +`scripts/ado-script/test/azure-wif-isolation.test.ts`. No Azure service +connection or pipeline registration is needed: + +- A Linux Docker test runs the compiler's directory/FIFO setup and the bundled + refresher with a fake clock/provider. A persistent, different-UID consumer + uses the compiler-generated read-only mount and observes atomic rotation. + The test also checks denied writes, inaccessible private sibling files, + and denied access for an unrelated UID through the original directory tree. +- A Linux AWF test captures the compiled agent invocation, replaces the AI + command with a file/environment probe, and runs the compiler-pinned AWF + binary (downloaded with checksum verification). It checks normal and + `/host` paths plus a workspace symlink, and verifies that internal identity + variables are excluded. It omits MCPG network attachment because this probe + has no MCPG service. + +Both use synthetic values only. They do not prove Azure token issuance or +Entra exchange; the manual credentialed test above covers that boundary. + +After building the compiler and refresher bundle, run the Docker regression: + +```bash +cargo build +cd scripts/ado-script +npm run build:azure-wif-refresh +ADO_AW_TEST_DOCKER=1 npx vitest run -c vitest.config.smoke.ts test/azure-wif-isolation.test.ts +``` + +On Linux, also set `ADO_AW_TEST_AWF=1` to run the real AWF probe. It requires +Docker, access to GitHub release assets/GHCR, and no concurrent AWF session +(AWF owns fixed container names). Private fixture data is placed outside +`/tmp` and the workspace; AWF intentionally exposes those locations. diff --git a/tests/azure-wif-refresh-e2e/azure-pipelines.yml b/tests/azure-wif-refresh-e2e/azure-pipelines.yml new file mode 100644 index 000000000..26c657460 --- /dev/null +++ b/tests/azure-wif-refresh-e2e/azure-pipelines.yml @@ -0,0 +1,179 @@ +# Manual credentialed proof for mcp-servers..azure-auth. +# +# The service connection is a queue-time parameter so no environment-specific +# credential name is committed. The pipeline emits an instructions-only job +# when queued without it. + +trigger: none +pr: none + +parameters: + - name: serviceConnection + displayName: ARM workload-identity service connection + type: string + default: "" + +pool: + vmImage: ubuntu-22.04 + +jobs: + - ${{ if eq(parameters.serviceConnection, '') }}: + - job: Instructions + steps: + - script: | + echo "Queue this pipeline with the serviceConnection parameter set." + displayName: Explain required parameter + + - ${{ if ne(parameters.serviceConnection, '') }}: + - job: DelayedFirstExchange + timeoutInMinutes: 25 + steps: + - checkout: self + fetchDepth: 1 + + - task: UseNode@1 + inputs: + version: "20.x" + displayName: Use Node.js 20 + + - script: | + set -euo pipefail + npm ci + npm run build:azure-wif-refresh + workingDirectory: $(Build.SourcesDirectory)/scripts/ado-script + displayName: Build candidate refresher + + - task: AzureCLI@3 + displayName: Verify refresh after initial assertion expiry + inputs: + connectionType: azureRM + azureSubscription: ${{ parameters.serviceConnection }} + scriptType: bash + scriptLocation: inlineScript + addSpnToEnvironment: true + visibleAzLogin: false + inlineScript: | + set -euo pipefail + + GUID_RE='^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$' + SERVICE_CONNECTION_ID="${AZURESUBSCRIPTION_SERVICE_CONNECTION_ID:-}" + if [ -z "${idToken:-}" ] \ + || ! [[ "${servicePrincipalId:-}" =~ $GUID_RE ]] \ + || ! [[ "${tenantId:-}" =~ $GUID_RE ]] \ + || ! [[ "$SERVICE_CONNECTION_ID" =~ $GUID_RE ]] \ + || [ -z "${SYSTEM_OIDCREQUESTURI:-}" ]; then + echo "Required workload-identity metadata is unavailable" >&2 + exit 1 + fi + + ROOT="$(Agent.TempDirectory)/azure-wif-refresh-e2e" + rm -rf "$ROOT" + mkdir -p "$ROOT/token.d" + chmod 700 "$ROOT" + chmod 755 "$ROOT/token.d" + FIFO="$ROOT/material" + mkfifo -m 600 "$FIFO" + LOG="$ROOT/refresher.log" + + node "$(Build.SourcesDirectory)/scripts/ado-script/azure-wif-refresh.js" \ + < "$FIFO" >"$LOG" 2>&1 & + REFRESH_PID=$! + cleanup() { + kill -TERM "$REFRESH_PID" 2>/dev/null || true + wait "$REFRESH_PID" 2>/dev/null || true + rm -rf "$ROOT" + } + trap cleanup EXIT + + MATERIAL_FIFO="$FIFO" \ + INITIAL_ID_TOKEN="$idToken" \ + SYSTEM_ACCESSTOKEN="$SYSTEM_ACCESSTOKEN" \ + SYSTEM_OIDCREQUESTURI="$SYSTEM_OIDCREQUESTURI" \ + SERVICE_CONNECTION_ID="$SERVICE_CONNECTION_ID" \ + node -e ' + const fs = require("node:fs"); + const e = process.env; + fs.writeFileSync(e.MATERIAL_FIFO, JSON.stringify({ + initialIdToken: e.INITIAL_ID_TOKEN, + systemAccessToken: e.SYSTEM_ACCESSTOKEN, + oidcRequestUri: e.SYSTEM_OIDCREQUESTURI, + serviceConnectionId: e.SERVICE_CONNECTION_ID, + tokenPath: process.argv[1] + "/token.d/token", + readyPath: process.argv[1] + "/ready.json", + statusPath: process.argv[1] + "/status.json" + })); + ' "$ROOT" + rm -f "$FIFO" + + for _i in $(seq 1 30); do + if [ -s "$ROOT/token.d/token" ] \ + && jq -e '.state == "ready"' "$ROOT/ready.json" >/dev/null 2>&1; then + break + fi + if ! kill -0 "$REFRESH_PID" 2>/dev/null; then + cat "$LOG" >&2 + exit 1 + fi + sleep 1 + done + test -s "$ROOT/token.d/token" + + INITIAL_HASH=$(printf '%s' "$idToken" | sha256sum | cut -d' ' -f1) + INITIAL_EXP=$(INITIAL_ID_TOKEN="$idToken" node -e ' + const p = process.env.INITIAL_ID_TOKEN.split(".")[1]; + const claims = JSON.parse(Buffer.from(p, "base64url")); + if (!Number.isSafeInteger(claims.exp)) process.exit(1); + process.stdout.write(String(claims.exp)); + ') + NOW=$(date +%s) + WAIT_SECONDS=$((INITIAL_EXP - NOW + 5)) + if [ "$WAIT_SECONDS" -lt 1 ] || [ "$WAIT_SECONDS" -gt 900 ]; then + echo "Unexpected initial assertion lifetime: ${WAIT_SECONDS}s" >&2 + exit 1 + fi + echo "Initial assertion expires at $(date -u -d "@$INITIAL_EXP" --iso-8601=seconds)" + sleep "$WAIT_SECONDS" + + REFRESHED=$(cat "$ROOT/token.d/token") + REFRESHED_HASH=$(printf '%s' "$REFRESHED" | sha256sum | cut -d' ' -f1) + REFRESHED_EXP=$(REFRESHED="$REFRESHED" node -e ' + const p = process.env.REFRESHED.split(".")[1]; + const claims = JSON.parse(Buffer.from(p, "base64url")); + if (!Number.isSafeInteger(claims.exp)) process.exit(1); + process.stdout.write(String(claims.exp)); + ') + if [ "$REFRESHED_HASH" = "$INITIAL_HASH" ] || [ "$REFRESHED_EXP" -le "$INITIAL_EXP" ]; then + echo "Assertion was not refreshed before the initial expiry" >&2 + cat "$LOG" >&2 + exit 1 + fi + echo "Refreshed assertion expires at $(date -u -d "@$REFRESHED_EXP" --iso-8601=seconds)" + + REFRESHED_ASSERTION="$REFRESHED" \ + CLIENT_ID="$servicePrincipalId" \ + TENANT_ID="$tenantId" \ + node --input-type=module -e ' + const form = new URLSearchParams({ + client_id: process.env.CLIENT_ID, + client_assertion: process.env.REFRESHED_ASSERTION, + client_assertion_type: + "urn:ietf:params:oauth:client-assertion-type:jwt-bearer", + grant_type: "client_credentials", + scope: "https://management.azure.com/.default" + }); + const response = await fetch( + `https://login.microsoftonline.com/${encodeURIComponent(process.env.TENANT_ID)}/oauth2/v2.0/token`, + { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: form + } + ); + const body = await response.json(); + if (!response.ok || typeof body.access_token !== "string" || body.access_token === "") { + throw new Error(`Entra exchange failed with HTTP ${response.status}`); + } + ' + echo "Refreshed assertion successfully exchanged for an Azure access token" + env: + SYSTEM_ACCESSTOKEN: $(System.AccessToken) diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index 991f20c4c..1f5433b24 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -2395,6 +2395,87 @@ fn test_mcpg_config_container_based_mcp() { let _ = fs::remove_dir_all(&temp_dir); } +#[test] +fn test_mcpg_container_azure_auth_emits_refresher_and_rotating_token_mount() { + let temp_dir = std::env::temp_dir().join(format!( + "agentic-pipeline-mcpg-azure-auth-{}", + std::process::id() + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + + let input = "---\nname: \"Azure Auth MCP Test\"\ndescription: \"Tests renewable Azure workload identity\"\nmcp-servers:\n kusto:\n container: \"node:22-slim\"\n azure-auth:\n service-connection: \"my-arm-sc\"\n mount-path: \"/var/run/custom-azure\"\n---\n\n## Test\n"; + let input_path = temp_dir.join("azure-auth-mcp.md"); + let output_path = temp_dir.join("azure-auth-mcp.yml"); + fs::write(&input_path, input).unwrap(); + + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + input_path.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + output.status.success(), + "Compiler should succeed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let compiled = fs::read_to_string(&output_path).unwrap(); + assert!(compiled.contains("task: AzureCLI@3")); + assert!(compiled.contains("connectionType: azureRM")); + assert!(compiled.contains("azureSubscription: my-arm-sc")); + assert!(compiled.contains("azure-wif-refresh.js")); + assert!(compiled.contains("\"AZURE_FEDERATED_TOKEN_FILE\": \"/var/run/custom-azure/token\"")); + assert!(compiled.contains("$(Agent.TempDirectory)/ado-aw-azure-auth/")); + assert!(compiled.contains("/token.d:/var/run/custom-azure:ro")); + assert!(compiled.contains("MCPG_REQUIRED_ENV_NAMES")); + assert!(compiled.contains("Stop Azure auth refresher (kusto)")); + let pipeline: serde_yaml::Value = serde_yaml::from_str(&compiled).unwrap(); + let jobs = pipeline["jobs"].as_sequence().unwrap(); + let refresh = jobs + .iter() + .flat_map(|job| job["steps"].as_sequence().unwrap()) + .find(|step| step["displayName"].as_str() == Some("Start Azure auth refresher (kusto)")) + .unwrap(); + let refresh_script = refresh["inputs"]["inlineScript"].as_str().unwrap(); + let identity_keys: Vec<_> = refresh_script + .lines() + .filter_map(|line| { + line.strip_prefix("CLIENT_VARIABLE='") + .or_else(|| line.strip_prefix("TENANT_VARIABLE='")) + .and_then(|value| value.strip_suffix('\'')) + }) + .collect(); + assert_eq!(identity_keys.len(), 2); + for name in ["Agent", "Detection"] { + let job = jobs.iter().find(|job| job["job"].as_str() == Some(name)).unwrap(); + let run = job["steps"] + .as_sequence() + .unwrap() + .iter() + .filter_map(|step| step["bash"].as_str()) + .find(|script| script.contains("AWF_ARGS+=(--skip-pull --env-all)")) + .unwrap(); + for key in &identity_keys { + assert!( + run.contains(&format!("--exclude-env {key}")), + "{name} must exclude internal identity {key}" + ); + } + } + assert!( + !compiled.contains("initialIdToken: \""), + "generated YAML must not contain a federated assertion" + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + /// Test that HTTP-based MCPs generate correct MCPG config JSON structure #[test] fn test_mcpg_config_http_based_mcp() { @@ -6372,12 +6453,11 @@ safe-outputs: Replace the managed issue comment. "#, ); + assert!(compiled.contains("--actor-output-var 'ADO_AW_SAFE_OUTPUTS_GITHUB_APP_ACTOR_LOGIN'")); assert!( - compiled.contains("--actor-output-var 'ADO_AW_SAFE_OUTPUTS_GITHUB_APP_ACTOR_LOGIN'") + compiled + .contains("ADO_AW_GITHUB_ACTOR_LOGIN: $(ADO_AW_SAFE_OUTPUTS_GITHUB_APP_ACTOR_LOGIN)") ); - assert!(compiled.contains( - "ADO_AW_GITHUB_ACTOR_LOGIN: $(ADO_AW_SAFE_OUTPUTS_GITHUB_APP_ACTOR_LOGIN)" - )); } /// The example file in `examples/dogfood-failure-reporter.md` must compile