feat: Download releases from GitHub.

[jamesadevine]

• also align ado-aw release model

[github-actions]

🔍 Rust PR Review

Summary: Looks good overall — clean migration from internal ADO artifacts to GitHub Releases. One security concern worth addressing, one testing gap.

Findings

🔒 Security Concerns

• templates/base.yml:72, base.yml:344 — sha256sum -c checksums.txt --ignore-missing silently succeeds if the downloaded binary's filename doesn't appear in checksums.txt at all. The --ignore-missing flag suppresses errors for files listed in the checksum file but not present locally — but it provides no protection if the binary is simply absent from checksums.txt entirely (e.g. a naming mismatch between awf-linux-x64 and whatever the release actually names the entry). The old per-binary .sha256 approach was more explicit. A hardening option: pipe the output through a grep to confirm at least one line was verified:

  sha256sum -c checksums.txt --ignore-missing | grep -q ": OK"

  Or capture the exit behavior with --strict. In practice this is likely fine if the releases use consistent naming, but it's worth a comment in the script acknowledging the assumption.

⚠️ Suggestions

• tests/compiler_tests.rs:163 — The assertion template_content.contains("{{ firewall_version }}") validates the raw template, not compiled output. There's no test confirming the marker gets replaced in the final YAML (i.e., that AWF_VERSION actually appears in compiled output and no {{ firewall_version }} placeholder leaks through). Consider adding a compiled-output fixture test alongside the existing template structure checks.

• src/compile/onees.rs:120 — AWF_VERSION is imported and used in the replacement table for the 1ES compiler, but 1es-base.yml doesn't appear to have a {{ firewall_version }} marker (1ES handles network isolation via OneBranch, not AWF). This is a no-op replacement — harmless, but potentially confusing. Worth either adding a comment or guarding the replacement.

✅ What Looks Good

• Replacing the hardcoded ADO pipeline ID (2450, runVersion: latestFromBranch) with a versioned AWF_VERSION constant is a significant improvement — previously every deployment was pinned to whatever happened to be the latest build on ms/main, which is non-reproducible and opaque to external consumers.
• AWF_VERSION constant is well-documented with a link to the releases page and a clear update instruction.
• The download + verify + rename + prependpath sequence follows the same pattern already established for ado-aw, keeping the templates consistent.
• Release workflow correctly switches from per-binary .sha256 to checksums.txt, matching what the templates expect.

Generated by Rust PR Reviewer for issue #17 · ◷

[github-actions]

🔍 Rust PR Review

Summary: Looks good — clean migration from internal ADO artifact to public GitHub Releases for AWF. One subtle security observation worth considering.

Findings

⚠️ Suggestions

• templates/base.yml:188, templates/1es-base.yml (multiple) — The pattern sha256sum -c checksums.txt --ignore-missing | grep -q ": OK" is correct for ADO pipelines (which run bash with -eo pipefail by default), but the --ignore-missing flag introduces a subtle assumption: it passes silently if no files from checksums.txt are present locally. The grep -q ": OK" guards against this case — if no files were verified, sha256sum outputs nothing and grep exits 1, failing the step. This is correct for the current use case, but the logic is non-obvious. A comment explaining the intent would help future maintainers:

  # --ignore-missing allows multi-arch checksums.txt; grep ensures at least one file was verified OK
  sha256sum -c checksums.txt --ignore-missing | grep -q ": OK"

• templates/base.yml:192 — ./awf --version || echo "AWF binary ready" unconditionally suppresses AWF launch failures. Since the checksum was already verified above, a failing --version only indicates the binary doesn't support that flag — the || fallback is intentional. However, if someone adds --ignore-missing to the checksum step without the grep guard in the future, a corrupted binary would silently pass through. The current combination is safe; the concern is maintainability. Already noted above via comment.

• src/compile/onees.rs:120 — The comment # No-op for 1ES (template doesn't use AWF), but included for forward-compatibility is clear and correct. Confirmed {{ firewall_version }} is absent from templates/1es-base.yml, so the replacement is truly a no-op with no risk of stale markers. Good defensive practice.

✅ What Looks Good

• AWF_VERSION as a typed constant in common.rs (with a clear update comment and link to releases) is the right pattern — same as compiler_version from CARGO_PKG_VERSION.
• The test_compiled_output_no_unreplaced_markers loop (lines 352–358) provides a strong regression guard: any future marker added to the template but not wired up in Rust will be caught automatically.
• Replacing DownloadPipelineArtifact@2 (which required authenticated access to an internal 4x4 project) with a public GitHub Releases download is the right move for an open-source compiler — pipelines using this template no longer need access to internal infrastructure to download AWF.
• The checksums.txt naming convention (shared file across architectures) aligns with standard multi-arch release tooling and is consistent across both the ado-aw and AWF download steps.

Generated by Rust PR Reviewer for issue #17 · ◷