Skip to content

restoreVersion() must validate the snapshot and restore its typeId #62

Description

@cuibonobo

Problem

restoreVersion() writes a historical snapshot back with no schema validation and without restoring the snapshot's type:

return this.adapter.updateRecord(id, {
  content: target.content,
  updatedAt: new Date(),
  version: existing.version + 1,
});

(packages/core/src/stack.ts:579–583)

Two defects, one of which forces the shape of the fix:

  1. No validation. Every other write path validates against the type schema; restore is the one door where arbitrary historical content walks back in unchecked. Usually the snapshot was valid when written — but "usually" breaks under in-place schema drift (the thing schemaHash exists to catch) or a buggy adapter, and restore is precisely a recovery path, where silent corruption is most expensive.
  2. typeId is not restored, which makes honest validation impossible. Restore a pre-migration snapshot onto a record that has since migrated to @2, and the result is @1-shaped content labeled @2. Validating that against @2's schema would rightly reject it — so the fix isn't to skip validation, it's to stop mislabeling: restore the snapshot's typeId along with its content.

Decided direction

  1. Restore content + typeId from the snapshot (hard dependency: RFC: replace lazy migration with explicit owner-driven migration + additive cross-app evolution #47's typeId field on RecordVersion).
  2. Validate the snapshot's content against its own stored type's schema — the type it claims, not the record's current type. Failure → StackValidationError (422 over the wire per APIAdapter collapses the error taxonomy: Stack* errors never survive the wire round trip #53's mapping).
  3. No forward-migration at restore time. The server executing PATCH contract mismatch: spec defines content-only merge patch, APIAdapter sends record-field envelopes #52's restore endpoint cannot migrate — migration functions are app code (RFC: replace lazy migration with explicit owner-driven migration + additive cross-app evolution #47) — and core shouldn't behave differently locally than the server does remotely. A restored old-version record is simply stale, which is legal and self-healing: the owning app's migrateAll() re-migrates it on next run. Same principle as undelete (Undelete API: the spec promises soft-delete recovery, nothing implements it #60): neither operation migrates; both may produce stale records; one mechanism heals both.
  4. Associations restore per One versioning rule: every record mutation bumps version and snapshots full prior state (associations and permissions included) #61's rule (when present in the snapshot); permissions never do (see One versioning rule: every record mutation bumps version and snapshots full prior state (associations and permissions included) #61's rationale).
  5. Server-side restore endpoint (PATCH contract mismatch: spec defines content-only merge patch, APIAdapter sends record-field envelopes #52) performs the same validation, pinned by a conformance fixture: restore of a drifted/invalid snapshot must 422 identically through adapter-local and adapter-api.

Work items

Refs #47 (RecordVersion.typeId dependency; stale-is-legal; migrations-are-app-code), #61 (snapshot contents and restore scope), #60 (shared no-migration-at-restore principle), #52 (server-side endpoint + fixtures), #53 (error round-trip).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions