ext package: bake resolved version into the published avocado.yaml

[mobileoverlord]

Problem

The in-source program extensions (avocado-ext-cli, avocado-ext-connect, avocado-ext-tunnels) keep the extension version in sync with Cargo.toml by templating it from an env var:

extensions:
  avocado-ext-connect:
    version: '{{ env.AVOCADO_EXT_VERSION }}'   # CI exports it from [package] version

That template only resolves while AVOCADO_EXT_VERSION is set — i.e. at package / PR-test time. The ext package RPM payload copied the raw on-disk avocado.yaml, so the literal template survived into the published package. Later, a runtime build consuming that extension has no AVOCADO_EXT_VERSION in scope, so it interpolated to '' and failed:

Extension 'avocado-ext-connect' has invalid version ''. Version must be in
semantic versioning format (e.g., '1.0.0', '2.1.3')

Fix

Resolve the required version field at package time and bake the concrete semver into the avocado.yaml that ships in the RPM payload. The resolved value is metadata.version — already interpolated through the normal config pipeline and semver-validated before packaging — so no new resolution logic, just persistence.

Only extensions.<ext>.version is rewritten, via a comment-preserving, indentation-scoped line edit (no serde round-trip, so comments and quoting are untouched). Every other template ({{ avocado.target }}, {{ env.AVOCADO_DISTRO_RELEASE }}, …) is intentionally left for the consumer to resolve at their build time — matching the "interpolate the required fields, defer the rest" model.

Implementation notes:

• The bake rewrites the single small staged avocado.yaml (≈1 KB) that's already in the staging dir — it adds no copy of the (potentially large) source tree.
• Remote/unreadable source configs (e.g. packaging a fetched remote ext) are left untouched rather than failing the package.

Tests

New unit tests for the bake_extension_version helper:

• version-only replacement, with other templates and comments preserved
• nested version: keys (e.g. under packages:) left untouched
• multi-extension files bake only the named extension
• not-found error path

Local: cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings, and cargo test all green (17 package tests, incl. 4 new).

Follow-up (not in this PR)

ext package currently copies the selected files twice (src → staging → rpmbuild buildroot). A separate refactor can have %install copy straight from the source into the buildroot (dropping the intermediate staging dir) to cut one copy for large source trees; it touches the rpmbuild flow so it should be validated against a real avocado ext package run.

[jetm]

@mobileoverlord before the review below: I've now gone through every PR you asked me to look at, so I'd appreciate the same in return. Mine have been sitting without a review and they're the foundation the CI/CD lab work depends on. Fair's fair - here are mine, roughly in priority order:

• avocado-cli Add --connect-sign flag to avocado deploy #158: Add --connect-sign flag to avocado deploy #158
• meta-avocado #232: Add TPM2 LUKS sealing, dm-verity, and Secure Boot key management meta-avocado#232
• meta-avocado #229: Redesign packagegroup-avocado-extra into opt-in feature groups meta-avocado#229
• meta-avocado #230: snap-on: add smartcard and printing feature groups meta-avocado#230
• meta-avocado #227: recipes-ml: add ExecuTorch 1.3.1 (+ torchgen-native) meta-avocado#227
• meta-avocado #226: Add avocado-rzv2h-rdk (Renesas RZ/V2H RDK) board meta-avocado#226
• avocado-mcp fix inclusion of repo_url and repo_release across commands #8: Add yocto-engineer recipe-authoring tools and skill avocado-mcp#8

Now to #161. The direction is right and the base64 transport is a clean way to dodge shell-escaping the YAML, but I think a few things need addressing before this merges - two of them turn a currently-working ext package into a hard failure, and one is a command-injection path.

Critical

1. Version sourced from a target/kernel override block hard-fails. metadata.version comes from the merged config, so the resolved version can originate from a target-<name>.version sub-block. bake_extension_version only matches a version: that's a direct child of the extension, so in that case it bail!s and the ? aborts a package that built fine before. It can also half-bake: a base version: template gets resolved while a target-*.version template survives into the payload and still interpolates to '' downstream - the exact failure this is meant to prevent.

2. Command injection through the version string. validate_semver only checks the three numeric components; pre-release/build metadata after -/+ isn't validated. metadata.version is then interpolated into the double-quoted echo "Baked resolved version '...'" in the rpmbuild script, so a version like 1.0.0-$(...) executes in the build container. The baked YAML rides safely through base64 already; the version shouldn't be interpolated into the shell at all.

3. Templated extension keys hard-fail. Extension keys can be templates (avocado-bsp-{{ avocado.target }}), and find_ext_in_mapping resolves them by interpolating. bake_extension_version compares the key literally, so any template-keyed extension never matches and bail!s.

Warning

4. .yml vs .yaml silently skips the bake. cfg_basename follows the on-disk name, but staging copies the hardcoded avocado.yaml. If the config is avocado.yml, the if [ -f "$STAGING_DIR/avocado.yml" ] guard is false and the bake is skipped with no error, shipping the unresolved template.

5. Remote extensions never get baked. The host can't read the container-volume config path, so the bake section is empty and the template ships. It's noted as intentional, but it's the same downstream breakage, just for remote ext.

Suggestion

6. Reuse src/utils/config_edit.rs. We already have a comment-preserving avocado.yaml line-editor there, with quoted-key handling and the compact-indent fix from ENG-1868, plus tests. A set_extension_field built on find_top_level_key would replace the hand-rolled state machine and inherit those fixes instead of drifting from them. Resolving against the merged config (which fixes 1 and 3) fits naturally there too.

The tests are good - the nested-version: and multi-extension cases are exactly the right edges. Happy to pair on the config_edit.rs rework if useful.

[mobileoverlord]

Thanks @jetm — this was a thorough review and most of it landed. Pushed f8b8d78 addressing it. Summary of what changed and where I pushed back:

Root cause (your #1 + #3): the bake matched the raw on-disk text literally while metadata.version is resolved from the merged config (find_ext_in_mapping + target overrides). Fixed at the source — matching now mirrors find_ext_in_mapping (direct key, then {{ avocado.target }} interpolation), so:

• wip #3 templated keys (avocado-bsp-{{ avocado.target }}) resolve instead of bail!ing. This was the widest regression, so good catch.
• workflow test #1 target-block version: when there's no direct-child version: we now insert a concrete base version instead of aborting, and we also bake version: inside target-*/kernel-* override blocks so a surviving template can't interpolate to '' downstream. Deeper packages.*.version stays untouched.

#2 command injection: fixed at the injection point — the version string is no longer interpolated into the rpmbuild echo; the baked value rides the base64 payload only. I'd gently downgrade this from "critical": the baked YAML was already base64-safe, only the cosmetic log line was exposed, and version originates from CI's AVOCADO_EXT_VERSION/Cargo.toml rather than an untrusted party. Real defense-in-depth, worth fixing because it's free, but not a live exploit in the intended flow. (Left validate_semver alone — tightening it would reject legitimate pre-release/build metadata; removing the shell interpolation is the right layer.)

#4 .yml/.yaml: the on-disk basename now drives both staging and the bake guard, so a .yml ext stages and bakes under its real name.

#6 reuse config_edit.rs: done — the helper moved there next to the other comment-preserving editors, reusing find_top_level_key/extract_package_name/leading_spaces. Tests moved with it and gained template-key + target-block coverage. Resolving against the merged config (your #1/#3 fix) fits naturally there as you predicted.

#5 remote ext: left as-is/documented — the host can't read the container-volume config, and the empty-bake path avoids failing the package. Same downstream caveat as before, no regression. Open to revisiting if it bites in practice.

One note on #1: no config in the tree currently puts version in a target block, so the half-bake was theoretical — but I implemented the override-block baking anyway since it's cheap and makes the helper match the merge semantics rather than drift from them.

cargo fmt / clippy -D warnings / cargo test green (1070 tests; 7 bake tests incl. the new template-keyed and target-block cases).

And yes — pulling up your stack now, will get reviews back to you today.

[jetm]

@mobileoverlord Approving. Appreciate the quick turnaround and picking up the stack.