feat(compile): add internal supply-chain feed/registry mirror

[jamesadevine]

Summary

Adds an optional supply-chain: front-matter section that lets a compiled agentic pipeline pull its artifacts from an internal Azure DevOps Artifacts feed and/or internal container registry instead of GitHub Releases / GHCR — for supply-chain-hardened environments where the build agent pool cannot reach GitHub/GHCR. When the section is omitted, the generated pipeline is byte-for-byte unchanged.

The four rerouted fetch points:

#	Artifact	Default source	Internal source
1	ado-aw compiler	github.com/githubnext/ado-aw releases	feed (NuGet)
2	AWF firewall binary	github.com/github/gh-aw-firewall releases	feed (NuGet)
3	ado-script.zip bundle	github.com/githubnext/ado-aw releases	feed (NuGet)
4	AWF + MCPG images	ghcr.io/github/...	registry (ACR)

Configuration

supply-chain:
  feed:                          # mirrors binaries #1–#3
    name: my-project/my-feed     # scalar `feed: my-feed` shorthand also works
    service-connection: feed-conn  # optional; same-org feeds use $(System.AccessToken)
  registry:                      # mirrors images #4
    name: myacr.azurecr.io/oss-mirror  # registry host OR base path
    service-connection: acr-conn   # required when registry is set
  service-connection: shared-conn  # optional shared fallback for both targets

Design highlights

• Binary mirror: NuGetAuthenticate@1 + DownloadPackage@1 + unzip/relocate, preserving the existing sha256sum -c checksums.txt verification. Same-org feeds authenticate via $(System.AccessToken) (no service connection needed).
• Image mirror: az acr login (AzureCLI@2) + registry rewrite in both the pre-pull step and the MCPG docker run, via a shared image_ref helper so the two call sites can't drift. registry.name accepts a full registry host or base path (e.g. myacr.azurecr.io/oss-mirror, contoso.azurecr.io/team/oss/mirror) — since teams generally cannot publish under GHCR's github/... namespace, the original prefix is not preserved; only the artifact name (squid, agent, gh-aw-mcpg) is placed directly under the configured base path at the same tag. az acr login derives the ACR name from the host portion. The local :latest aliases are still tagged to the GHCR names AWF resolves with --skip-pull.
• Asymmetric auth: the feed defaults to $(System.AccessToken); the registry requires a service connection (ACR has no $(System.AccessToken) path) — compilation fails otherwise. feed/registry each accept their own service-connection, with a top-level fallback, so the two targets can use different connections.
• Typed/validated: new FeedRef, RegistryRef, and ServiceConnection newtypes (src/secure.rs / src/validate.rs); the RegistryRef validator allows multi-segment base paths while rejecting .., //, leading/trailing /, ports, and shell metacharacters. SupplyChainConfig uses deny_unknown_fields and validated newtypes per the repo's parse-don't-validate convention.
• Universal: works across all four targets (standalone / 1es / job / stage) since they share build_pipeline_context.

Finding (documented, no code change)

The AWF firewall wraps only the copilot agent command (awf … -- '<engine_run>'); the mirror fetches run as ordinary ADO steps outside the sandbox. They therefore need no --allow-domains allowlist entry, and stripping github.com would be wrong. Documented in AGENTS.md (Security Considerations → Network Isolation) and docs/supply-chain.md.

Test plan

• cargo build ✓
• cargo clippy --all-targets --all-features — clean ✓
• cargo test — 2043 unit tests + all integration suites, 0 failures ✓
  • 5 integration tests in tests/compiler_tests.rs (full reroute, absent-section default, feed-only/SAT, registry-without-connection error, top-level fallback); the full-reroute test uses a registry base path (myacr.azurecr.io/oss-mirror) and guards that the GHCR github/... prefix is not carried under it
  • new unit tests for SupplyChainConfig deserialization/resolution/validation (including registry base-path acceptance) and the FeedRef/RegistryRef/ServiceConnection validators
• cargo test --test bash_lint_tests (shellcheck) ✓ — tests/fixtures/supply-chain-agent.md exercises the new bash bodies
• Docs site npm run build ✓ — all internal links valid; new reference/supply-chain page added to the sidebar

Docs

• New docs/supply-chain.md and site/src/content/docs/reference/supply-chain.mdx (including the registry base-path contract)
• Updated docs/front-matter.md grammar and the AGENTS.md doc index

[github-actions]

🔍 Rust PR Review

Summary: Good design and solid validation, but one likely runtime bug in the AWF image tagging strategy that the compile-time tests won't catch.

---

Findings

🐛 Bugs / Logic Issues

• src/compile/agentic_pipeline.rs — prepull_images_step creates wrong local :latest tags for AWF

  When an internal registry is configured, prepull_images_step builds squid_latest and agent_latest using image_ref(..., registry_host), so the local aliases become myacr.azurecr.io/github/gh-aw-firewall/squid:latest etc. But run_agent_step invokes AWF with --skip-pull and no explicit image-name flags — AWF therefore uses its built-in defaults, which are the ghcr.io/github/gh-aw-firewall/{squid,agent}:latest names. Those names are not present in the local Docker cache when the internal registry path is taken, so AWF would fail to start the firewall containers at runtime.

  The compile-time integration test (test_supply_chain_full_reroutes_all_artifacts) passes because it only checks the generated YAML; it cannot observe what Docker finds at runtime.

  Suggested fix — additionally tag the pulled images under the GHCR names that AWF expects, regardless of pull source:

  # After pulling from internal registry and creating the internal :latest alias,
  # also create the GHCR-named local aliases that AWF uses with --skip-pull:
  docker tag myacr.azurecr.io/github/gh-aw-firewall/squid:<ver> ghcr.io/github/gh-aw-firewall/squid:latest
  docker tag myacr.azurecr.io/github/gh-aw-firewall/agent:<ver>  ghcr.io/github/gh-aw-firewall/agent:latest

  Alternatively, consider adding --awf-squid-image/--awf-agent-image flags to the AWF invocation so the compiled YAML stays authoritative about which image names are used, making the choice visible and testable.

  If AWF actually reads the image names from an environment variable that's set elsewhere, please add a comment in run_agent_step explaining the mechanism — it's not obvious from the code.

🔒 Security Concerns

• src/compile/agentic_pipeline.rs:extract_package_payload_bash — unvalidated &str parameters interpolated into shell script

  All four parameters (staging, dest_dir, payload, tail) are directly interpolated into a format!() shell body with no validation. Today every call site passes hardcoded constants, so there's no immediate risk, but the function signature creates a footgun: a future caller that passes user-controlled data would silently introduce command injection. Consider making this a private const-parametrised helper or adding a // SAFETY: comment asserting the invariant that callers must satisfy.

⚠️ Suggestions

• src/compile/agentic_pipeline.rs / src/compile/extensions/ado_script.rs — NuGetAuthenticate@1 emitted 2–3 times per job

  When supply-chain.feed is set, the agent job emits a NuGetAuthenticate@1 step from each of download_compiler_step, download_awf_step, and install_and_download_steps_typed (ado-script extension). The task is idempotent so this won't break anything, but it creates redundant pipeline steps and noise in logs. Consider hoisting a single shared NuGetAuthenticate@1 step (keyed on the resolved connection) and skipping per-artifact auth steps.

✅ What Looks Good

• image_ref helper centralises registry rewriting: MCPG (start_mcpg_step) and the pre-pull script both call image_ref, so the two reference sites can't drift — exactly what the doc comment promises.
• FeedRef and HostName validated newtypes: values that flow into bash script bodies (feed name in DownloadPackage@1, --name arg to az acr login) are constrained by strict allowlists at deserialisation time; no injection path via user-controlled YAML.
• Cross-field validation for registry-without-connection: correctly placed in build_pipeline_context before job construction, fails fast with an actionable message.
• SanitizeConfigTrait impl is a correct no-op: the comment explaining why is exactly right — newtypes enforce constraints at parse time, nothing left to sanitise.
• Backward compatibility: absent supply-chain: leaves every existing pipeline byte-for-byte unchanged (confirmed by test_supply_chain_absent_uses_github_and_ghcr).
• deny_unknown_fields on SupplyChainConfig: good defensive serialisation practice; the unit test for it is a nice touch.

Generated by Rust PR Reviewer for issue #1080 · 320.8 AIC · ⌖ 12.9 AIC · ⊞ 35.6K · ◷

[jamesadevine]

Thanks for the review — all three points addressed in c33f671:

🐛 Bug — AWF :latest tags pointed at the internal registry. Fixed. prepull_images_step now always tags the :latest aliases to the GHCR names AWF resolves by default with --skip-pull (run_agent_step passes no image-name flags), tagged from the internally-pulled image:

docker pull myacr.azurecr.io/github/gh-aw-firewall/squid:0.27.3
docker tag  myacr.azurecr.io/github/gh-aw-firewall/squid:0.27.3 ghcr.io/github/gh-aw-firewall/squid:latest

So the firewall containers are present in the local Docker cache at runtime. Added regression assertions (test_supply_chain_full_reroutes_all_artifacts now checks the exact docker tag … ghcr.io/…:latest lines and that nothing is pulled from GHCR).

🔒 extract_package_payload_bash unvalidated &str params. Added a SAFETY: doc comment asserting the invariant — all callers must pass compile-time-constant, trusted strings only (today: hardcoded ADO macro paths + literal payload names); never user/front-matter-controlled data.

⚠️ Redundant NuGetAuthenticate@1. Hoisted a single per-job auth step (feed_auth_step) ahead of the binary downloads and dropped the per-artifact auth from download_compiler_step/download_awf_step. Auth steps per pipeline dropped from 6 → 4. Added an invariant assertion (auth_count < download_count). The one remaining ado-script auth lives in a separate extension whose Setup-job path has no sibling binary downloads, so it keeps its own auth.

Validation: cargo build, cargo clippy --all-targets --all-features (clean), full cargo test (2029 unit + all integration suites, 0 failures), and bash_lint_tests (shellcheck) all pass.

[github-actions]

🔍 Rust PR Review

Summary: Looks good overall — well-structured feature with solid security hygiene, but one logic inconsistency in the agent job's auth step count that contradicts the PR's own stated design.

---

Findings

🐛 Bugs / Logic Issues

src/compile/agentic_pipeline.rs:747 + src/compile/extensions/ado_script.rs:351 — Agent job gets two NuGetAuthenticate@1 steps in the common case

The PR description says "Hoist one NuGetAuthenticate@1 for the whole job," and the inline comment at line 1261 in agentic_pipeline.rs says "emit once per job." In practice, the agent job gets two:

1. feed_auth_step called at line 747 of build_agent_job (authenticates DownloadPackage for ado-aw + awf)
2. install_and_download_steps_typed in ado_script.rs line 351 also calls nuget_authenticate_step (authenticates DownloadPackage for ado-script)

runtime_imports_active() returns !inlined_imports, which is true for any pipeline that hasn't set inlined-imports: true. So for the overwhelming majority of pipelines, both paths trigger. I compiled the fixture to confirm — the agent job emits NuGetAuthenticate at lines 63 and 250 of the output.

This doesn't cause a runtime failure (NuGetAuthenticate@1 is idempotent), but it's a concrete gap between the design claim and the generated output. The test assertion auth_count < download_count (2 < 3) passes without enforcing the "one per job" invariant.

The fix: since feed_auth_step is hoisted at job level before the DownloadPackage steps it needs to authenticate, the second nuget_authenticate_step inside install_and_download_steps_typed is redundant for the agent/detection/safeoutputs jobs. The install_and_download_steps_typed path could either accept a feed_already_authenticated: bool flag and skip its own auth step, or the job-level feed_auth_step could be removed and each extension could be made self-contained (with the tradeoff that auth then appears after the ado-aw/awf downloads).

⚠️ Suggestions

src/compile/agentic_pipeline.rs:1186 — unquoted --name argument in bash inlineScript

.with_input("inlineScript", format!("az acr login --name {name}\n"))

name is derived from acr_registry_name(login_server) where login_server is HostName validated to [A-Za-z0-9.-], so it's safe in practice. But quoting defensively aligns with the rest of the generated bash in this file:

.with_input("inlineScript", format!("az acr login --name \"{name}\"\n"))

---

✅ What Looks Good

• FeedRef validator is properly allowlist-based ([A-Za-z0-9._/-]), rejects .., leading/trailing /, and more than one separator — shell metacharacters blocked before values ever reach generated bash or YAML
• image_ref helper centralises the GHCR→ACR rewrite so prepull_images_step and start_mcpg_step can't drift on the rewritten reference — the regression test at test_supply_chain_full_reroutes_all_artifacts explicitly guards this (d2 assertion)
• :latest re-tagging correctly preserves GHCR names for AWF's --skip-pull path — this is a subtle correctness requirement and the test guards it
• SupplyChainConfig::validate() is called early in build_pipeline_context before any codegen, so registry-without-connection is a clean compile-time error
• deny_unknown_fields on SupplyChainConfig, FeedConfig, and RegistryConfig
• SAFETY doc on extract_package_payload_bash is correct; shellcheck exercises the generated bodies via tests/bash_lint_tests.rs
• ServiceConnection deny-list rationale is sound — ADO connection display names legitimately contain spaces so a strict allowlist would reject valid inputs; the deny-list blocks the actual YAML-breaking characters (quotes, control chars)
• Zero-cost default: when supply-chain: is omitted the generated YAML is byte-for-byte unchanged (confirmed by test_supply_chain_absent_uses_github_and_ghcr)

Generated by Rust PR Reviewer for issue #1080 · 674.9 AIC · ⌖ 12.5 AIC · ⊞ 34K · ◷

[jamesadevine]

@copilot we need to consider that the configured internal feed may have a different "prefix" than GitHub. It is unlikely teams will be able to publish to the GitHub acr host. We should allow users to provide the full registry base path. Our contract is the artifact names must remain the same under that path.

[Copilot]

@copilot we need to consider that the configured internal feed may have a different "prefix" than GitHub. It is unlikely teams will be able to publish to the GitHub acr host. We should allow users to ...

Done in the latest commit. registry.name now accepts a full registry host or base path (e.g. myacr.azurecr.io/oss-mirror, contoso.azurecr.io/team/oss/mirror). The original GHCR github/... prefix is no longer carried over — only the artifact name (squid, agent, gh-aw-mcpg) is placed directly under the configured base path at the same tag, so the images resolve to e.g. myacr.azurecr.io/oss-mirror/squid:0.27.3.

Changes:

• New validated RegistryRef newtype (src/secure.rs) + is_valid_registry_ref (src/validate.rs) — allows multi-segment paths, rejects .., //, leading/trailing /, ports, and shell metacharacters.
• image_ref now appends only the final image-name segment under the base path; acr_registry_name derives the ACR name from the host portion of the base path.
• :latest aliases still tag to the GHCR names AWF expects.
• Updated tests (fixture uses myacr.azurecr.io/oss-mirror) and docs to reflect the contract: artifact names stay the same, the prefix is whatever you provide.

[jamesadevine]

/rust-review

[github-actions]

✅ Rust PR Reviewer completed successfully!

[github-actions]

🔍 Rust PR Review

Summary: Looks good overall — well-structured, secure design with typed newtypes and validated inputs. Two issues worth fixing before merge.

Findings

🐛 Bugs / Logic Issues

tests/compiler_tests.rs:6123,6129 — hardcoded AWF_VERSION will cause spurious test failures on every version bump

assert!(
    compiled.contains(
        "docker tag myacr.azurecr.io/oss-mirror/squid:0.27.3 ghcr.io/..."
    ),

This is a binary-only crate, so AWF_VERSION can't be imported in integration tests. When common.rs::AWF_VERSION is bumped the assertion fails even though the code is correct. The rest of the test already uses prefix-based assertions (e.g. contains("docker pull myacr.azurecr.io/oss-mirror/squid:")) — apply the same pattern here:

// Instead of an exact version match, check both halves independently:
assert!(
    compiled.contains("docker tag myacr.azurecr.io/oss-mirror/squid:")
        && compiled.contains(" ghcr.io/github/gh-aw-firewall/squid:latest"),
    "AWF squid image must be re-tagged to the GHCR :latest name AWF expects"
);

🔒 Security Concerns

src/validate.rs:131 — is_valid_service_connection does not block ADO macro expressions

The validator uses a deny-list (no quotes/control chars) but doesn't filter $(, ${{, or ##vso[. A user writing:

supply-chain:
  registry:
    name: myacr.azurecr.io
    service-connection: "$(my.shared.conn)"

produces azureSubscription: $(my.shared.conn) in the YAML, which ADO expands as a pipeline variable at queue time. The field is author-controlled compile-time config (not an agent-runtime injection vector), so this is low severity, but the codebase convention is reject_pipeline_injection everywhere. Calling validate::contains_ado_expression in the validator (or adding !s.contains("$(") && !s.contains("${{") to the deny-list) would make it consistent.

⚠️ Suggestions

src/compile/agentic_pipeline.rs:1178 — acr_registry_name fallback undocumented for Private Link

For ACR instances accessed over Azure Private Link with a custom hostname (e.g. myacr.internal.contoso.com), host.strip_suffix(".azurecr.io") fails and .split('.').next() returns myacr, which may not be the correct registry name for az acr login --name. Worth a doc note in docs/supply-chain.md (or a comment in the function) that registry.name must use the standard *.azurecr.io login-server hostname when Private Link is involved.

src/compile/extensions/ado_script.rs:324 — redundant NuGetAuthenticate@1 per agent/detection job

install_and_download_steps_typed(supply_chain) emits its own NuGetAuthenticate@1 inline, while the job builder in agentic_pipeline.rs also hoists one via feed_auth_step. Result: the agent and detection jobs each contain two auth steps and three download steps. NuGetAuthenticate@1 is idempotent so this is functionally fine, but the generated YAML is slightly noisier than described by the hoisting comment. No action required unless YAML compactness is a goal.

✅ What Looks Good

• deny_unknown_fields on SupplyChainConfig + typed FeedRef/RegistryRef/ServiceConnection newtypes follow the parse-don't-validate convention cleanly.
• image_ref centralisation prevents the pre-pull and docker run image references from drifting — exactly the right design.
• acr_login_step correctly derives the ACR registry name before the docker pull step so the daemon is pre-authenticated.
• validate() enforces the registry-requires-connection invariant at compile time with a clear error message.
• The :latest alias logic (comment + None for GHCR local tags) is well-reasoned and the regression test at lines 6123/6129 guards the right behaviour — just use a version-agnostic assertion form.
• extract_package_payload_bash SAFETY comment is accurate and the callers honour it today.

Generated by Rust PR Reviewer for issue #1080 · 984.5 AIC · ⌖ 12.8 AIC · ⊞ 35.4K · ◷
_{Comment /rust-review to run again}

[jamesadevine]

Thanks — addressed in ec36cf8:

🐛 Hardcoded AWF_VERSION in tests. Fixed. The :latest re-tag assertions now use version-agnostic split contains() checks ("docker tag …/squid:" + " ghcr.io/github/gh-aw-firewall/squid:latest"), matching the prefix-based style used elsewhere in the test, so an AWF_VERSION bump won't cause spurious failures.

🔒 is_valid_service_connection didn't block ADO expressions. Fixed. The validator now also rejects $(, ${{, $[, ##vso[, and ##[ via the existing contains_ado_expression / contains_pipeline_command detectors, so a connection value can't be expanded as a pipeline variable at queue time — consistent with the reject_pipeline_injection convention. Added unit-test cases.

⚠️ Private Link caveat for acr_registry_name. Documented. Added a note to the function comment and docs/supply-chain.md that registry.name should use the canonical *.azurecr.io login server when ACR is reached over Private Link with a custom domain.

⚠️ Redundant NuGetAuthenticate@1 in install_and_download_steps_typed. Left as-is per your "no action required" note. The ado-script extension also runs in the Setup job (synthetic-PR / filter gates), where there are no sibling binary downloads and thus no hoisted feed_auth_step, so it keeps its own idempotent auth; deduping it would require coupling across the extension boundary.

Validation: cargo build, cargo clippy --all-targets --all-features (clean), full cargo test (2031 unit + all integration suites, 0 failures).

[github-actions]

🔍 Rust PR Review

Summary: Looks good — well-structured feature with no blocking issues. A few minor concerns worth addressing.

Findings

⚠️ Suggestions

• src/validate.rs — RegistryRef accepts underscores in the host segment
  is_valid_registry_ref allows _ (e.g. my_acr.azurecr.io passes), but acr_registry_name strips the .azurecr.io suffix and passes my_acr to az acr login --name my_acr. ACR registry names are [a-z0-9-]{5,50} only — underscores are rejected by the service and the pipeline would fail at runtime with a cryptic ACR error rather than a clear compile-time message.
  Consider additionally validating the host segment before the first . with [A-Za-z0-9-]+ (matching az acr login's requirements), or at minimum add a note in the error message.
  (The existing test assert!(!is_valid_registry_ref("host:443")) demonstrates intent to catch runtime-invalid names at parse time, so this seems consistent with the PR's approach.)

• src/compile/agentic_pipeline.rs — Duplicate NuGetAuthenticate@1 in the agent job
  When both supply-chain.feed and the ado-script extension are active, the agent job ends up with two NuGetAuthenticate@1 steps: one hoisted by feed_auth_step at step 5 (for compiler + AWF downloads) and a second emitted by install_and_download_steps_typed inside ext_agent_prepare at step ~14 (for the ado-script download). Both resolve the same connection, so this is provably idempotent — but it shows up as redundant auth noise in pipeline logs. The ado_script extension already has a feed_auth_step-aware path for the setup job where no duplicate exists; the same dedup could be applied in install_and_download_steps_typed when the agent job is the caller.

• src/compile/agentic_pipeline.rs — No timeout on DownloadPackage@1 in download_compiler_step / download_awf_step
  The ado-script extension sets t.timeout = Some(Duration::from_secs(300)) on its DownloadPackage@1 tasks (see install_and_download_steps_typed). The download_package_step helper used in download_compiler_step and download_awf_step silently inherits the ADO task default (no explicit timeout). Minor inconsistency — a hung NuGet feed can stall the pipeline indefinitely on those steps.

✅ What Looks Good

• image_ref centralisation correctly prevents the pre-pull step and start_mcpg_step from drifting on the rewritten image reference — a real correctness risk that the PR explicitly calls out.
• validate()-early in build_pipeline_context properly short-circuits compilation when registry is set without a resolvable service connection, matching the documented "fail otherwise" guarantee.
• extract_package_payload_bash SAFETY contract is clearly documented: all callers pass compile-time-constant strings, preventing shell injection through the unescaped format!() interpolation.
• ##vso[task.complete result=Failed] in extract_package_payload_bash is always paired with exit 1, so the step actually fails rather than silently continuing.
• The :latest tag aliases in prepull_images_step correctly use the GHCR names (not the internal registry names) for the docker tag step, which is the right behaviour because AWF resolves its images by GHCR name via --skip-pull — easy regression to introduce, good that it's guarded by the test at lines 1599–1608.
• Test coverage is solid: full-reroute, absent-section, feed-only/$(System.AccessToken), registry-without-connection (negative test), and shared-fallback tests all present.

Generated by Rust PR Reviewer for issue #1080 · 323 AIC · ⌖ 12.9 AIC · ⊞ 35.6K · ◷