Skip to content

Scaffold hardened Axum application Docker recipes - #401

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

ChristianPavilonis wants to merge 14 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 advisories remain a complete lock-bound report-only inventory, separate from strict High/Critical image policy.
  • 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.

Current 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 Rust advisories, with no exceptions or filtering. Its runtime child is sha256:cfc14c62b697225036d242674494d4b10e80c5a9525aa8d42c81489c1234aac5. This is point-in-time evidence, not a permanent vulnerability-free claim.
  • Current-head hosted native amd64/arm64 and handoff results require confirmation after this push. Earlier-head results are not substituted for the changed runtime and verifier.

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.

@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.

🔧 Review: request changes

Reviewed all 33 changed files at 1f244a7c1b78dafd5bc9291ad86c385360636f72, against 87456c708ffe9edf86b3a0f8b06dfd15d60b07b5. One existing merge-metadata blocker remains; one new, nonblocking Compose smoke finding is attached inline. No additional blocking defect was confirmed in scaffolding, toolchain validation, container hardening, TLS checks, publishing permissions, or deployment contracts.

🔧 P2 — Merging would prematurely close unfinished issues (existing finding, still unresolved)

GitHub currently lists #398, #399, and #400 as closing references. The PR explicitly says #399/#400 must remain open, while #398 still has outstanding production acceptance. Merging would therefore close work the PR acknowledges is incomplete.

Remove #399/#400 from the closing associations and retain #398 until its remaining acceptance is complete, or explicitly separate the delivered packaging slice. The PR also remains marked ready for review despite its description calling it draft.

😃 The artifact handoff is carefully designed. The publisher independently recomputes committed source identities before registry login, verifies both architecture bundles and their metadata, and preserves tested child digests. Report-only scans cannot authorize GHCR publication.

Validation: Rust 1.98.1 workspace tests passed (1,553 passed, one ignored), together with formatting, strict all-feature Clippy, native feature compilation, and Spin WASM compilation. The helper suite passed 77 tests, followed by all three prepared-context checks passing. Local Node was 24.12.0; hosted checks used the declared 24.20.0. ShellCheck, actionlint, and diff checks passed. Hosted native amd64/arm64 container and handoff checks passed; their merge-result file tree exactly matches the reviewed head.

Local Docker execution was unavailable because the daemon was stopped. Protected GHCR publication, attestations, Kubernetes, and AWS acceptance remain unexecuted; the docs explicitly defer those claims. Green packaging checks do not establish release security approval: current hosted logs retain one High OS finding and five Rust advisories.

@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

Container scaffolding, native multi-arch CI and the handoff verification are solid and hosted CI is green, but there are two correctness gaps (builder digest/version drift, vacuous lockfile check) and an OIDC scoping question to resolve before merge.

Findings

Blocking

  • 🔧 Rust version and builder digest can drift (Dockerfile.hbs:4)
  • 🔧 Lockfile check is vacuous (prepare_generated_axum_container.sh:23)
  • ❓ OIDC token scoped to the whole publisher job (release.yml:121)

Non-blocking

  • 🤔 Scope: ~470 lines of draft AWS/Kubernetes design (not #398) plus a 539-line hand-rolled tar/OCI verifier. Suggest splitting the design docs into their own PR under #391/#400, and weighing whether Skopeo/oras verification could replace some of the custom parsing.
  • ⛏ Pinning consistency: setup-node comment is only # v6; the recipe uses Node 24.12.0 vs the repo's 24.20.0; cargo install cargo-audit has no checksum while gh is checksum-verified.
  • ⛏ Docs hygiene: new docs/superpowers/ files are undated (others use YYYY-MM-DD-); the K8s spec uses VitePress ::: callouts but is excluded from the site, so it renders raw on GitHub (use > [!WARNING]); the guide links blob/main/... URLs that 404 until merge; the PR body cites docs/specs/aws-reference-deployment.md but the file is under docs/superpowers/plans/.
  • ⛏ Head commit message ("Implemented all 18 accepted plan items… Changes remain uncommitted…") should be squashed/reworded.
  • 📝 The runtime base distroless/cc-debian12:nonroot is intentionally floating; consequently the attested base_images record a tag, not runtime bytes. Consider recording the resolved digest from BuildKit metadata.

CI Status

  • fmt: PASS
  • clippy: PASS
  • tests: PASS (hosted CI on 1f244a7; local Node script suite 77 pass / 3 skip)

Comment thread scripts/axum_container_helpers.mjs Resolved
Comment thread .github/workflows/axum-container.yml Resolved
Comment thread .github/workflows/axum-container.yml Resolved
@aram356 aram356 added this to the 202611 milestone Oct 29, 2026
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.

./scripts/test_generated_axum_container.sh /path/to/container-probe
```

See the [smoke script prerequisites](https://github.com/IABTechLab/edgezero/blob/d7c4ee9e4a9db1d547272b3f13a6177f10098dba/scripts/test_generated_axum_container.sh)

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.

🤔 Commit permalinks will go stale after merge

This link (and the two in docs/guide/deploying-axum-containers.md at lines 132 and 150) now points at branch commits d7c4ee9e and 91cb3e65. They load today, but after merge they will show those exact old versions permanently. This one calls the script "the authoritative tool list", so once scripts/test_generated_axum_container.sh changes on main, the guide will point readers at an outdated prerequisite list.

blob/main/... links are only broken until this PR merges, and after that they stay current. Suggestion: switch these three links back to blob/main/... as the last change before merge, or drop "authoritative" if pinned links are intended.

cancel-in-progress: true

env:
ACTIONLINT_VERSION: 1.7.7

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.

⛏ Duplicated tool pins, and the static checks now gate the native builds

ACTIONLINT_VERSION, ZIZMOR_VERSION and YQ_VERSION are now declared identically here and in deploy-action.yml, so a bump in one file can silently miss the other. Also, prepare now compiles Zizmor before the native jobs can start: prepare took about 1m50s at d7c4ee9e and 5m04s at this head, which adds roughly three minutes to the critical path.

Suggestion: move the actionlint/pin/Zizmor/ShellCheck steps into a separate job that runs in parallel with native (the handoff can still depend on it if desired), and add a comment pairing these pins with deploy-action.yml.

@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 on lines +39 to +40
[[ "$(docker inspect "$CONTAINER" --format '{{.Config.User}} {{.HostConfig.ReadonlyRootfs}} {{.HostConfig.CapDrop}} {{.HostConfig.SecurityOpt}}')" \
== '10001:10001 true [ALL] [no-new-privileges:true]' ]]

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.

🔧 On macOS's default bash, these security checks can't fail

macOS ships bash 3.2 as /bin/bash, which #!/usr/bin/env bash resolves to unless a newer bash is earlier on PATH. Bash before 4.1 does not apply set -e to a failing [[ … ]] statement, so a bare [[ … ]] assertion is a no-op there.

I ran this script under /bin/bash (3.2.57) with the test suite's own mocks, making docker inspect report 0:0 false [] []: root user, writable root, no dropped capabilities, no no-new-privileges. It printed PASS: no-store Compose config, restricted HTTP startup, stop/start and teardown. and exited 0. The four Compose smoke rejects insecure configuration tests fail on macOS for this reason. The two failed-teardown tests also fail there: the failure is still detected, but the diagnostics they expect are not written.

The same bare pattern appears in scripts/test_generated_axum_container.sh:

  • line 140: the image's Config.User
  • lines 187-188: the tmpfs settings
  • lines 199 and 203: the TLS rejection status (the grep on the body after each still catches a handshake that wrongly succeeds)

Linux CI runs bash 5 and is unaffected, but the guides tell people to run these smokes from a checkout.

Fix: give each assertion an explicit failure, as line 42 already does:

[[ "$(docker inspect "$CONTAINER" --format '{{.Config.User}} {{.HostConfig.ReadonlyRootfs}} {{.HostConfig.CapDrop}} {{.HostConfig.SecurityOpt}}')" \
  == '10001:10001 true [ALL] [no-new-privileges:true]' ]] \
  || { echo 'Compose restrictions missing' >&2; exit 1; }

assert.match(native, /^\s*-\s+['"]?scripts\/package\*\.json['"]?\s*$/m);
});

test('Axum static validation moves with its triggers and retains repository-wide gates', () => {

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.

🔧 These tests assert workflow files whose changes never trigger them

This test and container helper CI has one suite owner and derives framework Node pins (line 232) assert exact lines, indentation, job text and validator pins in .github/workflows/deploy-action.yml and .github/workflows/format.yml. They run only in axum-container.yml's prepare job (line 242 asserts that), and that workflow's paths filter includes neither file.

So a PR that edits deploy-action.yml, for example by reflowing a run: block, changing how Zizmor is installed or removing a step, passes its own CI. The next PR that touches crates/** or Cargo.lock then fails on an assertion it had nothing to do with.

The assertions also force configuration into a workflow that other work owns:

  • lines 245-248 require deploy-action.yml to keep a setup-node step that nothing under .github/actions uses;
  • lines 295-302 require its validator pins and install commands to match this workflow's, which is the duplication prk-Jr raised.

Fix: assert only files that axum-container.yml triggers on (axum-container.yml, examples/container-publishing/release.yml, tools.env). If the deploy and docs workflows need these guarantees, put the checks in tests those workflows run.

if (policy === 'report-only') console.error(`WARNING: report-only scan: ${counts.HIGH} HIGH, ${counts.CRITICAL} CRITICAL package findings, ${fixable} with fixes. Packaging evidence only; not approved for release.`);
}
// Advisory inventory is report-only; OS release policy remains a separate gate.
export function check_advisories(record, inputs) {

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.

🤔 Strict release policy doesn't cover the Rust advisory inventory

With the distroless runtime, the strict OS gate has little left to block: the hosted bundles at this head (run 41481693509) report 0 High/Critical OS findings on both architectures. The locked crates are now the main dependency surface, but check_advisories only validates the report's shape, so any number of RustSec vulnerabilities passes verify and release-record under strict.

It also ignores cargo-audit's warnings. Both hosted bundles carry RUSTSEC-2026-0386 (tokio 1.53.1, "AsyncFd::new and related constructors violate I/O safety", patched in 1.53.3) as an unsound warning, with no effect on the verdict.

The guide currently leaves advisory policy to application owners. Shipping the safe default in the copyable recipe seems better: under strict, require vulnerabilities.found === false, decide explicitly whether unsound warnings block, and take exceptions from the same owner-reviewed, expiring file, keyed by advisory ID and package. Framework CI can stay report-only.

scripts/install-actionlint.sh \
scripts/install-yq.sh

- uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6.5.0

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.

⛏ Leftover changes in this workflow

Nothing under .github/actions runs Node, so this setup-node step is unused. It stays only because scripts/tests/test_axum_container_publishing.test.mjs:245-248 requires it (see the comment there).

The other changes in this file are leftovers too: the comment block at lines 6 and 29 was removed on main by #417 and is re-added by the merge, and the pin-gate step's run: | reflow changes nothing. With the helper suite now owned by axum-container.yml, this PR's changes to deploy-action.yml can be reverted.

curl --fail --silent --show-error --location \
"https://static.crates.io/crates/cargo-audit/cargo-audit-${CARGO_AUDIT_VERSION}.crate" \
--output "$WORK/cargo-audit.crate"
printf '%s %s\n' "$CARGO_AUDIT_SHA256" "$WORK/cargo-audit.crate" | sha256sum --check --strict

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.

⛏ GNU-only checksum check

macOS's sha256sum rejects --strict and can't read the checksum list from stdin with any flag, so a local build run of this helper stops here with a usage error. The two cargo-audit source installation tests fail on macOS for the same reason.

Fix: compare the computed hash directly, as line 78 of this script already does, and update the assertion at scripts/tests/test_axum_container_advisories.test.mjs:38:

[[ "$(sha256sum "$WORK/cargo-audit.crate" | awk '{print $1}')" == "$CARGO_AUDIT_SHA256" ]] \
  || { echo 'cargo-audit checksum mismatch' >&2; exit 1; }

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