fix(sbom): report the pm package catalogue version the build installed with

[reyreavman]

Summary

An os-pm build no longer resolves PACKAGES_VERSION and REGISTRY from build secrets whose id happens to match those names; both come from the base image environment or from an explicit %secret:ID% reference in packages[].env. The same change fixes the SBOM reporting a package catalogue version the build did not install with: packages[].env.PACKAGES_VERSION never reached the guard that writes /var/lib/pm/container-factory-version, so a scratch base failed as if the variable were unset, and when the base image also set it the containerfactoryversion purl qualifier carried the base value while pm installed with the env one.

secrets:
  - env: PACKAGES_VERSION
packages:
  - type: os-pm
    env:
      PACKAGES_VERSION: "%secret:PACKAGES_VERSION%"
    spec:
      - curl==8.12.1

What

• BREAKING: a config that declared a secret with id PACKAGES_VERSION or REGISTRY and relied on werf wiring it into pm loses that value — pm falls back to its own default registry and the stage fails on a missing version. The way out is one packages[].env entry referencing the secret.
• packages[].env.PACKAGES_VERSION now satisfies the packages stage on a base image that sets no such variable; before, the stage failed as if the variable were unset.
• When both the base image environment and packages[].env set PACKAGES_VERSION, the provenance file and pm install use the env value; the SBOM qualifier and the installed catalogue can no longer disagree.
• A packages[].env value referencing a secret is now assigned by its own shell statement instead of riding in the command prefix, and the whole directive runs in a subshell: an unreadable secret file fails the stage, where before pm ran with the variable empty and the stage exited 0, and the assignment stays local to its directive — the next one in the same stage script still sees the variable the base image exports. Only an entry that references a secret changes shape and rebuilds; a literal value keeps its inline prefix.
• The message that stops the build when PACKAGES_VERSION is missing now names the fix: PACKAGES_VERSION: werf records it in the SBOM; set it in packages[].env, e.g. PACKAGES_VERSION: "%secret:PACKAGES_VERSION%". No warning keyed on secret ids: REGISTRY is a generic name with an unrelated meaning in .Values.werf.image, and pm reports an unreachable registry itself.
• The generated packages command no longer mentions /run/secrets unless the config references a secret, so the stage digest of every os-pm image changes once and the packages stage rebuilds.
• The os-pm docs (en + ru) now state that PACKAGES_VERSION is mandatory, show how to supply both variables when the base image provides neither, and name the migration for a build that relied on the implicit lookup.
• The docs no longer promise that a referenced secret never reaches the resulting image: PACKAGES_VERSION is written into the image and the SBOM by design.
• A directive without a secret-referencing value produces the same command as before, for os-pm and for the file-based types alike.

Why

formatVersionFileCommand built its own copy of the value from the base image environment and the implicit secret lookup, while pm install got the directive env — one variable resolved twice from different inputs, which is why an explicitly set version could be both ignored and contradicted in the SBOM at once. The implicit lookup itself dates to 6a744aa, when a scratch base had no other way to supply the variables; since %secret:ID% landed it is a second, undocumented mechanism for the same job, and it reserves two very common secret ids — a secret named REGISTRY declared for anything else silently redirects pm.

Keeping the lookup for PACKAGES_VERSION only — narrower, and werf itself is what demands that variable — was the alternative. It was rejected because the argument for it is exactly the argument for documenting the explicit form instead: after the precedence fix, one env line covers the scratch case, and a single mechanism is what makes the behavior discoverable.

Fixes: 6a744aaf6e6f ("fix(config): resolve pm env vars from build secrets in packages stage")

[reyreavman]

Verification

• CI e2e_simple (runs 34809312857, 34813060301 34816511309 and 34859548570 attempt 1, which carries the review fixes): E2E SBOM suite passed — all 12 Docker entries of SBOM os-pm packages ran, including resolves pm env from referenced build secrets on a scratch base image without own coreutils on vanilla-docker and buildkit-docker with the fixture switched to explicit %secret:ID% references. This is what proves pm install needs no implicit lookup and that the containerfactoryversion qualifier still carries the secret value. Buildah entries are XEntry (pending) as before.

• Locally the same spec could not be exercised: the trusted builder base is linux/amd64 and the host is arm64, so the run failed on platform mismatch before reaching the stage under test.

• 845659b750 reverts the secret-id warning (309fb8dc39); 51aff24b51 moves the hint into the guard. The unit harness runs the generated command under bash -e and asserts the hint arrives verbatim, including the quoted "%secret:PACKAGES_VERSION%" — the message lives inside a "${VAR:?…}" expansion, so a broken escape would show up as garbled output there.

• 74fdbb7155: the version assignment was chained to the guard with &&, which bash exempts from set -e; an unreadable secret file skipped the guard and let pm install run with an empty PACKAGES_VERSION while the stage exited 0 (inherited from the formatSecretVar shape on main, reproduced with the generated command under bash -e). Now terminated with ; so the failed read aborts the stage. Mutation: restoring && fails only fails the stage when the referenced secret file is missing.

• Mutation: reverting formatVersionFileCommand to ignore env → records the same version it installs with failed on packages env only (guard rejected an explicitly set version) and on packages env wins over base image env (provenance file and install env disagreed).

• CI unit failed on the first attempt of runs 34809312857 and 34816511309 with failures detected: build ./pkg/build while every Build Suite spec was green; both reruns passed. Cause is outside this PR: pkg/build/graph_scheduler_integration_test.go TestDoImagesInParallel_DependentImageDoesNotWaitForUnrelatedSlowImage asserts wall-clock ordering (30 ms chain vs 150 ms sleep) and loses under 15-proc contention — it reproduces locally under CPU load ("3" is not less than "2", order [a b slow c]). Ginkgo's parallel runner swallows plain-test output, which is why the log shows no --- FAIL. Same signature on main attempts 33632689293 and 33893236413 before this branch existed.

• Rerunning only the failed e2e jobs of 34859548570 is not meaningful: kind_cleanup had already torn down the test registry, so attempt 2 fails everything with dial tcp 127.0.0.1:32769: connect: connection refused before any build starts. Attempt 1 is the readable result.

• e2e_complex fails with FAIL! - Suite Timeout Elapsed -- Ran 6 of 95 Specs in 5069s, 4 Passed | 2 Failed in test/e2e/build staged-dockerfile specs (Signal: terminated, the rest Suite did not run because the timeout elapsed). main run 34590330771 fails with the same line and the same counts, and the sbom suite never runs in that job at all.

• e2e_extra fails on Buildah storage errors in test/e2e/build (overlay-layers/tmp/...: no such file or directory, image not known), which also fail on main (run 34590330771); nothing in this PR is in that path.

• ec12f25c87 came out of a review pass (self-review plus an independently spawned reviewer). Three findings were fixed here rather than deferred:

  • a secret-backed value in the command prefix repeated the set -e hole 74fdbb7155 closed for PACKAGES_VERSION, for every other variable — verified as FOO="$(</nonexistent)" cmd ⇒ stderr message, empty value, exit 0. Secret-backed values now get their own statement. Mutation: forcing every value back into the prefix fails 7 specs, including fails the stage when a secret another variable references cannot be read.
  • :? → ? (accepting a set-but-empty version) survived the old suite. New entries the value is set but empty and the referenced secret is empty catch it; the empty value is reachable through env: {PACKAGES_VERSION: ""} or an empty base image ENV.
  • dropping strings.TrimSpace changed the generated command — and with it every os-pm stage digest — while the suite stayed green, because every assertion was ContainSubstring. produces exactly this command, byte for byte now pins the whole string for three env shapes; it catches that mutation and the secret_path values staying inline.

Review focus

• Whoever relied on the implicit lookup now learns about it from the failing guard, not from a warning; the guard fires only when PACKAGES_VERSION is missing, so a build that lost REGISTRY alone now fails on its unreadable secret if it referenced one, and otherwise gets pm's own default registry.
• The docs guarantee for %secret:ID% was too broad — it promised the value never reaches the resulting image, while PACKAGES_VERSION is written into the image and the SBOM on purpose. Corrected in both languages (2a461a26b2).

[Fral738]

Requesting changes because secret-backed packages[].env assignments now outlive their directive. When the base image exports the variable, subsequent package managers inherit the previous directive's secret value even if their own env does not reference it. This changes package-source/proxy selection for otherwise unchanged later entries. Please isolate these assignments before merging.

The description's claim that packages[].env handling is otherwise unchanged does not hold for multiple directives sharing the packages-stage shell. The inline comment includes the reproduction and suggested correction.

Validation at 2eef160e: task build and full task test:unit passed. A temporary two-command regression probe failed for a secret reference (private-proxy / private-proxy instead of private-proxy / base-proxy) and passed for the equivalent literal value. Mutations of version precedence, the empty-version guard, and unreadable-secret handling were detected; the last required system bash 3.2 because Homebrew bash 5.2.37 already aborts on the old prefix form. All temporary changes were restored. The focused scratch/Vanilla Docker e2e did not reach the packages stage: pushing its builder image to the local registry failed with a connection error, so that e2e and its fixture mutation remain unverified locally.

[Fral738]

These standalone assignments overwrite the shell environment for subsequent packages directives. All commands run in one stage script (legacy executor, Buildah executor). If the base image exports GOPROXY=base-proxy, a first go-mod entry with GOPROXY: "%secret:proxy%" permanently replaces it; a second entry without env then installs through that private proxy as well. Previously the command prefix kept this override local to the first package manager.

A two-command formatWorkdirCommand probe under bash -e, using printenv GOPROXY as the manager command and a secret containing private-proxy, prints private-proxy\nprivate-proxy\n; the same probe with a literal env value correctly prints private-proxy\nbase-proxy\n.

Wrap the complete generated directive in a subshell when it needs standalone assignments, keeping the assignments and install inside it (and the version guard/provenance write for os-pm). This preserves fail-fast secret reads without modifying the next directive's environment. Add a regression with an exported base variable, a secret-backed override in the first directive, and no override in the second. For example:

(
  GOPROXY="$(</run/secrets/proxy)"
  cd "/app" && GOPROXY="$GOPROXY" go mod download
)
cd "/tools" && go mod download

[reyreavman]

Confirmed and fixed in 03ae8cd. Reproduced your probe first: two go-mod directives under bash -e with GOPROXY=base-proxy exported printed private-proxy / private-proxy for the secret reference and private-proxy / base-proxy for the literal — the standalone assignment introduced in ec12f25c87 outlived its directive exactly as you describe.

The directive now runs in a subshell whenever it has such an assignment (formatWorkdirCommand and formatInstallCommand share joinDirective); a directive without one keeps its previous single-line form, so its digest does not move again. For os-pm the mkdir, guard, provenance write and pm install all sit inside the subshell.

Regression added as formatWorkdirCommand › two directives in one stage script: exported base variable, secret-backed override in the first directive, no env in the second, asserting private-proxy\nbase-proxy\n, plus the literal variant and an unreadable-secret case asserting the stage dies before the second directive runs. Mutation check: removing the parentheses from joinDirective fails the secret-backed entry (and the byte-for-byte golden), while the literal entry stays green — the same split your probe showed.

The PR description no longer claims the handling is otherwise unchanged; it now says so only for directives that reference no secret.