Skip to content

Scaffold hardened Axum application Docker recipes - #401

Open
ChristianPavilonis wants to merge 17 commits into
mainfrom
issue-398-docker-spec
Open

ChristianPavilonis wants to merge 17 commits into
mainfrom
issue-398-docker-spec

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Scaffold hardened Axum application Dockerfiles and .dockerignore files using the application's committed lockfile and declared Rust toolchain, a paired Rust builder version/digest, a digest-pinned Dockerfile frontend, BuildKit dependency caches, CA trust, numeric non-root identity and explicit container binding.
  • Include Axum: validate multi-platform containers and provide an application publishing workflow #399 preparation: native amd64/arm64 container CI, verified OCI artifact handoff, and an inactive, copyable application-owned GHCR publishing recipe that preserves tested image bytes.
  • Include Axum: document Docker, Kubernetes, and managed-cloud deployments #400 preparation: Docker/Compose guidance and packaging smoke, plus internal Kubernetes and AWS deployment designs. Runnable Kubernetes manifests and authorized cloud execution are not delivered here.
  • Keep framework packaging scans report-only and application-release verification strict. No public application image, external registry publication, production deployment, runtime API change or vulnerability waiver is included.

Scope and issue relationships

Part of #391. Related to #398, #399, #400 and #410, with provider implementation tracked separately in #414. All four issues remain open for their outstanding acceptance. This PR delivers packaging preparation and the related CI/documentation preparation; it does not redefine the issues' completion criteria.

The current combined PR scope and custom OCI verifier are deliberately retained after review. The publishing recipe also retains its combined publication/attestation job. Its whole-job OIDC signing-authority risk is explicitly accepted and documented; credentials and OIDC are not available to the native build jobs.

Issue Delivered preparation Acceptance still outstanding
#398 Hardened Docker scaffolding, portable-consumer instructions and constrained packaging checks Accepted-runtime required-config failure, durable restart/recovery, readiness and bounded SIGTERM/draining
#399 Native architecture CI, tested-artifact handoff/index checks and application-owned GHCR recipe Authorized protected GHCR publication/attestation verification and accepted-runtime final-image cases
#400 Docker/Compose guide and no-store smoke, internal Kubernetes design and selected ECS/Fargate target Production Docker/Compose cases, runnable kind/Envoy Gateway manifests and checks, and authorized ECS/Fargate deployment
#410 AWS reference design linking independent config/secrets/KV selection and acceptance requirements Provider integration in #414 and separately authorized real IAM/resource/version/DynamoDB acceptance

Applications own their release artifacts, registry approval, credentials, configuration, secrets, storage policy and deployment. Trusted Server's images and releases remain downstream. Resolved review threads do not establish production acceptance or authorize merging unfinished issue criteria.

Main changes

  • Conditional root container templates and generator regression tests. GNU binaries build on the pinned Bookworm Rust builder and run on the compatible shell-free gcr.io/distroless/cc-debian13:nonroot runtime. Runtime updates are pulled and tested; publication and rollback use the exact tested output digests.
  • Disposable container-probe preparation stages framework sources deliberately. It labels its derived fixture lock, records both seed and prepared lock hashes, rejects changed registry name/version/source tuples, and keeps the separate application-owned committed consumer-lock contract intact.
  • Native packaging checks cover context exclusions, ELF/native libraries, CA trust, UID/GID 10001, read-only root, dropped capabilities, mount configuration, published HTTP, controlled positive/negative TLS, and clean no-store Compose stop/start/teardown. STOPSIGNAL SIGINT uses the existing graceful path, not bounded SIGTERM acceptance.
  • OCI verification checks manifests, blobs, inputs, source/run/attempt identity and scans; preserves optional supplied platform variants; and records BuildKit-resolved image materials in retained provenance and release acceptance records.
  • The application-owned publishing recipe independently reconstructs committed inputs before registry login, selects exact same-run immutable artifacts, preserves child/index digests, and verifies expected attestation subjects and workflow identity. SCAN_EXCEPTIONS does not enable an automatic waiver path in this sample workflow.
  • cargo-audit 0.22.2 is installed from a checksum-pinned source archive with its packaged lockfile. Rust advisory collection and framework verification remain report-only. Strict application verification rejects Rust vulnerabilities and unsound warnings as well as High/Critical image findings; no automatic Rust advisory waiver path is provided.
  • Axum-only checks no longer trigger unrelated deployment smokes. Recipe actionlint, action-pin, Zizmor and ShellCheck coverage moves with the Axum workflow, the helper suite keeps one owner, and docs build in the existing docs-format job.
  • Internal designs are dated and excluded from public navigation: docs/superpowers/specs/2026-10-14-deploying-axum-kubernetes.md and docs/superpowers/plans/2026-10-14-aws-reference-deployment.md.

Review follow-through at 1a6ae861

Committed and pushed the approved remaining fixes in 1a6ae861. The resolution comment accounts for the latest summary-only points and approved deferrals.

  • Shared base extraction handles lowercase/indented FROM and platform flags without changing the runtime-tag exception.
  • Length-prefixed context hashing and extracted-context verification run before builder creation. Prepared receipts require regeneration for the new encoding.
  • Bash 3.2 cleanup preserves the original status for empty container lists. Compose retains log-only evidence under the uploaded failure directory, without rendered config or response bodies.
  • CRLF toolchain handling retains exact-pin checks; archive tests isolate path, type and link rejection; both guides explicitly describe ignored SIGTERM as PID 1.
  • Cumulative PAX resource limits, broader remote-source restrictions and all-materials/declarations matching remain deferred. Existing PR scope, verifier, OIDC trade-off, scan policy and outstanding production acceptance are unchanged.

Validation of the tested working changes committed here:

  • All 159 helper tests with freshly prepared context, zero skips; 87 focused Bash 3.2 checks on Linux.
  • Final CLI suite and serial workspace/all-target tests, with 1,554 passed and one ignored. An earlier parallel config-test failure remains unconfirmed after a clean-baseline full run and eight baseline config runs passed; details are in the resolution comment.
  • Rust formatting, strict workspace/all-target/all-feature Clippy, native feature compilation and Spin WASM compilation.
  • Docs format/lint/build, changed-script Bash syntax, ShellCheck, actionlint, action pins and Zizmor.
  • Local pre-commit native amd64 build, context/ELF/HTTP/TLS, Compose stop/start/teardown, OCI copy/pull identity and direct TERM versus configured SIGINT checks.
  • Final local Trivy scan/seal did not complete before the command timeout. The Rust inventory still contains one unsound warning, which strict verification blocks. No release approval or clean full-scan claim is made.
  • Hosted checks at this head are running as of this update. Local native arm64, native macOS, protected GHCR/OIDC and production/cloud acceptance were not exercised in this pass.

All 49 review threads have explanatory responses and are resolved. Re-review is requested from prk-Jr, aram356 and dhruv8sh. This does not dismiss their verdicts or close any issue acceptance.

Review follow-through at 6ef7f999

  • Fixed explicit smoke failures and config/teardown diagnostics on Bash 3.2; added source-backed assertion regressions and direct source-archive checksum comparison.
  • Moved Axum static validation into an independent job, with handoff depending on static validation and native builds. Removed cross-owner workflow assertions and unused deployment Node setup. Deployment workflow changes relative to the PR baseline are now only a reciprocal pin-maintenance comment.
  • Restored maintained blob/main guide links. Strict application verification now rejects Rust vulnerabilities and unsound warnings. Both policies require unfiltered warning-category selection. Framework collection/verification remains report-only; OS exceptions cannot waive Rust findings, and the Tokio update remains separate.

Validation of these changes:

  • All 130 helper tests with a freshly prepared context, zero skips.
  • Bash 3.2 focused shell/deployment/advisory checks, including security rejection, visible diagnostics and failed checksum handling.
  • Native amd64 generated-context/ELF/HTTP/TLS and Compose stop/start/teardown smokes under Bash 3.2, using the exact retained image from run 41481693509, not a rebuilt release candidate.
  • The retained arm64 bundle is rejected by strict verification on its unsound warning and accepted by report-only verification.
  • Scoped CLI tests and strict all-feature Clippy, Rust formatting, docs format/lint/build, Bash syntax, ShellCheck, actionlint, recipe action pins and Zizmor.
  • All 660 deployment contracts and existing Rust build-cache action contracts.
  • Hosted native amd64/arm64 and handoff checks subsequently passed at 6ef7f999, run 42162617461. This is historical evidence, not current-head verification.

Native macOS userland, protected GHCR/OIDC publication and cloud/runtime production acceptance were not exercised. Two read-only workers checked shell portability and CI/policy behavior. The policy review found an informational-warning filter gap; it was fixed and covered before committing. No repository protection settings, dependency upgrade or Rust waiver path was added.

Earlier validation

Local checks at implementation commit 827317aa6eb0797c62d8f1469d089b7a9df0b3c1, including unchanged relevant inputs tested immediately before committing:

  • Workspace/all-target tests and strict all-feature Clippy.
  • Rust formatting, native adapter feature compilation, Spin WASM compilation and locked app-demo tests.
  • All 114 helper tests passed with a freshly prepared context, with no skipped prepared-context checks.
  • Docs formatting, lint and build; Bash syntax, ShellCheck, actionlint, action-reference checks and Zizmor.
  • All 660 deploy-action contracts passed with fixture commit signing disabled and LC_ALL=C, matching the test's deterministic sorting contract.
  • Native amd64 final-image build, constrained HTTP/TLS execution, clean Compose stop/start/teardown, and exact OCI registry-copy/Docker-pull identity passed under SCAN_POLICY=strict.
  • That exact amd64 image reported zero High/Critical OS findings and zero cargo-audit vulnerabilities, with no exceptions or filtering. Its runtime child is sha256:cfc14c62b697225036d242674494d4b10e80c5a9525aa8d42c81489c1234aac5. This is point-in-time evidence, not a permanent vulnerability-free claim. The retained hosted inventories also include an unsound Tokio warning, distinct from the zero-vulnerability count. Current strict verification rejects that warning.
  • Hosted native amd64/arm64 and handoff subsequently passed at 827317aa, run 41481693509. These earlier results do not substitute for current-head verification above.

The local handoff used the pinned BuildKit image and local Buildx 0.38.1. Hosted workflows pin Buildx 0.37.0. Both must retain the observed resolved-material/config-digest evidence and exact tested child identities. No QEMU execution substitutes for native arm64 acceptance.

Remaining acceptance outside this preparation

  • Integrate accepted runtime/lifecycle/configuration contracts and exercise required-input failure before readiness, mounted-state persistence/recovery, competing-writer rejection, and bounded SIGTERM/draining through the actual candidate images on both native architectures.
  • Execute separately authorized protected, app-owned GHCR publication and attestation verification without rebuilding tested artifacts. No registry publication or release waiver has been authorized by this review cleanup.
  • Deliver and exercise plain Kubernetes manifests with kind and Envoy Gateway, including probes, storage, HTTPS/proxy trust, shutdown, upgrade and rollback. Local packaging does not qualify EKS or AWS services.
  • Integrate accepted AppConfig Agent, Secrets Manager and DynamoDB providers, then obtain separately authorized real resource/IAM/version/table evidence. Ordinary KV proof does not certify Trusted Server identity/CAS. Proposed ECR transfer remains unimplemented and untested.
  • Exercise stateless ECS/Fargate only with explicit account/resource/credential/budget/cleanup approval.

Packaging preparation is not production certification. No end-client-delivery guarantee, multi-writer embedded-store support, Kubernetes/EKS acceptance, Fargate acceptance, real AWS provider acceptance or identity/CAS certification is claimed.

Use a concrete final squash subject such as Scaffold and verify hardened Axum application containers; earlier implementation-progress commit subjects are not release-status evidence.

Retain full vulnerability evidence in framework packaging checks without treating the fixture as an approved release. Keep strict defaults and reject report-only GHCR publication. Update inherited Bookworm packages and cover the policy boundary with regression tests.
Reject empty and non-string CVE/package identities in both scan policies. Record the successful hosted native builds and artifact/index handoff.
Replace Python helpers and tests with dependency-free Node.js tooling, update workflows and documentation, and preserve OCI artifact verification contracts.

Accept valid repeated separators in image repository names and share Compose startup polling with regression coverage.
@aram356
aram356 requested a review from dhruv8sh October 26, 2026 15:51

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Please resolve the integration conflicts against current main and validate the resulting revision before merging.

The reviewed PR head is 493644242f5839afbe5d37a032e08c13a381dc04; current main was checked at bc4aeb850c74ba982bdb4ad45309520b2ebb07b5. The merge check identifies conflicts in crates/edgezero-cli/src/generator.rs, .github/workflows/deploy-action.yml, and the location of docs/specs/aws-reference-deployment.md.

When resolving the generator conflict, keep the generated Rust version and the Docker builder digest aligned with main's toolchain upgrade. This PR introduces GENERATED_RUST_VERSION = "1.95.0" and a pinned 1.95 builder, while current main uses 1.98.1. Update the toolchain declaration and builder digest together; changing the image tag alone leaves the old digest selected. Rerun the affected checks on the integrated revision, including container preparation and native image execution.

No additional confirmed code defects were found in this review pass across the 29 changed files and relevant consumers.

Validation at the reviewed PR head: workspace tests passed (1,424 passed, one ignored), formatting and strict all-feature Clippy passed, native feature compilation and Spin WASM compilation passed, container helper tests passed including the prepared-fixture checks, and docs lint/format/build passed. Workflow and shell static checks passed, with a different local actionlint version from CI. These results do not validate a resolved merge with current main. Real Docker execution could not be repeated because the local daemon was unavailable; no current-head hosted checks were listed.

😃 The publisher independently recomputes committed source identities before registry login, verifies both architecture bundles, and prevents report-only scans from authorizing GHCR publication.

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Adds a hardened Axum Dockerfile scaffold, native amd64/arm64 container CI with an OCI artifact handoff, an app-owned GHCR publishing recipe, and Docker/Kubernetes/AWS deployment docs. The verification tooling is careful, but the generated image ignores SIGTERM, its runtime base means the shipped strict release gate rejects every build, and several integration hazards with current main merge without a textual conflict. This review adds to the existing conflict request rather than repeating it.

I built the fixture image from this head locally (native arm64) and measured its stop behavior and scan results; the numbers are in the inline comments.

Findings

Blocking

  • 🔧 The container ignores SIGTERM, so every stop ends in SIGKILL (crates/edgezero-cli/src/templates/root/Dockerfile.hbs:64, inline)
  • 🔧 The runtime base makes the shipped strict release gate unpassable (crates/edgezero-cli/src/templates/root/Dockerfile.hbs:52, inline)
  • 🔧 New stackpop links after the repository transfer; the generated README's docs link returns 404 (crates/edgezero-cli/src/templates/root/README.md.hbs:57, inline)
  • 🔧 The toolchain test breaks on the required Rust 1.98.1 bump (crates/edgezero-cli/src/generator.rs:1141, inline)
  • 🔧 Merging would close #398, #399 and #400: GitHub lists all three as closing references for this PR, but the description says #399 and #400 must stay open and that #398's production acceptance is still outstanding. The description also calls the PR a draft, but it is not marked as one. Remove the #399/#400 links from the Development sidebar, decide explicitly whether this merge should close #398, and convert the PR to draft if it is not ready to merge.
  • 🔧 The spec relocation breaks guide links (in addition to the existing conflict request): on main, docs/specs moved under docs/superpowers/, which docs/.vitepress/config.mts excludes with srcExclude: ['superpowers/**']. Once aws-reference-deployment.md moves there, the relative links at docs/guide/deploying-axum-containers.md:150 and docs/guide/deploying-axum-kubernetes.md:160,240 point at pages the site no longer builds, and VitePress fails the build on dead links by default. Link to the file on GitHub instead, or drop those links. Also note that no hosted CI has run on 49364424: GitHub does not run pull_request workflows while a PR has merge conflicts, so the integrated revision will be the first to exercise the new workflows on this head.

Non-blocking

  • ♻️ Split this PR: 29 files and +4,325 lines combine about 340 lines of scaffold change (#398), about 3,000 lines of CI, verification and publishing tooling with tests (#399), and about 1,000 lines of deployment and AWS design docs (#400/#410). Each part has different reviewers, risk and acceptance criteria. Suggested split: (1) the Dockerfile, .dockerignore and README templates, generator tests and the Axum guide section; (2) native CI, the handoff helpers and the publishing recipe; (3) the design docs under docs/superpowers/specs/.
  • ♻️ Keep point-in-time status out of committed docs: several pages record review-time state that will be wrong soon after merge. Examples: dated scan counts and a specific CI run link (docs/guide/publishing-axum-containers.md:208-226); this PR's own heads and CI status (docs/specs/aws-reference-deployment.md:24-36); a developer's local, uncommitted worktree (issue-410-native-store-providers, docs/specs/aws-reference-deployment.md:32 and :215); and issue-ownership notes in the Axum guide's "Packaging verification and limits" section. That belongs in the PR description and the issues. The docs should state the contract and its current limits.
  • ♻️ Keep the Kubernetes draft out of the published navigation (docs/.vitepress/config.mts:61, inline)
  • ♻️ Replace the hand-written TOML parser (scripts/axum_container_toml.mjs:3, inline)
  • ♻️ One implementation of the context identity, not three (examples/container-publishing/release.yml:46, inline)
  • ♻️ Every source edit reinstalls cmake and rebuilds all dependencies (crates/edgezero-cli/src/templates/root/Dockerfile.hbs:40, inline)
  • 🤔 The vulnerability gate can't see the Rust dependencies (crates/edgezero-cli/src/templates/root/Dockerfile.hbs:46, inline)
  • 🤔 The container CI resolves a fresh dependency graph on every run (scripts/prepare_generated_axum_container.sh:22, inline)
  • 🤔 Downstream-specific build steps in the framework guide (docs/guide/adapters/axum.md:305, inline)
  • ⛏ Non-hermetic unit test fails on hosts without readelf (scripts/tests/test_axum_container_artifacts.test.mjs:325, inline)
  • ⛏ Stale Python references after the Node migration (scripts/axum_container_helpers.mjs:87, inline)
  • ⛏ Prerequisite lists disagree (scripts/test_generated_axum_container.sh:7, inline)
  • ⛏ Absolute symlink targets never resolve (scripts/axum_container_helpers.mjs:171, inline)
  • ⛏ Duplicate BuildKit pin for an unused builder (.github/workflows/axum-container.yml:104, inline)
  • ⛏ Docs-only edits run both native container builds (.github/workflows/axum-container.yml:18, inline)
  • ⛏ Drop bzip2/xz decompression of artifact input (examples/container-publishing/verify_artifacts.mjs:189, inline)

CI Status

Local runs at 49364424:

  • fmt: PASS
  • clippy (--workspace --all-targets --all-features -- -D warnings): PASS
  • tests (cargo test --workspace --all-targets): PASS
  • Node helper tests: 50 pass, 1 fail on macOS (non-hermetic; see inline), 3 skipped by default. The 3 prepared-fixture tests pass with AXUM_PREPARED_CONTEXT set.
  • shellcheck, actionlint and zizmor on the new workflows and scripts: clean
  • Fixture docker build (native arm64): PASS
  • Hosted CI on this head: none has run (blocked by merge conflicts)

Comment thread crates/edgezero-cli/src/generator.rs Resolved
Comment thread docs/.vitepress/config.mts Resolved
Comment thread scripts/axum_container_helpers.mjs Resolved
Comment thread .github/workflows/axum-container.yml Resolved
Comment thread .github/workflows/axum-container.yml Resolved

@dhruv8sh dhruv8sh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Solid, security-conscious Axum container packaging, but the branch is stale against main (missing #366 and #419), conflicts, and reintroduces stackpop references. Rebase and the items below are needed before merge.

😃 Praise

  • Strong hardening: non-root UID 10001, --read-only, cap-drop=ALL, no-new-privileges; digest-pinned base images and actions with persist-credentials: false; toolchain verified before rustup; strict scan policy and post-push digest verification for GHCR; Dockerfile/.dockerignore generated only for the Axum adapter, with good unit tests (mock-rustup toolchain check, digit-leading app names).

Findings

Blocking

  • 🔧 Stale base / merge conflict: branch lacks #366 (Rust 1.98.1, Node 24.20.0) and #419 (IABTechLab transfer); git merge-tree reports a conflict. GENERATED_RUST_VERSION = "1.95.0" in generator.rs while main emits rust 1.98.1. Rebase, bump to 1.98.1, re-resolve the rust:1.98.1-bookworm@sha256:… digest in Dockerfile.hbs, and update the hard-coded "1.95.0" in the new tests.
  • 🔧 stackpop references reintroduced (inline on the README template). Also in docs/guide/publishing-axum-containers.md (git dependency examples), issue/PR links in the Kubernetes guide and docs/specs/aws-reference-deployment.md, and the org.opencontainers.image.source fallback stackpop/edgezero in examples/container-publishing/release.yml. Switch to IABTechLab/edgezero / iabtechlab.github.io/edgezero.
  • 🔧 Hard-coded Node 24.12.0 in workflows (inline).
  • ❓ Draft docs in main nav (inline).

Non-blocking

  • 🤔 Bespoke tooling scope: scripts/axum_container_toml.mjs is a 182-line hand-rolled TOML parser and examples/container-publishing/verify_artifacts.mjs is 524 lines of OCI/tar verification; with release.yml and publish.sh that is a lot of security-sensitive code for the framework to own as an "example". Consider a small TOML dep (e.g. smol-toml) for tests, or splitting the publishing recipe into its own PR.
  • 🤔 Missing # syntax= directive for heredoc RUN (inline).
  • ⛏ No dependency layer caching in the Dockerfile (inline).
  • ⛏ rmSync without force (inline).
  • 📝 Gate toolchain: clippy on 1.98.1 fails in untouched edgezero-macros (inline_trait_bounds) because the branch still pins 1.95.0; resolved by the rebase.

CI Status

  • fmt: PASS
  • clippy: PASS on branch pin 1.95.0; FAIL on 1.98.1 (pre-existing lint in untouched crate, fixed by rebase)
  • tests: PASS
  • Docker/OCI workflows and Node helper tests: not run locally (require Docker/buildx)

Comment thread .github/workflows/axum-container.yml Resolved
Comment thread .github/workflows/deploy-action.yml Resolved
Comment thread docs/.vitepress/config.mts Resolved
Comment thread scripts/axum_container_helpers.mjs Resolved
Resolve merge conflicts, preserve combined CI coverage, align container scaffolding with Rust 1.98.1, and repair relocated documentation links.
…ps. Changes remain uncommitted; generated checkout artifacts were removed. Validation remains incomplete for native arm64 and hosted release flows.
ChristianPavilonis and others added 2 commits October 30, 2026 10:43
…, paginated artifact selection, registry guards, simplified Node setup, focused tests and documentation cleanup. No new dependencies.
@ChristianPavilonis

Copy link
Copy Markdown
Collaborator Author

Review reconciliation at d7c4ee9

I checked all 39 review threads and all five nonempty submitted review summaries against the current branch. No pending review was exposed by the API. This pass changes GitHub discussion state only, not source code or release policy.

Replied and resolved

  • Toolchain test fixture: 66c73e9 makes the test use its synthetic Rust pin rather than inheriting the production 1.98.1 pin. The focused regression passed.
  • Duplicate BuildKit pin: 4ea53d6 removes the unused image setting from both workflows. Only the handoff helper creates the pinned builder.
  • Compose teardown: 4ea53d6 checks teardown before PASS, preserves failures and diagnostics, and adds failure-path regressions.

There are now 29 resolved threads and 10 open threads. Previously resolved threads already had explanatory replies, so I did not duplicate them or dismiss any reviewer's change request.

The review-summary integration request is also addressed. Main was merged in 66c73e9, production Rust is 1.98.1, Node is aligned to 24.20.0, and GitHub reports MERGEABLE. The cited point-in-time documentation status was removed, internal drafts were taken out of published navigation, relocated documentation links were repaired, and the Kubernetes draft now uses GitHub-compatible warning syntax. These do not clear the separate future builder-digest maintenance or merge-acceptance requests below.

Remaining inline requests

Request Status and proposed follow-through
Runtime base / strict release gate Partial. 1f244a7 switched to distroless, removed runtime apt tooling, moved TLS diagnostics to a separate image, and adapted the smokes. Current native scans still report one fixable High OS finding; the Rust inventory reports five advisories. Keep this open rather than equating report-only packaging success with release approval. No waiver or weaker gate is assumed.
Source-edit rebuilds and dependency caching Two related threads. cmake installation already precedes source COPY, but Cargo cache mounts are still absent. Batch the remaining build-cache improvement.
Dockerfile frontend directive Still absent. Add the reviewed frontend pin and appropriate regression.
Rust version / builder digest drift Still open. Pair the maintained version/digest and make the upgrade instructions explicitly require updating the digest. The current integration fix alone does not prevent future drift.
Derived fixture lock Partial. Hosted prepared-context tests already reject registry versions outside the seed lock. The preparation script itself still reconciles before its locked check. Make the derived-lock identity and what that check proves explicit, preserving the separate committed consumer-lock contract.
Whole-job OIDC authority Requires an explicit security choice. Separate copying from signing authority, or retain the combined job and explicitly accept that any compromised step in it can request OIDC. Merely delaying registry login does not isolate OIDC.
Over-broad deploy workflow triggers Still open. Move or gate Axum-only checks without losing existing actionlint, pin and Zizmor coverage for the inactive release recipe.
arm64 variant Still open. A focused Node reproduction confirms that description and index construction discard an explicitly supplied v8 variant. Preserve optional supplied metadata without inventing a variant for images that omit it.
SCAN_EXCEPTIONS plumbing Partial. The sample performs strict verification and record construction before publication, so the alleged publish-then-record failure is not its current execution order. The standalone record command supports --exceptions. Clarify that exporting SCAN_EXCEPTIONS does not enable exceptions in the sample's direct verifier calls, preserving its documented no-automatic-waiver policy.

Remaining summary-only requests

Evidence and limits

  • Local helper suite: 88 passed, zero failed, three prepared-context checks skipped.
  • Focused Rust toolchain regression: one passed.
  • Current-head hosted container run: all 91 helper checks passed, including prepared context; native amd64/arm64 HTTP/TLS/Compose checks and handoff passed.
  • The hosted checkout was merge 316a4e2. Its Git tree exactly equals d7c4ee9, so that execution evidence applies to the current source tree. All 33 reported PR checks are successful.
  • Protected GHCR publication/attestation and production Kubernetes/AWS/runtime acceptance were not newly exercised. Packaging success does not clear their outstanding acceptance requirements.

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Follow-up review at d7c4ee9e4a9db1d547272b3f13a6177f10098dba (base 87456c708ffe9edf86b3a0f8b06dfd15d60b07b5), covering the changes since my previous review at 1f244a7c. 4ea53d66 and d7c4ee9e do what they set out to do, and I found no new code defects in this delta. One earlier blocker is still open: it concerns PR metadata, not code.

Findings

Blocking

  • 🔧 Merging would still close unfinished issues (existing finding, still unresolved): GitHub currently lists #398, #399 and #400 as closing references. The PR description says #399 and #400 must stay open and that #398 still has production acceptance outstanding (required-input startup failure, durable restart, readiness and bounded SIGTERM). Once the code threads are settled, merging would close all three automatically. Fix: remove #399 and #400 from the Development sidebar. Either change the #398 link to a non-closing reference or have an owner explicitly approve closing it on the packaging slice. The description also calls the PR a draft, but it is still marked ready for review. This is a metadata change only.

😃 Praise

  • The Compose smoke test now checks compose down before reporting PASS. It keeps the primary failure status if cleanup also fails and preserves diagnostics. I made the success-path teardown ignore failures, and both new teardown tests failed, so the regression is covered.
  • Artifact selection now reads every page of results, and a duplicate accepted artifact that only appears on a later page is rejected rather than silently selected.

Checked in this delta, no findings

  • node-version-file: .tool-versions with an exact # v6.5.0 pin comment. The standalone recipe's 24.20.0 pin is now kept in sync with .tool-versions by a test.
  • gh api --paginate --slurp | jq: the pinned publisher gh (2.102.0) supports --slurp, and the default Actions bash shell uses pipefail, so an API failure fails the step.
  • Registry readiness now fails the job if the registry never starts. Buildx setup uses driver: docker, and only the handoff helper creates the pinned BuildKit builder.
  • The runtime-tar check now rejects .git at the root and in nested paths while still allowing .gitignore/.gitkeep. The GHCR preflight fails with a clear message, before any network call, when inline credentials are missing.

The other reviewers' open threads are not repeated here.

CI Status

  • Hosted CI at d7c4ee9e: all 33 checks passed, including native amd64/arm64 container execution and the handoff (run 41463920215, Node 24.20.0).
  • Local helper suite: 88 passed, 0 failed, 3 prepared-context tests skipped. Local Node was 24.12.0, not the declared 24.20.0.
  • Not rerun locally: Rust fmt/clippy/tests (no Rust changes since my last review; hosted CI covers them) and Docker execution. GHCR publication/attestation, Kubernetes and AWS acceptance remain unexercised, as the PR states.

@ChristianPavilonis

Copy link
Copy Markdown
Collaborator Author

Review follow-through completed

Pushed 91cb3e6 and 827317a. Every one of the 39 review threads is now resolved, with an explanatory reply. The 26 threads already resolved before this audit retained their existing fix explanations; this audit resolved three previously fixed threads, then replied to and resolved the ten remaining threads after implementing or recording the agreed response.

The owner's decisions also apply to the summary-only PR split/custom-verifier request and later scope request: retain this PR's current scope and verifier. The combined OIDC publisher is retained with its whole-job signing-authority risk explicitly accepted in the guide and thread reply. Review-summary text has no separate thread-resolution control; these decisions are recorded here and in the updated PR description.

The remaining fixes include:

  • Paired generator Rust/builder pins, explicit digest-upgrade instructions, the pinned Dockerfile frontend and Cargo cache mounts.
  • Standalone derived-lock checks and seed/prepared hash labeling, optional platform-variant preservation, BuildKit-resolved base provenance and checksum-pinned cargo-audit source installation.
  • Narrower CI ownership/triggers with retained static/security checks and docs-build coverage.
  • Dated internal design paths, pushed commit-permalink guide links, exception-policy clarification and current PR status/evidence.
  • A compatible Debian 13 distroless runtime and three targeted patch-level lockfile updates that clear the previously recorded image/advisory findings in the tested local candidate.

I removed the closing associations for #398, #399 and #400 and verified that all remain open. This preserves their outstanding acceptance criteria rather than closing unfinished work. The PR body now agrees with its ready-for-review metadata and explicitly distinguishes delivered preparation from production acceptance. Concrete new commit subjects and the proposed final squash subject replace progress prose as the description of this delivery; old tested revision hashes were preserved.

Local validation at 827317a passed:

  • All 114 helper tests with a fresh prepared context, with no skips.
  • Workspace/all-target tests, strict all-feature Clippy, formatting, native feature checks, Spin WASM compilation and locked app-demo tests.
  • Docs formatting/lint/build, Bash syntax, ShellCheck, actionlint, action pin checks, Zizmor and all 660 deploy-action contracts. The action contracts require C locale and fixture commit signing disabled in this local environment.
  • The full native amd64 final-image build, constrained HTTP/TLS and clean Compose checks, followed by separate strict import, preserved the exact runtime digest. Its strict scan had zero High/Critical OS findings and its lock-bound Rust inventory had zero advisories, without exceptions.
  • Actual retained BuildKit metadata includes the resolved runtime digest and config binding. Both pushed design permalinks were verified through the GitHub contents API.

Current-head hosted checks, including native arm64 and the two-platform handoff, are still running. These are not claimed as passed. Protected GHCR/OIDC publication and production Kubernetes/AWS/runtime acceptance were not exercised or waived. Resolving review discussions does not dismiss reviewer verdicts, grant release approval or complete those acceptance criteria.

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Re-review at 827317aa6eb0797c62d8f1469d089b7a9df0b3c1 (base 87456c708ffe9edf86b3a0f8b06dfd15d60b07b5), covering 91cb3e65 and 827317aa. My earlier blocker is resolved: GitHub no longer lists any closing issues, and the description now says #398, #399, #400 and #410 remain open. I found no new defects. The two inline comments are non-blocking. Approving, which replaces my previous change request.

😃 Praise

  • The remaining review threads were closed with real mechanisms, not just replies: the paired Rust version/builder digest constant, the derived-lock registry subset check, the platform-bound runtime material in the signed release record, and the checksum-verified cargo-audit source install. I disabled the subset check as a test, and both of its regression tests failed, so the check is covered.
  • The cleared scan comes from the Debian 13 runtime and the patch-level lockfile updates, not from a waiver. The hosted native logs at this head show 0 High and 0 Critical OS findings and 0 Rust advisories on both architectures.

Verified in this delta

  • The Dockerfile renders the builder digest from the same constant as RUST_VERSION. The frontend is pinned, and the cache mounts still copy the binary out within the same RUN. .dockerignore excludes **/target.
  • The cargo-audit 0.22.2 checksum matches the checksum crates.io publishes for that version.
  • The moved trigger paths cover everything removed from deploy-action.yml. The release recipe keeps actionlint, the pin gate, Zizmor and ShellCheck.
  • The derived-lock check applies only to the framework fixture. The app-owned recipe still uses prepare_inputs.mjs and its committed lock.
  • An optional image variant is carried from the config into the receipt and the index, with a check that they match.

Findings

Non-blocking

  • 🤔 Guide links are pinned to pre-merge commits (inline, docs/guide/adapters/axum.md:342; also docs/guide/deploying-axum-containers.md:132,150)
  • ⛏ Duplicated validation tool versions; the static checks gate the native builds (inline, .github/workflows/axum-container.yml:51)

CI Status

  • Hosted CI at 827317aa: all 33 checks passed, including native amd64/arm64 and the handoff (run 41481693509).
  • Local: fmt passed. cargo test -p edgezero-cli passed. Clippy (-p edgezero-cli --all-targets --all-features -D warnings) passed on the pinned Rust 1.98.1. The container helper suite had 111 passed, 0 failed and 3 prepared-context tests skipped, on local Node 24.12.0.
  • Not exercised: local Docker, GHCR/OIDC publication, and Kubernetes/AWS acceptance, which the PR explicitly leaves outstanding.

Comment thread docs/guide/adapters/axum.md Resolved
Comment thread .github/workflows/axum-container.yml Resolved

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Re-review at 827317aa. I checked all 22 findings from my previous review against this head: 21 are fixed in code, and the PR split was declined as an owner decision, which I'm not raising again. The runtime now passes the strict image gate and stops cleanly, and the integration problems with main are resolved. Two new blocking problems remain: the smoke scripts' security assertions can't fail under macOS's default bash, and two tests assert workflow files whose changes never trigger them. prk-Jr's two open threads from yesterday (commit permalinks, duplicated tool pins on the critical path) are not repeated here.

Previous findings, verified

  • Stop signal: on a native arm64 build of the fixture, docker stop now takes 1 s and exits 0 (previously 20 s, exit 137); the Compose example's stop takes 1 s and exits 0 (previously 31 s, exit 137).
  • Runtime base: the arm64 image is 64 MB (previously 177 MB) and has no shell. The hosted bundles from run 41481693509 report 0 High/Critical OS findings on both architectures.
  • Floating runtime provenance: both hosted bundles record the resolved cc-debian13:nonroot digest in their BuildKit materials, and the handoff verified them.
  • Confirmed in code: IABTechLab links, the toolchain test's synthetic pin, no closing-issue associations, internal designs out of the published nav, smol-toml instead of the hand-written parser, the shared context hash, cmake before COPY plus cache mounts, the seeded fixture lockfile, the generic asset-stage example, the stubbed readelf test, the Python references, absolute symlink resolution, the single BuildKit pin, docs paths out of the container triggers, and the removal of bzip2/xz.

Findings

Blocking

  • 🔧 On macOS's default bash, the smoke scripts' security checks can't fail (scripts/test_axum_container_deployment.sh:39-40, plus four lines in scripts/test_generated_axum_container.sh; inline)
  • 🔧 Tests assert workflow files whose changes never trigger them (scripts/tests/test_axum_container_publishing.test.mjs:252 and :232; inline)

Non-blocking

  • 🤔 Strict release policy doesn't cover the Rust advisory inventory (examples/container-publishing/verify_artifacts.mjs:307; inline)
  • ⛏ Leftover changes in deploy-action.yml (.github/workflows/deploy-action.yml:107; inline)
  • ⛏ GNU-only checksum check (scripts/test_axum_container_handoff.sh:103; inline)

📌 Out of Scope

  • Main's Cargo.lock also pins tokio 1.53.1, which RUSTSEC-2026-0386 (AsyncFd::new and related constructors violate I/O safety) marks unsound; it is patched in 1.53.3. The fixture lock inherits it from there. Bump it in a separate change.

CI Status

Local runs at 827317aa:

  • fmt: PASS
  • clippy (--workspace --all-targets --all-features -- -D warnings): PASS
  • tests (cargo test --workspace --all-targets): PASS (1,554 passed, 1 ignored)
  • feature check (--features "fastly cloudflare spin") and Spin wasm32-wasip2 check: PASS
  • Node helper tests on macOS (Node 24.20.0, /bin/bash 3.2): 103 pass, 8 fail (explained inline), 3 skipped. The 3 prepared-context tests pass with AXUM_PREPARED_CONTEXT set.
  • shellcheck, actionlint and zizmor on the new and changed workflows and scripts: clean
  • Native arm64 fixture build, HTTP, docker stop and Compose stop: PASS
  • Hosted CI at this head: all checks pass, including native amd64/arm64 and the handoff

Comment thread .github/workflows/deploy-action.yml Resolved
Comment thread scripts/test_axum_container_handoff.sh Resolved
@ChristianPavilonis

Copy link
Copy Markdown
Collaborator Author

Review follow-through at 6ef7f99

Implemented the approved responses in 6ef7f99, replied to all seven newly open threads, and resolved them. Earlier resolved threads and their explanations were preserved.

  • The Bash 3.2 security assertions and visible config/teardown diagnostics are fixed, with regressions and actual native amd64 HTTP/TLS/Compose execution under Bash 3.2 against the retained tested image.
  • Axum tests no longer enforce unrelated deployment/docs workflow text. Static recipe/security validation runs independently, and handoff needs both static checks and native builds. The deployment workflow differs from the PR baseline only by the pin-maintenance comment.
  • Guide links now follow main. Source archive verification compares the computed hash directly before extraction/install.
  • Strict application verification blocks Rust vulnerabilities and unsound warnings. Collection and framework verification remain report-only. Filtered warning-category settings are rejected under both policies. OS exceptions do not waive Rust findings; no Rust waiver mechanism or dependency upgrade was added.

The review summary's Tokio warning is real: the retained arm64 bundle now fails strict verification on that warning and still passes report-only. Zero vulnerabilities is not zero advisory warnings. The Tokio update remains separate, and the PR description now makes that distinction.

Local validation passed: all 130 helper tests with fresh prepared context and no skips; Bash 3.2 focused regressions; native amd64 HTTP/TLS/Compose; CLI tests and strict all-feature Clippy; Rust formatting; docs formatting/lint/build; Bash syntax, ShellCheck, actionlint, recipe action pins and Zizmor; all 660 deployment contracts and the existing Rust build-cache action contracts. Two read-only workers reviewed the changes; their one policy-filter finding was fixed and covered before committing.

Current-head hosted checks are running at https://github.com/IABTechLab/edgezero/actions/runs/42162617461. Preparation passed and both native jobs have started independently of static validation, confirming the intended scheduling change. No hosted result is claimed before completion.

This does not dismiss reviewer verdicts, close outstanding acceptance issues, approve registry publication, change repository protection, or establish native macOS/GHCR/OIDC/Kubernetes/AWS production acceptance.

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Full re-review at 89d50968f4c958b1229230e63f8856e90ec83aba (base cf2c57b1), covering all 37 changed files rather than only the delta since my approval at 827317aa. The Rust generator, the Dockerfile template, the workflows and the verifier's OCI/scan handling hold up. Two small defects contradict claims this PR makes, so I'm moving from approve to request changes. Both fixes are a line or two.

😃 Praise

  • The verifier is careful: digest regexes are anchored, tar members are checked against an allowlist of names, blob reads use O_NOFOLLOW, exceptions must match exactly, and strict mode can't be bypassed through Trivy severity casing or a filtered cargo-audit report.
  • Native build jobs get no credentials or OIDC, every action is pinned to a full SHA, and no ${{ }} expression is interpolated into a run: block. The builder digest and Rust version come from one paired constant.

Findings

Blocking

  • 🔧 Bash 3.2 cleanup aborts when the smoke fails before any container exists (scripts/test_generated_axum_container.sh:52; inline)
  • 🔧 Lowercase or indented FROM lines escape the pinned-base check (examples/container-publishing/prepare_inputs.mjs:42, verify_artifacts.mjs:450; inline)

Non-blocking

  • 🤔 Compose-check diagnostics are lost in CI (scripts/test_axum_container_deployment.sh:28; inline)
  • 📝 The docs could say SIGTERM is ignored outright. The app runs as PID 1 and installs only tokio::signal::ctrl_c() (dev_server.rs:314), so SIGTERM does nothing until the runtime's kill timeout. STOPSIGNAL SIGINT is right for Docker and Compose, but docker kill -s TERM or an orchestrator stop-signal override gets no reaction, then a SIGKILL. One sentence in axum.md and deploying-axum-containers.md would make that explicit.
  • 🌱 Hardening (low; producer-controlled or fails closed):
    • hashContext hashes name\0bytes\0 with no length prefix, so two different contexts can produce the same hash, and the native build never re-hashes the extracted context before docker buildx build. The separate git-archive hash still binds the source.
    • readTar accumulates PAX metadata without a total limit and re-spreads the object on every header, so time and memory grow without bound. extract/runtime-tar don't checksum first.
    • The derived-lock subset check (axum_container_helpers.mjs:104) compares only registry+ sources, so an added git+/sparse+ package passes.
    • Every "unsafe archive members" test case also sets link, so removing the traversal checks would still pass.
    • A CRLF .tool-versions fails the toolchain check with a misleading "exact X.Y.Z" message.

CI Status

  • Hosted at 89d50968: all 33 checks pass, including native amd64/arm64 and the handoff (run 42185282051).
  • Local on Rust 1.98.1: fmt passed, cargo test -p edgezero-cli passed (227), clippy (-p edgezero-cli --all-targets --all-features -D warnings) passed. Helper suite: 127 passed, 0 failed, 3 prepared-context tests skipped, on local Node 24.12.0. actionlint, ShellCheck and the action-pin check were clean, and the docs build succeeded.
  • Not exercised: local Docker build, GHCR/OIDC publication, Kubernetes/AWS.

@ChristianPavilonis

Copy link
Copy Markdown
Collaborator Author

Review follow-through at 1a6ae86

Pushed 1a6ae86. I reread all 49 threads, all nine nonempty review summaries and all conversation comments, with pagination checked. No pending review was exposed by the API; other reviewers' private drafts are not visible. The 46 previously resolved threads retain their existing explanations. The three newly open threads have individual replies for cleanup, base extraction and Compose CI diagnostics.

Remaining points from the latest review

  • SIGTERM wording, fixed. Both guides now state that the current entrypoint as container PID 1 ignores SIGTERM. A local native amd64 check confirmed it stayed running after TERM and exited 0 after Docker's configured SIGINT stop. This is not bounded draining or a runtime API change.
  • Context identity and pre-build binding, fixed. Shared hashing now length-prefixes UTF-8 names and payload bytes. The handoff rechecks the extracted context before creating the builder, excluding only the root input receipt. Tests distinguish the reproduced embedded-NUL/file-boundary collision, reject changed/added/removed/linked input, preserve nested receipt-named assets and check that a mismatched context never reaches the builder. Prepared receipts must be regenerated for the new encoding.
  • Unsafe-archive tests, fixed. Path rejection, unexpected types/names and link-target rejection now have separate fixtures and expected error assertions, so an unrelated link error cannot mask missing traversal validation.
  • CRLF toolchain files, fixed. The Dockerfile guards and handoff token parser normalize trailing CR while retaining exact-release, duplicate-entry, conflicting-toolchain and installed-compiler checks. Raw input bytes remain bound by their hashes.
  • Cumulative PAX metadata/resource limits, deferred. The cumulative allocation/re-spread concern remains. No resource-exhaustion failure was demonstrated in this pass, and arbitrary thresholds were not added to this producer-controlled path. Tar headers still have checksum validation. Receipt verification checks the archive hash before unpacking, but initial extract/runtime inspection does not acquire an independently authenticated whole-archive checksum by reading it first. These limitations are not claimed fixed.
  • Non-registry entries in the derived fixture lock, deferred. The documented check remains a registry name/version/source subset check. New git+/sparse+ tuples are outside that check; the current framework seed has only registry sources. A broader remote-source restriction would be a separate policy change. Application-owned committed-lock reconstruction is unchanged.
  • Optional all-materials/declarations matching, deferred. See the base-extraction reply. Existing runtime-material and config-digest binding remains; no broader provenance policy was introduced.

Earlier summary-only points remain handled by the previous reconciliation and 6ef7f999 follow-through. The owner retains this PR's scope, custom verifier and explicitly accepted whole-job OIDC authority. The Tokio upgrade remains separate, with strict Rust-advisory rejection and no automatic Rust waiver. I rechecked that #398, #399, #400 and #410 are open and that this PR has no closing-issue associations. Review-time design links, dated internal files, tool pins/checksums and the proposed concrete final squash subject retain their documented dispositions. No new follow-up issue was created for the optional hardening items.

Validation and limits

The committed diff byte-matches the tested working changes.

  • All 159 helper tests passed with a freshly prepared context, zero skips.
  • All 87 focused Bash 3.2 checks passed on Linux. The shipping cleanup and CRLF guards are exercised directly.
  • Workspace/all-target tests passed serially: 1,554 passed, one ignored. One earlier parallel run failed in unchanged config::tests::typed_push_envelope_sha_matches_hand_computed_hash on a process-environment selector. It did not reproduce in a clean-baseline full run or eight baseline config runs, so its cause remains unconfirmed. The final CLI suite passed separately.
  • Rust formatting, strict workspace/all-target/all-feature Clippy, native feature compilation and Spin WASM compilation passed.
  • Docs formatting/lint/build, changed-script Bash syntax on Bash 3.2 and modern Bash, ShellCheck, actionlint, action-pin checks and Zizmor passed. The unchanged Linux publisher is not certified for Bash 3.2.
  • Local native amd64 image build, context/ELF/user/restriction/HTTP/TLS checks, Compose stop/start/teardown and OCI registry-copy/Docker-pull identity passed. This was the pre-commit working fixture, not a released candidate at the new commit. The full local handoff timed out during Trivy database setup, so final scan/seal completion is not claimed. Its Rust inventory reported zero vulnerabilities and one unsound warning; strict verification continues to block that warning. Disposable run resources were removed.
  • Hosted checks at this head are running, not yet claimed passed. Native arm64, native macOS userland, protected GHCR/OIDC publication and production Kubernetes/AWS/runtime acceptance were not exercised locally in this pass.

Thread resolution does not dismiss reviewer verdicts, grant release approval, close outstanding issue acceptance or authorize registry/cloud operations. Re-review is requested from prk-Jr, aram356 and dhruv8sh.

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Follow-up review at 1a6ae8610e12c9b6631e1b8f9ae2fcff889d52e8 (base cf2c57b1, unchanged since my full review at 89d50968). This pass covers the one new commit and rechecks my earlier findings. Both blockers are fixed, and each fix has a regression test that fails when the fix is reverted. The one new comment below is non-blocking. Approving, which replaces my previous change request.

😃 Praise

  • Every fix comes with a test that catches its removal. I reverted each one locally and the matching test failed:
    • Removing the handoff --verify: 1 test fails.
    • Restoring the NUL-separated context hash: 2 tests fail.
    • Dropping the root-receipt exclusion: 2 tests fail.
    • Restoring the old FROM regex: 4 tests fail.
    • Restoring the old CONTAINERS loop under /bin/bash 3.2: 4 cleanup tests fail.
    • Removing either \r strip in Dockerfile.hbs: the Rust generator test panics.
  • The Compose evidence directory is kept tight: it must not already exist, only *.log files and exit-status are copied (not the rendered config or response bodies), and the original exit status is preserved.

My previous findings, verified

  • 🔧 Bash 3.2 cleanup with an empty CONTAINERS: fixed with the ${CONTAINERS[@]+"${CONTAINERS[@]}"} expansion and covered by tests under Bash 3.2.
  • 🔧 Lowercase, indented or --platform FROM lines: fixed. Preparation, fixture recording and the release record now share one baseImages().
  • 🤔 Compose diagnostics in CI: fixed. The handoff passes --evidence-dir "$OUTPUT/compose" and the failure upload collects it.
  • 📝 SIGTERM wording: fixed in both guides.
  • 🌱 Hardening: done: the length-prefixed context hash, the pre-build context re-check, separate unsafe-archive cases and CRLF handling. The cumulative PAX limit and the non-registry derived-lock entries are deferred with reasons, which is fine for producer-controlled inputs.

Findings

Non-blocking

  • 🤔 The FROM regex also matches heredoc lines (inline, examples/container-publishing/prepare_inputs.mjs:24)

CI Status

  • Hosted at 1a6ae861: all 33 checks pass, including native amd64/arm64 and the handoff (run 42701012913).
  • Local on Rust 1.98.1: fmt passed, cargo test -p edgezero-cli passed (227), and clippy (-p edgezero-cli --all-targets --all-features -D warnings) passed.
  • Helper suite: 156 passed, 0 failed, 3 prepared-context tests skipped, under both macOS /bin/bash 3.2 and Bash 5.2, on local Node 24.12.0.
  • Not exercised locally: Docker builds (hosted native jobs cover them), the CRLF awk under Debian mawk, GHCR/OIDC publication, and Kubernetes/AWS.

}

export function baseImages(dockerfile) {
return [...dockerfile.matchAll(/^\s*FROM\s+(?:--\S+\s+)*(\S+)/gim)].map(match => match[1]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 The new pattern also matches from lines inside heredoc bodies

This is a side effect of the regex I suggested last round. Because it ignores case and indentation, lines inside RUN <<EOF … EOF bodies are now read as base images, even though Docker doesn't treat them as instructions:

Heredoc line base_images
from os import path (Python) ["gcr.io/distroless/cc-debian13:nonroot","os"]
FROM users (SQL) ["a@sha256:1","users"]

check_inputs then rejects the build with "Pinned base trace missing", which is a wrong error for a valid application Dockerfile. It only fails closed: the new pattern accepts a superset of the old one's inputs, so no unpinned base gets through. The scaffold template has no such lines, so CI is unaffected.

Fix (optional): skip heredoc bodies before matching, i.e. lines from a <<[-]?['"]?WORD marker through the line that is exactly WORD. Alternatively, note in the publishing guide that heredoc lines starting with from are treated as FROM instructions.

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Re-review at 1a6ae861, covering the three commits since my review at 827317aa. All five of my previous findings are fixed. The new strict Rust advisory gate exposes two problems at this head: the guide's local reproduction commands fail, and no scaffolded app can currently pass the gate, because of a dependency that is not in the image and that consumers can't upgrade themselves. prk-Jr's open heredoc note on prepare_inputs.mjs:24 is not repeated here; my FROM finding below is a separate case with the same root cause.

Previous findings, verified

  • Bare [[ … ]] assertions under Bash 3.2: fixed. Every one now has an explicit failure exit, and test_axum_container_shell.test.mjs runs the shipping assertions under the bash on PATH. On macOS (/bin/bash 3.2.57, Node 24.20.0) the helper suite now passes 156 tests with 0 failures; 8 failed at 827317aa.
  • Tests asserting workflows outside their triggers: fixed. The suite now reads only axum-container.yml and release.yml, both inside that workflow's paths.
  • Strict policy and Rust advisories: implemented. Strict verify, index and release-record block Rust vulnerabilities and unsound warnings; inventory collection and framework packaging stay report-only.
  • deploy-action.yml leftovers: fixed. Relative to the merge base, only the comment pairing the validator pins remains.
  • GNU-only sha256sum --strict: fixed with a direct hash comparison.

Findings

Blocking

  • 🔧 The guide's local reproduction commands now fail (docs/guide/publishing-axum-containers.md:51 and :85; inline)
  • ❓ The strict Rust gate currently blocks every scaffolded app, over a crate the image doesn't contain (examples/container-publishing/verify_artifacts.mjs:325-327; inline)

Non-blocking

  • 🤔 Valid Dockerfiles are rejected as unpinned (examples/container-publishing/verify_artifacts.mjs:338; inline)

📌 Out of Scope

  • On main, upgrade rusqlite to a release that allows libsqlite3-sys 0.39 or later (RUSTSEC-2026-0459), and tokio to 1.53.3 or later (RUSTSEC-2026-0386). Main's Cargo.lock still has libsqlite3-sys 0.38.2 and tokio 1.53.1, the fixture lock inherits both, and both now block the strict gate.

CI Status

Local runs at 1a6ae861:

  • fmt: PASS
  • clippy (--workspace --all-targets --all-features -- -D warnings): PASS
  • tests (cargo test --workspace --all-targets): PASS (1,554 passed, 1 ignored)
  • feature check (--features "fastly cloudflare spin") and Spin wasm32-wasip2 check: PASS
  • Node helper tests on macOS (/bin/bash 3.2.57, Node 24.20.0): 156 passed, 0 failed, 3 skipped. The 3 prepared-context tests pass with AXUM_PREPARED_CONTEXT set.
  • shellcheck, actionlint and zizmor on the changed workflows and scripts: clean
  • Native arm64 fixture build: PASS. The image is 64 MB, runs as 10001:10001 with STOPSIGNAL SIGINT, serves HTTP, and docker stop exits 0 immediately.
  • Hosted CI at this head: all 33 checks pass, including native amd64/arm64 and the handoff (run 42701012913).

```sh
npm ci --prefix scripts --ignore-scripts
./scripts/prepare_generated_axum_container.sh /tmp/axum-context
./scripts/test_axum_container_handoff.sh build \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 The guide's local reproduction commands now fail

This command, and the import example at line 85, run the handoff helper with its default SCAN_POLICY=strict (scripts/test_axum_container_handoff.sh:15). Strict verification now also gates Rust advisories, and the fixture this guide prepares can't pass that gate: its lockfile is seeded from main's Cargo.lock, which currently resolves libsqlite3-sys 0.38.2 (RUSTSEC-2026-0459) and tokio 1.53.1 (RUSTSEC-2026-0386, unsound).

I downloaded the accepted arm64 bundle from this head's run 42701012913 and ran the same verification the helper performs:

verify --scan-policy strict       -> exit 1: Artifact verification failed: Blocked Rust vulnerabilities; see rust-advisories.json
verify --scan-policy report-only  -> exit 0

In build mode that strict check is the final step, so a local run fails only after the full build, smokes and scans. In import mode it runs first and fails immediately.

Fix: run both documented commands under the policy framework CI already uses:

SCAN_POLICY=report-only ./scripts/test_axum_container_handoff.sh build \
  /tmp/axum-context/container-probe /tmp/axum-amd64 \
  linux/amd64 "$(git rev-parse HEAD)" local

Comment on lines +325 to +327
if (policy === 'strict') {
require(!findings.found, 'Blocked Rust vulnerabilities; see rust-advisories.json');
require(!warnings.unsound?.length, 'Blocked Rust unsound advisories; see rust-advisories.json');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ The strict Rust gate currently blocks every scaffolded app, over a crate the image doesn't contain

The vulnerability that fails strict verification at this head is RUSTSEC-2026-0459 in libsqlite3-sys 0.38.2 (patched in 0.39.0). In a prepared fixture it comes in only through rusqlite 0.40.2, which the Spin adapter's cli feature uses:

libsqlite3-sys v0.38.2
└── rusqlite v0.40.2
    └── edgezero-adapter-spin v0.1.0
        ├── container-probe-adapter-spin v0.1.0
        └── edgezero-cli v0.1.0
            └── container-probe-cli v0.1.0

cargo tree -p container-probe-adapter-axum -i libsqlite3-sys reports that no package matches, so the binary in the image doesn't link it.

A consumer can't clear the finding either. rusqlite 0.40.2 requires libsqlite3-sys ^0.38.1, which excludes the fixed 0.39.0, so a fresh cargo generate-lockfile in a generated app still resolves 0.38.x. With no Rust waiver path, no app generated from this scaffold can release through the recipe until EdgeZero moves to a newer rusqlite. That applies even though its image doesn't contain the affected code.

The publishing guide states the whole-lockfile scope deliberately. Is this outcome intended? If not, two options gate on what the image actually ships:

  • limit strict findings to packages reachable from the image's binary package through normal and build dependencies, using the resolve graph from the cargo metadata output the preparation already produces; or
  • build with cargo auditable and audit the exported binary with cargo audit bin.

Either way, upgrading rusqlite on main clears today's instance; I've noted that as out of scope in the review summary.

require(typeof inputs.toolchain === 'string' && /^rust [0-9]+\.[0-9]+\.[0-9]+$/m.test(inputs.toolchain), 'Toolchain trace missing');
// The scaffold deliberately follows this runtime tag; all other bases remain pinned.
require(Array.isArray(inputs.base_images) && inputs.base_images.length && inputs.base_images.every(base => typeof base === 'string' &&
(TAGGED_RUNTIMES.has(base) || /^[^\s@]+@sha256:[0-9a-f]{64}(?![\s\S])/.test(base))), 'Pinned base trace missing');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Valid Dockerfiles are rejected as unpinned

baseImages() records every FROM operand as a base image, including operands that pull nothing. I ran check_inputs on base images taken from two Dockerfiles that use only pinned or allowlisted images:

Added stage base_images Result
FROM runtime AS debug (an earlier stage) […builder…, "gcr.io/distroless/cc-debian13:nonroot", "runtime"] Pinned base trace missing
FROM scratch […builder…, "scratch"] Pinned base trace missing

Both fail closed, like prk-Jr's heredoc case on prepare_inputs.mjs:24, but the error is wrong, and a stage-reference or scratch final stage is a common pattern.

Suggestion: one change covers all three cases. Decide pinning from the BuildKit materials that resolved_base_images already loads and verifies. Require every pkg:docker/ material to carry digest=sha256: in its URI, except the allowlisted runtime tag; the builder and frontend materials already do, as in pkg:docker/rust@1.98.1-bookworm?digest=sha256:…. Materials list only the images BuildKit actually pulled, so stage names, scratch and heredoc text never appear. base_images can stay as the recorded trace.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants