feat(apply): apply-time safety gates — declared-resource existence + drift preview/verify

[Aleksei Sviridkin (lexfrei)]

Summary

Implements the apply-time safety gates umbrella with four gates plus the talm init --update non-tty UX fix surfaced during real-cluster validation:

Phase 1 — declared-resource existence (block-by-default, --skip-resource-validation opt-out): walks the rendered MachineConfig and verifies every declared link / disk reference against the node's live COSI snapshot. v1.12 multi-doc form covers LinkConfig.name, DHCPv4/6Config.name, EthernetConfig.name, BondConfig.links[], BridgeConfig.links[], VLANConfig.parent, Layer2VIPConfig.link, HCloudVIPConfig.link, and UserVolumeConfig.provisioning.diskSelector. v1.11 nested form covers machine.network.interfaces[].interface, machine.install.disk, machine.install.diskSelector, and machine.disks[].device. Virtual-creator documents (BondConfig.name, VLANConfig.name, etc.) are intentionally skipped — their .name describes a new resource being created, not an existing-link reference. Findings cite the source JSONPath so operators can locate the offending value directly. Hint candidate lists are capped at 10 entries with a ... and N more suffix.

Phase 2A — pre-apply drift preview (--skip-drift-preview opt-out): reads on-node MachineConfig via COSI and diffs against what's about to be sent, rendering +/-/~/= per (kind, name) pair. Informational — never blocks. Runs on --dry-run too. Slice values surface as set-diff buckets (removed [127.0.0.1] for certSANs duplicate-cleanup) rather than full-slice dumps; map / nested values render in YAML flow style. Multi-node output is per-node-distinguished via (node X.Y.Z.W) in the header so multi-node stderr is disambiguated.

Phase 2B — post-apply state verification (--skip-post-apply-verify opt-out, default off until the Talos-mutated-field allowlist lands — see #172): re-reads on-node MachineConfig after ApplyConfiguration success and structurally compares against the bytes sent. Blocks on divergence. Auto-skipped on --mode=staged / --mode=try / --mode=reboot / --mode=auto / --dry-run — each for a documented reason. The AUTO skip in particular is non-obvious: Talos's apply server promotes AUTO to REBOOT internally when the change requires it, so the verify would race the reboot.

Phase 2C — post-upgrade version verify (closes #175, --skip-post-upgrade-verify opt-out, default on): after talosctl upgrade reports success, waits the reconcile window, reads runtime.Version via COSI, and compares the running version's (Major, Minor) contract against the contract parsed from the target image tag. Catches the silent A/B rollback case where Talos rolls back to the previous partition (cross-vendor image, missing extensions, failed boot readiness check) yet the upgrade RPC has already acked. Cross-minor mismatch surfaces as a hint-bearing blocker with a two-hypothesis hint (rollback OR slow boot beyond the reconcile window). Multi-node verify is collect-then-block via errors.Join. Best-effort surrender on digest-pinned images and unparseable tags. Skipped on --insecure (auth-only COSI path) and --stage (new partition not yet booted).

Both Phase 2 gates that run on the auth path degrade gracefully on the insecure / maintenance connection: MachineConfig is meta.Sensitive, the reader returns ok=false deterministically (the maintenance-vs-auth path is statically known from --insecure), and the gate prints one explanatory line without blocking.

talm init --update non-tty handling (closes #174): the preset-template overwrite prompt used to wedge on EOF when stdin wasn't a tty, silently leaving the project on a stale preset. Now resolves to one of three policies via a pure resolveOverwritePolicy(force, isTTY) predicate: --force accepts all overwrites with a per-file log line; tty + no --force keeps the historical interactive prompt; non-tty + no --force errors with a hint pointing at --force. stdinIsTTY uses term.IsTerminal instead of the naive ModeCharDevice check that falsely accepted /dev/null as a tty. --image runs through validateImageRefShape before writing, rejecting malformed values (::malformed, no-slash:tag) up front instead of leaving a broken values.yaml.

Tests

TDD red-first for every contract-level predicate. ~3,400 LOC of test code across pkg/applycheck (walker, validator, differ, selector contract tests) and pkg/commands (preflight hook integration via function-type fakes, mode-gating matrices, multi-node collect-then-block, wall-clock regression pin for the post-upgrade reconcile-window placement). Lint clean on host and GOOS=windows, go vet clean, race detector clean. TestNoWorkflowLeakageInRepoSource is extended with dev17 / lexfrei to ban private operator identifiers from committed source.

Real-cluster validation

Exercised against a 3-node OCI Talos v1.12.6 reference stand. Each gate's behavior was verified end-to-end through the auth and insecure paths; Phase 2C was confirmed against an intentional cross-vendor upgrade (cozystack-bundled → vanilla siderolabs installer) that produced the documented two-hypothesis blocker. Test plan reference at docs/apply-safety-gates-test-plan.md for future re-validation.

Closes

Closes #172. Closes #174. Closes #175.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR implements apply-time and upgrade-time safety gates for the talm CLI, including host-resource existence validation, configuration drift preview, post-apply state verification, and post-upgrade version verification. It adds companion CLI skip flags and integrates reference extraction, disk selector matching, and multi-document YAML diffing infrastructure to support these phases. Additionally, it fixes the non-TTY issue with talm init --update by introducing an overwrite policy abstraction and TTY detection.

Changes

Apply-time safety gates and init improvements

Layer / File(s)	Summary
Reference extraction and host-snapshot types pkg/applycheck/doc.go, pkg/applycheck/refs.go, pkg/applycheck/refs_test.go	RefKind, DiskSelector, and Ref types to model host-resource references extracted from v1.11 nested and v1.12 multi-document YAML. WalkRefs decodes documents and emits refs for network links, disk literals, and disk selectors with JSONPath-like source paths.
Disk selector matching and validation pkg/applycheck/selector.go, pkg/applycheck/validate.go, pkg/applycheck/validate_test.go	Disk selector predicates (glob/exact/type/size) with exclusion rules (readonly, CD-ROM, virtual), deterministic hint formatting with "… and N more" truncation, and ValidateRefs entry point that checks ref resolution against host snapshots and produces blocker/warning Finding results.
Multi-document YAML diff engine pkg/applycheck/diff.go, pkg/applycheck/diff_test.go	Diff parses current/desired snapshots by DocID, emits per-document Change entries (add/remove/update/equal), and performs leaf-level FieldChange diffing that distinguishes absent fields from present-but-null. FilterChanged and FormatChange utilities support downstream use.
Phase 1: Host-resource validation pkg/commands/preflight_apply_safety.go (lines 1–141), pkg/commands/preflight_apply_safety_test.go (lines 72–165)	preflightValidateResources walks rendered config refs, reads COSI host snapshot (links and disks), validates all refs resolve, and returns blocker errors with truncated hint lists when validation fails. Gracefully handles reader unavailability on insecure paths.
Phase 2A/2B: Drift preview and post-apply verification pkg/commands/preflight_apply_safety.go (lines 159–618), pkg/commands/preflight_apply_safety_test.go (lines 166–871)	Phase 2A previews drift via previewDrift with on-node MachineConfig diff, multiset-aware slice set-diffing, and YAML flow-style single-line field rendering. Phase 2B re-reads post-apply state via verifyAppliedState with reconcile window and blocks on structural divergence. Both support per-node headers and graceful degradation on maintenance paths. Tests validate all scenarios including scheduling predicates and rendering edge cases.
Phase 2C: Post-upgrade version verification pkg/commands/preflight_upgrade_verify.go, pkg/commands/preflight_upgrade_verify_test.go	parseTargetVersion extracts version literals from image tags, verifyPostUpgradeVersion reads running version and enforces major/minor match, distinguishes connection errors from slow-boot scenarios in hints, and skips silently on digest pins or missing tags.
Integration into apply and preflight commands pkg/commands/apply.go, pkg/commands/apply_test.go, pkg/commands/preflight.go, pkg/commands/preflight_test.go	Threads resolved node ID through cosiPreflightContext returning (ctx, nodeID, error), wires Phase 1 and Phase 2 gates per-node in both template-render and direct-patch modes, adds --skip-resource-validation, --skip-drift-preview, --skip-post-apply-verify flags. Updates versionReader contract to (version, ok, err) 3-tuple with test stub support.
Integration into upgrade command pkg/commands/upgrade_handler.go, pkg/commands/upgrade_handler_test.go	Captures pre-upgrade --image, --insecure, --stage, implements 90s post-upgrade reconcile window, resolves target nodes from CLI or talosconfig, runs per-node version verification collecting all failures, and gates via shouldRunPostUpgradeVerify predicate.
Init --update overwrite policy and TTY detection pkg/commands/init.go, pkg/commands/init_overwrite_test.go	Introduces overwritePolicy enum (force/ask/non-interactive) with TTY detection via golang.org/x/term. Refactors askUserOverwrite to auto-overwrite under --force, block in non-interactive mode with clear --force hint, or prompt on TTY. Centralizes "Created …" reporting via createReportedFile. Tests validate all policy branches and TTY/non-TTY behavior.
Init image reference validation pkg/commands/init.go, pkg/commands/init_test.go	Tightens imageLineRe to require space after colon in YAML image: keys. Adds validateImageRefShape to enforce OCI structure (requires / and :tag or @sha256:/sha512: digest), integrated into validateImageOverride before preset scanning. Tests validate malformed rejection and error reporting.
Documentation, test infrastructure, and CI README.md, docs/apply-safety-gates-test-plan.md, docs/manual-test-plan.md, go.mod, .github/workflows/pr.yml, pkg/engine/engine_test.go	README section describing all four safety gates, their defaults, and per-phase skip flags. Comprehensive safety gates reference test plan (Phase 1–2C, real-Talos validation steps, limitations). Manual end-to-end test matrix for talm workflows. Promotes go-humanize and golang.org/x/term to direct dependencies. Increases golangci-lint timeout from 5m to 10m. Extends banned-phrase list for workflow leakage checks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A rabbit's ode to safety gates:
Four phases guard thy config's fate,
With refs extracted, diffs displayed,
And TTY prompts that won't degrade—
Now preset updates reach the gate,
And upgrades verify their state! 🎉

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/apply-safety-gates

[gemini-code-assist]

Code Review

This pull request introduces the applycheck package to implement safety gates for Talos MachineConfig applications, including host-resource reference validation and structural diffing. These checks are integrated into the apply command with new flags to control resource validation, drift previews, and post-apply verification. A review comment identifies an improvement opportunity in error handling within cosiMachineConfigReader to prevent transient errors from being incorrectly reported as maintenance connection issues.

[gemini-code-assist]

The current implementation swallows the error from safe.StateGet. This can lead to misleading output in previewDrift and verifyAppliedState, where a transient network or API error is reported as a maintenance connection issue. It's better to return the error so the caller can provide accurate feedback.

Suggested change

	if err != nil {
	// PermissionDenied on the insecure path is the documented
	// Sensitive-resource degrade signal. Distinguishing transient
	// failures from auth-side denial would require parsing the
	// gRPC status; for the pre-merge cut we treat any read
	// failure as "could not read" and let the caller decide
	// whether to surface that as a blocker (Phase 2B never does;
	// Phase 2A never does either).
	return nil, false, nil
	}
	if err != nil {
	// PermissionDenied on the insecure path is the documented
	// Sensitive-resource degrade signal. Distinguishing transient
	// failures from auth-side denial would require parsing the
	// gRPC status; for the pre-merge cut we treat any read
	// failure as "could not read" and let the caller decide
	// whether to surface that as a blocker (Phase 2B never does;
	// Phase 2A never does either).
	return nil, false, err
	}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/commands/init.go`:
- Around line 1089-1092: The current error wrapping around
askUserOverwrite(filePath, policy) replaces the original error (e.g.,
overwritePolicyNonInteractive) with "failed to read user input", which is
misleading for non-interactive flows; update the error handling in the caller of
askUserOverwrite to preserve the original error context: detect or allow the
sentinel error returned by askUserOverwrite (such as
overwritePolicyNonInteractive) to be returned directly (or wrapped with a
message that includes the original error), and only use "failed to read user
input" for genuine stdin read failures; reference askUserOverwrite,
overwritePolicyNonInteractive, filePath and policy when making this change so
the non-tty path propagates its true error.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f332d6f-b787-4a4f-a1c5-c4ba2e2968ba

📥 Commits

Reviewing files that changed from the base of the PR and between ed3b820 and 7868946.

📒 Files selected for processing (27)

• .github/workflows/pr.yml
• README.md
• docs/apply-safety-gates-test-plan.md
• docs/manual-test-plan.md
• go.mod
• pkg/applycheck/diff.go
• pkg/applycheck/diff_test.go
• pkg/applycheck/doc.go
• pkg/applycheck/refs.go
• pkg/applycheck/refs_test.go
• pkg/applycheck/selector.go
• pkg/applycheck/validate.go
• pkg/applycheck/validate_test.go
• pkg/commands/apply.go
• pkg/commands/apply_test.go
• pkg/commands/init.go
• pkg/commands/init_overwrite_test.go
• pkg/commands/init_test.go
• pkg/commands/preflight.go
• pkg/commands/preflight_apply_safety.go
• pkg/commands/preflight_apply_safety_test.go
• pkg/commands/preflight_test.go
• pkg/commands/preflight_upgrade_verify.go
• pkg/commands/preflight_upgrade_verify_test.go
• pkg/commands/upgrade_handler.go
• pkg/commands/upgrade_handler_test.go
• pkg/engine/engine_test.go

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve policy error context instead of forcing a “read input” message.

When overwritePolicyNonInteractive is hit, no stdin read occurs, but Line 1091 rewrites that path to "failed to read user input". This makes non-tty failures misleading.

Suggested fix

 	overwrite, err := askUserOverwrite(filePath, policy)
 	if err != nil {
-		return errors.Wrap(err, "failed to read user input")
+		return err
 	}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	overwrite, err := askUserOverwrite(filePath, policy)
	if err != nil {
	return errors.Wrap(err, "failed to read user input")
	}
	overwrite, err := askUserOverwrite(filePath, policy)
	if err != nil {
	return err
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/commands/init.go` around lines 1089 - 1092, The current error wrapping
around askUserOverwrite(filePath, policy) replaces the original error (e.g.,
overwritePolicyNonInteractive) with "failed to read user input", which is
misleading for non-interactive flows; update the error handling in the caller of
askUserOverwrite to preserve the original error context: detect or allow the
sentinel error returned by askUserOverwrite (such as
overwritePolicyNonInteractive) to be returned directly (or wrapped with a
message that includes the original error), and only use "failed to read user
input" for genuine stdin read failures; reference askUserOverwrite,
overwritePolicyNonInteractive, filePath and policy when making this change so
the non-tty path propagates its true error.