diff --git a/internal/cli/devices.go b/internal/cli/devices.go index d4f41f2..71efb53 100644 --- a/internal/cli/devices.go +++ b/internal/cli/devices.go @@ -15,6 +15,7 @@ import ( "github.com/Reederey87/DevStrap/internal/pairing" "github.com/Reederey87/DevStrap/internal/state" dssync "github.com/Reederey87/DevStrap/internal/sync" + "github.com/Reederey87/DevStrap/internal/workspacekeys" "github.com/spf13/cobra" "golang.org/x/term" ) @@ -302,6 +303,11 @@ func newDeviceTrustCommand(stdout io.Writer, opts *options, use, trustState stri } } if trustState == "revoked" || trustState == "lost" { + // Issue #134: preflight the post-revoke rotation's recipients + // BEFORE the trust write so the by-far-likeliest rotation + // failure (a malformed recipient on a REMAINING device) is + // named while the operator is still looking at the revoke. + warnMalformedRemainingRecipients(cmd.Context(), stderr, store, args[0]) // TRUST-01: the trust flip and the synced device.revoked/lost // event land in ONE transaction (P6-DATA-03), BEFORE the WCK // rotation below — a rotation failure can never orphan the @@ -816,12 +822,46 @@ func rotateWorkspaceKeyOnRevoke(ctx context.Context, stderr io.Writer, opts *opt // the revoked device (it still holds the old WCK). The trust flip is // deliberately NOT rolled back: refusing the revoke because rotation // failed would keep a compromised device APPROVED, which is worse. - // Be loud about the confidentiality gap and the remedy instead. + // Be loud about the confidentiality gap and record the owed rotation + // so sync's rotation gate retries it every cycle (issue #134). _, _ = fmt.Fprintf(stderr, "warning: workspace key rotation FAILED: %v\n", rerr) - _, _ = fmt.Fprintf(stderr, "warning: events pushed before a successful rotation remain readable by the revoked device (it holds the current epoch key). Run 'devstrap keys rotate' until it succeeds, then 'devstrap sync'.\n") + if merr := markWCKRotationPending(ctx, store, epoch); merr != nil { + _, _ = fmt.Fprintf(stderr, "warning: could not record the owed rotation (%v); sync will NOT auto-retry — run 'devstrap keys rotate' until it succeeds, then 'devstrap sync'\n", merr) + return + } + _, _ = fmt.Fprintf(stderr, "warning: events pushed before a successful rotation remain readable by the revoked device (it holds the current epoch key). The next 'devstrap sync' retries the rotation automatically; run 'devstrap keys rotate' to close the gap now.\n") return } + // A successful Rotate excludes every locally-revoked device, so it also + // satisfies any rotation still owed from an earlier failed revoke. + if cerr := clearWCKRotationPending(ctx, store); cerr != nil { + _, _ = fmt.Fprintf(stderr, "warning: could not clear the owed-rotation marker (%v); sync may rotate once more\n", cerr) + } if len(grants) > 0 { _, _ = fmt.Fprintf(stderr, "Rotated workspace key to epoch %d; granted to %d remaining device(s); run 'devstrap sync' to publish\n", newEpoch, len(grants)) } } + +// warnMalformedRemainingRecipients preflights the post-revoke rotation (issue +// #134): the by-far-likeliest Rotate failure is a malformed age recipient on a +// REMAINING approved device, and by the time Rotate reports it the trust flip +// is already committed. Parsing each remaining recipient up front names the +// offending device while the operator is still looking at the revoke. +// Advisory only — the revoke proceeds regardless (refusing to revoke because a +// bystander's row is malformed would keep the compromised device approved), +// and Rotate's wrap-first ordering remains the enforcement. Empty recipients +// are skipped to mirror ApprovedRecipients. +func warnMalformedRemainingRecipients(ctx context.Context, stderr io.Writer, store *state.Store, target string) { + devices, err := store.ListDevices(ctx) + if err != nil { + return + } + for _, dev := range devices { + if dev.ID == target || dev.TrustState != "approved" || strings.TrimSpace(dev.PublicKey) == "" { + continue + } + if err := workspacekeys.ValidateRecipient(dev.PublicKey); err != nil { + _, _ = fmt.Fprintf(stderr, "warning: remaining device %s has a malformed age recipient (%v); the post-revoke key rotation will fail until it is re-enrolled with a valid recipient\n", dev.ID, err) + } + } +} diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index c051a14..29b291f 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -228,6 +228,7 @@ func runDoctorChecks(ctx context.Context, opts *options) []checkResult { results = append(results, checkSkippedEvents(ctx, store)...) results = append(results, checkWorkspaceKeyAge(ctx, opts, store)...) + results = append(results, checkWCKRotationPending(ctx, store)...) results = append(results, checkForgeCLIs(ctx, opts, store)...) results = append(results, checkAgentRunSweep(ctx, opts, store)...) results = append(results, checkSandboxViolations(ctx, store)...) @@ -472,6 +473,28 @@ func gradeWorkspaceKeyAge(epoch int64, created time.Time, maxAge time.Duration, return checkResult{Name: "workspace key age", Status: checkOK, Detail: detail} } +// checkWCKRotationPending surfaces an owed WCK rotation (issue #134): a device +// revoke could not rotate the epoch, so events keep sealing under a key the +// revoked device still holds until a rotation lands. Silent when nothing is +// owed — the marker exists only after a failed revoke-path rotation and is +// cleared only by this device's own successful Rotate (see wck_rotation.go). +func checkWCKRotationPending(ctx context.Context, store *state.Store) []checkResult { + since, pending, err := wckRotationPendingSince(ctx, store) + if err != nil || !pending { + return nil + } + detail := "a device revoke could not rotate the workspace key; events remain readable by the revoked device" + if !since.IsZero() { + detail = fmt.Sprintf("owed since %s: %s", since.Format(time.RFC3339), detail) + } + return []checkResult{{ + Name: "workspace key rotation", + Status: checkWarn, + Detail: detail, + Remedy: "run 'devstrap sync' (retries the rotation automatically) or 'devstrap keys rotate'", + }} +} + // checkSkippedEvents surfaces the durable P6-SYNC-02 skip records: hub // objects this device's pulls keep dropping, each holding its origin device's // cursor at a seq gap until it applies. The remedy depends on the reason. diff --git a/internal/cli/keys.go b/internal/cli/keys.go index a0691db..86e82d4 100644 --- a/internal/cli/keys.go +++ b/internal/cli/keys.go @@ -57,6 +57,11 @@ re-encrypts blobs and flags secrets for source rotation.`, if err != nil { return err } + // A successful Rotate excludes every locally-revoked device, so + // it also satisfies a rotation owed from a failed revoke (#134). + if cerr := clearWCKRotationPending(cmd.Context(), store); cerr != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "warning: could not clear the owed-rotation marker (%v); sync may rotate once more\n", cerr) + } _, err = fmt.Fprintf(stdout, "Rotated workspace key to epoch %d; queued %d grant event(s); run 'devstrap sync' to publish\n", newEpoch, len(grants)) return err }, diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 7f2c50e..65b6542 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -323,31 +323,69 @@ func keyRotateMaxAge(opts *options) time.Duration { // registry is per-device), so a fleet device unknown to the rotator lands on // the P6-SEC-03 grace→quarantine→replay path until any device that knows it // re-approves it. +// +// Issue #134: it also retries a rotation OWED from a failed revoke-path +// rotation (the wck_rotation_pending marker). The owed retry runs even when +// keys.rotate_max_age=0 — disabling PERIODIC rotation must not disable the +// revoke containment a device already committed to. func maybeRotateWorkspaceKey(ctx context.Context, stdout io.Writer, opts *options, store *state.Store) (bool, error) { - maxAge := keyRotateMaxAge(opts) - if maxAge <= 0 { - return false, nil + pendingSince, pending, err := wckRotationPendingSince(ctx, store) + if err != nil { + return false, err } epoch, created, err := store.ActiveKeyEpochAge(ctx) if err != nil { return false, err } - if epoch == 0 || time.Since(created) < maxAge { + if epoch == 0 { + return false, nil + } + maxAge := keyRotateMaxAge(opts) + aged := maxAge > 0 && time.Since(created) >= maxAge + if !pending && !aged { return false, nil } kr := buildKeyring(ctx, opts, store) newEpoch, grants, rerr := kr.Rotate(ctx) if rerr != nil { - // FATAL for the cycle (post-#56 Codex review, P1): Rotate wraps every - // grant before writing any state, so a failure here is either - // harmless-and-early (nothing recorded; a malformed recipient row - // needs fixing, not retrying) or a rare DB/custody fault mid-commit — - // and in the latter case pushing would seal this cycle's events under - // a half-minted epoch whose grants never published, while the fresh - // created_at suppresses the retry. Aborting keeps the cycle's events - // queued and the failure loud. + // FATAL for the cycle when the failure could be a mid-commit + // half-mint (post-#56 Codex review, P1): if the active epoch advanced + // under this failed Rotate, pushing would seal this cycle's events + // under an epoch whose grants never fully published. Detect it by + // re-reading the epoch rather than assuming. + if after, aerr := store.CurrentKeyEpoch(ctx); aerr != nil || after != epoch { + return false, fmt.Errorf("workspace key rotation failed mid-commit (epoch advanced %d→%d; grants may not have published): %w", epoch, after, rerr) + } + if pending { + // Early failure (nothing recorded — the malformed-recipient + // class). Do NOT fail the cycle: aborting here would also block + // pushing the device.revoked event itself, so the fleet never + // learns about the revoke — strictly worse than pushing under the + // old epoch, which is the already-documented exposure + // (adversarial-review finding, issue #134). Warn loudly, keep the + // marker, retry next cycle; deliberately NOT progressf-gated. + _, _ = fmt.Fprintf(stdout, "warning: workspace key rotation owed since %s still failing (events remain readable by the revoked device until it succeeds): %v\n", + pendingSince.Format(time.RFC3339), rerr) + _, _ = fmt.Fprintf(stdout, "warning: check 'devstrap devices list' for a malformed age recipient, or run 'devstrap keys rotate' after fixing it\n") + return false, nil + } + // Age-triggered early failure keeps the shipped fatal semantics: a + // malformed recipient row needs fixing, not retrying, and there is no + // queued revoke event whose propagation the abort would block. return false, fmt.Errorf("periodic workspace key rotation failed (fix the cause or disable via keys.rotate_max_age=0 / --key-max-age 0): %w", rerr) } + if pending { + // The ONLY resolution path for the owed marker (see wck_rotation.go: + // a newer epoch alone is not proof the revoked device was excluded; a + // local Rotate is). A delete failure surfaces as a cycle error so the + // marker cannot silently outlive its rotation. + if cerr := clearWCKRotationPending(ctx, store); cerr != nil { + return true, cerr + } + opts.progressf(stdout, "Rotated workspace key to epoch %d (rotation owed since %s after a device revoke); %d grant event(s) ride this push\n", + newEpoch, pendingSince.Format(time.RFC3339), len(grants)) + return true, nil + } opts.progressf(stdout, "Rotated workspace key to epoch %d (epoch %d exceeded keys.rotate_max_age %s); %d grant event(s) ride this push\n", newEpoch, epoch, maxAge, len(grants)) return true, nil diff --git a/internal/cli/wck_rotation.go b/internal/cli/wck_rotation.go new file mode 100644 index 0000000..d8bf116 --- /dev/null +++ b/internal/cli/wck_rotation.go @@ -0,0 +1,64 @@ +package cli + +import ( + "context" + "encoding/json" + "time" + + "github.com/Reederey87/DevStrap/internal/state" +) + +// wckRotationPendingMetaKey is the local_meta row recording that a WCK +// rotation is OWED: a `devices revoke`/`lost` could not rotate the epoch, so +// events keep sealing under a key the revoked device still holds until a +// rotation lands (issue #134). Machine-local retry bookkeeping only — it never +// rides the event log. +const wckRotationPendingMetaKey = "wck_rotation_pending" + +// wckRotationPendingRecord is the JSON value of the pending row. Epoch records +// the epoch that was ACTIVE when the rotation failed (diagnostic only — see +// wckRotationPendingSince for why it must NOT drive resolution). +type wckRotationPendingRecord struct { + Epoch int64 `json:"epoch"` + Since time.Time `json:"since"` +} + +// markWCKRotationPending persists the owed-rotation marker after a failed +// revoke-path rotation so sync's rotation gate retries it on every cycle. +func markWCKRotationPending(ctx context.Context, store *state.Store, epoch int64) error { + raw, err := json.Marshal(wckRotationPendingRecord{Epoch: epoch, Since: time.Now().UTC()}) + if err != nil { + return err + } + return store.SetLocalMeta(ctx, wckRotationPendingMetaKey, string(raw)) +} + +// wckRotationPendingSince reports whether a rotation is still owed. The marker +// resolves ONLY via clearWCKRotationPending after THIS device's own successful +// Rotate — never by observing a newer epoch (adversarial-review HIGH, issue +// #134): a peer that has not yet pulled the revoke can rotate for age reasons +// and grant the new epoch to the still-approved-in-its-registry revoked +// device, so "any newer epoch is active" is NOT proof the revoked device was +// excluded. A locally-run Rotate always excludes locally-revoked devices, +// which is exactly the proof the marker needs; the worst case of ignoring a +// legitimate peer rotation is one redundant epoch. A marker that fails to +// parse stays pending with a zero Since (fail-closed). +func wckRotationPendingSince(ctx context.Context, store *state.Store) (time.Time, bool, error) { + raw, ok, err := store.GetLocalMeta(ctx, wckRotationPendingMetaKey) + if err != nil || !ok { + return time.Time{}, false, err + } + var rec wckRotationPendingRecord + if jerr := json.Unmarshal([]byte(raw), &rec); jerr != nil { + return time.Time{}, true, nil + } + return rec.Since, true, nil +} + +// clearWCKRotationPending removes the marker. Called ONLY after a successful +// local Rotate (sync's owed retry, `keys rotate`, or a later revoke whose +// rotation succeeded) — every local Rotate wraps to ApprovedRecipients, which +// excludes all locally-revoked devices, satisfying the owed containment. +func clearWCKRotationPending(ctx context.Context, store *state.Store) error { + return store.DeleteLocalMeta(ctx, wckRotationPendingMetaKey) +} diff --git a/internal/cli/wck_rotation_test.go b/internal/cli/wck_rotation_test.go new file mode 100644 index 0000000..53633e5 --- /dev/null +++ b/internal/cli/wck_rotation_test.go @@ -0,0 +1,250 @@ +package cli + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/Reederey87/DevStrap/internal/platform" + "github.com/Reederey87/DevStrap/internal/state" + "github.com/spf13/viper" +) + +// Issue #134: a `devices revoke` whose WCK rotation fails must (a) preflight- +// name the malformed remaining recipient, (b) persist the owed rotation, and +// (c) have the next sync cycle's rotation gate retry it — even with periodic +// rotation disabled — clearing the marker on success. + +// openTestStore opens the state DB under home for assertions. +func openTestStore(t *testing.T, home string) *state.Store { + t.Helper() + opts := &options{v: viper.New()} + opts.v.Set("home", home) + st, err := state.Open(context.Background(), opts.paths().StateDB()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { closeStore(st) }) + return st +} + +func TestDeviceRevokeRotationFailureMarksPendingAndSyncRetries(t *testing.T) { + t.Setenv(platform.NoKeychainEnv, "1") + ctx := context.Background() + // rotateTestHome: init + EnsureBootstrap(epoch 1) + one approved peer with + // VALID keys ("dev_rotate_peer") + a captured env binding. + home, root := rotateTestHome(t) + + st := openTestStore(t, home) + // A second approved bystander with a MALFORMED age recipient: Rotate + // wrap-first fails on it, which is exactly the #134 failure class. + if err := st.UpsertDevice(ctx, state.Device{ + ID: "dev_badrec", Name: "badrec", OS: "linux", Arch: "arm64", + PublicKey: "not-an-age-recipient", SigningPublicKey: "irrelevant", TrustState: "approved", + }); err != nil { + t.Fatal(err) + } + + _, stderr, err := executeForTest("--home", home, "--root", root, "devices", "revoke", "dev_rotate_peer") + if err != nil { + t.Fatalf("revoke: %v (%s)", err, stderr) + } + // (a) The preflight names the malformed REMAINING device before the flip. + if !strings.Contains(stderr, "remaining device dev_badrec has a malformed age recipient") { + t.Fatalf("stderr = %q, want preflight warning naming dev_badrec", stderr) + } + // The rotation itself failed loudly and promised the auto-retry. + if !strings.Contains(stderr, "workspace key rotation FAILED") { + t.Fatalf("stderr = %q, want rotation FAILED warning", stderr) + } + if !strings.Contains(stderr, "retries the rotation automatically") { + t.Fatalf("stderr = %q, want auto-retry promise", stderr) + } + // (b) The owed rotation is persisted with the epoch active at failure. + raw, ok, err := st.GetLocalMeta(ctx, wckRotationPendingMetaKey) + if err != nil || !ok { + t.Fatalf("GetLocalMeta(%s) = %q, %v, %v; want persisted marker", wckRotationPendingMetaKey, raw, ok, err) + } + var rec wckRotationPendingRecord + if err := json.Unmarshal([]byte(raw), &rec); err != nil { + t.Fatal(err) + } + if rec.Epoch != 1 || rec.Since.IsZero() { + t.Fatalf("pending record = %+v; want epoch 1 with a timestamp", rec) + } + if epoch, err := st.CurrentKeyEpoch(ctx); err != nil || epoch != 1 { + t.Fatalf("CurrentKeyEpoch = %d, %v; want 1 (rotation failed)", epoch, err) + } + + // Fix the bystander's recipient, then run the sync rotation gate with + // periodic rotation DISABLED: the owed retry must rotate anyway (c). + goodDev, err := deviceByID(ctx, st, "dev_rotate_peer") + if err != nil { + t.Fatal(err) + } + if err := st.UpsertDevice(ctx, state.Device{ + ID: "dev_badrec", Name: "badrec", OS: "linux", Arch: "arm64", + PublicKey: goodDev.PublicKey, SigningPublicKey: goodDev.SigningPublicKey, TrustState: "approved", + }); err != nil { + t.Fatal(err) + } + opts := &options{v: viper.New()} + opts.v.Set("home", home) + opts.v.Set("root", root) + opts.v.Set("keys.rotate_max_age", "0") + var out strings.Builder + rotated, err := maybeRotateWorkspaceKey(ctx, &out, opts, st) + if err != nil { + t.Fatalf("maybeRotateWorkspaceKey: %v", err) + } + if !rotated { + t.Fatal("maybeRotateWorkspaceKey did not rotate; owed retry must run even with keys.rotate_max_age=0") + } + if !strings.Contains(out.String(), "rotation owed since") { + t.Fatalf("progress = %q, want owed-rotation wording", out.String()) + } + if epoch, err := st.CurrentKeyEpoch(ctx); err != nil || epoch != 2 { + t.Fatalf("CurrentKeyEpoch = %d, %v; want 2 after retry", epoch, err) + } + if _, ok, err := st.GetLocalMeta(ctx, wckRotationPendingMetaKey); err != nil || ok { + t.Fatalf("pending marker still present (%v, %v); want cleared on success", ok, err) + } +} + +// TestMaybeRotateWarnsAndContinuesCycleOnEarlyOwedFailure (adversarial-review +// fix): an owed retry that fails EARLY (nothing recorded — the malformed- +// recipient class) must NOT fail the sync cycle, or the device.revoked event +// itself never pushes and the fleet never learns about the revoke. It warns, +// keeps the marker, and lets the cycle proceed. +func TestMaybeRotateWarnsAndContinuesCycleOnEarlyOwedFailure(t *testing.T) { + t.Setenv(platform.NoKeychainEnv, "1") + ctx := context.Background() + home, root := rotateTestHome(t) + st := openTestStore(t, home) + if err := st.UpsertDevice(ctx, state.Device{ + ID: "dev_badrec", Name: "badrec", OS: "linux", Arch: "arm64", + PublicKey: "not-an-age-recipient", SigningPublicKey: "irrelevant", TrustState: "approved", + }); err != nil { + t.Fatal(err) + } + if err := markWCKRotationPending(ctx, st, 1); err != nil { + t.Fatal(err) + } + opts := &options{v: viper.New()} + opts.v.Set("home", home) + opts.v.Set("root", root) + var out strings.Builder + rotated, err := maybeRotateWorkspaceKey(ctx, &out, opts, st) + if rotated || err != nil { + t.Fatalf("maybeRotateWorkspaceKey = %v, %v; want warn-and-continue on an early owed failure", rotated, err) + } + if !strings.Contains(out.String(), "rotation owed since") || !strings.Contains(out.String(), "still failing") { + t.Fatalf("output = %q; want loud owed-rotation warning", out.String()) + } + if epoch, err := st.CurrentKeyEpoch(ctx); err != nil || epoch != 1 { + t.Fatalf("CurrentKeyEpoch = %d, %v; want unchanged 1", epoch, err) + } + // Marker survives for the next cycle. + if _, ok, gerr := st.GetLocalMeta(ctx, wckRotationPendingMetaKey); gerr != nil || !ok { + t.Fatalf("pending marker gone (%v, %v); must survive a failed retry", ok, gerr) + } +} + +// TestWCKRotationPendingSurvivesNewerEpoch (adversarial-review HIGH): a newer +// active epoch is NOT proof the revoked device was excluded — a peer that has +// not pulled the revoke yet can rotate and grant the revoked device the new +// epoch. The marker must survive until THIS device's own Rotate succeeds. +func TestWCKRotationPendingSurvivesNewerEpoch(t *testing.T) { + t.Setenv(platform.NoKeychainEnv, "1") + ctx := context.Background() + home, _ := rotateTestHome(t) // epoch 1 active + st := openTestStore(t, home) + if err := markWCKRotationPending(ctx, st, 0); err != nil { // recorded epoch 0 < active 1 + t.Fatal(err) + } + since, pending, err := wckRotationPendingSince(ctx, st) + if err != nil { + t.Fatal(err) + } + if !pending || since.IsZero() { + t.Fatalf("pending = %v since %v; a newer epoch must NOT resolve the owed rotation", pending, since) + } + if _, ok, err := st.GetLocalMeta(ctx, wckRotationPendingMetaKey); err != nil || !ok { + t.Fatalf("marker deleted by a read (%v, %v); resolution is Rotate-only", ok, err) + } +} + +// TestKeysRotateClearsOwedRotation: the manual remedy also resolves the owed +// marker (a local Rotate always excludes locally-revoked devices). +func TestKeysRotateClearsOwedRotation(t *testing.T) { + t.Setenv(platform.NoKeychainEnv, "1") + ctx := context.Background() + home, root := rotateTestHome(t) + st := openTestStore(t, home) + if err := markWCKRotationPending(ctx, st, 1); err != nil { + t.Fatal(err) + } + if _, stderr, err := executeForTest("--home", home, "--root", root, "keys", "rotate"); err != nil { + t.Fatalf("keys rotate: %v (%s)", err, stderr) + } + if _, ok, err := st.GetLocalMeta(ctx, wckRotationPendingMetaKey); err != nil || ok { + t.Fatalf("owed marker survives a successful keys rotate (%v, %v)", ok, err) + } +} + +// TestWCKRotationPendingMalformedRecordStaysPending: garbage in the marker +// must fail closed (still pending) — the row only exists because a rotation +// failed, and a successful retry clears it explicitly. +func TestWCKRotationPendingMalformedRecordStaysPending(t *testing.T) { + t.Setenv(platform.NoKeychainEnv, "1") + ctx := context.Background() + home, _ := rotateTestHome(t) + st := openTestStore(t, home) + if err := st.SetLocalMeta(ctx, wckRotationPendingMetaKey, "{garbage"); err != nil { + t.Fatal(err) + } + _, pending, err := wckRotationPendingSince(ctx, st) + if err != nil { + t.Fatal(err) + } + if !pending { + t.Fatal("malformed marker treated as not pending; must fail closed") + } +} + +func TestDoctorWarnsWCKRotationPending(t *testing.T) { + t.Setenv(platform.NoKeychainEnv, "1") + ctx := context.Background() + home, root := rotateTestHome(t) + st := openTestStore(t, home) + if err := markWCKRotationPending(ctx, st, 1); err != nil { // epoch 1 is active → still owed + t.Fatal(err) + } + stdout, _, _ := executeForTest("--home", home, "--root", root, "doctor") + if !strings.Contains(stdout, "workspace key rotation") || !strings.Contains(stdout, "owed since") { + t.Fatalf("doctor output missing owed-rotation warning:\n%s", stdout) + } + if !strings.Contains(stdout, "retries the rotation automatically") { + t.Fatalf("doctor output missing auto-retry remedy:\n%s", stdout) + } +} + +func TestDeleteLocalMetaIdempotent(t *testing.T) { + t.Setenv(platform.NoKeychainEnv, "1") + ctx := context.Background() + home, _ := rotateTestHome(t) + st := openTestStore(t, home) + if err := st.SetLocalMeta(ctx, "test_key", "v"); err != nil { + t.Fatal(err) + } + if err := st.DeleteLocalMeta(ctx, "test_key"); err != nil { + t.Fatal(err) + } + if _, ok, err := st.GetLocalMeta(ctx, "test_key"); err != nil || ok { + t.Fatalf("row survives delete (%v, %v)", ok, err) + } + if err := st.DeleteLocalMeta(ctx, "test_key"); err != nil { + t.Fatalf("deleting an absent key must be a no-op, got %v", err) + } +} diff --git a/internal/state/store.go b/internal/state/store.go index 5837c9d..03f9fb8 100644 --- a/internal/state/store.go +++ b/internal/state/store.go @@ -2920,6 +2920,16 @@ ON CONFLICT(key) DO UPDATE SET value = excluded.value, updated_at = excluded.upd return nil } +// DeleteLocalMeta removes a local, never-synced key/value metadata row. +// Deleting an absent key is a no-op. Used to clear self-resolved flags such as +// the owed-WCK-rotation marker (issue #134). +func (s *Store) DeleteLocalMeta(ctx context.Context, key string) error { + if _, err := s.db.ExecContext(ctx, `DELETE FROM local_meta WHERE key = ?;`, key); err != nil { + return fmt.Errorf("delete local meta %q: %w", key, err) + } + return nil +} + // ApprovedDeviceSigningKey returns the signing public key of a locally known, // APPROVED device, or ok=false when the device is unknown, not approved, or has // no signing key recorded. It is the trust gate for snapshot recovery diff --git a/internal/workspacekeys/keyring.go b/internal/workspacekeys/keyring.go index 92c8c79..aa260dc 100644 --- a/internal/workspacekeys/keyring.go +++ b/internal/workspacekeys/keyring.go @@ -520,6 +520,16 @@ func (k *Keyring) cached(epoch int64, kid string) ([]byte, bool) { return entry.wck, true } +// ValidateRecipient reports whether recipient parses as an age X25519 +// recipient — the same parse wrapWCK performs. `devices revoke` preflights the +// remaining approved recipients with it BEFORE the trust write so the +// by-far-likeliest rotation failure (a malformed recipient row) is named up +// front (issue #134); Rotate's wrap-first ordering remains the enforcement. +func ValidateRecipient(recipient string) error { + _, err := age.ParseX25519Recipient(recipient) + return err +} + // wrapWCK age-encrypts a WCK to a single X25519 recipient and returns base64. func wrapWCK(wck []byte, recipient string) (string, error) { r, err := age.ParseX25519Recipient(recipient) diff --git a/spec/07_NAMESPACE_AND_SYNC_MODEL.md b/spec/07_NAMESPACE_AND_SYNC_MODEL.md index f295da6..9d65ed2 100644 --- a/spec/07_NAMESPACE_AND_SYNC_MODEL.md +++ b/spec/07_NAMESPACE_AND_SYNC_MODEL.md @@ -636,7 +636,7 @@ Lifecycle: - **Init** (`devstrap init`): the **founder** mints the workspace id; a second device does not mint its own but adopts the founder's with `devstrap init --join --workspace-id ` (`--workspace-id` implies `--join`; the R2/S3 hub keys under `workspaces//`, so a self-minted id keys a disjoint prefix — see the pairing runbook in `19_CLOUD_PROVISIONING_GUIDE.md` §E). Adopting is born-correct: a mismatch against an already-initialized store is refused with a remove-and-reinit remedy, never a post-hoc rewrite. Init does **not** mint a WCK (`P6-SEC-02`, shipped). Founding is deferred to the first `devstrap sync`: the founder/join gate in `runSyncCycle` mints epoch 1 (`EnsureBootstrap`) only when the hub is genuinely empty — **both** the pull and push cursors are 0 (this device has never observed hub content) **and** the pull returned zero raw objects (`EncryptedHub.PullStats.RawSeen == 0`; `RawSeen` alone only proves "nothing new after the pull cursor", so a keyless device whose pull cursor already advanced past quarantined hub events must not found) — and the device did not `init --join`. A device that JOINS an existing workspace (`init --join`, or any device whose first pull sees a non-empty hub) never self-mints a key nobody else holds, so it can never seal its pre-approval events under a never-granted WCK — the SEC-02 data loss. The joiner receives the fleet WCK via `devices approve` (`GrantAllEpochs`) on an existing device; approving another device from a never-synced **founder** still founds defensively, but a **joiner** approving a device while holding no key never self-mints (`grantWorkspaceKeyToApprovedDevice` is founder-gated — it warns and grants nothing, closing the last self-mint path a joiner could reach). WCKs are stored in the OS keychain / 0600 file fallback (`devicekeys.HybridStore`). - **Approve** (`devices approve` / `enroll --approve`): gated on out-of-band **fingerprint confirmation** before any trust-state write (`P4-SEC-04`, shipped) — the full 256-bit fingerprint binding the device's signing key and age recipient is computed from the row/flags/code being approved and must be confirmed via `--fingerprint`, an interactive `yes`, or (non-TTY) a copy-paste remedy; a keyless placeholder row is refused outright (`SECU-05`). The exchanged values may ride in an unauthenticated `devstrap-pair1:` blob (`devices pairing-code`, `init --join --code`, `devices enroll --code`); integrity still comes from the derived fingerprint, never from the blob. Once confirmed, `GrantAllEpochs(recipient)` wraps every held epoch's WCK to the newly-approved device and emits one `device.key.granted` event per epoch. The new device ingests them on its first pull and decrypts the entire history. On a keyless **joiner**, approve grants nothing (`grantWorkspaceKeyToApprovedDevice` is founder-gated) but the approved row still pins the device's keys and flips verification fail-closed — the `P4-SEC-04` founder-pinning ceremony a joiner runs before its first sync. - **Periodic rotation** (`keys rotate`, or automatically during `sync` once the active epoch is older than `keys.rotate_max_age`, default 90d): a pure `Rotate` — mint epoch+1, grant to every locally-known approved device, publish on the next push. Deliberately none of revoke's side effects (no secret flags, no blob rewrap, no hub deletes): it bounds FORWARD exposure of a silently compromised key and nothing else. The auto-rotate check runs AFTER the pull (a freshly ingested grant resets the local age, so at most one device in a fleet rotates per deadline instead of storming) and BEFORE the push (grants + events sealed under the new epoch ride one cycle; the peer converges in one pull because grants ingest before decryption — pinned by `sync_rotate_converge.txtar`). Concurrent mints from two devices racing the same deadline are harmless by design: `(epoch, kid)` keying lets both keys coexist at the epoch and each device keeps PUSHING under its own mint until it ingests the other's grant — push-key selection converges on `grant`-origin keys, and readers hold both keys either way, so do not "fix" the transient push-key divergence. Residual (also spec/15): the rotator grants only to approved devices its LOCAL registry knows; an unknown fleet device rides the `P6-SEC-03` grace→quarantine→replay path until re-approved by a device that knows it. -- **Revoke / lost** (`devices revoke` / `lost`): `Rotate` mints a fresh WCK at epoch+1 and wraps it to the remaining `ApprovedRecipients` (the revoked device is already excluded), emitting grant events. Go-forward events encrypt under the new epoch, giving forward secrecy without re-encrypting past events (a revoked device keeps its already-downloaded history — the residual risk age's no-native-revocation model accepts, bounded by secret rotation). The existing blob re-encryption pass runs after the rotate. +- **Revoke / lost** (`devices revoke` / `lost`): `Rotate` mints a fresh WCK at epoch+1 and wraps it to the remaining `ApprovedRecipients` (the revoked device is already excluded), emitting grant events. Go-forward events encrypt under the new epoch, giving forward secrecy without re-encrypting past events (a revoked device keeps its already-downloaded history — the residual risk age's no-native-revocation model accepts, bounded by secret rotation). The existing blob re-encryption pass runs after the rotate. A FAILED revoke-path rotation is self-healing (issue #134): the revoke preflights the remaining approved recipients before the trust write (naming a malformed recipient row up front), records the owed rotation in the machine-local `wck_rotation_pending` marker (local_meta), and every subsequent `sync` cycle's rotation gate retries it — even with `keys.rotate_max_age=0`. The marker resolves ONLY on this device's own successful `Rotate` (sync retry, `keys rotate`, or a later revoke's rotation) — deliberately never by observing a newer epoch, because a peer that has not yet pulled the revoke can rotate for age reasons and grant the new epoch to the still-approved-in-its-registry revoked device (adversarial-review HIGH); the worst case of ignoring a legitimate peer rotation is one redundant epoch. A retry that fails EARLY (nothing recorded) warns loudly and lets the cycle continue so the `device.revoked` event itself still pushes — aborting would keep the fleet ignorant of the revoke, strictly worse; a failure with the epoch ADVANCED (a mid-commit half-mint whose grants may not have published) stays fatal for the cycle. `doctor` warns while the rotation is owed. - **Pull**: `EncryptedHub.Pull` primes the keyring, **verifies each `device.key.granted` carrier before ingesting its WCK** (`P6-SEC-01`, shipped), ingests the verified in-batch grants in HLC order (so a new device obtains its WCKs before decrypting history), then decrypts `enc.v2` envelopes. The verification uses the `EncryptedHub.Verify` seam wired by `hubFromOptions` to `(*state.Store).VerifyRemoteEvent` (the same content-hash self-consistency check plus `verifyEventSignature` the apply path runs, so the pre-ingest gate rejects exactly the apply-path permanent-failure set), so once any device is approved a grant forged by an unknown/unapproved/bad-signature device — e.g. a hostile hub wrapping an attacker-chosen WCK to the victim's own recipient — is refused and never reaches `StoreWCK`/`RecordKeyEpoch`/the cache; the refused carrier still flows to `ApplyEvents` and is quarantined as an `event_verification_failure` conflict. Before enrollment, grants are accepted (the `P4-SEC-04` bootstrap window). The key-overwrite refusal (`P6-SEC-01` steps b/c) is shipped via `(epoch, kid)` addressing (see the P6-SEC-02 section below): `IngestGrant` computes `kid = hex(sha256(wck))` from the unwrapped bytes, rejects a grant whose carried kid disagrees, and stores each key in its own `(epoch, kid)` slot — nothing ever displaces an existing key (a same-slot custody write additionally byte-compares and refuses a mismatch), and push-key selection prefers verified `grant`-origin keys over `self` mints over `legacy` backfills. Because the hub is untrusted, a single non-conforming object must never wedge sync, so Pull degrades instead of aborting the whole batch: an event whose **(epoch, kid) key has not yet been granted** — a missing epoch, or an unheld kid at a held epoch (the fleet key vs. a legacy self-mint collision) — *truncates* the batch **within a bounded grace window** (`P6-SEC-03`: the decryptable prefix is returned and applies; the cursor advances up to but not past it; the next sync retries once the grant arrives) and **quarantines past it** (the first sighting of the missing key is recorded in `key_grant_waits` through the `MissingKeyWait` seam — the earliest first-seen across every kid at the epoch, so re-pulls and hostile kid relabeling never restart the clock — and once `sync.key_grant_grace` (default 72h, `0` = immediate) has elapsed the still-encrypted carrier is forwarded to the same undecryptable quarantine as an AEAD failure, so the cursor advances, later held-epoch events in the batch still apply, and a grant that eventually arrives recovers the carrier via the replay path), while an **AEAD failure on the held candidate key(s)** (corruption, forgery, or a hub-side carrier mutation; kid-less envelopes try every held key at the epoch) *forwards the still-encrypted carrier* so `ApplyEvents` records a permanent `event_verification_failure` conflict of kind `undecryptable` (`P6-SYNC-04`): visible in `conflicts list`, it blocks `hub gc`, never enters the event log, is never replayed by `devices approve`, and the cursor advances past it — visible refusal without a wedge and without silent loss. Because the defer-vs-quarantine classification necessarily reads the untrusted kid hint, a hostile hub could steer a NOT-yet-granted event into this quarantine by stripping/relabeling the hint; every pull therefore replays open undecryptable conflicts against the keys held then (`ReplayUndecryptableConflicts` in `pullAndApplyEvents`) — once the grant lands the carrier decrypts, applies through the normal verified path, and the conflict auto-resolves, so kid tampering delays that event but cannot destroy it. A **malformed/unknown envelope**, **retired `enc.v1` traffic** (the remedy is re-founding the hub), or a **non-grant plaintext event** (anti-downgrade) is still *skipped with a loud warning* and Pull continues. Bad events are never applied — the security property (no unauthenticated data enters the log) is preserved. **P6-SYNC-02 (shipped):** every drop leaves a durable, self-clearing `sync_skipped_events` record; unknown envelope versions defer per-device within the grace window then quarantine; malformed envelopes forward straight to the undecryptable quarantine; retired v1 and anti-downgrade plaintext hold their origin device's cursor at the seq gap with `status`/`doctor` surfacing and a `hub gc` refusal. See the P6-SYNC-02 section below. (`ErrPlaintextEventFromHub`/`ErrUnknownEnvelopeVersion` still surface from `ParseEncryptedEnvelope`, and `ErrMissingWorkspaceKey` still guards `Push`.) SQLite holds only non-secret metadata (`workspace_keys`, `workspace_key_grants` — migrations 00013 + 00014, keyed `(workspace_id, epoch, kid)` with an `origin` column); the secret WCK lives only in the keychain / 0600 file fallback, addressed by the same `(epoch, kid)`. diff --git a/spec/13_CLI_DAEMON_API.md b/spec/13_CLI_DAEMON_API.md index d77a12c..74c3686 100644 --- a/spec/13_CLI_DAEMON_API.md +++ b/spec/13_CLI_DAEMON_API.md @@ -109,7 +109,7 @@ Rules: - `restore` (`P6-DATA-04`) extracts a `--full` archive and replaces only the captured paths (`state.db`, `config.yaml`, `blobs/`, `keys/`) **in place**, leaving un-captured state-dir contents (e.g. `quarantine/`, `logs/`) intact. It refuses when a captured target already exists unless `--force`. Extraction is staged in a sibling temp dir, guarded against path traversal (zip-slip: absolute paths and `..` components are rejected, entries are confined to the `state.db`/`config.yaml`/`blobs/`/`keys/` layout), the staged DB is validated (`quick_check` + `foreign_key_check`) before any swap, and each captured path is swapped in via a `.bak` sibling + rename with rollback (`0600`); a restore of a keychain-custody DB prints the file-custody reconciliation guidance; - state DB and backups are mode `0600`. -`doctor` (`PROD-02`) is a severity-graded health report: each check returns `{name, status: ok|warning|error, detail, remedy}`, rendered as a graded table with a summary line and a non-zero exit code when any check is error (so it can gate CI). Checks cover git/gh/go tools (git required, gh/go optional), state home + permissions, schema version, SQLite `quick_check`/`foreign_key_check`, dangling blob refs (`P6-DATA-04`: every `age_blob:` ref the DB holds must have its ciphertext present under `blobs/`; a missing one is an error whose remedy points at a `db backup --full` restore), secrets needing rotation, the recorded key-custody backend (`P6-XP-04`: a `key custody` row reporting `keychain` or `file`, warning when the recorded backend is currently unreachable, is being overridden by `DEVSTRAP_NO_KEYCHAIN`, or has not been recorded yet on a pre-`P6-XP-04` store), local age + Ed25519 device-key health, workspace keys awaiting grants (`P6-SEC-03`: each open `key_grant_waits` row with epoch/kid/first-seen and the re-approve remedy), the active workspace key's age against `keys.rotate_max_age` (`P4-SEC-07`: ok at epoch 0, warn past the deadline with the `keys rotate` remedy), agent-run dead-PID reconciliation (`P6-GIT-06`: `agent run sweep` reports rows flipped from `running` to `interrupted` and the remaining running count), and held repo locks (stale = warning). `--json` emits the check array; `--fix` applies safe remediations (create the missing state home, run pending migrations, clear stale repo locks) and re-runs the checks. `--remote` (`P5-PROD-05`) additionally probes the configured sync hub (reachability, pending push, queued deletes, device trust) and always reports a `workspace id` row (a warning row when the id is unreadable) so two devices can be compared directly; `--hub-file` selects the file-backed hub for that probe. For workspace-id-keyed remote hubs (R2/S3 and the git carrier), `--remote` also warns `workspace id match` when the local role is `joiner`, the pull cursor is still `0`, and the raw hub backend reports no events under this device's workspace-id prefix — the signature of a joiner reading its own empty `workspaces//...` prefix instead of the founder's populated prefix. The remedy text points operators to confirm the founder's workspace id with `devstrap doctor` on the founding device and re-init the joiner with `devstrap init --join --workspace-id ` (the adoption flag shipped with the P4-SEC-07 pairing wave — see the `init` section above; this change ships the detection and regression-test side). +`doctor` (`PROD-02`) is a severity-graded health report: each check returns `{name, status: ok|warning|error, detail, remedy}`, rendered as a graded table with a summary line and a non-zero exit code when any check is error (so it can gate CI). Checks cover git/gh/go tools (git required, gh/go optional), state home + permissions, schema version, SQLite `quick_check`/`foreign_key_check`, dangling blob refs (`P6-DATA-04`: every `age_blob:` ref the DB holds must have its ciphertext present under `blobs/`; a missing one is an error whose remedy points at a `db backup --full` restore), secrets needing rotation, the recorded key-custody backend (`P6-XP-04`: a `key custody` row reporting `keychain` or `file`, warning when the recorded backend is currently unreachable, is being overridden by `DEVSTRAP_NO_KEYCHAIN`, or has not been recorded yet on a pre-`P6-XP-04` store), local age + Ed25519 device-key health, workspace keys awaiting grants (`P6-SEC-03`: each open `key_grant_waits` row with epoch/kid/first-seen and the re-approve remedy), the active workspace key's age against `keys.rotate_max_age` (`P4-SEC-07`: ok at epoch 0, warn past the deadline with the `keys rotate` remedy), an owed post-revoke workspace-key rotation (issue #134: warn while the machine-local `wck_rotation_pending` marker is set — a `devices revoke`/`lost` could not rotate the epoch, so events stay readable by the revoked device; remedy names sync's automatic retry and `keys rotate`; silent when nothing is owed), agent-run dead-PID reconciliation (`P6-GIT-06`: `agent run sweep` reports rows flipped from `running` to `interrupted` and the remaining running count), and held repo locks (stale = warning). `--json` emits the check array; `--fix` applies safe remediations (create the missing state home, run pending migrations, clear stale repo locks) and re-runs the checks. `--remote` (`P5-PROD-05`) additionally probes the configured sync hub (reachability, pending push, queued deletes, device trust) and always reports a `workspace id` row (a warning row when the id is unreadable) so two devices can be compared directly; `--hub-file` selects the file-backed hub for that probe. For workspace-id-keyed remote hubs (R2/S3 and the git carrier), `--remote` also warns `workspace id match` when the local role is `joiner`, the pull cursor is still `0`, and the raw hub backend reports no events under this device's workspace-id prefix — the signature of a joiner reading its own empty `workspaces//...` prefix instead of the founder's populated prefix. The remedy text points operators to confirm the founder's workspace id with `devstrap doctor` on the founding device and re-init the joiner with `devstrap init --join --workspace-id ` (the adoption flag shipped with the P4-SEC-07 pairing wave — see the `init` section above; this change ships the detection and regression-test side). ### scan diff --git a/spec/15_SECURITY_THREAT_MODEL.md b/spec/15_SECURITY_THREAT_MODEL.md index 600f10c..3812ad0 100644 --- a/spec/15_SECURITY_THREAT_MODEL.md +++ b/spec/15_SECURITY_THREAT_MODEL.md @@ -177,7 +177,7 @@ Attacker = a mistaken operator, or the untrusted hub steering a device onto the ### Threat: one bad signed event wedges every device's sync (`P6-SYNC-01`, mitigated) -Attacker = a revoked/lost device that keeps pushing signed events (or a hub replaying one). Previously any signature/trust failure in `ApplyEvents` aborted the whole batch before the cursor advanced, so a single poisoned event re-pulled and re-failed forever — a fleet-wide availability break requiring no key compromise. **Mitigated:** permanent verification failures (signature/trust/content-hash, wrapped in `state.ErrEventVerification`) and divergent duplicate IDs are now quarantined per-event as `event_verification_failure` conflicts (full event JSON retained for replay) while the rest of the batch applies and the cursor advances safely; `devices approve` replays a newly-approved device's quarantined events. The synced trust event is now shipped (TRUST-01): `devices revoke`/`lost` emit `device.revoked`/`device.lost` (mustVerify, same-tx with the local flip), every receiving device applies it sticky/monotonically (local device exempt; approval never propagates — it stays the local P4-SEC-04 ceremony) and flags `secret_bindings.needs_rotation` once. Residuals: mutual revocation across different pull windows can leave bystanders divergent (fail-closed either way, loud, recovered by the two-step local re-approval — re-approving one side replays its counter-revoke); a FAILED WCK rotation during revoke leaves the old epoch active, so events pushed before a later successful rotation (including the revoke event itself) stay readable by the revoked device — the trust flip is deliberately kept (refusing the revoke would keep a compromised device approved, which is worse) and the CLI warns loudly with the `keys rotate` remedy (adversarial-review finding; fail-closed rotation is a candidate follow-up); bounded conflict-row aggregation for a still-pushing revoked device. +Attacker = a revoked/lost device that keeps pushing signed events (or a hub replaying one). Previously any signature/trust failure in `ApplyEvents` aborted the whole batch before the cursor advanced, so a single poisoned event re-pulled and re-failed forever — a fleet-wide availability break requiring no key compromise. **Mitigated:** permanent verification failures (signature/trust/content-hash, wrapped in `state.ErrEventVerification`) and divergent duplicate IDs are now quarantined per-event as `event_verification_failure` conflicts (full event JSON retained for replay) while the rest of the batch applies and the cursor advances safely; `devices approve` replays a newly-approved device's quarantined events. The synced trust event is now shipped (TRUST-01): `devices revoke`/`lost` emit `device.revoked`/`device.lost` (mustVerify, same-tx with the local flip), every receiving device applies it sticky/monotonically (local device exempt; approval never propagates — it stays the local P4-SEC-04 ceremony) and flags `secret_bindings.needs_rotation` once. Residuals: mutual revocation across different pull windows can leave bystanders divergent (fail-closed either way, loud, recovered by the two-step local re-approval — re-approving one side replays its counter-revoke); a FAILED WCK rotation during revoke leaves the old epoch active, so events pushed before a later successful rotation (including the revoke event itself) stay readable by the revoked device — the trust flip is deliberately kept (refusing the revoke would keep a compromised device approved, which is worse), and the exposure window is now bounded by the next SUCCESSFUL rotation (issue #134, shipped): the revoke records the owed rotation in a machine-local `wck_rotation_pending` marker, every `devstrap sync` cycle's rotation gate retries it — even with `keys.rotate_max_age=0`, since disabling periodic rotation must not disable committed revoke containment — and the marker resolves ONLY on this device's own successful `Rotate`, never on merely observing a newer epoch (adversarial-review HIGH: an uninformed peer's age rotation can grant the new epoch to the revoked device, so "newer epoch" is not proof of exclusion). An early retry failure warns and lets the cycle continue so the `device.revoked` event still propagates (aborting would keep the fleet ignorant of the revoke — the availability regression the adversarial review flagged); a mid-commit failure (epoch advanced, grants possibly unpublished) stays fatal for the cycle. `devices revoke` preflights the remaining approved recipients so the likeliest failure (a malformed recipient row) is named before the trust write, and `doctor` warns `workspace key rotation` while the rotation is owed; bounded conflict-row aggregation for a still-pushing revoked device. **Related (owned elsewhere):** `P6-HUB-01` — hub-side blob GC data-loss (availability, owned in `spec/03`, shipped); `P6-SYNC-03` — revoking the last approved device previously reopened the fail-open bootstrap window (owned in `spec/07`, **shipped**: sticky enrollment, see above). diff --git a/spec/18_WORK_LOG.md b/spec/18_WORK_LOG.md index 7d7020a..068028b 100644 --- a/spec/18_WORK_LOG.md +++ b/spec/18_WORK_LOG.md @@ -31,6 +31,23 @@ Follow-ups: Entries are newest-first: each code-modifying cycle prepends ONE dated entry at the top. +## 2026-07-06 — feat(devices): self-healing WCK rotation after revoke (#134) + +Changed: +- `internal/cli/wck_rotation.go` (new): the owed-rotation marker — a `wck_rotation_pending` `local_meta` row (JSON `{epoch, since}`, NO schema migration: the generic `GetLocalMeta`/`SetLocalMeta` accessors from P4-SYNC-02 already exist). The marker resolves ONLY via `clearWCKRotationPending` after THIS device's own successful `Rotate` (sync's owed retry, `keys rotate`, or a later revoke's rotation — every local Rotate wraps to `ApprovedRecipients`, which excludes all locally-revoked devices, exactly the proof the marker needs); a marker that fails to parse stays pending (fail-closed). New `state.Store.DeleteLocalMeta` (idempotent). +- `internal/cli/devices.go`: `rotateWorkspaceKeyOnRevoke` records the marker on `Rotate` failure (warning promises the sync auto-retry; falls back to manual-only wording if even the marker write fails) and clears it on any later successful revoke rotation; new `warnMalformedRemainingRecipients` preflights the REMAINING approved recipients before the trust write (issue #134 option 3) via new `workspacekeys.ValidateRecipient` — advisory only, the revoke always proceeds (refusing would keep a compromised device approved, per the PR #132 adversarial ruling); Rotate's wrap-first ordering stays the enforcement. +- `internal/cli/sync.go`: `maybeRotateWorkspaceKey` rotates when a rotation is OWED regardless of epoch age — and even with `keys.rotate_max_age=0`, since disabling PERIODIC rotation must not disable committed revoke containment. An owed retry that fails EARLY (epoch unchanged — the malformed-recipient class) warns loudly and lets the cycle CONTINUE; a failure with the epoch advanced (mid-commit half-mint, grants possibly unpublished) is fatal for the cycle, detected by re-reading the epoch. Success clears the marker (a delete failure is a cycle error — the marker must not silently outlive its rotation). `keys rotate` clears it too. Epoch-0 skip unchanged (a joiner never self-mints). +- `internal/cli/doctor.go`: new `workspace key rotation` check — warns `owed since ` with the sync-auto-retry + `keys rotate` remedy; silent when nothing is owed. +- Specs: spec/07 (revoke lifecycle bullet: preflight + marker + Rotate-only resolution + early-vs-mid-commit failure split), spec/13 (doctor check inventory), spec/15 (the TRUST-01 failed-rotation residual is now bounded by the next successful rotation; issue #134 shipped). + +- Post-review (dual: Codex adversarial + opus, both converged on the same HIGH): the first draft self-resolved the marker when ANY epoch above the recorded one became active — unsound, because a peer that has not yet pulled the revoke (and it cannot have: the fatal-for-cycle draft also blocked pushing the `device.revoked` event) can rotate for AGE reasons and grant the new epoch to the still-approved-in-its-registry revoked device; the marker would clear while the revoked device holds the current key, and the self-resolve actively cancelled the retry that would have excluded it. Fixed: resolution is Rotate-only (above); the worst case of ignoring a legitimate peer rotation is one redundant epoch. The same fix covers the mid-commit-failure self-clear (Codex Medium). The fatal-for-cycle availability regression (Codex Medium / opus Major: a permanently-malformed bystander recipient blocked the revoke event from ever propagating AND run-loop aborted after 5 ticks) is fixed by the early-failure warn-and-continue split. Opus Minors: the doctor read-path DELETE side effect is gone with lazy resolution; the workspace-scoping mismatch is moot (the marker no longer compares epochs). + +Validated: +- New: `TestDeviceRevokeRotationFailureMarksPendingAndSyncRetries` (malformed bystander recipient → preflight warning names it, marker at epoch 1, then fixed recipient + `rotate_max_age=0` → owed retry mints epoch 2 and clears the marker), `TestMaybeRotateWarnsAndContinuesCycleOnEarlyOwedFailure` (cycle proceeds, marker survives, epoch unchanged), `TestWCKRotationPendingSurvivesNewerEpoch` (the HIGH's regression pin), `TestKeysRotateClearsOwedRotation`, `TestWCKRotationPendingMalformedRecordStaysPending` (fail-closed), `TestDoctorWarnsWCKRotationPending`, `TestDeleteLocalMetaIdempotent`. +- gofmt clean; `go test -race ./...` all ok; golangci-lint; spec-drift vs origin/main. + +Follow-ups: +- None (closes #134; the accepted residual — events pushed between the failed revoke rotation and this device's next successful rotation remain readable by the revoked device — is documented in spec/15). ## 2026-07-06 — fix(sync): draft-snapshot apply quarantines instead of aborting the pull batch (#133) Changed: