githubnext / githubnext/ado-aw

Reject unwireable cross-job provider-env macros on engine.env (follow-up to #1372)

Open
#1,375 1 comment 0 reactions 0 assignees View on GitHub
agentic-workflows enhancement rust
Dominant language
Rust
Stars
23
Forks
8
Avg merge
4d 9h
Merged PRs (30d)
22

Description

Follow-up to #1372.

#1372 is fixed by adding the typed `engine.provider` (BYOK) block with compiler-owned in-job Azure CLI token minting: a same-job secret `AW_PROVIDER_BEARER_TOKEN` is wired into `COPILOT_PROVIDER_API_KEY` (the credential env var the AWF api-proxy sidecar actually reads and forwards as `Authorization: Bearer `), so a Foundry token resolves at runtime. That change is additive: the raw `engine.env COPILOT_PROVIDER_*` passthrough (including the broken cross-job `$(Setup.FOUNDRY_TOKEN)` macro) still compiles.

This issue tracks two deferred hardenings of the **legacy raw `engine.env` provider path** (both out of scope for the additive #1372 fix, both low urgency since `engine.provider` is now the sanctioned surface).

## 1. Reject the unwireable cross-job `$(.)` macro

Reject the cross-job output-macro form on raw `engine.env` provider keys at compile time, with an actionable error pointing users to `engine.provider` (or a same-job / pipeline / variable-group `$(VAR)` secret).

Scope:
- In `validate_engine_env_entry` (`src/engine.rs`), for provider-expr keys, classify the macro: allow single-identifier `$(VAR)` (resolves same-job/pipeline), reject dotted cross-job `$(a.b)` (never resolves in a step `env:` block -> empty -> managed-identity 403).
- Keep `${{ }}` / `$[...]` / `##vso[` rejections unchanged.
- Tests + docs note.

Explicitly NOT a codemod: rewriting the old `$(Setup.*)` + `setup:` mint pattern into `engine.provider` would require parsing an arbitrary inline `az` bash script (fragile, cannot produce a complete block); there is no live in-repo usage; and the pattern is already runtime-broken. The compile-time error is the migration nudge.

## 2. Drop `COPILOT_PROVIDER_BEARER_TOKEN` from the raw-env allowlist

The AWF api-proxy sidecar (which ado-aw always enables for BYOK) has **no `COPILOT_PROVIDER_BEARER_TOKEN` concept** — its BYOK credential is read exclusively from `COPILOT_PROVIDER_API_KEY` (verified against AWF v0.27.9 `containers/api-proxy/providers/copilot.js` + `provider-env-constants.js`, whose `COPILOT_ENV` lists only `GITHUB_TOKEN`, `PROVIDER_API_KEY`, `PROVIDER_TYPE`, `PROVIDER_BASE_URL`, `API_TARGET`, `API_BASE_PATH`).

Yet `COPILOT_PROVIDER_BEARER_TOKEN` is still listed in `COPILOT_PROVIDER_EXPR_ENV_KEYS` and `COPILOT_BYOM_CREDENTIAL_ENV_KEYS` (`src/engine.rs`) — carried over from #1264. A user who sets `COPILOT_PROVIDER_BEARER_TOKEN` directly in `engine.env` would have it `--exclude-env`'d from the container (so no leak) but **never consumed by the sidecar** -> silently unauthenticated (same 403 class). The `engine.provider.token` path was corrected to plumb `COPILOT_PROVIDER_API_KEY` instead; the raw-env allowlist should follow.

Scope:
- Remove `COPILOT_PROVIDER_BEARER_TOKEN` from `COPILOT_PROVIDER_EXPR_ENV_KEYS` and `COPILOT_BYOM_CREDENTIAL_ENV_KEYS` (`src/engine.rs`), so the raw-env path only advertises the keys the sidecar actually reads.
- If retained for any non-sidecar/direct-CLI edge case, at minimum document that it is inert through the api-proxy and emit a warning.
- Update `docs/engine.md` + `site/.../engine.mdx` (the `engine.env` exception note currently lists `COPILOT_PROVIDER_BEARER_TOKEN`) and tests.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/engine.rs at validate_engine_env_entry and the COPILOT_PROVIDER_EXPR_ENV_KEYS/COPILOT_BYOM_CREDENTIAL_ENV_KEYS definitions; inspect the existing tests covering engine.env provider expressions. Update docs/engine.md and the site engine page, run the relevant engine tests, and confirm dotted cross-job macros fail with guidance while valid $(VAR) remains accepted and the bearer-token key is no longer advertised.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, documentation, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.