fix(packaging): preserve operator TLS cert across the cert-shipping -> rc.10 upgrade (#598)

The #596 generate-if-absent fix protects rc.10->onward upgrades, but the
one-time transition FROM a release that shipped the cert in its payload
(<= rc.9) removed the operator's cert entirely: rc.9 owned
/etc/openwatch/tls/cert.pem, rc.10 does not, so the package manager
reclaimed the orphaned file AFTER rc.10's provisioning had already run and
skipped it. RPM left it missing (service can't start TLS); DEB regenerated
a demo (operator cert lost). Found via the Stage-3 gap-closure RPM/DEB
install-transition tests.

Fix:
- RPM: declare the cert/key paths %ghost so rpm tracks them with no payload
  content and does not reclaim the operator's file on the transition.
- DEB: preinst stashes an existing cert/key to .dpkg-bak on upgrade (before
  dpkg removes the orphan); postinst restores it before provisioning.
- AC-22 updated: assert %ghost (flag 'g', no content) on RPM and the
  preinst/postinst preserve dance on DEB; upgrade-container-test.sh now
  asserts an operator cert survives the rpm -U upgrade.
- release-package-build spec v1.3.0 (C-05 + AC-22 extended).

Verified in containers: genuine published rc.9 -> rc.10 preserves the
operator cert+key on both RPM (%ghost) and DEB (preinst/postinst); fresh
install still generates a demo; steady-state reinstall preserves.

Author: remyluslosius

CHANGELOG.md

@@ -53,7 +53,9 @@ overwriting an operator's TLS certificate.
   certificate is now generated at install time only when the TLS files are
   absent (the same generate-if-absent model already used for the server's
   identity keys), so a certificate you put in place survives upgrades untouched
-  (#596).
+  (#596). This also covers the one-time upgrade from an earlier build that did
+  ship the demo certificate (rc.9 and before): your certificate is preserved
+  rather than removed during that transition too (#598).
 
 ---
 

packaging/deb/postinst

@@ -16,6 +16,16 @@ if [ "$1" = "configure" ]; then
     # failure should abort configure rather than leave an unbootable service.
     /usr/lib/openwatch/provision-identity-keys.sh
 
+    # Restore an operator TLS cert/key that preinst stashed before dpkg removed
+    # the orphaned demo cert during an upgrade from a cert-shipping release
+    # (<= rc.9). The restore runs BEFORE provisioning so the operator's file
+    # wins over a freshly generated demo.
+    for f in cert.pem key.pem; do
+        if [ -f "/etc/openwatch/tls/.$f.dpkg-bak" ]; then
+            mv -f "/etc/openwatch/tls/.$f.dpkg-bak" "/etc/openwatch/tls/$f"
+        fi
+    done
+
     # Provision a demo TLS cert + key (generate-if-absent). Not shipped in the
     # payload so an upgrade never overwrites an operator's replacement cert.
     /usr/lib/openwatch/provision-tls-cert.sh

packaging/deb/preinst

@@ -11,6 +11,20 @@
 
 set -e
 
+# On upgrade FROM a release that shipped the demo TLS cert in its payload
+# (<= rc.9), dpkg removes that now-orphaned cert/key during unpack because
+# rc.10+ no longer own those paths. Stash a copy first so postinst can restore
+# an operator's replacement certificate. rc.10+ generate the cert in postinst
+# and never ship it, so a steady-state upgrade has nothing to remove and this
+# is a harmless round-trip. (The RPM uses %ghost to the same end.)
+if [ "$1" = "upgrade" ]; then
+    for f in cert.pem key.pem; do
+        if [ -f "/etc/openwatch/tls/$f" ]; then
+            cp -p "/etc/openwatch/tls/$f" "/etc/openwatch/tls/.$f.dpkg-bak" 2>/dev/null || :
+        fi
+    done
+fi
+
 getent group openwatch >/dev/null || groupadd --system openwatch
 getent passwd openwatch >/dev/null || \
     useradd --system --gid openwatch \

packaging/rpm/openwatch.spec

@@ -145,7 +145,12 @@ systemctl daemon-reload || :
 # TLS dir ships empty (0750); %post generates the demo cert/key into it
 # generate-if-absent. The cert/key files are intentionally NOT packaged, so
 # a package upgrade cannot revert an operator's replacement certificate.
+# They are declared %ghost — rpm tracks the paths (no payload content, nothing
+# laid down or verified) so that on an upgrade FROM a release that DID ship the
+# cert (<= rc.9), rpm does not reclaim/erase the operator's file as an orphan.
 %dir %attr(0750, root, openwatch)   /etc/openwatch/tls
+%ghost %attr(0644, root, openwatch)      /etc/openwatch/tls/cert.pem
+%ghost %attr(0600, openwatch, openwatch) /etc/openwatch/tls/key.pem
 %config(noreplace) %attr(0640, root, openwatch) /etc/openwatch/openwatch.toml
 %config(noreplace) %attr(0640, root, openwatch) /etc/openwatch/upgrade.conf
 %attr(0644, root, root)             /etc/systemd/system/openwatch.service

packaging/tests/package_test.go

@@ -659,20 +659,30 @@ func TestTLS_PostInstallProvisions(t *testing.T) {
 			}
 		}
 
-		// Payload ships the empty tls dir + the helper, but NEVER the cert/key
-		// files themselves (that is what made upgrades clobber operator certs).
+		// Payload ships the empty tls dir + the helper, and carries NO real
+		// cert/key content. On RPM the cert/key paths are declared %ghost so
+		// rpm tracks them (flag 'g') without laying down content — this stops a
+		// package upgrade FROM a cert-shipping release (<= rc.9) from reclaiming
+		// the operator's file as an orphan, while still never shipping a cert.
 		const tlsDir = "/etc/openwatch/tls"
-		rpmFiles := rpmQuery(t, rpm, "[%{FILENAMES}\n]")
-		if !strings.Contains(rpmFiles, helperPath) {
+		if !strings.Contains(rpmQuery(t, rpm, "[%{FILENAMES}\n]"), helperPath) {
 			t.Errorf("RPM payload missing the TLS helper %s", helperPath)
 		}
-		if !strings.Contains(rpmFiles, tlsDir) {
+		rpmGhost := rpmQuery(t, rpm, "[%{FILENAMES} %{FILEFLAGS:fflags}\n]")
+		if !strings.Contains(rpmGhost, tlsDir) {
 			t.Errorf("RPM payload missing the %s directory", tlsDir)
 		}
-		if strings.Contains(rpmFiles, "tls/cert.pem") || strings.Contains(rpmFiles, "tls/key.pem") {
-			t.Errorf("RPM payload MUST NOT ship the TLS cert/key; got:\n%s", rpmFiles)
+		for _, p := range []string{"cert.pem", "key.pem"} {
+			re := regexp.MustCompile(`/etc/openwatch/tls/` + regexp.QuoteMeta(p) + `\s+\S*g\S*`)
+			if !re.MatchString(rpmGhost) {
+				t.Errorf("RPM tls/%s MUST be declared %%ghost (flag 'g', no payload content); file list:\n%s", p, rpmGhost)
+			}
 		}
 
+		// DEB has no %ghost: the cert/key are not in the payload, and preinst
+		// stashes an operator's cert before dpkg removes the orphan on upgrade,
+		// while postinst restores it. So the cert/key are NEVER real payload
+		// files in either format.
 		debFiles := debContents(t, deb)
 		if !strings.Contains(debFiles, helperPath) {
 			t.Errorf("DEB payload missing the TLS helper %s", helperPath)
@@ -684,6 +694,16 @@ func TestTLS_PostInstallProvisions(t *testing.T) {
 			t.Errorf("DEB payload MUST NOT ship the TLS cert/key; got:\n%s", debFiles)
 		}
 
+		// DEB preinst backs up an operator cert on upgrade; postinst restores it.
+		preinst := readPackagingFile(t, dir, "deb", "preinst")
+		if !strings.Contains(preinst, ".dpkg-bak") || !strings.Contains(preinst, `"$1" = "upgrade"`) {
+			t.Errorf("deb/preinst must back up the TLS cert/key (.dpkg-bak) on upgrade")
+		}
+		postinst := readPackagingFile(t, dir, "deb", "postinst")
+		if !strings.Contains(postinst, ".dpkg-bak") || !strings.Contains(postinst, "mv -f") {
+			t.Errorf("deb/postinst must restore the preserved TLS cert/key (.dpkg-bak)")
+		}
+
 		// The build scripts no longer stage a demo cert into the payload.
 		for _, bs := range []string{"rpm/build-rpm.sh", "deb/build-deb.sh"} {
 			b, err := os.ReadFile(filepath.Join(dir, "packaging", bs))
@@ -697,6 +717,16 @@ func TestTLS_PostInstallProvisions(t *testing.T) {
 	})
 }
 
+// readPackagingFile reads packaging/<sub>/<name> as a string.
+func readPackagingFile(t *testing.T, appDir, sub, name string) string {
+	t.Helper()
+	b, err := os.ReadFile(filepath.Join(appDir, "packaging", sub, name))
+	if err != nil {
+		t.Fatalf("read packaging/%s/%s: %v", sub, name, err)
+	}
+	return string(b)
+}
+
 // readDebControlScript extracts a named maintainer script from a DEB
 // using `dpkg-deb --ctrl-tarfile` piped to `tar`. Returns the script body
 // as a string.

packaging/tests/upgrade-container-test.sh

@@ -48,6 +48,13 @@ echo "### install OLD package (release 1)"
 rpm -i --nodeps /rpms/old/openwatch-*.rpm
 echo "OPENWATCH_DATABASE_DSN=$DSN" > /etc/openwatch/secrets.env
 
+# Simulate an operator who replaced the demo TLS cert with their own. It MUST
+# survive the upgrade — including the transition from a release that shipped
+# the cert in its payload (the cert/key are %ghost in current packages, so rpm
+# must not reclaim the operator's file). release-package-build C-05 / AC-22.
+echo "OPERATOR-TLS-CERT-SENTINEL-DO-NOT-CLOBBER" > /etc/openwatch/tls/cert.pem
+echo "OPERATOR-TLS-KEY-SENTINEL"                 > /etc/openwatch/tls/key.pem
+
 echo "### bring DB to head, then roll back the head migration ($HEAD_VER) to simulate the prior version"
 set -a; . /etc/openwatch/secrets.env; set +a
 openwatch migrate >/dev/null
@@ -76,8 +83,12 @@ fail=0
 ls /var/lib/openwatch/backups/openwatch-pre-upgrade-*.sql >/dev/null 2>&1 || { echo "FAIL: no pre-upgrade backup"; fail=1; }
 grep -q "stop openwatch.service"  /tmp/systemctl.log || { echo "FAIL: service not stopped"; fail=1; }
 grep -q "start openwatch.service" /tmp/systemctl.log || { echo "FAIL: service not restarted"; fail=1; }
+grep -q "OPERATOR-TLS-CERT-SENTINEL-DO-NOT-CLOBBER" /etc/openwatch/tls/cert.pem 2>/dev/null \
+    || { echo "FAIL: operator TLS cert was NOT preserved across the upgrade (cert.pem=$(head -1 /etc/openwatch/tls/cert.pem 2>/dev/null || echo MISSING))"; fail=1; }
+grep -q "OPERATOR-TLS-KEY-SENTINEL" /etc/openwatch/tls/key.pem 2>/dev/null \
+    || { echo "FAIL: operator TLS key was NOT preserved across the upgrade"; fail=1; }
 if [ "$fail" -eq 0 ]; then
-    echo "RESULT: PASS - the package upgrade migrated $PREV_VER -> $HEAD_VER with a backup and a stop/start"
+    echo "RESULT: PASS - the package upgrade migrated $PREV_VER -> $HEAD_VER with a backup, a stop/start, and preserved the operator TLS cert"
 else
     echo "RESULT: FAIL"
     exit 1

specs/release/package-build.spec.yaml

@@ -1,7 +1,7 @@
 spec:
   id: release-package-build
   title: Native RPM and DEB packaging
-  version: "1.2.0"
+  version: "1.3.0"
   status: approved
   tier: 2
 
@@ -73,7 +73,12 @@ spec:
         time by a post-install scriptlet, generate-if-absent (never overwrite).
         The packages ship only the empty /etc/openwatch/tls directory plus the
         provisioning helper. openssl MUST be a package dependency (shared with
-        C-11).
+        C-11). Additionally, an operator's certificate MUST survive the one-time
+        upgrade FROM a release that DID ship the cert in its payload (<= rc.9):
+        the RPM MUST declare the cert/key paths %ghost (rpm tracks the path with
+        no payload content, so it does not reclaim the operator's file as an
+        orphan), and the DEB MUST stash an existing cert/key in preinst (before
+        dpkg removes the orphaned file) and restore it in postinst.
       type: security
       enforcement: error
     - id: C-06
@@ -232,9 +237,13 @@ spec:
         root:openwatch, key.pem 0600 openwatch:openwatch) only when both files
         are absent. The package payloads ship the empty /etc/openwatch/tls
         directory and the helper at /usr/lib/openwatch/provision-tls-cert.sh,
-        but do NOT contain cert.pem or key.pem (verified via rpm -qpl and
-        dpkg-deb -c), and the build scripts no longer stage a cert into the
-        payload — so a package upgrade never reverts an operator's replacement
-        certificate.
+        but do NOT contain cert.pem or key.pem as real payload content (verified
+        via rpm -qpl and dpkg-deb -c), and the build scripts no longer stage a
+        cert into the payload. For the one-time upgrade from a cert-shipping
+        release (<= rc.9): the RPM declares the cert/key paths %ghost (file flag
+        'g', no content) so rpm does not reclaim the operator's orphaned file,
+        and the DEB preinst stashes the cert/key to a .dpkg-bak on upgrade while
+        the postinst restores them — so a package upgrade never reverts (or
+        removes) an operator's replacement certificate, including the transition.
       priority: critical
       references_constraints: [C-05]