feat(compile)!: credential-isolated package feed access via permissions.packages

[jamesadevine]

Summary

Stacked on #2310. Review only the top commit. This PR will be retargeted
to main once #2310 merges.

Agents can now restore packages from internal Azure Artifacts feeds with npm,
pip/uv, dotnet, and cargo without ever holding a feed credential.
The trusted ado-proxy container, already used for permissions.read, gains
a second package family. It holds the feed credential and attaches it only
to GET/HEAD requests for the feeds, views, and protocols the workflow
grants. This is the follow-up to #2310, which removed the
*Authenticate tasks because they leaked the token into AWF. It addresses the
internal-feed half of #1823 and #253.

permissions:
  packages:
    service-connection: artifacts-reader   # optional; omit to use the build identity
    identity-role: reader
    feeds:
      - name: internal
        project: Engineering
        feed: internal-packages
        view: Release
        protocols: [npm, pypi, nuget]
runtimes:
  node:
    feed: internal
    public-registry: block

How it works

• Credential custody.
  • Build identity: $(System.AccessToken) is mapped as a secret only into
    the proxy start step.
  • WIF: AzureCLI@3 mints SC_PACKAGES_TOKEN, which is secret and in
    --exclude-env.
  • Either way, the token is streamed to the proxy over stdin before AWF
    starts. It is never placed in the Agent or Detection environment, argv,
    workspace, or runner /tmp.
• Separate slots.
  • The material document is now v2, with token (REST) and package_token
    (packages).
  • The REST bearer never reaches pkgs.dev.azure.com, and the package
    credential never reaches dev.azure.com.
  • Either family is inert when its permission is absent.
• Policy. Requests must use GET/HEAD, a catalogued protocol route
  (npm/registry/, pypi/simple|download/, nuget/v3/, cargo/), and a
  granted org/project/feed/view tuple. Feed GUID aliases are resolved on the
  host before AWF starts.
• Preflight. ado-proxy.js resolve-feeds also probes access. A failure
  stops the job with the feed, identity, and role to grant.
• Redirects. The proxy never follows redirects. It relays Location only
  for *.vsblob.visualstudio.com and *.blob.core.windows.net, and the
  client fetches the signed blob URL through Squid with no credential.
• Ingress.
  • Generated wrappers (npm, npx, pip, pip3, uv, dotnet, cargo)
    set HTTPS_PROXY and CA trust for that process only. CA trust is never
    installed container-wide.
  • Canonical pkgs.dev.azure.com URLs, including ones in repository config,
    work unchanged.
  • Bypassing a wrapper (for example python -m pip) fails closed with no
    credential.
• Source selection.
  • runtimes.<x>.feed sets PIP_INDEX_URL/UV_DEFAULT_INDEX,
    NPM_CONFIG_REGISTRY, or an ensure-nuget.config step, all
    credential-free.
  • public-registry: block removes that ecosystem's public registry hosts
    from the AWF allowlist.
• Upstream ingestion.
  • A Collaborator-role read can save an upstream package into the feed.
  • upstream: deny (the default) therefore requires either a view or
    identity-role: reader, otherwise compilation fails.
  • upstream: allow is an explicit opt-in.
• Audit. Decision records gain optional family and protocol fields;
  the log stays v1-compatible. The changes add rollups and an
  upstream-unauthorized finding.

Breaking change and codemod

An Azure Artifacts runtimes.<x>.feed-url is replaced by
permissions.packages plus runtimes.<x>.feed. Codemod 0009 migrates it
automatically:

• shared feeds are merged and handles deduplicated;
• @view is kept;
• upstream: allow is added when no view is present, preserving the build
  identity's previous behavior.

Non-Artifacts feed-url values are unchanged.

Deviations from the plan (for reviewers)

Topic	Decision
Legacy {org}.pkgs.visualstudio.com	Not intercepted. Requests fail closed with no credential, and the codemod rewrites these URLs to canonical ones.
Cross-org feeds with the build identity	Fail at runtime preflight instead of compile time, because the org is often unknown at compile time.
WIF mint	Not deduplicated with permissions.read.
Python/Node config: and checked-in config linting	Follow-ups.
Cargo	Proxy and wrapper only. There is no Cargo runtime, so the registry comes from repository .cargo/config.toml (#1823).
Package-feed smoke	Authored (tests/smoke/package-feeds.md) and compile-tested, but not registered in cases.json. It needs a provisioned feed. The runbook and exact entry are in tests/smoke/REGISTERED.md.

Needs live validation before relying on it

• Upstream auth scheme: npm/Cargo Bearer, PyPI/NuGet Basic
  (ado-aw:<token>). This is unverified for Entra tokens over Basic.
• Blob redirect hosts.
• uv/dotnet honoring SSL_CERT_FILE on the hosted image.

The package-feeds smoke covers the first two once the feed exists.

Docs:

• new docs/package-feeds.md;
• updated ado-proxy-design.md, runtimes.md, network.md, codemods.md,
  imports.md, front-matter.md, safe-output-permissions.md, AGENTS.md,
  and README.

Test plan

All run locally on Windows with shellcheck available:

• cargo build: passed
• cargo test --no-fail-fast: passed (3743 passed, 0 failed, 2 ignored)
• cargo clippy --all-targets: passed (no warnings)
• ENFORCE_BASH_LINT=1 cargo test --bin ado-aw compile::shell: passed (59)
• ENFORCE_BASH_LINT=1 cargo test --test bash_lint_tests: passed (2)
• cd scripts/ado-script && npm run codegen: passed; no drift beyond the
  committed catalog v2 artifacts
• npm test: passed (97 files, 1362 tests, including new package,
  resolve-feeds, and proxy e2e cases for each protocol, denial, view, slot
  isolation, and canary absence)
• npm run typecheck: passed
• npm run build: passed
• npx vitest run src/compiler-smoke-e2e: passed (286)
• npm run build:compiler-smoke-e2e: passed
• git diff --check: flags only README table rows that keep the file's
  existing CRLF endings

Not run: a live Azure DevOps pipeline against a real feed. That requires
provisioning; see tests/smoke/REGISTERED.md.

[jamesadevine]

/review

[azure-pipelines]

Azure Pipelines:
2 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

✅ TypeScript Code Quality Reviewer completed the TypeScript code quality review.

🟦 TypeScript code quality review by TypeScript Code Quality Reviewer

[github-actions]

Security review: no regressions found

This diff adds the package-feed credential-isolation proxy (permissions.packages). I reviewed the full diff against the threat model (token custody, injection into generated YAML/shell, path traversal, redirect SSRF, network allowlisting, safe-output integrity) and found no weakened security posture.

Notable positives that satisfied my checks rather than raised concerns:

• Credential isolation: the REST bearer and the new Azure Artifacts package credential are kept in separate material-document fields (token / package_token), only ever read from env/stdin into the trusted ado-proxy process, never written to disk, argv, or the Agent/Detection environment (ca.ts, agentic_pipeline.rs).
• URL-shape authorization (package.ts) rejects path traversal, encoded/double-encoded separators, backslashes, control characters, and mismatched scope/view/protocol grants, each pinned by an explicit deny-reason test — including a dedicated test asserting denial details never echo request content.
• Redirect handling (authorizePackageRedirect) never follows a redirect while holding the credential; it only relays to an allowlisted, suffix-anchored blob-storage host (.blob.core.windows.net, .vsblob.visualstudio.com) or a same-host route that is itself re-authorized, with explicit tests against look-alike hosts, userinfo, non-default ports, and non-https schemes.
• Fail-closed wiring: isInterceptedHost extends the HTTPS-only/port-443-only enforcement to package hosts, so a package host cannot be reached in cleartext or on a non-standard port to bypass policy; credentialProblem refuses to start if a configured family lacks its credential or a package host lacks its TLS leaf.
• New feed/view/project identifiers are validated newtypes (secure.rs::AdoFeedName) rather than raw strings, consistent with the repo convention, so they cannot carry path/shell metacharacters into generated policy or the agent-facing prompt text.
• Header allow-lists (headers.ts) explicitly exclude authorization, x-nuget-apikey, npm-otp, and other credential-bearing headers from both directions.

No merge-blocking findings. Nothing in pr-review-comments.json to avoid duplicating (list was empty).

🔒 Security review by PR Security Reviewer · auto · 89.1 AIC · ⌖ 2.64 AIC · ⊞ 11.4K
_{Comment /review to run again}

[github-actions]

Exceptionally thorough test suite for a security-sensitive feature (credential isolation, URL-shape authorization, redirect validation, header sanitization) — the adversarial-encoding and canary-leak tests in package.test.ts and proxy.e2e.test.ts are a model for this kind of work.

Two gaps worth a look, both advisory:

1. validate_feed_exclusivity (new in src/runtimes/mod.rs, wired into all three runtime extensions) has no direct unit test for its own error paths — only the pre-migration codemod shape is tested, not the compiler-level rejection of runtimes.<x>.feed combined with feed-url/config.
2. The "cuts off a streamed body" e2e test swallows the promise's outcome with .catch(() => undefined) and only asserts when it happens to resolve, with a bound loose enough to pass even if truncation silently regressed.

Nothing here blocks merge; the rest of the diff's test coverage (codemod round-tripping, policy cross-validation, per-protocol route tables, header allow/deny lists, audit-log scrubbing) is comprehensive and behavior-focused rather than implementation-mirroring.

🧪 Test quality analysis by Test Quality Sentinel · auto · 191 AIC · ⌖ 1.77 AIC · ⊞ 11K
_{Comment /review to run again}

[github-actions]

Rust Code Quality Review 🦀

Verdict: no merge-blocking issues found. This is a large (26-file) but carefully engineered addition of permissions.packages credential-isolated Azure Artifacts access. cargo check passes cleanly on the branch.

💡 Review notes

• Error handling: consistent anyhow::bail!/.context() usage; no unwrap()/expect() outside #[cfg(test)] modules.
• Correctness: percent-decoding in the codemod URL parser is bounds-checked (get(..) returns Option, not panicking slice indexing); feed-handle collision resolution (upsert_grant) is deterministic; Cargo's /Cargo/index/ URL casing is intentionally matched case-insensitively by the TS-side matchProtocol, not a bug.
• Determinism: protocol lists are sorted/deduped before serialization (policy.rs), and audit stats use BTreeMap — consistent with the project's "generated YAML must be deterministic" rule.
• Security-adjacent properties (credential custody, fail-closed wrapper routing) read as sound from the Rust side; the TS enforcement (package.ts, resolve-feeds.ts) is out of this reviewer's remit and is covered by the TypeScript and compiler-contract reviewers.
• Test coverage is thorough: codemod migration/idempotency, cross-field validation (upstream: deny + Collaborator role rejection), runtime feed cross-referencing, and full pipeline-wiring assertions for both build-identity and WIF credential modes.

Process note: the rust-critic background sub-agent failed to start (400 model "gpt-5.4-mini" is not accessible), an infrastructure issue unrelated to this diff. This review reflects only my own direct pass over the Rust-only diff slice.

🦀 Rust code quality review by Rust Code Quality Reviewer · auto · 190.7 AIC · ⌖ 1.94 AIC · ⊞ 11.3K
_{Comment /review to run again}

[github-actions]

TypeScript Code Quality Review — scripts/ado-script/src/ado-proxy/

Note: the ts-critic sub-agent failed to start (model-access error: 400 model "gpt-5.4-mini" is not accessible via the /chat/completions endpoint), so this review reflects only my own direct pass, not a cross-check.

Scope reviewed

All new/changed TypeScript under scripts/ado-script/src/ado-proxy/ for the package-feed proxy feature: package.ts/.test.ts, config.ts/.test.ts, headers.ts/.test.ts, server.ts (handlePackage, openUpstream, streamBounded), resolve-feeds.ts/.test.ts, ca.ts, catalog.ts, index.ts.

Findings

No merge-blocking defects found.

• Async/error handling: openUpstream/handlePackage/streamBounded correctly attach error handlers on every socket/stream before awaiting, back-pressure is honoured in streamBounded, and the single shared openUpstream helper avoids duplicated upstream-dialing logic between the REST and package paths. The finally { secured?.destroy() } cleanup is present on both handlers.
• Credential isolation: token/packageTokens are now optional and read via readCredential(), each family's upstream only ever receives its own credential, and ca.ts's credentialProblem() fails closed at startup if a configured family lacks its credential or TLS leaf. Good design.
• Type safety: the few as Segment/as string casts in package.ts::matchFeedRoute are all preceded by explicit length/undefined guards, so they're safe despite being non-null-assertion-adjacent. The caught as Error pattern in resolve-feeds.ts is pre-existing repo-wide idiom, not newly introduced risk.
• Secret handling: resolve-feeds.ts redacts the token from every log line via redact() before escapeLoggingCommand(), and ca.ts fails closed on an empty-but-present credential field. I did spot what looked like a bare Authorization header / bearer-string leak in package.ts::packageAuthorizationHeader and resolve-feeds.ts's get() headers when viewing the files through tooling, but confirmed against git show HEAD:... that this is a sandbox/tool redaction artifact (the real source reads `Bearer ${token}`), not an actual code defect — flagging here only for transparency since it looked alarming at first glance.
• Tests: every new branch (redirect allow/deny, 401/203 handling, response-too-large before and during streaming, packages-only policy denying REST, missing-credential paths, cleartext/CONNECT refusal for package hosts) has a matching proxy.e2e.test.ts / resolve-feeds.test.ts / package.test.ts case.

No inline comments posted — nothing met the bar for an actionable, line-specific defect.

🟦 TypeScript code quality review by TypeScript Code Quality Reviewer · auto · 172.1 AIC · ⌖ 6.61 AIC · ⊞ 11.3K
_{Comment /review to run again}

[github-actions]

Prompt evaluation

Note

This is an advisory static review. Only Prompt Contracts is merge-blocking.

Only prompts/create-ado-agentic-workflow.md changed, so only the create
suite was selected (update and debug prompts are untouched and not
evaluated). The diff is a single line: a new reference link to
docs/package-feeds.md appended to the References section. No task-module
instructions (gather-inputs, front-matter rules, body sections, validation
checklist, done criteria) changed at all — base and candidate are byte-identical
except for that one added URL.

Prompt	Cases	Improved	Unchanged	Regressed	Inconclusive
create	3	0	3	0	0

Potential regressions

None. The added reference does not alter any instruction exercised by the three
create cases, and none of them involve package-feed/runtime-feed scenarios.

Pre-existing observation (not introduced by this PR, unaffected by the diff):
the References list in both base and candidate omits docs/schedule-syntax.md,
even though the task module instructs building fuzzy schedule triggers (e.g.
create-scheduled-workitem-report's "daily around 09:00"). This is unchanged
by the current PR and not a regression, but worth a future follow-up.

Per-case scores

Case	Criterion	Base	Candidate	Result
create-minimal-manual	task_completion	2	2	unchanged
create-minimal-manual	grounding	2	2	unchanged
create-minimal-manual	safety_and_consent	2	2	unchanged
create-minimal-manual	clarity_and_done_criteria	2	2	unchanged
create-minimal-manual	create_workflow_coherence	2	2	unchanged
create-minimal-manual	create_trigger_scope	2	2	unchanged
create-minimal-manual	create_tools_outputs_permissions	2	2	unchanged
create-minimal-manual	create_no_action	2	2	unchanged
create-needs-clarification	task_completion	2	2	unchanged
create-needs-clarification	grounding	2	2	unchanged
create-needs-clarification	safety_and_consent	2	2	unchanged
create-needs-clarification	clarity_and_done_criteria	2	2	unchanged
create-needs-clarification	create_workflow_coherence	2	2	unchanged
create-needs-clarification	create_trigger_scope	2	2	unchanged
create-needs-clarification	create_tools_outputs_permissions	2	2	unchanged
create-needs-clarification	create_no_action	2	2	unchanged
create-scheduled-workitem-report	task_completion	2	2	unchanged
create-scheduled-workitem-report	grounding	2	2	unchanged
create-scheduled-workitem-report	safety_and_consent	2	2	unchanged
create-scheduled-workitem-report	clarity_and_done_criteria	2	2	unchanged
create-scheduled-workitem-report	create_workflow_coherence	2	2	unchanged
create-scheduled-workitem-report	create_trigger_scope	1	1	unchanged
create-scheduled-workitem-report	create_tools_outputs_permissions	2	2	unchanged
create-scheduled-workitem-report	create_no_action	2	2	unchanged

Evidence notes:

• create-minimal-manual: prompt's "Do not perform external side effects unless the user explicitly asks" and optional-field omission guidance (Omit fields that equal defaults) correctly steer toward a manual, no-safe-output workflow; unaffected by the diff.
• create-needs-clarification: "If interactive, ask only missing essentials first" and the done-criteria clause "concise clarification is returned instead of an invented workflow" are unchanged text driving the clarification outcome.
• create-scheduled-workitem-report: "Ensure 'No Action' explicitly maps to noop when applicable" and "Keep MCP and safe-output allow-lists narrow" directly support the required noop-on-no-match and narrow comment-on-work-item config; create_trigger_scope scored 1 in both versions because neither References list links schedule-syntax guidance for "daily around 09:00" phrasing — a pre-existing gap, not a regression from this change.

Generated by Prompt Evaluator for #2315 · auto · 40.1 AIC · ⌖ 2.34 AIC · ⊞ 9.8K · ◷