Skip to content

fix(health): a pre-epoch newborn VM is not an owner-epoch violation - #145

Merged
colonelpanik merged 2 commits into
colonelpanik:mainfrom
livingstaccato:fix/owner-epoch-fresh-page
Aug 9, 2026
Merged

fix(health): a pre-epoch newborn VM is not an owner-epoch violation#145
colonelpanik merged 2 commits into
colonelpanik:mainfrom
livingstaccato:fix/owner-epoch-fresh-page

Conversation

@livingstaccato

Copy link
Copy Markdown
Contributor

A fresh VM create is born at vm_owner_epoch = 0 and graduates on the reconciler's next sweep, which also writes its first owner-epoch marker. In that window the VM is running on its owner with no marker — the expected newborn state — but the dual-run detector's epoch check (which requires a valid marker equal to the DB epoch) paged it as a critical owner_epoch_mismatch. Every fresh lv run flashed a critical condition for up to a sweep interval, and via the host-involvement admission clause it transiently refused unrelated creates on that host, before resolving on its own.

The exception is scoped as tightly as the regime allows: only a row at epoch 0 with a missing marker is skipped — a state that names exactly "awaiting backfill", since epochs are never reset. A present marker against an epoch-0 row still pages (the runtime claims a generation the DB does not know), and a missing marker on a graduated row remains the violation it always was. Both boundaries are pinned by tests; the newborn case was observed red before the fix.

Verified live on a 4-node lab: a fresh lv run polled for two minutes produces zero owner_epoch_mismatch flashes (the old build paged within one sweep), while epoch graduation and marker convergence are unaffected.

@livingstaccato

Copy link
Copy Markdown
Contributor Author

Heads up — I'm still working this one through (rebasing/verifying against the freshly-merged main and re-checking on the lab), so please hold off merging until I confirm it's final. I'll post when it's ready for review. Thanks!

@livingstaccato
livingstaccato force-pushed the fix/owner-epoch-fresh-page branch 2 times, most recently from de21096 to 65faf45 Compare August 8, 2026 18:55
@livingstaccato

livingstaccato commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/main (de655ea) to bring this current — it now sits directly on top of #146 (pki cert-serial re-recording) and #147 (CI release idempotency). Rebase only: the fix is unchanged (still one commit, same diff), no content edits, no merge commit.

Verified green on the rebased tip: go build ./..., go vet ./..., full go test ./..., and make ci-guards (schema-bump, ledger-drift, writecheck, stmtshapecheck — 336 statements registered, docs-truth).

Drafted and posted by Claude Code.

@livingstaccato

livingstaccato commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Adversarial review remediation — 124dacc

An independent adversarial pass flagged two findings on this branch. One was real and is fixed; one is closed as accepted risk with reasoning. The fix was mutation-verified — regression tests written first, observed failing, then passing.

Verification on 124dacc: go build ./... && go vet ./..., go test ./..., and BASE_REF=upstream/main make ci-guards all pass.

Fixed — a future created_at suppressed the owner-epoch finding forever

withinNewbornGrace tested only time.Since(t) < newbornEpochGrace. Since is negative for a timestamp in the future, and every negative duration is less than the grace, so a row stamped ahead of the evaluating host sat inside the exception indefinitely — the owner-epoch mismatch was skipped on every pass, and that VM lost owner-epoch protection permanently without ever paging.

This is the strongest finding in the review. It is reachable without an attacker — a creator whose clock is wrong — and with one: created_at is a replicated column and corrosion is last-writer-wins, so any peer can write it. A detector must not have an input that switches it off for good.

Why the obvious fix is wrong — the bound has to be two-sided

Requiring a non-negative age would reintroduce exactly the false page this branch exists to remove: a creator whose clock runs slightly ahead of the detector leader stamps created_at marginally in the future, and that is the common skew direction.

The window is instead bounded by the same constant on both sides. Honest skew still earns the newborn exception, worst-case suppression is capped at two grace windows instead of unbounded, and a wedged or forged row always surfaces. Empty and malformed timestamps keep failing toward paging.

Closed, accepted risk — negative clock skew shrinking the grace

Real, but not actionable as stated. An age inflated by a trailing creator clock and a genuinely old row are the same observation, so the only lever is enlarging the constant — which widens the very suppression window the other finding is trying to bound.

It also needs about 4m30s of inter-node skew before it bites, in a cluster whose gossip and lease machinery is already unsound at that magnitude, and the existing two-pass debounce absorbs the single-pass race the review constructs.

Revisit if the fleet ever gains a clock-skew monitor that makes the magnitude observable; the alternative would be keying the newborn exception off a monotonic sweep counter rather than wall-clock created_at.

Test approach

Tests drive the predicate directly across both bounds — including year 9999, empty, and malformed, closing the two gaps the review noted as untested — and drive the detector end to end with a post-dated row, which fails before this change.

No clock seam was added: the timestamps are SQL-stamped, matching the pattern TestEpochMismatch_WedgedPreEpochRowStillFires already uses. That keeps the tests deterministic without adding new production surface purely for testability.

Drafted and posted by Claude Code.

@livingstaccato

livingstaccato commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

@colonelpanik — ready for review whenever you have time.

This branch is done from our side for now: the adversarial-review remediation above is the last change we plan to make, CI is green on 124dacc (Build & Test, Schema version guard), and the branch is rebased on current upstream/main and mergeable. No further pushes are queued, so it will not move under you mid-review.

Happy to take it further if anything in the remediation — particularly the two-sided grace bound, or the decision to accept the negative-skew case rather than widen the window — reads differently to you.

Drafted and posted by Claude Code.

@livingstaccato

Copy link
Copy Markdown
Contributor Author

I agree that 124dacc fixes the static far-future timestamp bug and that rejecting every future timestamp would recreate the newborn false page under positive clock skew.

A High-severity suppression path remains under the stated replication threat model. created_at is peer-writable LWW input, so the two-sided bound limits one fixed value but does not limit renewal. Keep this state constant:

DB:      name=vmA, state=running, host_name=h1, vm_owner_epoch=0
runtime: h1 reports vmA running with MarkerMissing
gate:    owner_epoch_v1 enforced

If a peer rewrites vms.created_at to its current RFC3339 time before each detector pass, withinNewbornGrace returns true every time and the continue at dualrun.go:417 suppresses the finding without bound. A new violation never enters the lifecycle. If the condition was already confirmed, two complete skipped passes treat it as absent and resolve it through dualrun_lifecycle.go:218-237.

This invalidates the remediation claim that worst-case suppression is capped and that a forged row always surfaces. The tests assign created_at once, so they do not exercise renewal.

The grace needs detector-owned durable state, such as a finite complete-scan budget that peer row edits cannot reset. That preserves the newborn allowance without trusting remote wall time. The stronger end state assigns a positive epoch and writes both markers before publishing the VM as running. I would not merge this head until the renewable-grace path is addressed.

A fresh create is born at vm_owner_epoch 0 and graduates on the
reconciler's next sweep, which also writes its first runtime marker. In
that window the VM is RUNNING on its owner with no marker, and the
dual-run detector's epoch check — which requires a VALID marker equal to
the DB epoch — paged it as a critical owner_epoch_mismatch. Every fresh
`lv run` flashed a critical condition for up to a sweep interval (and,
via the host-involvement admission clause, transiently refused unrelated
creates on that host) before resolving on its own. Observed live on the
lab, 2026-08-05 and again 2026-08-06 after a migration test.

The exception is scoped as tightly as the regime allows: only a row at
epoch 0 with a MISSING marker is skipped — a state that names exactly
'awaiting backfill', since epochs are never reset. A PRESENT marker
against an epoch-0 row still pages (the runtime claims a generation the
DB does not know), and a missing marker on a graduated row remains the
violation it always was; both boundaries are pinned by tests.

The newborn case was observed red before the fix; full repo gate
(build, vet, go test ./..., ci-guards) green.
withinNewbornGrace tested only `time.Since(t) < newbornEpochGrace`. Since is
NEGATIVE for a created_at in the future and every negative duration is less
than the grace, so a row stamped ahead of the evaluating host sat inside the
exception indefinitely: the owner-epoch mismatch finding was skipped on every
pass, and that VM lost owner-epoch protection permanently without ever paging.

That is reachable without an attacker — a creator whose clock is wrong — and
with one: created_at is a replicated column and corrosion is last-writer-wins,
so any peer can write it. A detector must not have an input that switches it
off for good, which is exactly what an unbounded future timestamp was.

The window is now bounded in both directions by the same constant. A modest
future stamp is honest clock skew between the creator and whichever host holds
the detector lease, so it still earns the newborn exception and the fix this
branch landed is preserved; past one grace ahead it does not. Empty and
malformed timestamps keep failing toward paging.

Be precise about the bound: it caps what any ONE stamp buys at two grace
windows. It does NOT bound a peer that REWRITES created_at before each pass,
since the predicate is re-evaluated against freshly-read DB state every sweep.
That is the detector's general trust of replicated DB state rather than
anything specific to this predicate — the same writer suppresses the whole
epoch check more cheaply by setting state to a migrationState, setting
deleted_at (ListVMs filters it), or pointing host_name at an unprobed host,
none of which need renewing. Closing that class means detector-owned durable
state the peers cannot reset, or assigning a positive epoch and writing markers
before a VM is published as running, which removes the newborn window instead
of bounding it. Both are follow-ups, not this diff; the predicate now documents
the limit rather than overstating it.

Tests drive the predicate directly across both bounds (including year 9999,
empty, and malformed) and drive the detector end to end with a post-dated row,
which fails before this change.
@livingstaccato
livingstaccato force-pushed the fix/owner-epoch-fresh-page branch from 124dacc to 66f5801 Compare August 9, 2026 19:54
@livingstaccato

livingstaccato commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main (4d3c564) and pushed as 66f5801. Rebase plus one comment-only amendment — no behaviour change from 124dacc.

On the renewable-grace finding: it is correct, and my earlier claim above was wrong. I wrote that worst-case suppression is capped at two grace windows and that a forged row always surfaces. Both hold only for a stamp that is wrong once. withinNewbornGrace is re-evaluated against freshly-read DB state every sweep, so a peer that rewrites created_at before each pass keeps the exception open without bound. The two-sided bound does not address renewal and I should not have stated the cap unconditionally.

What changed here: the same overclaim was in the code comment, which is worse than in a PR thread — a comment asserting a safety property the code lacks discourages exactly the audit that would find the gap. The predicate now documents the real limit instead.

What did not change, and why. Renewal is not a weakness of the newborn grace specifically; it restates that the epoch check trusts replicated DB state. The same writer suppresses it more cheaply, with a single write and no renewal at all:

Lever Effect Cost
state = 'migrating' migrationStates[vmState[vm]]continue (dualrun.go:391) 1 write, permanent
deleted_at = <ts> ListVMs filters WHERE deleted_at IS NULL — the VM never enters the loop (corrosion/vms.go:255) 1 write, permanent
host_name = <unprobed host> !probedcontinue (dualrun.go:394) 1 write

The renewal path is strictly more expensive and achieves strictly less. Closing it alone would need new durable detector state — a schema bump, ledger entry and stmtshapecheck registration — and would still leave those three open, so it does not belong in a 20-line detector fix.

Worth adding: created_at is insert-only in litevirt. No code path updates it and it appears in no ledger statement, so renewal presupposes arbitrary SQL on a compromised peer, at which point the cheaper levers are already available.

Follow-ups, filed rather than folded in: detector-owned durable scan budget that peer row edits cannot reset; and the stronger end state you identified — assign a positive epoch and write both markers before publishing a VM as running, which removes the newborn window rather than bounding it. The second is the real fix and lives in the create path.

Gate on 66f5801: go build ./... && go vet ./..., full go test ./..., BASE_REF=upstream/main make ci-guards (stmtshapecheck 336/336) — all pass.

Drafted and posted by Claude Code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants