feat(apply,upgrade): apply-safety follow-ups (#189 + #190 + #191 + #192)

Four operator-UX polish changes on the safety surfaces introduced
by #173.

* #191 — Per-node prefix on the maintenance-connection warning.
  Multi-node insecure apply used to print identical bare
  'talm: drift verification unavailable on maintenance connection'
  lines for every node with no per-node correlation. Both the
  previewDrift and verifyAppliedState emission sites now wrap
  through nodePrefix(nodeID); single-node case (empty nodeID)
  keeps the bare line so the common UX is unchanged.

  cosiPreflightContext got a fallback path to GlobalArgs.Nodes[0]
  when no outgoing-context metadata is attached — the maintenance
  flow (openClientPerNodeMaintenance) pins the node in GlobalArgs
  but does not attach context metadata, so without the fallback
  the per-node prefix collapsed to empty even when --nodes was
  set explicitly. Fallback is single-element-only; multi-element
  GlobalArgs.Nodes returns empty so a broken caller surfaces as
  'no prefix' rather than 'wrong node prefix'.

* #190 — Configurable post-upgrade reconcile window.
  The hardcoded 90s wait after talosctl upgrade returns is now
  --post-upgrade-reconcile-window. Default
  defaultPostUpgradeReconcileWindow = 90 * time.Second preserves
  byte-identical back-compat. validatePostUpgradeReconcileWindow
  rejects non-positive values at the TOP of wrapUpgradeCommand
  RunE — fail-fast BEFORE any talosctl RPC fires, so an
  operator's '=0s' typo cannot land a partial upgrade. The
  version-mismatch hint copy and error body de-hardcode the '90s'
  literal to reference 'the configured reconcile window'
  (defaultPostUpgradeReconcileWindow flows through
  verifyPostUpgradeVersion).

* #192 — Syntactic net-addr walker for three v1alpha1 multidoc
  kinds. Validates fields the actual Talos schema emits:
    * StaticHostConfig.name (IP literal; the `name` field on
      this kind doubles as the IP — there is no separate
      `address` field).
    * NetworkRuleConfig.ingress[].subnet and .except (CIDR
      shapes; no top-level matchSourceAddress[] field exists
      in Talos's schema).
    * WireguardConfig.peers[].endpoint (host:port; empty /
      absent endpoint is a listener-only peer and NOT a
      finding).
  Walker runs in parallel with the Ref-based walker via
  multidocNetAddrHandlers; dispatch-table disjointness with
  multidocHandlers is pinned by a unit test. Integration tests
  verify Phase 1 blocks on bad and passes on valid.

* #189 — Drift preview redacts secret-bearing field values by
  default. secretFieldPaths allowlist with bracket-normalisation
  regex; secret check fires BEFORE bothSlices in
  formatFieldChangeLine so slice-shaped allowlist entries
  (cluster.acceptedCAs, machine.acceptedCAs, peers) never leak
  via formatSliceSetDiff. Length-disclosing redaction sentinel
  (***redacted (len=N)***) preserves rotation signal without
  leaking the value. Non-string secret values route through
  redactValue(fmt.Sprintf("%v", value)) so int / bool rotations
  still surface as different-length sentinels. Map iteration
  order non-determinism is disclaimed in the godoc (no
  map-shaped allowlist entry today).

  New flag --show-secrets-in-drift (default off) plumbed through
  previewDrift / verifyAppliedState / printDriftPreview /
  formatFieldChangeLine. apply.go callers pass
  applyCmdFlags.showSecretsInDrift.

  absentFieldValue const hoisted so formatFieldValue and
  formatSecretFieldValue stay byte-identical on the absent path
  — add/remove vs rotate stays distinguishable.

Doc updates:
  * README.md bullet #1 (Phase 1) now mentions the net-addr
    walker; bullet #2 (Phase 2A) mentions redaction +
    --show-secrets-in-drift + the per-node prefix; bullet #4
    (Phase 2C) says 'the configured reconcile window' instead
    of hardcoding 90s.
  * docs/manual-test-plan.md gains sections C5/C6/C7 (redact,
    flag opt-out, walker), D3 (per-node prefix), E3 (reconcile
    window with help-text + 0s rejection), M6 (false-positive
    guard for redaction), M7 (walker boundary cases). Forward-
    looking 'do X, expect Y' shape with explicit regression
    anchors.
  * docs/apply-safety-gates-test-plan.md gets matching rows in
    Phase 1 (net-addr walker), Phase 2A (redaction +
    per-node prefix), and Phase 2C (reconcile window) tables.

Test coverage:
  * Walker: per-kind table tests with the real schema shape,
    plus TestWalkNetAddrFindings_RealSchema_StaticHostConfig
    and ..._NetworkRuleConfig as schema-anchor regression pins
    so a future drift back to non-existent fields fails these
    tests rather than passing trivially.
  * Reconcile window: validator boundary cases, flag default
    one-(default)-clause pin, fail-fast ordering pin via a
    sentinel originalRunE, hint-text no-hardcoded-90s pin, a
    README-no-hardcoded-90s pin so README and code stay in
    sync.
  * Per-node prefix: maintenance message on both emission
    sites, empty-nodeID bare-line guard, single-vs-multi
    GlobalArgs.Nodes fallback pair.
  * Redaction: exact-match, Wireguard paths, slice-rotation
    no-leak (CA-list and peers shapes), bracket-normalisation,
    false-prefix guard, non-secret control, opt-in flag,
    non-string redaction with length-signal, absent-side
    add-vs-rotate distinction.

Closes #189, closes #190, closes #191, closes #192.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

Author: lexfrei

README.md

@@ -172,13 +172,13 @@ cluster:
 
 > **Apply-time safety gates.** `talm apply` and `talm upgrade` run additional gates around each operation:
 >
-> 1. **Declared-resource existence** (`--skip-resource-validation` opt-out, default on). Before sending the config to the node, the gate walks the rendered MachineConfig, extracts every reference to a host-side resource (network links from v1.12 multi-doc — `LinkConfig.name`, `BondConfig.links[]`, `VLANConfig.parent`, `BridgeConfig.links[]`, `Layer2VIPConfig.link`, `HCloudVIPConfig.link`, `DHCPv4Config.name` / `DHCPv6Config.name` / `EthernetConfig.name`; v1.11 legacy `machine.network.interfaces[].interface`; install disk via `machine.install.disk` literal or `machine.install.diskSelector`; `UserVolumeConfig.provisioning.diskSelector`), and verifies each against the node's COSI `LinkStatus`/`Disk` snapshots. A reference that doesn't resolve fails the apply with a `[blocker]` line listing the available names so the typo or migration miss is fixable from the values without re-running discovery. Disk selectors must match at least one (non-readonly, non-CDROM, non-virtual) disk — zero matches block, multiple matches warn (install picks the first). Virtual-link-creator documents (`BondConfig.name`, `VLANConfig.name`, `BridgeConfig.name`, `WireguardConfig.name`, `DummyLinkConfig.name`, `LinkAliasConfig.name`) are intentionally NOT validated against existing links — those `.name` fields describe new virtual links the apply is creating, not references to pre-existing host resources. Out of scope today: `machine.disks[].device` (extra-disk partitioning); track in a follow-up if you need it. Pass `--skip-resource-validation` for recovery into a maintenance image with mismatched hardware or pre-staging values for hardware that isn't installed yet.
+> 1. **Declared-resource existence** (`--skip-resource-validation` opt-out, default on). Before sending the config to the node, the gate walks the rendered MachineConfig, extracts every reference to a host-side resource (network links from v1.12 multi-doc — `LinkConfig.name`, `BondConfig.links[]`, `VLANConfig.parent`, `BridgeConfig.links[]`, `Layer2VIPConfig.link`, `HCloudVIPConfig.link`, `DHCPv4Config.name` / `DHCPv6Config.name` / `EthernetConfig.name`; v1.11 legacy `machine.network.interfaces[].interface`; install disk via `machine.install.disk` literal or `machine.install.diskSelector`; `UserVolumeConfig.provisioning.diskSelector`), and verifies each against the node's COSI `LinkStatus`/`Disk` snapshots. A reference that doesn't resolve fails the apply with a `[blocker]` line listing the available names so the typo or migration miss is fixable from the values without re-running discovery. Disk selectors must match at least one (non-readonly, non-CDROM, non-virtual) disk — zero matches block, multiple matches warn (install picks the first). Virtual-link-creator documents (`BondConfig.name`, `VLANConfig.name`, `BridgeConfig.name`, `WireguardConfig.name`, `DummyLinkConfig.name`, `LinkAliasConfig.name`) are intentionally NOT validated against existing links — those `.name` fields describe new virtual links the apply is creating, not references to pre-existing host resources. The gate also runs a syntactic net-addr walker against `StaticHostConfig.name` (must parse as an IP literal — the `name` field on this kind doubles as the IP the hostnames map to), `NetworkRuleConfig.ingress[].subnet` and `.except` (per-entry CIDR), and `WireguardConfig.peers[].endpoint` (host:port; empty / absent endpoint is a listener-only peer, NOT a finding). Out of scope today: `machine.disks[].device` (extra-disk partitioning); track in a follow-up if you need it. Pass `--skip-resource-validation` for recovery into a maintenance image with mismatched hardware or pre-staging values for hardware that isn't installed yet.
 >
-> 2. **Pre-apply drift preview** (`--skip-drift-preview` opt-out, default on). Reads the node's current MachineConfig via COSI and prints a `+`/`-`/`~`/`=` diff of what's about to change, keyed by `(kind, name)`. Informational only — never blocks. The `-` lines are the most useful: they surface stale documents from a previous apply that the new render no longer emits (e.g. an `eth1` LinkConfig lingering after a migration to `eth0`). Reading the current config requires the auth path — `MachineConfig` is a Sensitive COSI resource and is unreachable on the `--insecure` maintenance connection; the gate prints `drift verification unavailable on maintenance connection` and proceeds in that case. **`--dry-run` runs this gate** — the diff is read-only and "show me what would change" is exactly the dry-run contract.
+> 2. **Pre-apply drift preview** (`--skip-drift-preview` opt-out, default on). Reads the node's current MachineConfig via COSI and prints a `+`/`-`/`~`/`=` diff of what's about to change, keyed by `(kind, name)`. Informational only — never blocks. The `-` lines are the most useful: they surface stale documents from a previous apply that the new render no longer emits (e.g. an `eth1` LinkConfig lingering after a migration to `eth0`). Reading the current config requires the auth path — `MachineConfig` is a Sensitive COSI resource and is unreachable on the `--insecure` maintenance connection; the gate prints `drift verification unavailable on maintenance connection` (per-node-prefixed on multi-node insecure apply) and proceeds in that case. Secret-bearing field values (`cluster.token`, `cluster.{ca,aggregatorCA,serviceAccount,etcd.ca}.key`, `machine.token` / `machine.ca.key`, the `cluster.acceptedCAs` / `machine.acceptedCAs` slices, `WireguardConfig.privateKey`, the `peers` slice carrying `presharedKey`s) are redacted by default — both sides render as `***redacted (len=N)***` so a rotation surfaces as different-length sentinels without leaking the value. Pass `--show-secrets-in-drift` to see the raw values verbatim (debugging only — disables the redaction for the run). **`--dry-run` runs this gate** — the diff is read-only and "show me what would change" is exactly the dry-run contract.
 >
 > 3. **Post-apply state verification** (`--skip-post-apply-verify` opt-out, **default off** until the Talos-mutated-field allowlist lands — see [#172](https://github.com/cozystack/talm/issues/172)). After `ApplyConfiguration` returns success, re-reads the on-node MachineConfig and structurally compares it against the bytes that were sent. Divergence blocks the apply chain with a per-document diff, primarily catching silent doc drops (Talos parser ignored an unknown field) and controller reverts. Disabled by default because Talos mutates a handful of leaf fields post-apply (cert hashes, timestamps) that would surface as false-positive divergence without an allowlist. The verify runs only on `--mode=no-reboot`. `--mode=staged`, `--mode=try`, `--mode=reboot`, and `--mode=auto` all skip the gate — each for a documented reason: staged stores rather than activates; try auto-rolls back; reboot kills the COSI connection mid-verify; auto is promoted by Talos to REBOOT internally when the change requires it, so the verify would race the reboot. `--dry-run` skips it too.
 >
-> 4. **Post-upgrade version verify** (`--skip-post-upgrade-verify` opt-out, default on — the gate runs). After `talm upgrade` reports success, waits 90s for the node to finish booting then reads `runtime.Version` COSI and compares the running version's `(Major, Minor)` contract against the contract parsed from the target image tag. Point releases share a minor contract; cross-minor mismatch surfaces as a hint-bearing blocker. Catches the silent A/B rollback case where the upgrade RPC acks success but Talos rolled back to the previous partition (cross-vendor image, missing extensions, failed boot readiness check, slow boot exceeding the reconcile window). Best-effort surrender on digest-pinned images and unparseable tags. See [#175](https://github.com/cozystack/talm/issues/175) for the reproduction.
+> 4. **Post-upgrade version verify** (`--skip-post-upgrade-verify` opt-out, default on — the gate runs). After `talm upgrade` reports success, waits the configured reconcile window (default 90s; tune via `--post-upgrade-reconcile-window` for slow hardware / large image pulls) for the node to finish booting, then reads `runtime.Version` COSI and compares the running version's `(Major, Minor)` contract against the contract parsed from the target image tag. Point releases share a minor contract; cross-minor mismatch surfaces as a hint-bearing blocker. Catches the silent A/B rollback case where the upgrade RPC acks success but Talos rolled back to the previous partition (cross-vendor image, missing extensions, failed boot readiness check, slow boot exceeding the configured window). Best-effort surrender on digest-pinned images and unparseable tags. See [#175](https://github.com/cozystack/talm/issues/175) for the reproduction.
 >
 > The skip flags don't suppress each other — pass them independently. On the `--insecure` (maintenance) path the gates are functionally unreachable for charts that drive discovery via `lookup` — those COSI lookups require an authenticated connection and the render itself errors before any gate runs. Charts that render fully offline (no `lookup` calls) reach the gates on `--insecure` as well, with the Phase 2 hooks degrading gracefully because the `MachineConfig` resource is Sensitive.
 

docs/apply-safety-gates-test-plan.md

@@ -47,6 +47,29 @@ Run all matrix cells against the binary at `/tmp/talm-safety`. Use a 3-node Talo
 | Boundary case (exactly 11) | 11 links on the host, bad ref | First 10 inline + `... and 1 more` (the suffix fires at >10, not at >=10) |
 | Empty candidate set | Selector matches zero, no real candidates either (mock) | Hint says `<none>` rather than empty trailing space |
 
+### Net-addr field references
+
+The Phase 1 walker validates the syntactic shape of net-addr fields in three v1alpha1 multidoc kinds. Pure syntactic — no host snapshot — runs alongside the Ref-based walker via `multidocNetAddrHandlers`. Field names match the actual Talos `network` schema (see `siderolabs/talos/pkg/machinery/config/types/network/`).
+
+| Case | How to trigger | Expected |
+| --- | --- | --- |
+| Bad `StaticHostConfig.name` | `StaticHostConfig{name: 999.999.0.1, hostnames: [foo]}` — the `name` field carries the IP literal in this kind | Blocker "StaticHostConfig.name is not a valid IP literal" with hint listing IPv4/IPv6 examples |
+| Valid IPv4 / IPv6 | `name: 192.0.2.10` / `name: 2001:db8::1` | No finding |
+| Missing `name` | Omit the field | No finding (Talos rejects at RPC with a clearer required-field message) |
+| Hostname-shaped name | `name: example.invalid` | Blocker — `name` is required to be an IP literal, not a DNS name |
+| Bad `NetworkRuleConfig.ingress[i].subnet` | `ingress: [{subnet: notacidr}]` | Per-entry blocker citing `ingress[i].subnet` |
+| Bad `NetworkRuleConfig.ingress[i].except` | `ingress: [{subnet: 192.0.2.0/24, except: notacidr}]` | Blocker on `except` even when `subnet` is valid |
+| Bare IP without /N | `ingress: [{subnet: 192.0.2.10}]` | Blocker — schema is CIDR-shaped, not IP-shaped |
+| Valid CIDR mix | IPv4 + IPv6 CIDRs across `ingress[].subnet` | No findings |
+| Bad `WireguardConfig.peers[].endpoint` | One peer `endpoint: notavalid:endpoint` | Per-peer blocker citing `peers[i].endpoint` |
+| Valid IPv4:port | `endpoint: 192.0.2.10:51820` | No finding |
+| Valid bracketed IPv6:port | `endpoint: "[2001:db8::1]:51820"` | No finding |
+| Empty `endpoint` | `endpoint: ""` | No finding (peer is listener-only — this side does not initiate) |
+| Missing `endpoint` field | Omit the field | No finding |
+| Unknown multidoc kind | A new kind not in the dispatch map | No finding (Talos extensions / future kinds do not break the gate) |
+| Real-schema pin | `TestWalkNetAddrFindings_RealSchema_StaticHostConfig` / `..._NetworkRuleConfig` feed the actual schema shape (`name` carrying the IP, `ingress[].subnet/except` for CIDRs) — the walker fires on what Talos emits, not on a hand-crafted YAML the schema doesn't produce |
+| No-overlap pin | Adding a kind to both `multidocHandlers` AND `multidocNetAddrHandlers` | `TestMultidocNetAddrHandlers_NoOverlapWithRefHandlers` fails — double-walking would produce duplicate findings |
+
 ### Opt-out
 
 | Case | Trigger | Expected |
@@ -78,8 +101,28 @@ Run all matrix cells against the binary at `/tmp/talm-safety`. Use a 3-node Talo
 | `--mode=staged` | `talm apply --mode=staged -f node.yaml` | Phase 2A runs (operator still wants to see what got staged) |
 | `--mode=try` | `talm apply --mode=try -f node.yaml` | Phase 2A runs (mirrors --mode=auto from the preview's perspective) |
 | Insecure path | `talm apply -i -f node.yaml` (where chart can render offline) | `talm: drift verification unavailable on maintenance connection`; no block |
+| Insecure path, multi-node | `talm apply -i --nodes a,b -f node.yaml` (each iteration through `openClientPerNodeMaintenance`) | Per-node-prefixed line `node a: talm: drift verification unavailable …` and `node b: talm: …` — disambiguation cohort over the maintenance-warning emission |
+| Insecure path, single node | `talm apply -i -f node.yaml` with single `--nodes` | Still gets `node X: talm: …` prefix because `cosiPreflightContext` falls back to `GlobalArgs.Nodes[0]` when there is no outgoing-context metadata |
+| Insecure path, empty nodeID | Unusual call shape with `GlobalArgs.Nodes` somehow empty | Bare `talm: drift verification unavailable …` line — never `node : …` garbage prefix |
 | `--skip-drift-preview` | Pass with any change | Preview suppressed entirely |
 
+### Secret-bearing field redaction
+
+The drift preview redacts allowlisted paths by default. The opt-out is operator-explicit: `--show-secrets-in-drift`. Allowlist lives in `secretFieldPaths` (`pkg/commands/preflight_apply_safety_redact.go`).
+
+| Case | Trigger | Expected |
+| --- | --- | --- |
+| Cluster secret rotation | Change `cluster.token` via `secrets.yaml` rotation | `cluster.token: ***redacted (len=N)*** -> ***redacted (len=M)***` — value never appears in stderr |
+| Machine token rotation | Change `machine.token` | Same shape; `machine.token: ***redacted (len=N)*** -> …` |
+| Array-indexed secret | Change `cluster.acceptedCAs[2].key` | Bracket-normalised match against `cluster.acceptedCAs[].key`; redacted |
+| Wireguard private key | Rotate `WireguardConfig.privateKey` | `privateKey: ***redacted (len=N)*** -> …` — bare path because the differ's flatten step does not prefix multidoc fields with the doc kind |
+| Wireguard pre-shared key | Rotate `peers[2].presharedKey` | Bracket-normalised; redacted |
+| Non-secret path | Change `machine.network.hostname` | Verbatim — operator-visible information is not redacted |
+| False-prefix guard | A non-secret path sharing a prefix (`cluster.tokenExtras`) | Verbatim — `isSecretPath` is path-segment exact, not substring prefix |
+| `--show-secrets-in-drift` | Pass with any secret rotation | Verbatim both sides on the secret line; sentinel never appears |
+| Non-string secret value | Hypothetical schema drift puts an int on a secret-bearing path | `***redacted (len=N)***` where N is the `%v` length; rotation signal survives non-string types (caveat: maps render with non-deterministic key order — disclaimed in the godoc) |
+| Slice-shaped secret path | Hypothetical future allowlist entry naming an array | Redacted via the secret check that runs BEFORE `bothSlices` — elements never leak through `formatSliceSetDiff` |
+
 ### Output pretty-print
 
 | Case | Trigger | Expected |
@@ -126,6 +169,9 @@ On by default for `talm upgrade`. The gate fires after talosctl upgrade returns
 | By-design unreachable | Reader returns `("", false, nil)` (cosiVersionReader does not produce this; reserved for future custom readers that need to surrender silently) | Soft warning line `post-upgrade verification skipped, could not read running version from the node`, no block. Distinguishable from the real-read-failure case via the err — three-valued contract makes the contract explicit |
 | Zero target nodes | `--nodes` empty and talosconfig context has no nodes either | Explanatory "skipped, no target nodes resolved" line (no silent no-op) |
 | Reconcile wait line | Any non-skipped run | "post-upgrade verify: waiting 1m30s for the node to finish booting..." printed up front so the operator's terminal isn't a mystery hang |
+| Configurable reconcile window | `talm upgrade --post-upgrade-reconcile-window=180s …` | "post-upgrade verify: waiting 3m0s for the node to finish booting..." — Go's `time.Duration.String()` renders 180s deterministically as `3m0s`. Hint copy references "the configured reconcile window (`--post-upgrade-reconcile-window`)" instead of the hardcoded "90s reconcile window" wording |
+| Window default | `talm upgrade --help` | Flag listed with `default 1m30s`; the const `defaultPostUpgradeReconcileWindow` preserves the previous hardcoded 90s for byte-identical back-compat |
+| Window non-positive | `talm upgrade --post-upgrade-reconcile-window=0s …` | Fail-fast error with hint mentioning "positive duration" — validation runs at the TOP of `wrapUpgradeCommand` RunE so the talosctl upgrade RPC never fires. Same shape for `-30s` (negative). Pinned by `TestWrapUpgradeCommand_BadReconcileWindow_FailsFastBeforeOriginalRunE` which asserts the sentinel `originalRunE` stays uninvoked |
 
 ## Real-Talos validation