Skip to content

Commit 3b83403

Browse files
itcmsgrclaude
andauthored
feat(v1.100 PR-P2-1): prior-authority record hardening — 5-state classification + required RecordedAt/InstallerVersion/ActiveAtInstall (#484)
Pre-PR-23 blocker #1. Strengthens prior-authority records so PR-24 restore-enforcement can rely on them without inheriting weak semantics. **Scope-locked per repair contract:** - schema additions (recorded_at, installer_version, *bool active_at_install) - 5-state classification (NoRecord / Malformed / Incomplete / UsableActive / UsableInactive) - backward-safe degradation of old weak records to Incomplete - render/JSON symmetry — PriorActiveAtInstall field + render line - tests + contract/doc updates only - NO restore execution, NO lifecycle mutation, NO signature/checksum framework, NO unrelated refactors ## Schema additions (prior.go) ```go type PriorRecord struct { SchemaVersion string FirewallType string RecordedAt *time.Time // NEW — required for usable InstallerVersion string // NEW — required for usable ActiveAtInstall *bool // CHANGED from bool — tri-state: // nil = record writer did not commit // &true = prior was active // &false = prior was explicitly inactive } ``` ## 5-state classification | State | Condition | |---|---| | NoRecord | no artifact on disk | | RecordMalformed | JSON does not parse (was folded into Incomplete before) | | RecordIncomplete | parses but required field missing/invalid | | RecordUsableActive | all required + ActiveAtInstall=&true | | RecordUsableInactive | all required + ActiveAtInstall=&false | `PriorRecordState.IsUsable()` helper returns true for both Usable* variants — RestoreAuthorized uses it. `IncompleteReason` enum provides machine-consumable reason codes: Unreadable, SchemaMismatch, MissingFirewallType, UnknownFirewallType, MissingRecordedAt, MissingInstallerVersion, MissingActiveAtInstall. ## Backward-safety Older records from before PR-P2-1 that lack recorded_at / installer_version / explicit active_at_install are intentionally reclassified as RecordIncomplete. Migration-style silent upgrade is forbidden. Test `TestProbe_OldStyleRecord_DegradesToIncomplete` falsifies this path directly. > **Migration note:** Older prior-authority records may now classify > as Incomplete by design. This is intentional safety tightening, not > a functional regression. Records produced by the install-side writer > that lands alongside PR-23 will carry all required fields. ## Plan integration (plan.go) - `Plan.RestoreAuthorized` now uses `PriorState.IsUsable()` — both Usable variants authorize, active/inactive is separate dimension - new `Plan.PriorIncompleteReason` surfaces typed reason in JSON - new `Plan.PriorActiveAtInstall *bool` surfaces the tri-state (nil = unknown, &true/&false = explicit) — PR-24 uses this to decide whether restoration needs a second confirmation prompt - new Render line `Prior firewall at install : active|inactive` only emitted when record is usable — no defaulted values - new warning when restore requested on UsableInactive record ## Tests Added (uninstall_test.go): - `TestProbe_RecordUsableActive` (rename of TestProbe_RecordUsable) - `TestProbe_RecordUsableInactive` — active_at_install=false path - `TestProbe_RecordMalformed_BadJSON` — split from Incomplete - `TestProbe_RecordIncomplete_MissingRecordedAt` - `TestProbe_RecordIncomplete_MissingInstallerVersion` - `TestProbe_RecordIncomplete_MissingActiveAtInstall` - `TestProbe_OldStyleRecord_DegradesToIncomplete` — backward-safety - `TestBuildPlan_Restore_UsableInactive_AuthorizedButWarned` - All existing tests updated: fixtures carry all 5 required fields; JSON strings updated to `record_usable_active` Every Incomplete* test also asserts the specific IncompleteReason (not just the state) so regressions attribute cleanly. ## Doc update (contract.md) Added "PR-P2-1 hardening" section under the prior-authority record definition: describes the 5 states, what "usable" now means, explicit backward-safety rule. ## Non-goals (strict scope lock) - no restore execution - no lifecycle mutation - no signature/checksum framework - no unrelated refactors Refs: internal/installer/uninstall/contract.md §"Pre-PR-23 blockers" Authorization: locked repair contract (pre-PR-23 Phase 2 item #1) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ff22efa commit 3b83403

4 files changed

Lines changed: 453 additions & 87 deletions

File tree

internal/installer/uninstall/contract.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,40 @@ PR-23). PR-22 only reads — it does not write any prior-authority
8585
artifact (that's install-side expansion per v1.100 Q9, tracked for
8686
PR-23 or a companion install-mode PR).
8787

88+
### PR-P2-1 hardening (landed pre-PR-23)
89+
90+
The 3-state classification above was tightened to 5 states so PR-24
91+
restore-enforcement has a stricter foundation:
92+
93+
| State | Meaning |
94+
|---|---|
95+
| `NoRecord` | no artifact on disk |
96+
| `RecordMalformed` | artifact exists but JSON does not parse |
97+
| `RecordIncomplete` | JSON parses but required fields missing, unknown, or semantically unusable |
98+
| `RecordUsableActive` | all required fields present AND prior firewall was active at install time |
99+
| `RecordUsableInactive` | all required fields present AND prior firewall was explicitly NOT active at install time |
100+
101+
"Usable" now requires:
102+
103+
- JSON parses
104+
- schema version matches
105+
- firewall type is one of the known types
106+
- `recorded_at` timestamp present
107+
- `installer_version` present
108+
- `active_at_install` explicitly committed (pointer non-nil, not just
109+
Go's zero-value false)
110+
111+
**Usable does not imply "restore/re-enable now."** It means the record
112+
itself is trustworthy — PR-24 will still define its own restoration
113+
authorization rules on top of that.
114+
115+
**Backward-safety note:** older records from before PR-P2-1 that lack
116+
`recorded_at` / `installer_version` / explicit `active_at_install` are
117+
intentionally reclassified as `RecordIncomplete` by the new Probe logic.
118+
This is safety tightening, not a functional regression. Records
119+
produced by the install-side writer that lands alongside PR-23 will
120+
carry all required fields.
121+
88122
---
89123

90124
## Plan output — contract-language rendering

internal/installer/uninstall/plan.go

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,38 @@ type Plan struct {
7878
RestoreRequested bool `json:"restore_requested"`
7979

8080
// RestoreAuthorized is true only when RestoreRequested AND
81-
// PriorState == PriorRecordUsable. This is the PR-22 classifier
82-
// output; PR-24 enforces it.
81+
// PriorState.IsUsable() returns true (both UsableActive and
82+
// UsableInactive qualify — active/inactive is a separate semantic
83+
// dimension surfaced via PriorActiveAtInstall). PR-24 enforces
84+
// this gate.
8385
RestoreAuthorized bool `json:"restore_authorized"`
8486

85-
// PriorState is the 3-state probe result for the prior-authority
86-
// record. Populated regardless of RestoreRequested so the operator
87-
// always sees the underlying ambiguity.
87+
// PriorState is the 5-state probe result for the prior-authority
88+
// record (PR-P2-1 hardened). Populated regardless of RestoreRequested
89+
// so the operator always sees the underlying ambiguity.
8890
PriorState PriorRecordState `json:"prior_state"`
8991

92+
// PriorIncompleteReason is populated when PriorState ==
93+
// RecordIncomplete and gives a machine-consumable reason code so
94+
// downstream tooling doesn't have to substring-match human notes.
95+
// Empty string when the state is not Incomplete.
96+
PriorIncompleteReason IncompleteReason `json:"prior_incomplete_reason,omitempty"`
97+
98+
// PriorActiveAtInstall surfaces the active-at-install-time
99+
// distinction when the record is usable. Three values:
100+
//
101+
// *true — the prior firewall was actively holding authority at
102+
// install time; restore = re-enable a firewall that was
103+
// running
104+
// *false — the prior firewall was present but NOT active at install
105+
// time; restore = enable a firewall that was NOT running
106+
// (a different operation; operator should see this)
107+
// nil — unknown (usable state not reached)
108+
//
109+
// PR-24 uses this to decide whether restoration needs a second
110+
// confirmation prompt.
111+
PriorActiveAtInstall *bool `json:"prior_active_at_install,omitempty"`
112+
90113
// Warnings is the operator-visible list of concerns that do NOT
91114
// abort the plan but should be surfaced. Example: "restore flag
92115
// set but no prior-authority record on disk".
@@ -123,14 +146,15 @@ const PlanSchemaVersion = "1.100.0"
123146
// --restore-prior-authority flag at call time.
124147
func BuildPlan(mode Mode, auth *ClassifyResult, prior *ProbeResult, restoreRequested bool) *Plan {
125148
p := &Plan{
126-
SchemaVersion: PlanSchemaVersion,
127-
RequestedMode: mode,
128-
ArtifactPolicy: artifactPolicyFor(mode),
129-
CurrentAuthority: auth.State,
130-
DetectedExternal: auth.External,
131-
RestoreRequested: restoreRequested,
132-
PriorState: prior.State,
133-
RestoreAuthorized: restoreRequested && prior.State == PriorRecordUsable,
149+
SchemaVersion: PlanSchemaVersion,
150+
RequestedMode: mode,
151+
ArtifactPolicy: artifactPolicyFor(mode),
152+
CurrentAuthority: auth.State,
153+
DetectedExternal: auth.External,
154+
RestoreRequested: restoreRequested,
155+
PriorState: prior.State,
156+
PriorIncompleteReason: prior.IncompleteReason,
157+
RestoreAuthorized: restoreRequested && prior.State.IsUsable(),
134158
// Scope boundary is carried in the struct so JSON + text both see
135159
// it. No text/JSON drift (audit D.5).
136160
ScopeBoundary: "PR-22 scope: detect + plan only. No mutation code " +
@@ -139,6 +163,15 @@ func BuildPlan(mode Mode, auth *ClassifyResult, prior *ProbeResult, restoreReque
139163
NoMutationPerformed: true,
140164
}
141165

166+
// PR-P2-1: surface the active-at-install distinction when the
167+
// record is usable. Populated only when the record parsed and was
168+
// classified as UsableActive/UsableInactive — otherwise nil so
169+
// downstream consumers see "unknown," not a defaulted false.
170+
if prior.State.IsUsable() && prior.Record != nil && prior.Record.ActiveAtInstall != nil {
171+
v := *prior.Record.ActiveAtInstall
172+
p.PriorActiveAtInstall = &v
173+
}
174+
142175
// TargetAuthority — per Q2=C (frozen), default is None; restore is
143176
// opt-in AND authorized (needs recorded usable prior).
144177
switch {
@@ -154,9 +187,21 @@ func BuildPlan(mode Mode, auth *ClassifyResult, prior *ProbeResult, restoreReque
154187
p.Warnings = append(p.Warnings,
155188
"--restore-prior-authority requested but no prior-authority record exists — PR-24 will refuse this combination")
156189
}
190+
if restoreRequested && prior.State == PriorRecordMalformed {
191+
p.Warnings = append(p.Warnings,
192+
"--restore-prior-authority requested but prior-authority record is malformed (JSON parse failed) — PR-24 will refuse this combination")
193+
}
157194
if restoreRequested && prior.State == PriorRecordIncomplete {
158195
p.Warnings = append(p.Warnings,
159-
"--restore-prior-authority requested but prior-authority record is incomplete/unparseable — PR-24 will refuse this combination")
196+
"--restore-prior-authority requested but prior-authority record is incomplete — PR-24 will refuse this combination")
197+
}
198+
// PR-P2-1: restoring an inactive prior is a semantically different
199+
// operation (you're starting a firewall that was NOT running). Flag
200+
// it even when restore is authorized so the operator sees the
201+
// distinction before PR-24 runs.
202+
if restoreRequested && prior.State == PriorRecordUsableInactive {
203+
p.Warnings = append(p.Warnings,
204+
"--restore-prior-authority requested; prior-authority record is usable but records the prior firewall as NOT active at install time — restoration would re-enable a firewall the operator was not running")
160205
}
161206
if auth.State == AuthorityAmbiguous {
162207
p.Warnings = append(p.Warnings,
@@ -220,6 +265,20 @@ func (p *Plan) Render(w io.Writer) {
220265
fmt.Fprintf(w, " Restore requested : %s\n", yesNo(p.RestoreRequested))
221266
fmt.Fprintf(w, " Restore authorized : %s\n", yesNo(p.RestoreAuthorized))
222267
fmt.Fprintf(w, " Prior-authority record : %s\n", p.PriorState)
268+
// PR-P2-1: surface the active-at-install distinction when the
269+
// record is usable. Operator must see the semantic difference
270+
// between "restore a running firewall" and "enable a firewall that
271+
// was not running." Unknown states (non-usable record) intentionally
272+
// skip this line rather than display a defaulted value.
273+
switch {
274+
case p.PriorActiveAtInstall != nil && *p.PriorActiveAtInstall:
275+
fmt.Fprintln(w, " Prior firewall at install : active (was running when nftban installed)")
276+
case p.PriorActiveAtInstall != nil && !*p.PriorActiveAtInstall:
277+
fmt.Fprintln(w, " Prior firewall at install : inactive (was NOT running when nftban installed)")
278+
}
279+
if p.PriorIncompleteReason != "" {
280+
fmt.Fprintf(w, " Prior-record issue : %s\n", p.PriorIncompleteReason)
281+
}
223282
fmt.Fprintln(w, "")
224283

225284
if len(p.Warnings) > 0 {

0 commit comments

Comments
 (0)