Package payload, first-boot provisioning and mage build:packages

[bootc]

The shared half of the deb and rpm packages: everything both formats install, the provisioning that makes an installed CA actually serve, and the mage target that builds them.

Scope note, because this changed mid-review. #250 originally excluded the configuration file, on the grounds that it is where the two formats diverge. Chris then ruled that the 8141 default belongs here rather than being set twice downstream, and the exclusion did not survive contact with nfpm: one config|noreplace declaration renders as a dpkg conffile and rpm %config(noreplace), so the divergence the exclusion assumed does not exist here. The packages therefore ship /etc/puppet-ca/config.yaml, and #251/#252 extend it rather than each creating one — which makes their scope smaller than those issues currently describe.

This unblocks #266. That PR is approved and mergeable and has been held solely on this one: its packaging job calls mage build:packages once (release.yml:155), and until now that target had zero occurrences on main. Both constraints recorded in #266's body — do not merge #266 before #250, and do not push a v* tag until #250 lands — are lifted by this.

Closes #250

mage build:packages

Sits beside build:dist, and matches the contract #266 already committed to: it reads the variant tarballs already in dist/ and writes one package per format for each packaged variant into the same directory. The filenames are nfpm's conventional ones, which is what apt and dnf expect, and they carry no variant name.

It does not build binaries, and that is the point of it. The ones inside openvox-ca_VER_amd64.deb are literally taken out of openvox-ca_VER_linux_amd64.tar.gz, so a package is never a second compilation of the same source that might differ from the artefact that was tested, checksummed and attested. A missing tarball is an error naming the target that produces it, not a silent rebuild.

The FIPS variants are not packaged: nfpm runs neither dpkg-shlibdeps nor rpm's automatic requires, so a dynamically linked package's dependencies would have to be hand-written and kept true.

nfpm, not goreleaser. The FIPS binaries are cgo and dynamically linked; goreleaser would mean either dropping FIPS or reinstating cgo cross-compilation, and importing externally-built binaries needs its Pro-only prebuilt builder. nfpm is what goreleaser's nfpms: block uses underneath, so what release infrastructure ingests is unchanged either way.

Crypto trace for the new dependency

This branch adds two direct dependencies, both build-tooling only, and here is the trace rather than an assertion. Everything below is diffed against main and re-run after each rebase that touched go.mod.

Added	Why	Reaches a shipped binary?
github.com/goreleaser/nfpm/v2	builds the .deb and .rpm in mage build:packages	no
github.com/sassoftware/go-rpmutils	test only — reads the built .rpm back so its payload, modes and %config(noreplace) flags are asserted against the artefact rather than cited from nfpm's source	no

Both are reachable only from magefile.go and magefile_test.go, which are behind //go:build mage. Verified for both, and for everything they dragged in:

$ go list -deps ./cmd/openvox-ca ./cmd/openvox-ca-ctl \
    | grep -Ec "nfpm|rpmpack|blakesmith/ar|go-git|go-rpmutils|cavaliergopher/cpio|DataDog/zstd|xi2/xz"
0

1. Neither shipped binary's package set changes at all. Not "nfpm is absent" — the whole set is byte-identical:

$ go list -deps ./cmd/openvox-ca | sort > head.txt      # 864 packages
$ git archive origin/main | tar -x -C main/ && (cd main && go list -deps ./cmd/openvox-ca | sort > ../main.txt)
$ diff main.txt head.txt && echo identical
identical
$ # and the same for ./cmd/openvox-ca-ctl -- 514 packages, also identical

That is the strongest form of the claim: nfpm does not reach the product, and neither does anything it dragged in. magefile.go is behind //go:build mage, which is why.

2. The non-stdlib crypto importers are unchanged. Reverse-traced through the shipped binaries' graphs:

$ go list -deps -json ./cmd/openvox-ca ./cmd/openvox-ca-ctl \
    | jq -r 'select(.Imports != null) | . as $p | .Imports[]
             | select(startswith("golang.org/x/crypto/") or startswith("filippo.io/"))
             | "\($p.ImportPath) -> \(.)"' | sort -u
filippo.io/edwards25519        -> filippo.io/edwards25519/field
github.com/go-sql-driver/mysql -> filippo.io/edwards25519
.../internal/api               -> golang.org/x/crypto/ocsp
.../internal/ca                -> golang.org/x/crypto/argon2
.../internal/ca                -> golang.org/x/crypto/ocsp
golang.org/x/crypto/argon2     -> golang.org/x/crypto/blake2b
mellium.im/sasl                -> golang.org/x/crypto/pbkdf2

diff against the same command on main is empty. These are the four known pre-existing exceptions outside the boring module — the Argon2id key-at-rest KDF (internal/ca/keyenc.go), x/crypto/ocsp encoding, the mysql driver's edwards25519, and mellium's SASL pbkdf2. No new one is introduced, and none is removed.

3. The FIPS build still is one. Built as the release builds it, in a linux/amd64 container:

$ GOEXPERIMENT=boringcrypto CGO_ENABLED=1 GOOS=linux GOARCH=amd64 go build -trimpath ./cmd/openvox-ca
$ go version -m openvox-ca | grep build
	build	CGO_ENABLED=1
	build	GOARCH=amd64
	build	GOEXPERIMENT=boringcrypto
	build	GOOS=linux
$ go tool nm openvox-ca | grep -c _goboringcrypto_
474
$ go tool nm openvox-ca | grep -c crypto/internal/boring
488
$ go tool nm openvox-ca | grep -cE "goreleaser/nfpm|google/rpmpack|go-git/go-git"
0

The boring module is linked in and doing the work; nothing from the packaging dependency is in the binary.

The dependency bump that did bite, and where

Worth recording because the obvious check missed it. Adding nfpm raised charmbracelet/x/ansi to v0.11.0 module-wide, and charmbracelet/x/cellbuf@v0.0.13 — reached via lipgloss from mfridman/tparse — calls the pre-0.11 API and stopped compiling. tparse is a tool directive in go.mod and test:unit pipes go test -json into it, so the entire unit suite became unrunnable; the CI job sat at "Run unit tests" for two hours against a 3m49s baseline rather than failing cleanly. Fixed by raising x/cellbuf to v0.0.15 (its own commit).

The generalisation for whoever adds the next dependency: go list -deps ./cmd/... proves nothing about the build tooling. It was correct here and the product really is untouched — but the tooling shares the module graph, and a tool directive is built from it. A dependency can be absent from everything shipped and still stop the tests from running.

Sharing the format list with #266

packageFormats, packageExtensions(), packagedDistVariants() and the packaged field on distVariantSpec are defined here with the names and semantics #266 gives them. #266 adds the same symbols and lands immediately after this, so its rebase is a deletion of its own copies rather than a conflict to resolve — its call sites are unchanged, and the specs on both sides are what make the deletion safe.

Both branches carry a spec for each property, under different titles, and this section previously cited one of each without saying so — recounted here rather than patched:

Property	On this branch	On #266
Only the two pure-Go variants are packaged	It("packages the two pure-Go variants and neither FIPS variant")	same title
packageExtensions() derives from packageFormats rather than restating it	It("derives the extensions from packageFormats rather than restating them")	It("follows packageFormats rather than restating it")

The second title in the earlier version of this section was #266's, quoted as though it were here — so a reader following it into this branch would not have found it. The coverage it pointed at is real and is in both places; only the citation was wrong. Whichever of the two survives the rebase, the property stays pinned; they differ in wording, not in what they assert.

The alternative was for the packaging code to carry a private format list, which is exactly the drift #266's own comment warns about: verifyDistVariants holds release.yml to packageFormats, but it reads the workflows and cannot see the producer.

The unit becomes a template

ExecStart carries a placeholder rendered to /usr/local/bin for the release tarballs — what an unmanaged install prefix means — and to /usr/bin for the packages, which must not write outside /usr. mage build:unit <bindir> renders it for a from-source install.

The file keeps its path, packaging/systemd/openvox-ca.service, rather than becoming an .in: #261 is editing the same file (comment-only, around TimeoutStartSec), and a rename would turn a probable non-conflict into a certain one.

Rendering refuses a template with no placeholder. Without that check, deleting the placeholder or hard-coding a path back into ExecStart would render "successfully" and ship a unit naming the wrong prefix, with nothing at all to say so.

Storage, hardening and the account

StateDirectory= goes; the storage default becomes /etc/puppetlabs/puppet/ssl/ca, the Clojure CA's own layout.

The long-running service keeps a narrow ReadWritePaths=/etc/puppetlabs/puppet/ssl/ca. Under ProtectSystem=strict a filesystem-backend CA with no writable path cannot write a signed certificate, a serial or a CRL — it could not issue at all. The wider parent /etc/puppetlabs/puppet/ssl is granted to the oneshot alone, which needs it to link certs/ca.pem and crl.pem into the tree above the CA. That is the reading of "ReadWritePaths= … on the oneshot only" that makes both halves of the sentence do work.

The service account is puppet, created by the package. OpenVox Server already creates puppet:puppet and chowns the ssl tree to it, so the tree openvox-ca writes is already expected to be owned by that account. openvox-ca must work on a host running neither openvox-agent nor openvox-server, so it creates the account itself: sysusers.d is the declaration, the postinstall calls systemd-sysusers because Debian wires no reliable trigger for that directory, and it falls back to groupadd/useradd where systemd-sysusers is absent. Every branch is idempotent, which matters on upgrade and on a host where Server got there first.

Provisioning

A Type=oneshot, RemainAfterExit=yes unit ordered before the service, running as puppet, with every step guarded on absence — so it is idempotent and a takeover does nothing.

Its [Install] carries only RequiredBy=openvox-ca.service and no WantedBy=. Enabling it at install time therefore installs one symlink and provisions nothing: a CA is created the first time an operator runs systemctl start openvox-ca, not the first time the host reboots after an install. Installing a package is not consent to create a certificate authority. It also means provisioning that fails stops the service rather than letting it start against a half-provisioned directory.

Before the service, not beside it (#275). It writes to storage directly, and the packages default to the filesystem backend, which coordinates no writes across hosts and cannot append to its inventory atomically; two writers there leave an integrity record covering a state that never existed, after which the server refuses to start. The ordering is what makes that unreachable. #189's own per-backend table reaches the same verdict from the other direction — for filesystem and sqlite it says stop the server. A packaged install is single-instance by definition, and nothing in the unit, the oneshot or the docs implies otherwise.

The #189 dependency, and what step 3 does before it lands

Step 3 mints this host's node certificate with openvox-ca generate, which is #189: open, rebased, and not schedulable. Nothing here vendors its code and nothing blocks on it.

The choice made: probe for the subcommand, and fail with the reason named. Not a silent skip, and not an assumption that it is present.

The reasoning is that a skip is not actually the gentler option. Without a node certificate there is nothing for tls_cert to point at, and openvox-ca refuses to start on a non-loopback address without TLS. So the choice is not between "works" and "fails" — it is between a failure in systemctl status openvox-ca-first-boot that names the missing subcommand and the issue number, and a failure two units later that says only that plain HTTP on a non-loopback address is refused. The first is the one that tells an operator what to do. The CA itself is bootstrapped and intact either way, and the message says so, along with how to supply a certificate out of band.

Once #189 lands, the mint path works with no change here: the probe simply finds the subcommand.

The probe reads the command listing, and the obvious probes do not work. Both openvox-ca generate --help and openvox-ca help generate exit 0 on a build with no such subcommand — cobra answers --help from the root command before it validates arguments, and an unknown help topic is not an error to it. A probe on either would report every build as capable and turn the check into a no-op, which is worse than not checking. Verified in both directions against a real build rather than by inspection.

Flags used, from #189 as documented on that branch: --cadir, --certname, --ttl 43800h (the built-in leaf default), --key-out and --cert-out. --cert-out rather than redirecting stdout — it refuses a path that already exists and writes nothing on failure, where a shell redirection creates the file either way.

The mage build:packages existence guard

Added here, as #266's driver asked. verifyMageTargets is wired into mage dev:check and asserts that every mage target named outside Go resolves: the ones in requiredMageTargets (today, build:packages), and every statically resolvable mage <target> in the workflows.

release.yml names the target as a string, so deleting or renaming it would compile cleanly, pass every test, and fail at tag time — after the tag is pushed, and while container-images.yml and helm-chart.yml publish their images anyway, including the mutable latest tags. On #266 this check would have been red from the moment it was written and so indistinguishable from a broken guard; here it is green on arrival.

It has two floors, because a guard made entirely of membership tests passes vacuously when its parse returns nothing. The magefile parse must find build:dist; and a workflow whose text invokes mage must yield at least one invocation from its parsed run: steps. The second is matched with the invocation pattern rather than by searching for "mage " — "image " contains that substring, and the first version of the floor fired on container-images.yml, which is the other way a floor fails: it stops being believed.

The invocation scan reads parsed run: steps with comment lines stripped, not raw file bytes. The workflows carry long comments quoting the very commands the check looks for, including mage build:packages itself, so a byte search would be satisfied by prose describing a step instead of by the step.

Two things worth a maintainer's eye

Port 8141 is in this PR, by Chris's ruling on #250 — defined once here so #251 and #252 inherit it rather than each setting it. (This paragraph previously recorded the opposite as an assumption; the ruling settled that the value lives here, and the mechanism below is mine.)

The packages ship /etc/puppet-ca/config.yaml, and port: 8141 is one of the four keys in it (cadir, tls_cert, tls_key, port — the first three are what a packaged install cannot start without at all). Three things make a configuration file the right mechanism for the port rather than merely a tidy one:

• A flag in the unit or a variable in the environment would not be a default. The server resolves the port as file → environment → flag: applyServerEnv overwrites the parsed file, and the Changed("port") branch overwrites that. So --port 8141 in ExecStart, or PUPPET_CA_PORT in a drop-in, would beat the operator's own config file and ignore an edit made in the obvious place, silently. A default has to sit at the layer an operator can override.
• The reason the config file was out of scope does not survive contact with nfpm. The issue excluded it because "it is where the two formats diverge" — and with nfpm they do not. type: "config|noreplace" is one declaration that becomes a dpkg conffile and rpm %config(noreplace). Both halves are now asserted against built packages rather than against nfpm's source: a spec reads the .deb's control archive and requires conffiles to list /etc/puppet-ca/config.yaml and nothing else, and another reads the .rpm's file flags for %config(noreplace). Neither apt upgrade nor dnf update overwrites an operator's edit, on either format.
• The tarball channel is untouched. It ships no configuration file and the shared unit gains no port, so tarballs, containers and CA-only hosts stay on the binary's 8140. Templating the port the way @BINDIR@ templates the path would have moved both channels — which is the trap this deliberately avoids.

The file sets those four and no more, on purpose: what a packaged CA does should otherwise be the documented behaviour of the binary, not a second set of values to look up. It is 0640 root:puppet because it is the file that will hold credentials once a backend needs them (etcd_password, an inline OpenBao role_id), and getting the mode right now avoids a permissions change on upgrade.

Specs pin both halves, and the packaged-port one is mutation-checked: setting the file to 8140 fails it on the expected value rather than incidentally.

The documentation enumerates from git ls-files, not from a directory walk. docs/ legitimately holds untracked working files, and a walk would package them — an artefact whose contents depend on the tree it happened to be built in, complete in CI and carrying a maintainer's private drafts when built on a laptop, with nothing in the build output to distinguish the two. There is now a spec for that, premise included: it writes an untracked file under docs/, checks git agrees it is untracked, and requires it to be absent from the staged tree while its tracked neighbour is present.

The packages are byte-reproducible when SOURCE_DATE_EPOCH is set. This came out of writing the spec for it. nfpm stamps everything it generates from that variable, so the binaries, both units, the config file and all the metadata were already reproducible — but the documentation goes in as a tree of real files, and a tree entry keeps the mtime it finds on disk, which for files stageDocTree wrote moments earlier is "now". The doc tree was the only thing between these packages and a verifiable rebuild, so stageDocTree now stamps what it stages with SOURCE_DATE_EPOCH when that is set, and leaves it alone when it is not — the variable is the caller saying this build is meant to be reproducible, and inventing a timestamp otherwise would put a wrong date on installed documentation. #266's release job should export it; noted in docs/development/releasing.md.

The spec asserts both halves, because "two builds agree" is true on its own of two builds inside the same second — it passed with the variable unset — so it is paired with a build at a different epoch, which must differ.

What the review rounds produced, and one thing worth carrying forward

Six Review Council rounds plus a full-council pass on the finished branch.
Every finding accepted was real; two remedies were declined with public
reasoning and evidence — chmod -h, which coreutils 8.32 on RHEL/Rocky 9
rejects outright (and which || warn would have swallowed, leaving the mode
unset), and the account-provisioning branch matrix, which needs a real
/etc/passwd and belongs to #254.

The last pass produced three things worth naming, all now fixed. A HIGH:
systemctl enable for the provisioning oneshot sat behind the running-systemd
guard, so a chroot, image build or mounted-root install enabled nothing — and
since the oneshot's only [Install] directive is RequiredBy=, it then never
ran. Enabling is on-disk symlink work; it is now outside that guard, with a
marker under /var/lib/openvox-ca recording the first successful enable so an
upgrade cannot re-enable a unit the operator disabled, and a reinstall can. A
set of prose corrections where a comment or a paragraph described something
the code no longer did — "a takeover does nothing at all" chief among them,
which is wrong in the direction that matters. And the coverage above.

Two coverage findings on this PR turned out to matter, and they matter in
different ways — the pair is the argument, not either one alone.

• Build.Unit's success path was untested. Writing that test exposed a live
  defect immediately: the target printed the untrimmed bindir, reporting
  ExecStart=/opt/bin//openvox-ca for a unit it had rendered correctly.
• resolve_certname's fallback tiers were untested. Writing those tests exposed
  nothing at the time. What they produced was a technique — stubbing
  hostname and the CA binaries on PATH — and one round later that seam is what
  made the certname-collision HIGH reachable at all: a bug where a host named ca
  silently ended up serving its own leaf certificate from certs/ca.pem, the path
  every agent reads the CA certificate from.

The second is the harder one to argue for at triage, because at the moment you
take it there is nothing to show. That is exactly why it is worth stating: the
useful claim is not "coverage findings sometimes find bugs" but the finding
that returns nothing today is often the one that makes tomorrow's finding
possible. Neither of these would have survived a triage pass that asked which
findings sounded interesting.

Not in scope

• Storage backend settings beyond the defaults — Debian package: debconf, ucf, policy-rc.d-aware start, and a purge question #251 and EL package: presets, %config(noreplace), and an environment file #252. (The configuration file itself is in scope; see the scope note at the top. It ships with the four settings a packaged install cannot start without or would get wrong: cadir, tls_cert, tls_key and port.)
• A CI leg that installs a built package and asserts its layout — CI install-and-verify legs for the deb and rpm packages #254. Publish deb and rpm packages from the release workflow #266 says plainly that nothing verifies the packages the way verify-dist-artifact verifies the tarballs, and that remains true.
• FIPS packaging, which would need a separate openvox-ca-fips package name with Provides/Conflicts. Not designed in, not designed out.

🤖 Generated with Claude Code

[bootc]

Thanks — worked through all eleven at f74ea058f986. I checked each against the code before accepting it, and all eleven were real. Dispositions below; nothing is declined.

🟠 HIGH — mage build:packages has no test

Fixed. The point stands and it was the right thing to lead with: this is the branch's deliverable and the only code exercising nfpm end to end, and I had verified it by unpacking a .deb by hand — which leaves nothing behind for the next person.

• checkPackagingInputs, extracted from Packages() so both emptiness branches are reachable with synthetic input. The real lists are never empty, so a test that could not supply its own inputs would be asserting nothing. Both error messages are asserted, not merely that an error occurred.
• buildVariantPackages end to end against the real packaging/nfpm.yaml, from a staged tarball, with no compilation — the fixtures are the "binaries", which is what makes it cheap.
• The .deb's payload is read back (a small ar reader in the test file) and asserted: every installed path with its mode, the doc tree, the ssl directories. Without reading the archive, a packaging test can only assert that a file appeared with the right name, which a package containing nothing satisfies.
• One assertion earns its place beyond coverage: the fixture tarball deliberately carries the unit rendered for /usr/local/bin, so shipping the tarball's copy instead of re-rendering for /usr/bin now fails. I mutation-tested that one — swapping renderUnit(packageUnitBindir) for renderUnit(tarballUnitBindir) fails on the ExecStart=/usr/bin/openvox-ca assertion specifically, not on something incidental.

Named separately from #266's verifyPackageSetNonEmpty on purpose: that one takes workflow bytes and does the gate-side half. Two same-named functions across two branches collide at rebase; two differently-named ones are two checks.

🟡 Certificate name reaches a path unsanitised (CWE-22)

Fixed, and the diagnosis was exactly right. I confirmed it rather than taking it on trust — driving the old script with ../../../etc/x.example.com produced:

--cert-out .../ssl/certs/../../../etc/x.example.com.pem

The severity calibration is right too, and worth restating: the sandbox bounds it (unprivileged, ProtectSystem=strict, ReadWritePaths=/etc/puppetlabs/puppet/ssl) but does not bound it to nothing, because the CA's own ca_crt.pem lives inside that subtree.

is_safe_certname is an allow-list, not a search for .. — the way to be sure a string cannot leave a directory is to permit only characters that cannot express leaving one. It also refuses a leading dash, so a name can never be read as an option by the commands it is passed to. Every source is checked, including the two the original skipped entirely (OPENVOX_CA_CERTNAME and puppet.conf, which bypassed is_usable_fqdn altogether). An unsafe explicit answer stops rather than falling through: it is a mistake in a hand-written file, and provisioning under a different name would hide it.

🟡 Stale docs (×4)

All four fixed: the cadir note in docs/configuration.md (the unit's default and the example are now the same path, so the contrast it drew is backwards), the two sudo -u puppet-ca recipes whose own justification is "matching the User= in the unit", and the README documentation-table row.

One deliberate non-change: the cadir: /var/lib/puppet-ca examples elsewhere in docs/storage-backends.md are left alone. They are one valid choice of directory rather than a claim about the unit's default, and docs/systemd.md now states that cadir and ReadWritePaths= must name the same place.

🟡 requiredMageTargets comment misstates release.yml

Fixed, and this was the most useful finding after the traversal — it is my own prose drifting from the code, which is harder to see than a code defect. grep -c build:packages .github/workflows/release.yml is 0 in this tree, and docs/development/releasing.md does not mention packages at all; both are #266's. The comment asserted them in the present indicative, so a maintainer checking it would conclude the guard's justification was wrong rather than early. It now says the entry is deliberately ahead of its caller, and says why the workflow scan cannot substitute for it today: the scan can only see callers that exist.

🟡 stageDocTree floor untested · 🟡 Build.Unit validation untested · 🔵 DescribeTable

All fixed. The floor is extracted as checkDocTreeFloor so it can be exercised without renaming directories in the working tree — the enumeration is the hard part to arrange, the check over it is not — and both it and the happy path are now tested. Build.Unit's rejection path is tested; the asymmetry with Build.DistVariant was not explained by cost, as the finding said. The two in-test loops are now DescribeTable/Entry per AGENTS.md:120.

🟡 first-boot has no automated test

Partly addressed, and honestly labelled. The security fix above is exercised — I drove the real script with stub binaries against a scratch OPENVOX_CA_SSLDIR/OPENVOX_CA_BINDIR and confirmed it discriminates: the old script accepts the traversal name and writes to the traversed path, the new one exits 1. That is how I know the fix works rather than merely looks right.

That check is not committed as a CI leg, and the rest of the script — account creation, the CA bootstrap, the four-way $NAME fallback, the marker-file path — still has no automated coverage. It stays #254's, as the PR body already said. The finding is right that the script was designed to be driven this way; what is missing is somewhere to run it, not the ability to.

---

Local state at f74ea058f986: mage dev:check 0 issues, mage test:magefile and mage test:unit green.

[bootc]

Rebased onto current main at 95a50555db33, now that #189 has merged.

Step 3's mint path is exercised rather than assumed. Built both binaries from this branch on top of main and ran the real first-boot against a scratch ssldir:

subject=CN=ca.example.com, issuer=CN=Puppet CA: ca.example.com
X509v3 Subject Alternative Name: DNS:ca.example.com
notBefore 2026-08-30, notAfter 2031-08-30      (the 43800h asked for)
private key mode 0600; cert and key public keys identical
openssl verify against the bootstrapped ca_crt.pem: OK

A second run adopts what the first minted rather than minting again. So the open question in the PR body — what does step 3 do before #189 lands — is closed by #189 landing, and the answer is that it mints.

The capability probe stays, but it was naming the wrong cause. It cited openvox-ca#189, which now points at a merged PR and reads as a missing feature. Every build shipping this script has the subcommand, so what the probe catches from here is a mismatched pair — an older binary left on PATH by a tarball install, or a partial upgrade — which a package cannot prevent and which produces the same unusable install. The message says that now, and names the binary it actually probed. The probe itself is unchanged: it reads the root command listing, because generate --help and help generate both exit 0 on a build without the subcommand.

One thing operators will now see that needs explaining. The mint prints openvox-ca generate's warning that the filesystem backend coordinates no writes across processes and that the server should be stopped first. On a first boot that is expected, and it is exactly why this unit is ordered Before=openvox-ca.service: at the moment it runs there is no server to stop. Unexplained it looks like provisioning reporting that it should not have run, so docs/systemd.md now says when that warning is benign and when it is not.

Rebase checked for silent reverts — git diff origin/main...HEAD shows only my own deletions, and #261's TimeoutStartSec comment on the shared unit survived intact.

mage dev:check 0 issues, test:magefile green. Remaining open question for @bootc is unchanged: whether port 8141 belongs to #251/#252's config file, which is recorded in the body as an assumption.

[bootc]

Worked through all 22 at 00561e5eaf97 (rebased onto current main). Each was checked against the code before being accepted. All 22 were real. 21 are fixed; one is deferred with a reason.

🔴 CRITICAL — packaged install cannot start

Fixed, and the finding understated it. Confirmed first: there is no built-in default for CADir (it is absent from the defaults block in config.go), and both runtime.go:83 and main.go:358 refuse without one.

The cause is worth recording because it is a shape rather than an oversight: the config file was created in the previous commit to hold the port, and the port was the thing under discussion, so it held the port and nothing else.

Fixing cadir exposed a second instance of the same defect, which the review did not find. With cadir set the server got one step further and exited again — it binds 0.0.0.0 and refuses plain HTTP on a non-loopback address, so tls_cert/tls_key were missing for exactly the same reason. Fixing only what was reported would have left the package still unbootable and this comment claiming otherwise.

The credential provisioning mints is named for the host, which no shipped file can know. So the config names two stable paths (certs/openvox-ca-server.pem and the matching key) and the oneshot links them to this host's credential — which also makes re-minting under a corrected name a relink rather than a config edit.

Verified by running it, not by reading it. Provision into a scratch tree, start with the shipped config and no flags:

readiness endpoint:               HTTP 200
GET /puppet-ca/v1/certificate/ca: -----BEGIN CERTIFICATE-----
certificate presented:            subject=CN=ca.example.com
                                  issuer=CN=Puppet CA: ca.example.com

Specs now assert every setting the server refuses to start without, and cross-check that the paths the config names are ones the script actually links — if those drift, the service starts and exits.

🟠 HIGH (5) — all fixed

• Config owned by an account that does not exist at unpack time. Correct, and my existing postinstall chown did not cover it — it handled the ssl tree only. It now sets the config's owner too, after sysusers has created puppet.
• Removal behaviour undocumented. Added, including the consequence that matters more than the reassurance: removing the package does not decommission the CA. The key stays readable by anything running as puppet until someone deletes it deliberately.
• Unguarded chmod breaks guarded-on-absence. Right on both counts — it would change a mode this package did not set, or fail under set -e and abort provisioning that had nothing to do. Directories are now created with their final mode and existing ones are left alone.
• renderUnit's guard had no failure test. Split as renderUnitFrom so the branch is reachable without editing the real unit.
• Certname allow-list had no coverage. Fixed, and it runs the shipped shell function rather than a Go restatement — a table over is_safe_certname and is_localhost_name driven through sh. Mutation-checked: weakening the allow-list to accept everything fails every rejection entry.

🟡 MEDIUM (11) — 10 fixed, 1 deferred

Fixed: the puppet uid trust-boundary rationale (see below); chown now --no-dereference on both paths; step 3's two hard-failure paths documented; the nfpm header that still said the config file was not in this package; docTreeEntries consumed by fixed index — a fourth path really would have been silently unpackaged; verifyMageTargets now globs the workflow directory with a floor instead of a hand-maintained list of five; the unresolved-name marker now written only when this run created the CA; postinstall gained a maintainer-script argument guard; Build.Unit's success path; extractTarGz's traversal refusal.

On the shared puppet uid: the finding was right that the consequence was unstated, and right that it is a documentation matter rather than a design one — Chris ruled on the account. The docs now say plainly that on a host also running OpenVox Server the CA's private key sits inside Server's trust boundary, what reduces that (key encryption at rest; better, not co-locating), and why a private puppet-ca account was rejected.

Deferred: the account-provisioning branch matrix has no test. It needs a container to exercise systemd-sysusers against a real /etc/passwd, which is #254's leg. I would rather say that than add a test that stubs systemd-sysusers and asserts my own stub was called.

🔵 LOW (5) — all fixed

private_keys is created 0750 rather than narrowed afterwards; the README no longer calls the template the shipped unit; the OPENVOX_CA_SSLDIR/BINDIR seam is now genuinely exercised by the specs above, so its comment is true; the duplicated localhost list is one function — the copies had already drifted, so *.localdomain was rejected as an FQDN and accepted as a short hostname; preremove stops the oneshot as well as disabling it.

One defect found while fixing, not in the review

Writing Build.Unit's success-path test exposed what its absence had hidden: the message printed the untrimmed bindir, so a trailing slash reported ExecStart=/opt/bin//openvox-ca for a unit that had been rendered correctly. Cosmetic, but it is the kind of thing a missing test is for.

---

mage dev:check 0 issues, test:magefile and test:unit green. Rebased onto main and checked for silent reverts — only my own deletions.

[bootc]

Worked through all 11 at daf7a9b3f65c (rebased onto current main). Each checked against the code first. All 11 were real; all 11 are fixed. No declines this round.

🟠 HIGH (3)

RPM payload never inspected. Correct, and the asymmetry was worse than "one format untested": the PR body verified nfpm's config|noreplace rpm mapping by citing nfpm's source while verifying the deb half empirically. The rpm's payload is now read — with the reader nfpm already depends on, rather than a second hand-rolled header parser where the risk is getting it subtly wrong and asserting against the mistake — and every property asserted of the deb is asserted of it, plus RPMFILE_CONFIG/RPMFILE_NOREPLACE on the config file and their absence everywhere else.

Node-certificate path untested. Fixed: has_generate_subcommand in both directions, ensure_node_certificate adopting a pair, refusing half a credential either way round, and stopping when the build cannot mint. The probe reads the root command listing precisely because generate --help exits 0 on a build without the subcommand — the subtlety that was unprotected is now the thing two specs pin.

Lifecycle scripts untested. Fixed for everything that does not need a container: which arguments act at all, and that an upgrade does not re-enable a unit the operator disabled. External commands are stubs that log their arguments, so nothing touches this machine's accounts.

You were right to tell me to check this against my earlier deferral rather than reuse it. The earlier deferral does not cover this, and the finding says so plainly: the harness to drive these already existed in the same file. What remains deferred is genuinely narrower — whether systemd-sysusers creates the account it is handed needs a real /etc/passwd, which is #254. A test that stubbed systemd-sysusers and then asserted the stub had been called would be asserting its own fixture.

🟡 MEDIUM (5) — all fixed

resolve_certname's precedence now has end-to-end coverage, including that an unsafe explicit answer stops rather than falling through. Build.Unit's success path goes through writeRenderedUnit against a temp dir instead of the repository's dist/.

checkDocTreeFloor is the "guard that cannot fail" class, and it was exactly that. It required LICENSE and README.md — the two entries that cannot stop matching on their own — and never checked docs/, the one whose pathspec can. A renamed docs/ passed the floor and packaged the two files beside an empty tree. Verified before accepting: the floor's own error message named docs while checking only stageDocTreeFloor.

Both documentation findings fixed — and a third drift I found by reading around them rather than only at them: the certname paragraph still described the unresolved-name marker as written on every unresolved name, when I changed it last round to be written only when that run created the CA. That one was mine, from the previous round's fix, and no reviewer caught it.

🔵 LOW (3) — all fixed

preremove accepted an argument dpkg cannot send. Confirmed against dpkg's actual argument set rather than taking it on trust: purge goes to postrm, never prerm. The branch was unreachable and read as though purge were handled here, when the file that handles it is postremove. Accepted set is now exactly dpkg's remove and rpm's remaining-count 0, with a spec pinning it.

The ownership fixups reported nothing when they failed. Both sent stderr to /dev/null and continued, so a package could install "successfully" and leave a service unable to read its own configuration with nothing saying why. They now warn, naming the consequence — and remain non-fatal, because a maintainer script that exits non-zero leaves the package half-configured, which is worse to hand an operator than a working install with a loud warning.

---

On the prose class, since this is the third round it has appeared: the counter-check I have adopted is to re-read the surrounding paragraphs of every file touched, not just the diff, and to give the PR description the same pass — nothing diffs it. That is what turned up the fourth drift above.

go.mod gains go-rpmutils (test-only) plus two indirects already in the graph via nfpm. Re-verified rather than assumed, because the PR body makes a claim about it: go list -deps ./cmd/openvox-ca is still byte-identical to main at 864 packages.

mage dev:check 0 issues; test:magefile and test:unit green.

[bootc]

Worked through all of these at 3c072b89817f. Each checked against the code first. All were real; all are fixed. One remedy is deliberately not the one recommended — reasoning below, with evidence.

Two of the findings are one finding. "Build.Packages has no test of its own" and "Build.Packages(), the actual target, is never invoked by a test" both cite magefile.go:1487, both from Tester, and describe the same gap. Answering it once rather than inventing a second response: so eight distinct findings, not nine.

🟠 HIGH — resolve_certname tiers 2–4 and write_unresolved_marker unexercised

Fixed, and the finding's read of why was the useful part — a spec had no way to control what hostname returns, so tiers 2–4 were unreachable rather than overlooked. A hostname stub on PATH (the technique the maintainer-script specs already use) unlocks all of them: the dotted answer, the short-hostname warning, a localhost form correctly refused as an FQDN, and the localhost fallback.

write_unresolved_marker now has coverage too, and it matters more than the tiers: it is the only thing that says a CA is unreachable while systemctl status says it is healthy. Its assertions include that the marker states what re-minting costs, since its recovery steps destroy a CA.

Covering it exposed something the finding did not mention: the function sat below the -- Run -- banner, so it was part of the executable section rather than the definitions, and no spec could have reached it however it was written. Moved up with the other step functions.

🟡 MEDIUM — the chmod symlink hole: diagnosis accepted, remedy declined

The diagnosis is right and is fixed. Plain chmod follows a symlink, so postinstall was unguarded exactly where the adjacent chown --no-dereference was guarded.

chmod -h is not the fix, and applying it would have been worse than shipping the hole. Verified in a container rather than reasoned about:

$ docker run --rm --platform linux/amd64 rockylinux:9 chmod --version | head -1
chmod (GNU coreutils) 8.32
$ ... chmod -h 0640 /tmp/link
chmod: invalid option -- 'h'          exit 1

RHEL and Rocky 9 are primary targets here, and Debian bookworm's coreutils 9.1 rejects it too. The call site is chmod … || warn, so on those platforms the rejection is swallowed into a warning and 0640 is never applied at all — trading a symlink hole for not setting the mode on the most common targets.

Applied instead: the [ -L "$CONFIG" ] check the finding itself proposed and then talked itself out of ("rather than the more involved [ -L … ] check originally proposed"). It refuses when the path is a symlink, says why, and tells the operator what to set. POSIX, portable to every target, independent of coreutils vintage. -L is tested before -f because -f follows the link.

🟡 MEDIUM — the preremove table cannot tell its two cases apart

Fixed, and this is the one I would have most wanted caught. shouldAct was declared and never read; the body asserted only that the script exited 0, which a stubbed systemctl and a script wrapping every call in || : make true whichever way it behaves. Six entries, three of each polarity, none distinguishable — a table named "acts only on a real removal" that would have passed had preremove done nothing at all, or stopped the service on every upgrade.

It now reads shouldAct and asserts on the call log. Making the positive half assertable required the systemd runtime path to be overridable: the scripts guard their systemctl calls on /run/systemd/system, so on a host without systemd they correctly do nothing and that half could not be checked at all. Same seam and same caveat as first-boot's roots — nothing in the packages sets it, and neither package manager passes an environment.

Mutation-checked, because a table that could not fail once should not be trusted twice: making preremove accept upgrade and 1 fails exactly the two upgrade entries and nothing else.

🟡 MEDIUM — remaining

Build.Packages itself now has a spec for the promise that matters: that it refuses when the tarballs it consumes are absent, rather than silently rebuilding. AGENTS.md's command table and the Build namespace's inline listing both gained build:packages and build:unit.

The PR body contradiction is fixed in both places, which is the part worth noting: correcting only the "Not in scope" bullet would have left the same false claim standing in the opening sentence. The body now carries a scope note explaining that the exclusion did not survive Chris's ruling or contact with nfpm — one config|noreplace declaration renders as a dpkg conffile and rpm %config(noreplace), so the divergence the exclusion assumed does not exist here.

🔵 LOW

docs/systemd.md step 4 described the symlinks backwards: they live in the ssl tree and point into the CA directory, not the reverse.

---

Verified after the changes: provisioning still runs end to end against real binaries — bootstrap, mint, all four links including the serving credential, no marker when the name resolves, and an idempotent re-run that adopts. mage dev:check 0 issues; test:magefile green.

The Dependabot alert on this push (google.golang.org/grpc) is pre-existing and not from this branch: v1.82.1 on main and v1.82.1 here.