From f7fd874ff6ef0478b823f4909de7e593e92d9784 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 8 Oct 2026 14:53:56 -0400 Subject: [PATCH 01/10] feat: prepare guarded content snapshots and protect stale editor writes --- .github/workflows/ci.yml | 19 ++ adminui/static/admin.js | 14 +- .../2026-10-08-content-promotion-amendment.md | 61 ++++ docs/promotion.md | 72 ++++ docs/qa/content-promotion.md | 37 +++ internal/admin_http.go | 58 +++- internal/admin_http_test.go | 8 +- internal/editor_regression_test.go | 2 +- internal/page_concurrency_test.go | 68 ++++ promotion/bundle.go | 314 ++++++++++++++++++ promotion/plan.go | 138 ++++++++ promotion/snapshot.go | 186 +++++++++++ promotion/snapshot_test.go | 163 +++++++++ store/batch.go | 282 ++++++++++++++++ store/batch_memory.go | 74 +++++ store/batch_test.go | 11 + store/page.go | 23 +- store/page_test.go | 4 +- store/postgres/batch.go | 144 ++++++++ store/postgres/batch_integration_test.go | 88 +++++ store/postgres/editor_browser_test.go | 81 +++++ store/postgres/store.go | 66 +++- store/postgres/testdata/editor-version.cjs | 63 ++++ store/storetest/batch.go | 190 +++++++++++ 24 files changed, 2129 insertions(+), 37 deletions(-) create mode 100644 docs/plans/2026-10-08-content-promotion-amendment.md create mode 100644 docs/promotion.md create mode 100644 docs/qa/content-promotion.md create mode 100644 internal/page_concurrency_test.go create mode 100644 promotion/bundle.go create mode 100644 promotion/plan.go create mode 100644 promotion/snapshot.go create mode 100644 promotion/snapshot_test.go create mode 100644 store/batch.go create mode 100644 store/batch_memory.go create mode 100644 store/batch_test.go create mode 100644 store/postgres/batch.go create mode 100644 store/postgres/batch_integration_test.go create mode 100644 store/postgres/editor_browser_test.go create mode 100644 store/postgres/testdata/editor-version.cjs create mode 100644 store/storetest/batch.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4665f0f..c5b19a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,8 +11,27 @@ permissions: jobs: test: runs-on: ubuntu-latest + services: + postgres: + image: postgres:17-alpine + env: + POSTGRES_HOST_AUTH_METHOD: trust + ports: ['5432:5432'] + options: >- + --health-cmd "pg_isready -U postgres" + --health-interval 5s + --health-timeout 5s + --health-retries 10 + env: + CMS_TEST_DATABASE_URL: postgres://postgres@127.0.0.1:5432/postgres?sslmode=disable steps: - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + - name: Verify exact tested source + env: + EXPECTED_TEST_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + run: test "$(git rev-parse HEAD)" = "$EXPECTED_TEST_SHA" - uses: actions/setup-go@v6 with: go-version-file: go.mod diff --git a/adminui/static/admin.js b/adminui/static/admin.js index 526d9a9..54b4164 100644 --- a/adminui/static/admin.js +++ b/adminui/static/admin.js @@ -14,6 +14,10 @@ const api = { }); if (!res.ok) { const text = await res.text(); + if (res.status === 409) { + let error; try { error = JSON.parse(text); } catch {} + if (error?.error === "version_conflict") throw new Error("This page changed. Your draft is still here. Keep a copy of your changes, then reload to review the latest page before saving."); + } throw new Error(method + " " + path + " → " + res.status + ": " + text); } if (res.status === 204) return null; @@ -29,7 +33,7 @@ const api = { getPage(tid, pid) { return this.req("GET", "/tenants/" + tid + "/pages/" + pid); }, createPage(tid, b) { return this.req("POST", "/tenants/" + tid + "/pages", b); }, updatePage(tid, pid, b) { return this.req("PUT", "/tenants/" + tid + "/pages/" + pid, b); }, - deletePage(tid, pid) { return this.req("DELETE", "/tenants/" + tid + "/pages/" + pid); }, + deletePage(tid, pid, version) { return this.req("DELETE", "/tenants/" + tid + "/pages/" + pid, {expected_version:version}); }, permissions(tid, pid) { return this.req("GET", "/tenants/" + tid + "/pages/permissions" + (pid ? "?page_id="+pid : "")); }, templates(tid) { return this.req("GET", "/tenants/" + tid + "/pages/templates"); }, async preview(tid, body) { @@ -121,7 +125,7 @@ async function applyView(route){ const form=$("#page-editor-form");form.reset();pageField(form,"id").value=p.ID||""; for(const [field,key]of Object.entries({path:"Path",title:"Title",status:"Status",template_id:"TemplateID"}))pageField(form,field).value=p[key]|| (field==="status"?"draft":""); pageField(form,"publish_at").value=isoToDateTimeLocal(p.PublishAt);pageField(form,"unpublish_at").value=isoToDateTimeLocal(p.UnpublishAt); - const body=p.RenderedBodyHTML??p.BodyHTML??"";setRichEditorHTML(form,body);savedPageBodies.set(form,{html:body,blocks:p.BodyBlocks});pagePermissions=permissions;draftKey=pageKey(route);baselineDraft=JSON.stringify(pagePayload(form)); + const body=p.RenderedBodyHTML??p.BodyHTML??"";setRichEditorHTML(form,body);savedPageBodies.set(form,{html:body,blocks:p.BodyBlocks,version:p.Version});pagePermissions=permissions;draftKey=pageKey(route);baselineDraft=JSON.stringify(pagePayload(form)); $("#page-templates").replaceChildren(...(templates.templates||[]).map(name=>new Option(name,name))); $("#editor-tenant").textContent=(tenant.Label||tenant.Slug)+" · "+tenant.Slug;$("#editor-title").textContent=p.Title||"New page"; const pages=$("#pages-link");pages.dataset.route="/tenants/"+tenant.ID+"/pages";pages.href=cmsBase+pages.dataset.route; @@ -497,14 +501,14 @@ document.addEventListener("DOMContentLoaded",()=>{ if(!form.checkValidity()){await navigate(pagePath(route,"publishing"));form.reportValidity();return} const payload=pagePayload(form);saving=true;freezeDraft(true);$("#btn-save-page").disabled=true;updateSaveState(); try{ - const saved=route.pid==="new"?await api.createPage(tid,payload):await api.updatePage(tid,route.pid,payload); + const saved=route.pid==="new"?await api.createPage(tid,payload):await api.updatePage(tid,route.pid,{...payload,expected_version:savedPageBodies.get(form)?.version}); if(key!==draftKey||tid!==currentTenant?.ID||generation!==navigationGeneration)return; - savedPageBodies.set(form,{html:payload.body_html,blocks:payload.body_blocks});baselineDraft=JSON.stringify(payload);$("#editor-title").textContent=payload.title;toast("Page saved", "ok"); + savedPageBodies.set(form,{html:payload.body_html,blocks:payload.body_blocks,version:saved.Version});baselineDraft=JSON.stringify(payload);$("#editor-title").textContent=payload.title;toast("Page saved", "ok"); if(route.pid==="new"){freezeDraft(false);saving=false;discardDraft();await navigate("/tenants/"+tid+"/pages/"+saved.ID+"/content",true)} }catch(e){if(key===draftKey&&generation===navigationGeneration)fail(e)}finally{freezeDraft(false);saving=false;$("#btn-save-page").disabled=false;updateSaveState()} }; $("#btn-page-preview").onclick=()=>refreshPreview().catch(fail); - $("#btn-delete-page").onclick=async()=>{if(saving||!pagePermissions.delete||!confirm("Delete this page?"))return;const tid=currentTenant.ID,pid=currentRoute.pid; saving=true;try{await api.deletePage(tid,pid);discardDraft();saving=false;await navigate("/tenants/"+tid+"/pages")}catch(e){fail(e)}finally{saving=false;updateSaveState()}}; + $("#btn-delete-page").onclick=async()=>{if(saving||!pagePermissions.delete||!confirm("Delete this page?"))return;const tid=currentTenant.ID,pid=currentRoute.pid; saving=true;try{await api.deletePage(tid,pid,savedPageBodies.get($("#page-editor-form"))?.version);discardDraft();saving=false;await navigate("/tenants/"+tid+"/pages")}catch(e){fail(e)}finally{saving=false;updateSaveState()}}; }else{ $("#new-tenant-form").onsubmit=async ev=>{ev.preventDefault();const fd=new FormData(ev.target);try{await api.createTenant({slug:fd.get("slug"),label:fd.get("label"),theme_id:fd.get("theme_id")});ev.target.reset();toast("Tenant created","ok");await navigate("/tenants")}catch(e){fail(e)}}; $("#new-domain-form").onsubmit=async ev=>{ev.preventDefault();const tid=currentTenant.ID,generation=navigationGeneration,fd=new FormData(ev.target);try{await api.createDomain(tid,{host:fd.get("host"),subsite_label:fd.get("subsite_label"),kind:fd.get("kind")});if(generation!==navigationGeneration)return;ev.target.reset();toast("Domain added","ok");await refreshDomains(tid,generation)}catch(e){fail(e)}}; diff --git a/docs/plans/2026-10-08-content-promotion-amendment.md b/docs/plans/2026-10-08-content-promotion-amendment.md new file mode 100644 index 0000000..2399b37 --- /dev/null +++ b/docs/plans/2026-10-08-content-promotion-amendment.md @@ -0,0 +1,61 @@ +# Content snapshot promotion implementation amendment + +This additive CMS implementation follows the parent-authorized Tina publication +assessment in multisite `docs/runbook/tina-content-promotion.md`. It preserves +the locked Tina plan, CMS PR31 at `0ceed5592a335e16b2eb6ddc5148ef45f248d0a7`, +CMS main's separate editor changes, all consumer pins and active worktrees. + +## Scope and boundaries + +1. Typed content-only snapshot export/strict decoding, explicit target mappings, + deterministic dry-run diffs and immutable bundle verification. +2. Whole-tenant page baseline guarded atomic batches and content-only before/after + receipts. Guarded rollback refuses any intervening page mutation. +3. All ordinary CMS creates/updates/deletes serialize with batches. Update uses + the caller's loaded `Page.Version`; API save/delete requires the client's + `expected_version`, and the editor retains its draft on conflict. +4. Memory/API tests plus a real isolated Postgres schema prove stale saves, + racing writes, all-or-nothing failure, mappings, omission and rollback. + +There is no promotion HTTP route, production write, credential configuration, +host deployment, consumer-pin change or storage purchase in this amendment. +Passwords, hashes, tokens, domains, roles, memberships, sessions and repository +mapping are outside every promotion struct. Unknown JSON fields and malformed +payloads yield fixed errors without printing supplied values. + +`PageStore.Delete` gains a required expected-version argument. This source API +change and the mandatory HTTP precondition require a coordinated host/plugin +release; consumers must not discard the browser's version or fall back to an +unconditional write. The historical PR31 editor remains unchanged in its own +worktree. + +## Concurrency contract + +Postgres page mutations take one transaction-scoped advisory lock per tenant. +Batches read and compare the complete tenant page ID/version/content baseline +under that same lock and commit all mutations in one transaction. The lock is +cooperative, not an atomic cloud-spec CAS or a promise covering arbitrary direct +SQL writers. Per-row version predicates protect stale ordinary saves/deletes +after promotion releases the lock. No omitted page is deleted. + +Rollback compares the complete recorded post-state and restores only affected +content, retaining page IDs and increasing versions. Backups are content only; +their durable archive is the authorized publication operator's responsibility. + +## Publication remains disabled + +CMS database transactions cannot atomically switch filesystem bundles or cloud +bootstrap configuration. A live publication endpoint remains blocked on a host +write/publication fence, staged immutable bundle, verified bootstrap source, +durable backup, superadmin authority, approval digest and restart/redeploy proof. +Uploaded tenant media references are refused until durable copy/rewrite/retrieval +exists. Static files shadowing selected CMS routes refuse validation. + +## Verification and resource ownership + +Use the existing module/build caches, `GOWORK=off`, package-targeted tests with +`-p 2` at most. Parent authorized one focused local allocation and a fresh +`promotion_20261008_*` schema on existing task Postgres port 55432. No Docker, +new cache, full local race build, existing-schema mutation or process restart. +CI may run its existing full checks plus an isolated Postgres service. Record +exact-head checks and obtain independent source review before merge. diff --git a/docs/promotion.md b/docs/promotion.md new file mode 100644 index 0000000..fc5f6dd --- /dev/null +++ b/docs/promotion.md @@ -0,0 +1,72 @@ +# Preparing approved content publication + +The `promotion` package prepares frozen, content-only snapshots and explicit +production diffs. It has no network client, background synchronization or live +apply endpoint. Review edits stay in their review tenant. An approval must name +the frozen snapshot digest and production diff; later drafts require a new +approval. + +Export selected page IDs from an authorized review tenant using +`promotion.Export(ctx, pages, reviewID, sourceSite, ids, immutableBundleDir, now)`. +Archive the returned snapshot and exact bundle bytes together. The snapshot +includes source page ID/version keys, content and SHA-256 file inventory. Strict +`promotion.Decode` refuses unknown/duplicate JSON keys, trailing documents and +oversize input using fixed errors. It never imports tenant settings or authority. + +Read the production tenant's pages and inventory its immutable bundle. Review +`promotion.SuggestedMapping` path matches, then supply an explicit mapping for +every selected page to `promotion.DryRun`. Target page IDs are distinct from source +IDs. A zero target ID explicitly creates; moves require `AllowMove`; deletion +requires a separate `DeleteSelection`. Omission preserves existing pages. The plan +contains the complete target page baseline, both bundle digests and content diffs. +Dry run never writes. + +`store.PageBatchStore` supplies persistence primitives, not publication authority. +`ApplyPageBatch` atomically compares the entire page baseline and applies content +in one tenant. Ordinary creates, saves and deletes share its per-tenant lock. +Update requires the caller's loaded `Page.Version`. API PUT and DELETE require +`expected_version`; missing preconditions fail and stale versions return 409. +The editor keeps an unsaved draft on conflict and refreshes its version after a +successful save. Existing external API consumers must upgrade before this plugin +is adopted. `PageStore.Delete` also gains a required expected-version argument. + +Archive the content-only batch receipt before completing host publication. Its +before/after rows, explicit mapping and integrity digest support +`RollbackPageBatch`, which refuses any change to the recorded post-state and +restores affected content with increasing versions. The receipt digest detects +corruption; it is neither a signature nor approval. Durable backup storage and +operator authorization belong to the host publication workflow. + +`VerifyBundle` re-hashes regular files, verifies template/assets references and +refuses symlinks, missing assets, uploaded tenant media URLs, admin API references +and static files shadowing selected CMS routes. Bundled photos retain relative +references and exact hashes. The operator must stage those verified immutable +bytes in the target bundle. The library does not configure media storage, copy +uploaded objects, fetch third-party embeds or claim rights to external media. + +Live application remains blocked on host integration: current superadmin +publication authority, a named approval digest, an exclusive publication/write +fence, staged immutable assets, durable content/bundle backups, approved production +bootstrap source and actual restart/redeploy proof. A database transaction cannot +atomically swap a filesystem bundle or cloud bootstrap setting. Preserve the +existing domains, credentials, roles, memberships and repository mapping. If the +host cannot provide the fence and restart-safe bundle, use a separately reviewed +maintenance procedure and do not enable partial publication. + +## Verification + +`go test ./store ./promotion ./internal ./adminui ./store/postgres` runs focused +unit/API checks. Set `CMS_TEST_DATABASE_URL` to an isolated authorized test server +to run the real Postgres contract; each test creates and cleans only a fresh +`promotion_20261008_*` schema. CI provisions a disposable Postgres service and +checks the exact PR head before its existing vet/race checks. + +The optional actual-editor test uses already installed tools. Set +`CMS_TEST_PLAYWRIGHT_PATH` to the local Playwright package and +`CMS_TEST_BROWSER_EXECUTABLE` to the browser binary; `CMS_TEST_SCREENSHOT` is an +optional screenshot destination. `TestPostgresEditorVersionBrowser` launches +only a temporary browser context and ephemeral loopback listener. Its authority +and promotion helper exist only in the test binary. It demonstrates repeated +save version refresh, stale save/delete conflicts after a batch, retained draft, +wrong-tenant denial and save/reload through the real editor/API/Postgres boundary. +It does not prove hosted authentication or host bundle activation. diff --git a/docs/qa/content-promotion.md b/docs/qa/content-promotion.md new file mode 100644 index 0000000..e8ef5cd --- /dev/null +++ b/docs/qa/content-promotion.md @@ -0,0 +1,37 @@ +# Content promotion verification + +This verifies CMS persistence/preparation primitives, not hosted publication. +There is no live apply endpoint, consumer-pin update or production content write. +Host bundle fencing, durable backup/bootstrap integration and restart/redeploy +proof remain required before live publication can be enabled. + +## Local behavior demonstrated + +Focused tests use Go 1.27.1, existing shared module/native build caches, +`GOWORK=off`, offline dependencies and concurrency two. Real Postgres uses only +fresh `promotion_20261008_*` schemas on the existing task-owned local server; +each schema is cleaned by its test. No runtime tables, processes or ports change. + +| Boundary | Observed result | +| --- | --- | +| Snapshot export → strict decode | Frozen selection survives decode; later source edits do not affect approved content; authority fields, duplicate keys, trailing documents and malformed fields refuse with fixed errors | +| Source → target mapping/dryrun | IDs differ; explicit mapping required; implicit moves refused; dry run writes nothing; omitted target page retained | +| Batch → real Postgres → reload | Update/create/delete persist atomically; mapping names target identities; complete page baseline checked | +| Ordinary save/delete → batch | Stale loaded versions conflict; racing save/promotion and delete/promotion each admit one winner | +| Database failure mid-batch | Isolated trigger refuses the second item; earlier update rolls back and complete baseline remains unchanged | +| Receipt → rollback | Content restored with increasing versions; created page removed; deleted page restored; later edit refuses whole rollback | +| Bundle → validator | Changed bytes, missing assets, static homepage shadow, symlink, admin reference and encoded tenant-upload references refused | +| Actual Chrome editor → API → Postgres | Repeated saves send versions 1/2; fixture promotion changes version to 4; stale save/delete send 3 and return 409; unsaved draft retained; reload then version-4 save persists as version 5; wrong tenant returns 403 | + +The browser test launches an ephemeral loopback test server and a temporary +Chrome context using already installed Playwright. Authority and promotion helper +exist only in the test binary. It is not a hosted authentication or domain proof. +The task evidence directory retains the browser conflict screenshot and test logs. + +## Release limits + +`PageStore.Delete` now requires expected version; external API clients must send +`expected_version` for PUT/DELETE. The CMS host must consume this change in a +separate reviewed release. The historical PR31 worktree and current consumer +pin are preserved. Independent review and exact committed-head CI are recorded +separately; this document does not imply those gates passed before observation. diff --git a/internal/admin_http.go b/internal/admin_http.go index fd032ca..bf6bc5b 100644 --- a/internal/admin_http.go +++ b/internal/admin_http.go @@ -1,9 +1,11 @@ package internal import ( + "bytes" "context" "encoding/json" "errors" + "io" "net/http" "strconv" "strings" @@ -379,15 +381,16 @@ type pageBody struct { } type pageUpdateBody struct { - Subsite *string `json:"subsite"` - Path *string `json:"path"` - Title *string `json:"title"` - BodyHTML *string `json:"body_html"` - BodyBlocks json.RawMessage `json:"body_blocks"` - Status *string `json:"status"` - TemplateID *string `json:"template_id"` - PublishAt json.RawMessage `json:"publish_at"` - UnpublishAt json.RawMessage `json:"unpublish_at"` + ExpectedVersion int `json:"expected_version"` + Subsite *string `json:"subsite"` + Path *string `json:"path"` + Title *string `json:"title"` + BodyHTML *string `json:"body_html"` + BodyBlocks json.RawMessage `json:"body_blocks"` + Status *string `json:"status"` + TemplateID *string `json:"template_id"` + PublishAt json.RawMessage `json:"publish_at"` + UnpublishAt json.RawMessage `json:"unpublish_at"` } func (a *AdminAPI) createPage(w http.ResponseWriter, r *http.Request, tenantID int64) { @@ -477,6 +480,14 @@ func (a *AdminAPI) updatePage(w http.ResponseWriter, r *http.Request, tenantID, writeJSONError(w, http.StatusBadRequest, "bad_request", err.Error()) return } + if body.ExpectedVersion <= 0 { + writeJSONError(w, http.StatusPreconditionRequired, "precondition_required", "expected_version required; reload the page") + return + } + if body.ExpectedVersion != existing.Version { + statusFromPageErr(w, store.ErrVersionConflict) + return + } if body.Path != nil { existing.Path = *body.Path } @@ -540,7 +551,18 @@ func (a *AdminAPI) deletePage(w http.ResponseWriter, r *http.Request, tenantID, writeJSONError(w, http.StatusServiceUnavailable, "unavailable", "page store not configured") return } - if err := a.pages.Delete(r.Context(), tenantID, pageID); err != nil { + var body struct { + ExpectedVersion int `json:"expected_version"` + } + if err := decodeJSON(r, &body); err != nil { + writeJSONError(w, http.StatusBadRequest, "bad_request", "invalid JSON request") + return + } + if body.ExpectedVersion <= 0 { + writeJSONError(w, http.StatusPreconditionRequired, "precondition_required", "expected_version required; reload the page") + return + } + if err := a.pages.Delete(r.Context(), tenantID, pageID, body.ExpectedVersion); err != nil { statusFromPageErr(w, err) return } @@ -729,6 +751,8 @@ func statusFromPageErr(w http.ResponseWriter, err error) { switch { case errors.Is(err, store.ErrNotFound): writeJSONError(w, http.StatusNotFound, "not_found", err.Error()) + case errors.Is(err, store.ErrVersionConflict): + writeJSONError(w, http.StatusConflict, "version_conflict", err.Error()) case errors.Is(err, store.ErrPathConflict): writeJSONError(w, http.StatusConflict, "conflict", err.Error()) default: @@ -740,9 +764,19 @@ func statusFromPageErr(w http.ResponseWriter, err error) { func decodeJSON(r *http.Request, dst any) error { defer r.Body.Close() - dec := json.NewDecoder(r.Body) + b, err := io.ReadAll(io.LimitReader(r.Body, (8<<20)+1)) + if err != nil || len(b) > 8<<20 { + return errors.New("invalid JSON request") + } + dec := json.NewDecoder(bytes.NewReader(b)) dec.DisallowUnknownFields() - return dec.Decode(dst) + if err := dec.Decode(dst); err != nil { + return errors.New("invalid JSON request") + } + if err := dec.Decode(new(any)); err != io.EOF { + return errors.New("invalid JSON request") + } + return nil } func writeJSON(w http.ResponseWriter, status int, body any) { diff --git a/internal/admin_http_test.go b/internal/admin_http_test.go index 4c7ed3b..f98f3a5 100644 --- a/internal/admin_http_test.go +++ b/internal/admin_http_test.go @@ -48,7 +48,7 @@ func TestAdmin_PageTemplateAndScheduleFieldsPersist(t *testing.T) { pid := int64(gotP["ID"].(float64)) rec, gotP = doJSON(t, api, http.MethodPut, "/api/v1/admin/tenants/"+strconv.FormatInt(tid, 10)+"/pages/"+strconv.FormatInt(pid, 10), - map[string]any{"template_id": "alt-shell", "status": "published"}) + map[string]any{"expected_version": gotP["Version"], "template_id": "alt-shell", "status": "published"}) if rec.Code != http.StatusOK { t.Fatalf("update scheduled page: %d %v", rec.Code, gotP) } @@ -363,7 +363,7 @@ func TestAdmin_PageCRUD(t *testing.T) { // Update. rec, gotU := doJSON(t, api, http.MethodPut, "/api/v1/admin/tenants/"+strconv.FormatInt(tid, 10)+"/pages/"+strconv.FormatInt(pid, 10), - map[string]string{"title": "Updated"}) + map[string]any{"expected_version": gotP["Version"], "title": "Updated"}) if rec.Code != http.StatusOK { t.Fatalf("update page: %d %v", rec.Code, gotU) } @@ -373,7 +373,7 @@ func TestAdmin_PageCRUD(t *testing.T) { rec, gotU = doJSON(t, api, http.MethodPut, "/api/v1/admin/tenants/"+strconv.FormatInt(tid, 10)+"/pages/"+strconv.FormatInt(pid, 10), - map[string]string{"body_html": "", "subsite": ""}) + map[string]any{"expected_version": gotU["Version"], "body_html": "", "subsite": ""}) if rec.Code != http.StatusOK { t.Fatalf("clear page fields: %d %v", rec.Code, gotU) } @@ -404,7 +404,7 @@ func TestAdmin_PageCRUD(t *testing.T) { // Delete. rec, _ = doJSON(t, api, http.MethodDelete, - "/api/v1/admin/tenants/"+strconv.FormatInt(tid, 10)+"/pages/"+strconv.FormatInt(pid, 10), nil) + "/api/v1/admin/tenants/"+strconv.FormatInt(tid, 10)+"/pages/"+strconv.FormatInt(pid, 10), map[string]any{"expected_version": gotG["Version"]}) if rec.Code != http.StatusNoContent { t.Errorf("delete: %d want 204", rec.Code) } diff --git a/internal/editor_regression_test.go b/internal/editor_regression_test.go index 004872e..3bd3b59 100644 --- a/internal/editor_regression_test.go +++ b/internal/editor_regression_test.go @@ -13,7 +13,7 @@ func TestHTMLUpdateClearsCanonicalBlocksAndSchedule(t *testing.T) { _, tenant := doJSON(t, a, "POST", "/api/v1/admin/tenants", map[string]any{"slug": "review"}) base := "/api/v1/admin/tenants/" + strconv.Itoa(int(tenant["ID"].(float64))) + "/pages" _, p := doJSON(t, a, "POST", base, map[string]any{"path": "/", "title": "Home", "body_blocks": map[string]any{"type": "doc", "content": []any{map[string]any{"type": "paragraph", "content": []any{map[string]any{"type": "text", "text": "old"}}}}}, "publish_at": "2026-01-01T00:00:00Z"}) - rec, p := doJSON(t, a, "PUT", base+"/"+strconv.Itoa(int(p["ID"].(float64))), map[string]any{"body_html": "

Edited

", "body_blocks": nil, "publish_at": nil, "unpublish_at": nil}) + rec, p := doJSON(t, a, "PUT", base+"/"+strconv.Itoa(int(p["ID"].(float64))), map[string]any{"expected_version": p["Version"], "body_html": "

Edited

", "body_blocks": nil, "publish_at": nil, "unpublish_at": nil}) if rec.Code != http.StatusOK || p["BodyBlocks"] != nil || p["PublishAt"] != nil { t.Fatalf("stale canonical content or schedule: %d %#v", rec.Code, p) } diff --git a/internal/page_concurrency_test.go b/internal/page_concurrency_test.go new file mode 100644 index 0000000..66ab766 --- /dev/null +++ b/internal/page_concurrency_test.go @@ -0,0 +1,68 @@ +package internal + +import ( + "context" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +func TestAdminPageSaveDeleteClientPreconditions(t *testing.T) { + pages := store.NewMemoryPageStore() + api := NewAdminAPI(store.NewMemoryTenantAdminStore(), pages) + p := &store.Page{Path: "/", Title: "Initial", Status: store.StatusDraft} + if pages.Create(context.Background(), 1, p) != nil { + t.Fatal("create failed") + } + url := "/api/v1/admin/tenants/1/pages/" + strconv.FormatInt(p.ID, 10) + for _, method := range []string{"PUT", "DELETE"} { + w, _ := doJSON(t, api, method, url, map[string]any{}) + if w.Code != http.StatusPreconditionRequired { + t.Fatal("missing loaded version accepted") + } + } + content := store.ContentOf(p) + content.Title = "Approved promotion" + all, _ := pages.List(context.Background(), 1, "") + _, err := pages.ApplyPageBatch(context.Background(), 1, store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "approved", Kind: "update", TargetID: p.ID, ExpectedVersion: p.Version, Content: &content}}}) + if err != nil { + t.Fatal(err) + } + for _, method := range []string{"PUT", "DELETE"} { + payload := map[string]any{"expected_version": p.Version} + if method == "PUT" { + payload["title"] = "Stale browser draft" + } + w, _ := doJSON(t, api, method, url, payload) + if w.Code != http.StatusConflict { + t.Fatal("stale browser operation accepted") + } + } + got, _ := pages.Get(context.Background(), 1, p.ID) + if got.Title != content.Title { + t.Fatal("API used freshly fetched version to overwrite approved content") + } + w, saved := doJSON(t, api, "PUT", url, map[string]any{"expected_version": got.Version, "title": "Fresh browser save"}) + if w.Code != http.StatusOK { + t.Fatal("fresh version refused") + } + w, _ = doJSON(t, api, "DELETE", url, map[string]any{"expected_version": saved["Version"]}) + if w.Code != http.StatusNoContent { + t.Fatal("fresh delete refused") + } +} + +func TestAdminJSONRejectedFieldsNeverEchoValues(t *testing.T) { + api := newAdminTestAPI() + for _, body := range []string{`{"path":"/","title":"Page","rejected_marker":"credential_fixture"}`, `{"path":"/","title":"Page"} {"rejected_marker":"credential_fixture"}`} { + w := httptest.NewRecorder() + api.ServeHTTP(w, httptest.NewRequest("POST", "/api/v1/admin/tenants/1/pages", strings.NewReader(body))) + if w.Code != http.StatusBadRequest || strings.Contains(w.Body.String(), "credential_fixture") || strings.Contains(w.Body.String(), "rejected_marker") { + t.Fatal("invalid request leaked rejected fields") + } + } +} diff --git a/promotion/bundle.go b/promotion/bundle.go new file mode 100644 index 0000000..136610a --- /dev/null +++ b/promotion/bundle.go @@ -0,0 +1,314 @@ +package promotion + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "io" + "io/fs" + "net/url" + "os" + "path" + "path/filepath" + "regexp" + "sort" + "strings" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" + "golang.org/x/net/html" +) + +type BundleFile struct { + Path string `json:"path"` + SHA256 string `json:"sha256"` + Size int64 `json:"size"` +} +type BundleManifest struct { + Files []BundleFile `json:"files"` + Digest string `json:"digest"` +} + +func (m BundleManifest) contentDigest() string { m.Digest = ""; return sumJSON(m) } +func cleanFile(p string) bool { + return p != "" && !strings.HasPrefix(p, "/") && path.Clean(p) == p && p != "." && p != ".." && !strings.HasPrefix(p, "../") && !strings.ContainsAny(p, "\\\x00?#") && !strings.Contains(p, "/.") && !strings.HasPrefix(p, ".") +} +func (m BundleManifest) Validate() error { + if len(m.Files) == 0 || len(m.Files) > 4096 || m.Digest != m.contentDigest() { + return ErrBundle + } + previous := "" + var total int64 + for _, f := range m.Files { + b, err := hex.DecodeString(f.SHA256) + if !cleanFile(f.Path) || f.Path <= previous || f.Size < 0 || f.Size > 64<<20 || err != nil || len(b) != sha256.Size || strings.ToLower(f.SHA256) != f.SHA256 { + return ErrBundle + } + previous = f.Path + total += f.Size + if total > 64<<20 { + return ErrBundle + } + } + return nil +} + +// InventoryBundle hashes exact regular files only. Call with an immutable +// version directory, not a current symlink. No file content is logged. +func InventoryBundle(root string) (BundleManifest, error) { + info, err := os.Lstat(root) + if err != nil || !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + return BundleManifest{}, ErrBundle + } + m := BundleManifest{Files: []BundleFile{}} + var total int64 + err = filepath.WalkDir(root, func(name string, d fs.DirEntry, walkErr error) error { + if walkErr != nil || d.Type()&os.ModeSymlink != 0 { + return ErrBundle + } + if d.IsDir() { + return nil + } + info, err := d.Info() + if err != nil || !info.Mode().IsRegular() || info.Size() > 64<<20 { + return ErrBundle + } + rel, err := filepath.Rel(root, name) + if err != nil { + return ErrBundle + } + rel = filepath.ToSlash(rel) + if !cleanFile(rel) { + return ErrBundle + } + total += info.Size() + if total > 64<<20 || len(m.Files) >= 4096 { + return ErrBundle + } + file, err := os.Open(name) + if err != nil { + return ErrBundle + } + h := sha256.New() + n, err := io.Copy(h, io.LimitReader(file, 64<<20+1)) + closeErr := file.Close() + if err != nil || closeErr != nil || n != info.Size() { + return ErrBundle + } + m.Files = append(m.Files, BundleFile{rel, hex.EncodeToString(h.Sum(nil)), n}) + return nil + }) + if err != nil { + return BundleManifest{}, ErrBundle + } + sort.Slice(m.Files, func(i, j int) bool { return m.Files[i].Path < m.Files[j].Path }) + m.Digest = m.contentDigest() + if err = m.Validate(); err != nil { + return BundleManifest{}, err + } + return m, nil +} + +var cssReferences = regexp.MustCompile(`(?i)url\(\s*["']?([^\s"')]+)["']?\s*\)|@import\s+["']([^"']+)["']`) + +// VerifyBundle re-hashes the supplied directory, validates local references and +// rejects exact static route shadowing. Uploaded tenant media and admin paths +// remain unsupported; copying their URLs would leak review tenant identity. +func VerifyBundle(root string, manifest BundleManifest, pages []store.PageContent) error { + actual, err := InventoryBundle(root) + if err != nil || manifest.Validate() != nil || sumJSON(actual) != sumJSON(manifest) { + return ErrBundle + } + files := map[string]bool{} + routes := map[string]bool{} + for _, f := range manifest.Files { + files["/"+f.Path] = true + } + for _, p := range pages { + if p.Validate() != nil { + return ErrBundle + } + routes[p.Path] = true + exact := strings.TrimPrefix(p.Path, "/") + if exact == "" { + exact = "index.html" + } + if files["/"+exact] { + return ErrBundle + } + if p.TemplateID != "" && (!cleanFile(p.TemplateID) || strings.Contains(p.TemplateID, "/") || !files["/cms/templates/"+p.TemplateID+".html"]) { + return ErrBundle + } + } + checkRef := func(raw, base string, link bool) error { + if raw == "" || strings.HasPrefix(raw, "#") { + return nil + } + u, err := url.Parse(raw) + if err != nil || u.User != nil { + return ErrBundle + } + if u.Scheme != "" { + if link && (u.Scheme == "mailto" || u.Scheme == "tel") { + return nil + } + if u.Scheme != "https" || u.Host == "" || strings.Contains(strings.ToLower(u.Hostname()), "preview.") || strings.Contains(strings.ToLower(u.Hostname()), "-review.") { + return ErrBundle + } + if strings.HasPrefix(u.Path, "/media/") { + return ErrBundle + } + return nil + } + if u.Host != "" { + return ErrBundle + } + if strings.Contains(u.Path, "\\") || strings.Contains(u.Path, "\x00") { + return ErrBundle + } + resolved := path.Clean(u.Path) + if !strings.HasPrefix(u.Path, "/") { + resolved = path.Join(path.Dir(base), u.Path) + } + if strings.HasPrefix(resolved, "/media/") || strings.HasPrefix(resolved, "/api/") || strings.HasPrefix(resolved, "/admin") || strings.HasPrefix(resolved, "/cms/") { + return ErrBundle + } + if files[resolved] || link && routes[resolved] { + return nil + } + return ErrBundle + } + checkCSS := func(css, base string) error { + for _, m := range cssReferences.FindAllStringSubmatch(css, -1) { + ref := m[1] + if ref == "" { + ref = m[2] + } + if err := checkRef(ref, base, false); err != nil { + return err + } + } + return nil + } + checkHTML := func(source, base string) error { + doc, err := html.Parse(strings.NewReader(source)) + if err != nil { + return ErrBundle + } + var visit func(*html.Node) error + visit = func(n *html.Node) error { + if n.Type == html.ElementNode && n.Data == "base" { + return ErrBundle + } + for _, a := range n.Attr { + switch a.Key { + case "href", "src", "poster", "action": + if err := checkRef(a.Val, base, a.Key == "href"); err != nil { + return err + } + case "srcset": + for _, ref := range strings.Split(a.Val, ",") { + parts := strings.Fields(ref) + if len(parts) == 0 { + return ErrBundle + } + if err := checkRef(parts[0], base, false); err != nil { + return err + } + } + case "style": + if err := checkCSS(a.Val, base); err != nil { + return err + } + } + } + if n.Type == html.ElementNode && n.Data == "style" { + for c := n.FirstChild; c != nil; c = c.NextSibling { + if c.Type == html.TextNode { + if err := checkCSS(c.Data, base); err != nil { + return err + } + } + } + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + if err := visit(c); err != nil { + return err + } + } + return nil + } + return visit(doc) + } + for _, p := range pages { + if len(p.BodyBlocks) > 0 { + var blocks any + if json.Unmarshal(p.BodyBlocks, &blocks) != nil { + return ErrBundle + } + var checkBlock func(any) error + checkBlock = func(value any) error { + switch v := value.(type) { + case string: + if strings.Contains(v, "/media/") { + return ErrBundle + } + if strings.HasPrefix(v, "/") || strings.HasPrefix(v, "https:") { + return checkRef(v, p.Path, false) + } + if strings.Contains(v, "<") { + return checkHTML(v, p.Path) + } + case []any: + for _, item := range v { + if err := checkBlock(item); err != nil { + return err + } + } + case map[string]any: + for _, item := range v { + if err := checkBlock(item); err != nil { + return err + } + } + } + return nil + } + if err := checkBlock(blocks); err != nil { + return err + } + } + if err := checkHTML(p.BodyHTML, p.Path); err != nil { + return err + } + } + for _, f := range manifest.Files { + if strings.HasSuffix(f.Path, ".css") || strings.HasSuffix(f.Path, ".html") { + b, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(f.Path))) + if err != nil { + return ErrBundle + } + if strings.HasSuffix(f.Path, ".css") { + if err := checkCSS(string(b), "/"+f.Path); err != nil { + return err + } + } else { + if strings.HasPrefix(f.Path, "cms/templates/") && strings.Count(string(b), "") != 1 { + return ErrBundle + } + // Shell relative references are interpreted at public page paths. + if strings.HasPrefix(f.Path, "cms/templates/") { + for _, p := range pages { + if p.TemplateID+".html" == path.Base(f.Path) { + if err := checkHTML(string(b), p.Path); err != nil { + return err + } + } + } + } else if err := checkHTML(string(b), "/"+f.Path); err != nil { + return err + } + } + } + } + return nil +} diff --git a/promotion/plan.go b/promotion/plan.go new file mode 100644 index 0000000..48433b1 --- /dev/null +++ b/promotion/plan.go @@ -0,0 +1,138 @@ +package promotion + +import ( + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +type Mapping struct { + Key string `json:"key"` + TargetID int64 `json:"target_id"` // zero explicitly creates a new page + AllowMove bool `json:"allow_move"` +} +type DeleteSelection struct { + Key string `json:"key"` + TargetID int64 `json:"target_id"` +} +type Diff struct { + Key string `json:"key"` + TargetID int64 `json:"target_id"` + Action string `json:"action"` + OldPath string `json:"old_path,omitempty"` + NewPath string `json:"new_path,omitempty"` + BeforeDigest string `json:"before_digest,omitempty"` + AfterDigest string `json:"after_digest,omitempty"` +} +type Plan struct { + Digest string `json:"digest"` + SnapshotDigest string `json:"snapshot_digest"` + BundleDigest string `json:"bundle_digest"` + BundleBaselineDigest string `json:"bundle_baseline_digest"` + Batch store.PageBatch `json:"batch"` + Diffs []Diff `json:"diffs"` +} + +func (p Plan) ContentDigest() string { p.Digest = ""; return sumJSON(p) } + +// DryRun requires an explicit mapping for every selected page. SuggestedMapping +// offers reviewable path matches, but never silently approves a rename/deletion. +// The caller must separately verify both on-disk bundles before publication. +func DryRun(s Snapshot, current []*store.Page, mappings []Mapping, deletes []DeleteSelection, targetBundle BundleManifest) (Plan, error) { + if err := s.Validate(); err != nil { + return Plan{}, err + } + if targetBundle.Validate() != nil { + return Plan{}, ErrBundle + } + byID := map[int64]*store.Page{} + for _, p := range current { + if p == nil || p.ID <= 0 || byID[p.ID] != nil { + return Plan{}, ErrMapping + } + byID[p.ID] = p + } + byKey := map[string]Mapping{} + targets := map[int64]bool{} + for _, m := range mappings { + if m.Key == "" || m.TargetID < 0 { + return Plan{}, ErrMapping + } + if _, ok := byKey[m.Key]; ok { + return Plan{}, ErrMapping + } + byKey[m.Key] = m + } + if len(mappings) != len(s.Pages) { + return Plan{}, ErrMapping + } + plan := Plan{SnapshotDigest: s.Digest, BundleDigest: s.Bundle.Digest, BundleBaselineDigest: targetBundle.Digest, Batch: store.PageBatch{Baseline: store.Baseline(store.States(current))}, Diffs: []Diff{}} + for _, page := range s.Pages { + m, ok := byKey[page.Key] + if !ok { + return Plan{}, ErrMapping + } + content := page.Content + diff := Diff{Key: page.Key, TargetID: m.TargetID, NewPath: content.Path, AfterDigest: content.Digest()} + mutation := store.PageMutation{Key: page.Key, TargetID: m.TargetID, Content: &content} + if m.TargetID == 0 { + diff.Action = "create" + mutation.Kind = "create" + } else { + p := byID[m.TargetID] + if p == nil || targets[p.ID] { + return Plan{}, ErrMapping + } + targets[p.ID] = true + if (p.Path != content.Path || p.Subsite != content.Subsite) && !m.AllowMove { + return Plan{}, ErrMapping + } + diff.Action = "update" + diff.OldPath = p.Path + diff.BeforeDigest = store.ContentOf(p).Digest() + mutation.Kind = "update" + mutation.ExpectedVersion = p.Version + if diff.BeforeDigest == diff.AfterDigest { + diff.Action = "unchanged" + } + } + plan.Diffs = append(plan.Diffs, diff) + if diff.Action != "unchanged" { + plan.Batch.Mutations = append(plan.Batch.Mutations, mutation) + } + } + for _, d := range deletes { + p := byID[d.TargetID] + if d.Key == "" || p == nil || targets[p.ID] { + return Plan{}, ErrMapping + } + targets[p.ID] = true + plan.Batch.Mutations = append(plan.Batch.Mutations, store.PageMutation{Key: d.Key, Kind: "delete", TargetID: p.ID, ExpectedVersion: p.Version}) + plan.Diffs = append(plan.Diffs, Diff{Key: d.Key, TargetID: p.ID, Action: "delete", OldPath: p.Path, BeforeDigest: store.ContentOf(p).Digest()}) + } + if len(plan.Batch.Mutations) > 0 { + if err := store.CheckPageBatch(store.States(current), plan.Batch, false); err != nil { + return Plan{}, err + } + } + plan.Digest = plan.ContentDigest() + return plan, nil +} + +func SuggestedMapping(s Snapshot, current []*store.Page) ([]Mapping, error) { + if s.Validate() != nil { + return nil, ErrInvalid + } + out := []Mapping{} + for _, page := range s.Pages { + m := Mapping{Key: page.Key} + for _, p := range current { + if p.Subsite == page.Content.Subsite && p.Path == page.Content.Path { + if m.TargetID != 0 { + return nil, ErrMapping + } + m.TargetID = p.ID + } + } + out = append(out, m) + } + return out, nil +} diff --git a/promotion/snapshot.go b/promotion/snapshot.go new file mode 100644 index 0000000..5c9f178 --- /dev/null +++ b/promotion/snapshot.go @@ -0,0 +1,186 @@ +// Package promotion prepares content-only publication artifacts. It deliberately +// has no network client or apply endpoint: host authority, bundle fencing and +// restart-safe activation are separate mandatory publication gates. +package promotion + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "io" + "regexp" + "sort" + "strconv" + "time" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +var ErrInvalid = errors.New("content snapshot invalid") +var ErrBundle = errors.New("content bundle invalid or unsupported reference") +var ErrMapping = errors.New("content mapping invalid or ambiguous") + +const SchemaVersion = 1 +const maxSnapshotBytes = 8 << 20 + +var sitePattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,62}$`) + +type SnapshotPage struct { + Key string `json:"key"` + SourceID int64 `json:"source_id"` + SourceVersion int `json:"source_version"` + Content store.PageContent `json:"content"` +} +type Snapshot struct { + Schema int `json:"schema"` + SourceSite string `json:"source_site"` + CreatedAt time.Time `json:"created_at"` + Pages []SnapshotPage `json:"pages"` + Bundle BundleManifest `json:"bundle"` + Digest string `json:"digest"` +} + +func sumJSON(value any) string { + b, _ := json.Marshal(value) + h := sha256.Sum256(b) + return hex.EncodeToString(h[:]) +} +func (s Snapshot) ContentDigest() string { s.Digest = ""; return sumJSON(s) } + +func (s Snapshot) Validate() error { + if s.Schema != SchemaVersion || !sitePattern.MatchString(s.SourceSite) || s.CreatedAt.IsZero() || s.CreatedAt.Location() != time.UTC || len(s.Pages) == 0 || len(s.Pages) > 1000 || s.Digest != s.ContentDigest() { + return ErrInvalid + } + keys := map[string]bool{} + paths := map[string]bool{} + for _, p := range s.Pages { + if p.SourceID <= 0 || p.SourceVersion <= 0 || p.Key != s.SourceSite+":"+strconv.FormatInt(p.SourceID, 10) || keys[p.Key] { + return ErrInvalid + } + keys[p.Key] = true + if err := p.Content.Validate(); err != nil { + return ErrInvalid + } + key := p.Content.Subsite + "\x00" + p.Content.Path + if paths[key] { + return ErrInvalid + } + paths[key] = true + } + if err := s.Bundle.Validate(); err != nil { + return err + } + return nil +} + +// Export freezes selected rows. It reads the supplied authorized tenant scope; +// it never consults a domain, credential, tenant setting or editor membership. +func Export(ctx context.Context, pages store.PageStore, sourceTenantID int64, sourceSite string, ids []int64, bundleRoot string, now time.Time) (Snapshot, error) { + if !sitePattern.MatchString(sourceSite) || sourceTenantID <= 0 || len(ids) == 0 { + return Snapshot{}, ErrInvalid + } + all, err := pages.List(ctx, sourceTenantID, "") + if err != nil { + return Snapshot{}, ErrInvalid + } + available := map[int64]*store.Page{} + for _, p := range all { + available[p.ID] = p + } + s := Snapshot{Schema: SchemaVersion, SourceSite: sourceSite, CreatedAt: now.UTC(), Pages: []SnapshotPage{}} + seen := map[int64]bool{} + for _, id := range ids { + p := available[id] + if p == nil || seen[id] { + return Snapshot{}, ErrInvalid + } + seen[id] = true + s.Pages = append(s.Pages, SnapshotPage{sourceSite + ":" + strconv.FormatInt(id, 10), id, p.Version, store.ContentOf(p)}) + } + sort.Slice(s.Pages, func(i, j int) bool { return s.Pages[i].SourceID < s.Pages[j].SourceID }) + s.Bundle, err = InventoryBundle(bundleRoot) + if err != nil { + return Snapshot{}, err + } + contents := []store.PageContent{} + for _, p := range s.Pages { + contents = append(contents, p.Content) + } + if err = VerifyBundle(bundleRoot, s.Bundle, contents); err != nil { + return Snapshot{}, err + } + s.Digest = s.ContentDigest() + if err = s.Validate(); err != nil { + return Snapshot{}, err + } + return s, nil +} + +// Decode rejects unknown fields, duplicate JSON keys, trailing documents and +// oversize input. Errors are fixed; rejected keys/values are never returned. +func Decode(r io.Reader) (Snapshot, error) { + b, err := io.ReadAll(io.LimitReader(r, maxSnapshotBytes+1)) + if err != nil || len(b) > maxSnapshotBytes { + return Snapshot{}, ErrInvalid + } + if !uniqueJSON(b) { + return Snapshot{}, ErrInvalid + } + var s Snapshot + d := json.NewDecoder(bytes.NewReader(b)) + d.DisallowUnknownFields() + if d.Decode(&s) != nil || d.Decode(new(any)) != io.EOF { + return Snapshot{}, ErrInvalid + } + if s.Validate() != nil { + return Snapshot{}, ErrInvalid + } + return s, nil +} + +func uniqueJSON(b []byte) bool { + d := json.NewDecoder(bytes.NewReader(b)) + d.UseNumber() + var value func() bool + value = func() bool { + t, err := d.Token() + if err != nil { + return false + } + delim, ok := t.(json.Delim) + if !ok { + return true + } + switch delim { + case '{': + keys := map[string]bool{} + for d.More() { + key, err := d.Token() + s, ok := key.(string) + if err != nil || !ok || keys[s] { + return false + } + keys[s] = true + if !value() { + return false + } + } + end, err := d.Token() + return err == nil && end == json.Delim('}') + case '[': + for d.More() { + if !value() { + return false + } + } + end, err := d.Token() + return err == nil && end == json.Delim(']') + default: + return false + } + } + return value() && func() bool { _, err := d.Token(); return err == io.EOF }() +} diff --git a/promotion/snapshot_test.go b/promotion/snapshot_test.go new file mode 100644 index 0000000..84e1320 --- /dev/null +++ b/promotion/snapshot_test.go @@ -0,0 +1,163 @@ +package promotion + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +func fixture(t *testing.T) (Snapshot, string, *store.MemoryPageStore, *store.Page) { + t.Helper() + root := t.TempDir() + write := func(p, s string) { + name := filepath.Join(root, p) + if err := os.MkdirAll(filepath.Dir(name), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name, []byte(s), 0600); err != nil { + t.Fatal(err) + } + } + write("assets/site.css", "body{color:#111;background:#fff}") + write("assets/photo.jpg", "owned-photo-bytes") + write("cms/templates/main.html", `
`) + pages := store.NewMemoryPageStore() + p := &store.Page{Path: "/", Title: "Approved home", BodyHTML: `PortraitHome`, Status: store.StatusPublished, TemplateID: "main"} + if err := pages.Create(context.Background(), 10, p); err != nil { + t.Fatal(err) + } + s, err := Export(context.Background(), pages, 10, "test-review", []int64{p.ID}, root, time.Date(2026, 10, 8, 18, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatal(err) + } + return s, root, pages, p +} + +func TestSnapshotFreezeExplicitMappingAndDryRun(t *testing.T) { + s, root, pages, review := fixture(t) + b, _ := json.Marshal(s) + decoded, err := Decode(bytes.NewReader(b)) + if err != nil || decoded.Digest != s.Digest { + t.Fatal("strict roundtrip failed") + } + for _, needle := range []string{`"tenant_id"`, `"password"`, `"domains"`, `"editors"`, `"repository"`, `"token"`} { + if bytes.Contains(b, []byte(needle)) { + t.Fatal("authority field leaked into snapshot") + } + } + review.Title = "Later unapproved draft" + if err := pages.Update(context.Background(), 10, review); err != nil { + t.Fatal(err) + } + if s.Pages[0].Content.Title == review.Title { + t.Fatal("export was not frozen") + } + target := &store.Page{Path: "/", Title: "Old live home", Status: store.StatusPublished} + if err := pages.Create(context.Background(), 20, target); err != nil { + t.Fatal(err) + } + keep := &store.Page{Path: "/keep", Title: "Keep", Status: store.StatusDraft} + if err := pages.Create(context.Background(), 20, keep); err != nil { + t.Fatal(err) + } + current, _ := pages.List(context.Background(), 20, "") + mappings, err := SuggestedMapping(s, current) + if err != nil || mappings[0].TargetID != target.ID || target.ID == s.Pages[0].SourceID { + t.Fatal("path suggestion/mapping failed") + } + plan, err := DryRun(s, current, mappings, nil, s.Bundle) + if err != nil { + t.Fatal(err) + } + if len(plan.Batch.Mutations) != 1 || len(plan.Batch.Baseline) != 2 || plan.Diffs[0].Action != "update" { + t.Fatal("dry run omitted baseline or updated omission") + } + before, _ := pages.Get(context.Background(), 20, target.ID) + if before.Title != target.Title { + t.Fatal("dry run wrote content") + } + receipt, err := pages.ApplyPageBatch(context.Background(), 20, plan.Batch) + if err != nil { + t.Fatal(err) + } + got, _ := pages.Get(context.Background(), 20, target.ID) + if got.Title != s.Pages[0].Content.Title { + t.Fatal("apply used unapproved source edit") + } + if err := VerifyBundle(root, s.Bundle, []store.PageContent{store.ContentOf(got)}); err != nil { + t.Fatal(err) + } + if _, err := pages.RollbackPageBatch(context.Background(), 20, receipt); err != nil { + t.Fatal(err) + } + move := s + move.Pages = append([]SnapshotPage(nil), s.Pages...) + move.Pages[0].Content.Path = "/moved" + move.Digest = move.ContentDigest() + if _, err := DryRun(move, current, mappings, nil, s.Bundle); !errors.Is(err, ErrMapping) { + t.Fatal("implicit move accepted") + } + if _, err := DryRun(s, current, nil, nil, s.Bundle); !errors.Is(err, ErrMapping) { + t.Fatal("implicit target mapping accepted") + } +} + +func TestDecodeRejectsAuthorityUnknownDuplicateAndTrailingWithoutValues(t *testing.T) { + s, _, _, _ := fixture(t) + b, _ := json.Marshal(s) + base := string(b) + for _, payload := range []string{strings.Replace(base, `"schema":1`, `"schema":1,"credentials":"rejected_marker"`, 1), strings.Replace(base, `"schema":1`, `"schema":1,"schema":1`, 1), base + base, strings.Replace(base, `"title":"Approved home"`, `"title":"Approved home","domains":["rejected_marker"]`, 1), strings.Replace(base, `"size":`, `"extra":"rejected_marker","size":`, 1)} { + _, err := Decode(strings.NewReader(payload)) + if err != ErrInvalid || strings.Contains(err.Error(), "rejected_marker") { + t.Fatal("strict decoder leaked or accepted invalid payload") + } + } +} + +func TestBundleHashesReferencesStaticCollisionsAndSymlinks(t *testing.T) { + s, root, _, _ := fixture(t) + contents := []store.PageContent{s.Pages[0].Content} + if err := os.WriteFile(filepath.Join(root, "assets/photo.jpg"), []byte("changed"), 0600); err != nil { + t.Fatal(err) + } + if VerifyBundle(root, s.Bundle, contents) != ErrBundle { + t.Fatal("changed bundle bytes accepted") + } + manifest, _ := InventoryBundle(root) + for _, ref := range []string{``, ``, ``, ``, `Review`} { + p := contents[0] + p.BodyHTML = ref + if VerifyBundle(root, manifest, []store.PageContent{p}) != ErrBundle { + t.Fatal("unsupported reference accepted") + } + } + p := contents[0] + p.BodyBlocks = json.RawMessage(`{"type":"doc","url":"\u002fmedia/10/hash.jpg"}`) + if VerifyBundle(root, manifest, []store.PageContent{p}) != ErrBundle { + t.Fatal("escaped upload reference accepted") + } + if err := os.WriteFile(filepath.Join(root, "index.html"), []byte("Static shadow"), 0600); err != nil { + t.Fatal(err) + } + manifest, _ = InventoryBundle(root) + if VerifyBundle(root, manifest, contents) != ErrBundle { + t.Fatal("static homepage shadow accepted") + } + if err := os.Remove(filepath.Join(root, "index.html")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(root, "assets/photo.jpg"), filepath.Join(root, "assets/alias.jpg")); err != nil { + t.Fatal(err) + } + if _, err := InventoryBundle(root); err != ErrBundle { + t.Fatal("bundle symlink accepted") + } +} diff --git a/store/batch.go b/store/batch.go new file mode 100644 index 0000000..81420b4 --- /dev/null +++ b/store/batch.go @@ -0,0 +1,282 @@ +package store + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "path" + "reflect" + "sort" + "strconv" + "strings" + "time" +) + +func idString(id int64) string { return strconv.FormatInt(id, 10) } + +var ErrBatchInvalid = errors.New("page batch invalid") + +// PageContent is an explicit content allowlist; it carries no tenant authority. +type PageContent struct { + Subsite string `json:"subsite"` + Path string `json:"path"` + Title string `json:"title"` + BodyHTML string `json:"body_html"` + BodyBlocks json.RawMessage `json:"body_blocks,omitempty"` + Status PageStatus `json:"status"` + TemplateID string `json:"template_id,omitempty"` + PublishAt *time.Time `json:"publish_at,omitempty"` + UnpublishAt *time.Time `json:"unpublish_at,omitempty"` +} + +func ContentOf(p *Page) PageContent { + c := PageContent{p.Subsite, p.Path, p.Title, p.BodyHTML, append(json.RawMessage(nil), p.BodyBlocks...), p.Status, p.TemplateID, cloneTime(p.PublishAt), cloneTime(p.UnpublishAt)} + return c +} + +func cloneTime(t *time.Time) *time.Time { + if t == nil { + return nil + } + v := t.UTC() + return &v +} + +// Page reconstructs content in the caller's authorized tenant, not a payload tenant. +func (c PageContent) Page(tenantID, id int64, version int) *Page { + return &Page{ID: id, TenantID: tenantID, Subsite: c.Subsite, Path: c.Path, Title: c.Title, BodyHTML: c.BodyHTML, BodyBlocks: append(json.RawMessage(nil), c.BodyBlocks...), Status: c.Status, TemplateID: c.TemplateID, PublishAt: cloneTime(c.PublishAt), UnpublishAt: cloneTime(c.UnpublishAt), Version: version} +} + +func (c PageContent) Validate() error { + if !strings.HasPrefix(c.Path, "/") || path.Clean(c.Path) != c.Path || strings.ContainsAny(c.Path, "?#\\\x00") || strings.TrimSpace(c.Title) == "" || strings.TrimSpace(c.Subsite) != c.Subsite { + return ErrBatchInvalid + } + if len(c.BodyBlocks) > 0 && !json.Valid(c.BodyBlocks) { + return ErrBatchInvalid + } + if c.Status != StatusDraft && c.Status != StatusPublished && c.Status != StatusScheduled && c.Status != StatusArchived { + return ErrBatchInvalid + } + if c.Status == StatusScheduled && c.PublishAt == nil { + return ErrBatchInvalid + } + if c.PublishAt != nil && c.PublishAt.IsZero() || c.UnpublishAt != nil && c.UnpublishAt.IsZero() { + return ErrBatchInvalid + } + if c.PublishAt != nil && c.UnpublishAt != nil && !c.UnpublishAt.After(*c.PublishAt) { + return ErrBatchInvalid + } + return nil +} + +func (c PageContent) Digest() string { + // JSONB may reorder object keys; normalize blocks before computing a baseline. + if len(c.BodyBlocks) > 0 { + var value any + d := json.NewDecoder(strings.NewReader(string(c.BodyBlocks))) + d.UseNumber() + if d.Decode(&value) == nil { + c.BodyBlocks, _ = json.Marshal(value) + } + } + c.PublishAt, c.UnpublishAt = cloneTime(c.PublishAt), cloneTime(c.UnpublishAt) + b, _ := json.Marshal(c) + hash := sha256.Sum256(b) + return hex.EncodeToString(hash[:]) +} + +type PageBaseline struct { + ID int64 `json:"id"` + Version int `json:"version"` + Digest string `json:"digest"` +} + +type PageState struct { + ID int64 `json:"id"` + Version int `json:"version"` + Content PageContent `json:"content"` +} + +func States(pages []*Page) []PageState { + out := make([]PageState, 0, len(pages)) + for _, p := range pages { + out = append(out, PageState{p.ID, p.Version, ContentOf(p)}) + } + sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) + return out +} +func Baseline(states []PageState) []PageBaseline { + out := make([]PageBaseline, 0, len(states)) + for _, p := range states { + out = append(out, PageBaseline{p.ID, p.Version, p.Content.Digest()}) + } + sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) + return out +} + +// PageMutation never copies a source ID into a target ID implicitly. +type PageMutation struct { + Key string `json:"key"` + Kind string `json:"kind"` // create, update, delete; restore is rollback-only + TargetID int64 `json:"target_id"` + ExpectedVersion int `json:"expected_version"` + Content *PageContent `json:"content,omitempty"` +} +type PageBatch struct { + Baseline []PageBaseline `json:"baseline"` + Mutations []PageMutation `json:"mutations"` +} +type PageBatchReceipt struct { + Before []PageState `json:"before"` + After []PageState `json:"after"` + Mapping map[string]int64 `json:"mapping"` + Digest string `json:"digest"` +} + +// Seal detects archive corruption. It is not a signature or authorization. +func (r *PageBatchReceipt) Seal() { + r.Digest = "" + b, _ := json.Marshal(r) + h := sha256.Sum256(b) + r.Digest = hex.EncodeToString(h[:]) +} + +// PageBatchStore is internal persistence, not an authorized publication API. +// The host must provide approval, fencing, verified bundles and durable backups. +type PageBatchStore interface { + ApplyPageBatch(context.Context, int64, PageBatch) (PageBatchReceipt, error) + RollbackPageBatch(context.Context, int64, PageBatchReceipt) (PageBatchReceipt, error) +} + +// CheckPageBatch compares the entire tenant baseline and checks the final paths +// before any mutation. Omitted pages remain untouched. restore is never accepted +// by ordinary apply; it is generated only by the guarded rollback implementation. +func CheckPageBatch(current []PageState, batch PageBatch, allowRestore bool) error { + if !reflect.DeepEqual(Baseline(current), batch.Baseline) { + return ErrVersionConflict + } + if len(batch.Mutations) == 0 { + return ErrBatchInvalid + } + pages := map[int64]PageState{} + for _, p := range current { + pages[p.ID] = p + } + keys := map[string]bool{} + targets := map[int64]bool{} + creates := []PageContent{} + for _, m := range batch.Mutations { + if m.Key == "" || keys[m.Key] { + return ErrBatchInvalid + } + keys[m.Key] = true + if m.TargetID != 0 && targets[m.TargetID] { + return ErrBatchInvalid + } + targets[m.TargetID] = true + switch m.Kind { + case "create": + if m.TargetID != 0 || m.ExpectedVersion != 0 || m.Content == nil { + return ErrBatchInvalid + } + creates = append(creates, *m.Content) + case "update", "delete": + p, ok := pages[m.TargetID] + if !ok || m.ExpectedVersion <= 0 || p.Version != m.ExpectedVersion { + return ErrVersionConflict + } + if m.Kind == "delete" { + if m.Content != nil { + return ErrBatchInvalid + } + delete(pages, m.TargetID) + } else { + if m.Content == nil { + return ErrBatchInvalid + } + p.Content = *m.Content + pages[m.TargetID] = p + } + case "restore": + if !allowRestore || m.TargetID <= 0 || m.ExpectedVersion <= 0 || m.Content == nil { + return ErrBatchInvalid + } + if _, ok := pages[m.TargetID]; ok { + return ErrVersionConflict + } + pages[m.TargetID] = PageState{m.TargetID, m.ExpectedVersion, *m.Content} + default: + return ErrBatchInvalid + } + } + paths := map[string]bool{} + check := func(c PageContent) error { + if err := c.Validate(); err != nil { + return err + } + key := c.Subsite + "\x00" + c.Path + if paths[key] { + return ErrPathConflict + } + paths[key] = true + return nil + } + for _, p := range pages { + if err := check(p.Content); err != nil { + return err + } + } + for _, c := range creates { + if err := check(c); err != nil { + return err + } + } + return nil +} + +func RollbackBatch(r PageBatchReceipt) (PageBatch, error) { + copy := r + copy.Seal() + if r.Digest == "" || r.Digest != copy.Digest { + return PageBatch{}, ErrBatchInvalid + } + before := map[int64]PageState{} + after := map[int64]PageState{} + for _, p := range r.Before { + if p.ID <= 0 || p.Version <= 0 || p.Content.Validate() != nil { + return PageBatch{}, ErrBatchInvalid + } + if _, ok := before[p.ID]; ok { + return PageBatch{}, ErrBatchInvalid + } + before[p.ID] = p + } + for _, p := range r.After { + if p.ID <= 0 || p.Version <= 0 || p.Content.Validate() != nil { + return PageBatch{}, ErrBatchInvalid + } + if _, ok := after[p.ID]; ok { + return PageBatch{}, ErrBatchInvalid + } + after[p.ID] = p + } + b := PageBatch{Baseline: Baseline(r.After)} + for _, p := range r.After { + old, ok := before[p.ID] + if !ok { + b.Mutations = append(b.Mutations, PageMutation{Key: "rollback:" + idString(p.ID), Kind: "delete", TargetID: p.ID, ExpectedVersion: p.Version}) + } else if old.Content.Digest() != p.Content.Digest() { + c := old.Content + b.Mutations = append(b.Mutations, PageMutation{Key: "rollback:" + idString(p.ID), Kind: "update", TargetID: p.ID, ExpectedVersion: p.Version, Content: &c}) + } + } + for _, p := range r.Before { + if _, ok := after[p.ID]; !ok { + c := p.Content + b.Mutations = append(b.Mutations, PageMutation{Key: "rollback:" + idString(p.ID), Kind: "restore", TargetID: p.ID, ExpectedVersion: p.Version + 1, Content: &c}) + } + } + return b, nil +} diff --git a/store/batch_memory.go b/store/batch_memory.go new file mode 100644 index 0000000..2ec590e --- /dev/null +++ b/store/batch_memory.go @@ -0,0 +1,74 @@ +package store + +import ( + "context" + "time" +) + +func (s *MemoryPageStore) statesLocked(tenantID int64) []PageState { + var pages []*Page + for _, p := range s.pages { + if p.TenantID == tenantID { + pages = append(pages, p) + } + } + return States(pages) +} + +func (s *MemoryPageStore) ApplyPageBatch(_ context.Context, tenantID int64, batch PageBatch) (PageBatchReceipt, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.applyBatchLocked(tenantID, batch, false) +} + +func (s *MemoryPageStore) RollbackPageBatch(_ context.Context, tenantID int64, receipt PageBatchReceipt) (PageBatchReceipt, error) { + batch, err := RollbackBatch(receipt) + if err != nil { + return PageBatchReceipt{}, err + } + s.mu.Lock() + defer s.mu.Unlock() + return s.applyBatchLocked(tenantID, batch, true) +} + +func (s *MemoryPageStore) applyBatchLocked(tenantID int64, batch PageBatch, restore bool) (PageBatchReceipt, error) { + if tenantID <= 0 { + return PageBatchReceipt{}, ErrBatchInvalid + } + before := s.statesLocked(tenantID) + if err := CheckPageBatch(before, batch, restore); err != nil { + return PageBatchReceipt{}, err + } + result := PageBatchReceipt{Before: before, Mapping: map[string]int64{}} + now := time.Now().UTC() + for _, m := range batch.Mutations { + if m.Kind == "delete" { + delete(s.pages, m.TargetID) + result.Mapping[m.Key] = m.TargetID + continue + } + id, version := m.TargetID, m.ExpectedVersion+1 + if m.Kind == "create" { + s.nextID++ + id = s.nextID + version = 1 + } + if m.Kind == "restore" { + version = m.ExpectedVersion + if id > s.nextID { + s.nextID = id + } + } + p := m.Content.Page(tenantID, id, version) + p.CreatedAt = now + p.UpdatedAt = now + if old := s.pages[id]; old != nil { + p.CreatedAt = old.CreatedAt + } + s.pages[id] = p + result.Mapping[m.Key] = id + } + result.After = s.statesLocked(tenantID) + result.Seal() + return result, nil +} diff --git a/store/batch_test.go b/store/batch_test.go new file mode 100644 index 0000000..785b05c --- /dev/null +++ b/store/batch_test.go @@ -0,0 +1,11 @@ +package store_test + +import ( + "github.com/GoCodeAlone/workflow-plugin-cms/store" + "github.com/GoCodeAlone/workflow-plugin-cms/store/storetest" + "testing" +) + +func TestMemoryPageBatchContract(t *testing.T) { + storetest.PageBatches(t, store.NewMemoryPageStore(), 1, 2) +} diff --git a/store/page.go b/store/page.go index a8afdee..9ed3f4a 100644 --- a/store/page.go +++ b/store/page.go @@ -92,6 +92,9 @@ var ErrNotFound = errors.New("page not found") // duplicate (tenant_id, subsite, path) tuple. var ErrPathConflict = errors.New("page path conflict") +// ErrVersionConflict refuses stale or missing write preconditions. +var ErrVersionConflict = errors.New("page changed; reload before saving or deleting") + // PageStore persists Page records scoped by tenant. // // Every method takes tenantID as the FIRST arg AFTER ctx — making @@ -101,7 +104,7 @@ type PageStore interface { Get(ctx context.Context, tenantID int64, id int64) (*Page, error) GetByPath(ctx context.Context, tenantID int64, subsite, path string) (*Page, error) Update(ctx context.Context, tenantID int64, p *Page) error - Delete(ctx context.Context, tenantID int64, id int64) error + Delete(ctx context.Context, tenantID int64, id int64, expectedVersion int) error List(ctx context.Context, tenantID int64, subsite string) ([]*Page, error) } @@ -145,6 +148,8 @@ func (s *MemoryPageStore) Create(_ context.Context, tenantID int64, p *Page) err p.Version = 1 } cp := *p + cp.BodyBlocks = append(json.RawMessage(nil), p.BodyBlocks...) + cp.PublishAt, cp.UnpublishAt = cloneTime(p.PublishAt), cloneTime(p.UnpublishAt) s.pages[p.ID] = &cp return nil } @@ -159,6 +164,8 @@ func (s *MemoryPageStore) Get(_ context.Context, tenantID, id int64) (*Page, err return nil, ErrNotFound } cp := *p + cp.BodyBlocks = append(json.RawMessage(nil), p.BodyBlocks...) + cp.PublishAt, cp.UnpublishAt = cloneTime(p.PublishAt), cloneTime(p.UnpublishAt) return &cp, nil } @@ -169,6 +176,8 @@ func (s *MemoryPageStore) GetByPath(_ context.Context, tenantID int64, subsite, for _, p := range s.pages { if p.TenantID == tenantID && p.Subsite == subsite && p.Path == path { cp := *p + cp.BodyBlocks = append(json.RawMessage(nil), p.BodyBlocks...) + cp.PublishAt, cp.UnpublishAt = cloneTime(p.PublishAt), cloneTime(p.UnpublishAt) return &cp, nil } } @@ -192,6 +201,9 @@ func (s *MemoryPageStore) Update(_ context.Context, tenantID int64, p *Page) err if !ok || existing.TenantID != tenantID { return ErrNotFound } + if p.Version <= 0 || p.Version != existing.Version { + return ErrVersionConflict + } if s.duplicateLocked(tenantID, p.Subsite, p.Path, p.ID) { return ErrPathConflict } @@ -200,18 +212,23 @@ func (s *MemoryPageStore) Update(_ context.Context, tenantID int64, p *Page) err p.CreatedAt = existing.CreatedAt p.UpdatedAt = time.Now().UTC() cp := *p + cp.BodyBlocks = append(json.RawMessage(nil), p.BodyBlocks...) + cp.PublishAt, cp.UnpublishAt = cloneTime(p.PublishAt), cloneTime(p.UnpublishAt) s.pages[p.ID] = &cp return nil } // Delete removes the page. ErrNotFound on miss / wrong tenant. -func (s *MemoryPageStore) Delete(_ context.Context, tenantID, id int64) error { +func (s *MemoryPageStore) Delete(_ context.Context, tenantID, id int64, expectedVersion int) error { s.mu.Lock() defer s.mu.Unlock() existing, ok := s.pages[id] if !ok || existing.TenantID != tenantID { return ErrNotFound } + if expectedVersion <= 0 || expectedVersion != existing.Version { + return ErrVersionConflict + } delete(s.pages, id) return nil } @@ -231,6 +248,8 @@ func (s *MemoryPageStore) List(_ context.Context, tenantID int64, subsite string continue } cp := *p + cp.BodyBlocks = append(json.RawMessage(nil), p.BodyBlocks...) + cp.PublishAt, cp.UnpublishAt = cloneTime(p.PublishAt), cloneTime(p.UnpublishAt) out = append(out, &cp) } // Stable sort by Path for deterministic output. diff --git a/store/page_test.go b/store/page_test.go index 55b29a1..d4f3921 100644 --- a/store/page_test.go +++ b/store/page_test.go @@ -52,7 +52,7 @@ func TestPageStore_CreateGetUpdateDelete(t *testing.T) { } // Delete - if err := s.Delete(ctx, 1, got.ID); err != nil { + if err := s.Delete(ctx, 1, got.ID, got.Version); err != nil { t.Fatalf("Delete: %v", err) } if _, err := s.Get(ctx, 1, got.ID); !errors.Is(err, ErrNotFound) { @@ -96,7 +96,7 @@ func TestPageStore_DeleteCrossTenantNoLeak(t *testing.T) { p := makePage(1, "/x", "P") _ = s.Create(ctx, 1, p) - if err := s.Delete(ctx, 2, p.ID); !errors.Is(err, ErrNotFound) { + if err := s.Delete(ctx, 2, p.ID, p.Version); !errors.Is(err, ErrNotFound) { t.Errorf("cross-tenant Delete: got %v want ErrNotFound", err) } // Sanity: page still exists for owner. diff --git a/store/postgres/batch.go b/store/postgres/batch.go new file mode 100644 index 0000000..c02719e --- /dev/null +++ b/store/postgres/batch.go @@ -0,0 +1,144 @@ +package postgres + +import ( + "context" + "errors" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" +) + +type pageQueries interface { + QueryRow(context.Context, string, ...any) pgx.Row + Exec(context.Context, string, ...any) (pgconn.CommandTag, error) +} + +// This lock covers all CMS page mutation paths. It is cooperative, not a claim +// that arbitrary external SQL writers participate or that bundle swaps are atomic. +func (s *Store) pageTransaction(ctx context.Context, tenantID int64) (pgx.Tx, error) { + if tenantID <= 0 { + return nil, store.ErrNotFound + } + tx, err := s.pool.BeginTx(ctx, pgx.TxOptions{}) + if err != nil { + return nil, err + } + if _, err = tx.Exec(ctx, `SELECT pg_advisory_xact_lock(hashtextextended('cms.pages:' || $1::bigint::text, 0))`, tenantID); err != nil { + _ = tx.Rollback(ctx) + return nil, err + } + return tx, nil +} + +func pageMiss(ctx context.Context, q pageQueries, tenantID, id int64) error { + var exists bool + if err := q.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM pages WHERE tenant_id=$1 AND id=$2)`, tenantID, id).Scan(&exists); err != nil { + return err + } + if exists { + return store.ErrVersionConflict + } + return store.ErrNotFound +} + +func statesInTransaction(ctx context.Context, tx pgx.Tx, tenantID int64) ([]store.PageState, error) { + rows, err := tx.Query(ctx, pageSelectSQL()+` WHERE tenant_id=$1 ORDER BY id FOR UPDATE`, tenantID) + if err != nil { + return nil, err + } + defer rows.Close() + var pages []*store.Page + for rows.Next() { + p := &store.Page{} + if err := scanPage(rows, p); err != nil { + return nil, err + } + pages = append(pages, p) + } + if err := rows.Err(); err != nil { + return nil, err + } + return store.States(pages), nil +} + +func (s *Store) ApplyPageBatch(ctx context.Context, tenantID int64, batch store.PageBatch) (store.PageBatchReceipt, error) { + return s.applyBatch(ctx, tenantID, batch, false) +} + +func (s *Store) RollbackPageBatch(ctx context.Context, tenantID int64, receipt store.PageBatchReceipt) (store.PageBatchReceipt, error) { + batch, err := store.RollbackBatch(receipt) + if err != nil { + return store.PageBatchReceipt{}, err + } + return s.applyBatch(ctx, tenantID, batch, true) +} + +func (s *Store) applyBatch(ctx context.Context, tenantID int64, batch store.PageBatch, restore bool) (store.PageBatchReceipt, error) { + tx, err := s.pageTransaction(ctx, tenantID) + if err != nil { + return store.PageBatchReceipt{}, err + } + defer tx.Rollback(ctx) + before, err := statesInTransaction(ctx, tx, tenantID) + if err != nil { + return store.PageBatchReceipt{}, store.ErrBatchInvalid + } + if err = store.CheckPageBatch(before, batch, restore); err != nil { + return store.PageBatchReceipt{}, err + } + result := store.PageBatchReceipt{Before: before, Mapping: map[string]int64{}} + // Vacate moved paths inside this transaction so swaps work despite immediate + // unique constraints. Validated final content replaces every temporary path. + for _, m := range batch.Mutations { + if m.Kind == "update" { + if _, err = tx.Exec(ctx, `UPDATE pages SET path='/__cms_batch_internal__/' || id::text WHERE tenant_id=$1 AND id=$2 AND version=$3`, tenantID, m.TargetID, m.ExpectedVersion); err != nil { + return store.PageBatchReceipt{}, store.ErrBatchInvalid + } + } + } + for _, m := range batch.Mutations { + if m.Kind == "delete" { + if err = deletePage(ctx, tx, tenantID, m.TargetID, m.ExpectedVersion); err != nil { + return store.PageBatchReceipt{}, batchError(err) + } + result.Mapping[m.Key] = m.TargetID + } + } + for _, m := range batch.Mutations { + if m.Kind == "delete" { + continue + } + p := m.Content.Page(tenantID, m.TargetID, m.ExpectedVersion) + switch m.Kind { + case "create": + err = createPage(ctx, tx, tenantID, p) + case "update": + err = updatePage(ctx, tx, tenantID, p) + case "restore": + err = scanPage(tx.QueryRow(ctx, `INSERT INTO pages (id,tenant_id,subsite,path,title,body_html,body_blocks,status,template_id,publish_at,unpublish_at,version) VALUES($1,$2,$3,$4,$5,$6,$7,$8,NULLIF($9,''),$10,$11,$12) RETURNING id,tenant_id,COALESCE(subsite,''),path,title,COALESCE(body_html,''),body_blocks,status,COALESCE(template_id,''),publish_at,unpublish_at,version,created_at,updated_at`, p.ID, tenantID, p.Subsite, p.Path, p.Title, p.BodyHTML, nullableJSON(p.BodyBlocks), p.Status, p.TemplateID, nullableTime(p.PublishAt), nullableTime(p.UnpublishAt), m.ExpectedVersion), p) + } + if err != nil { + return store.PageBatchReceipt{}, batchError(err) + } + result.Mapping[m.Key] = p.ID + } + result.After, err = statesInTransaction(ctx, tx, tenantID) + if err != nil { + return store.PageBatchReceipt{}, store.ErrBatchInvalid + } + if err = tx.Commit(ctx); err != nil { + return store.PageBatchReceipt{}, store.ErrBatchInvalid + } + result.Seal() + return result, nil +} + +func batchError(err error) error { + if errors.Is(err, store.ErrVersionConflict) || errors.Is(err, store.ErrPathConflict) || errors.Is(err, store.ErrNotFound) { + return err + } + return store.ErrBatchInvalid +} + +var _ store.PageBatchStore = (*Store)(nil) diff --git a/store/postgres/batch_integration_test.go b/store/postgres/batch_integration_test.go new file mode 100644 index 0000000..88d76f6 --- /dev/null +++ b/store/postgres/batch_integration_test.go @@ -0,0 +1,88 @@ +package postgres + +import ( + "context" + "fmt" + "os" + "reflect" + "testing" + "time" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" + "github.com/GoCodeAlone/workflow-plugin-cms/store/storetest" + "github.com/jackc/pgx/v5/pgxpool" +) + +// Uses a uniquely named schema and drops only that schema. Never uses the +// application's search_path/tables. Connection values are not printed. +func isolatedPostgres(t *testing.T) *Store { + t.Helper() + uri := os.Getenv("CMS_TEST_DATABASE_URL") + if uri == "" { + t.Skip("CMS_TEST_DATABASE_URL not configured") + } + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + t.Cleanup(cancel) + admin, err := pgxpool.New(ctx, uri) + if err != nil { + t.Fatal("test database configuration invalid") + } + t.Cleanup(admin.Close) + schema := fmt.Sprintf("promotion_20261008_%d", time.Now().UnixNano()) + if _, err = admin.Exec(ctx, `CREATE SCHEMA `+schema); err != nil { + t.Fatal("could not create isolated test schema") + } + t.Cleanup(func() { + if _, err := admin.Exec(context.Background(), `DROP SCHEMA `+schema+` CASCADE`); err != nil { + t.Error("isolated test schema cleanup failed") + } + }) + config, err := pgxpool.ParseConfig(uri) + if err != nil { + t.Fatal("test configuration invalid") + } + config.ConnConfig.RuntimeParams["search_path"] = schema + config.MaxConns = 4 + pool, err := pgxpool.NewWithConfig(ctx, config) + if err != nil { + t.Fatal("test pool initialization failed") + } + t.Cleanup(pool.Close) + _, err = pool.Exec(ctx, `CREATE TABLE tenants(id BIGSERIAL PRIMARY KEY,slug TEXT UNIQUE NOT NULL,label TEXT NOT NULL DEFAULT '',theme_id BIGINT,created_at TIMESTAMPTZ NOT NULL DEFAULT now(),updated_at TIMESTAMPTZ NOT NULL DEFAULT now()); CREATE TABLE pages(id BIGSERIAL PRIMARY KEY,tenant_id BIGINT NOT NULL REFERENCES tenants(id),subsite TEXT NOT NULL DEFAULT '',path TEXT NOT NULL,title TEXT NOT NULL,body_html TEXT,body_blocks JSONB,status TEXT NOT NULL CHECK(status IN ('draft','published','scheduled','archived')),template_id TEXT,publish_at TIMESTAMPTZ,unpublish_at TIMESTAMPTZ,version INT NOT NULL DEFAULT 1,created_at TIMESTAMPTZ NOT NULL DEFAULT now(),updated_at TIMESTAMPTZ NOT NULL DEFAULT now(),UNIQUE(tenant_id,subsite,path));`) + if err != nil { + t.Fatal("isolated test tables failed") + } + return NewWithPool(pool) +} + +func TestPostgresPageBatchContract(t *testing.T) { + s := isolatedPostgres(t) + ctx := context.Background() + pool := s.pool + target, source := &store.Tenant{Slug: "test-target"}, &store.Tenant{Slug: "test-review"} + if s.CreateTenant(ctx, target) != nil || s.CreateTenant(ctx, source) != nil { + t.Fatal("isolated tenants failed") + } + storetest.PageBatches(t, s, target.ID, source.ID) + // A database-side failure after an earlier update must roll back every item. + _, err := pool.Exec(ctx, `CREATE FUNCTION refuse_test_insert() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN IF NEW.title='DB refusal test' THEN RAISE EXCEPTION 'test refusal' USING ERRCODE='23514'; END IF; RETURN NEW; END $$; CREATE TRIGGER refuse_test_insert BEFORE INSERT ON pages FOR EACH ROW EXECUTE FUNCTION refuse_test_insert();`) + if err != nil { + t.Fatal("isolated failure trigger failed") + } + all, err := s.List(ctx, target.ID, "") + if err != nil || len(all) == 0 { + t.Fatal("failure fixture unavailable") + } + before := store.Baseline(store.States(all)) + changed := store.ContentOf(all[0]) + changed.Title = "Must roll back" + refused := store.PageContent{Path: "/database-rejected", Title: "DB refusal test", Status: store.StatusDraft} + batch := store.PageBatch{Baseline: before, Mutations: []store.PageMutation{{Key: "updated-first", Kind: "update", TargetID: all[0].ID, ExpectedVersion: all[0].Version, Content: &changed}, {Key: "refused-second", Kind: "create", Content: &refused}}} + if _, err := s.ApplyPageBatch(ctx, target.ID, batch); err != store.ErrBatchInvalid { + t.Fatal("database failure was not safely reported") + } + all, err = s.List(ctx, target.ID, "") + if err != nil || !reflect.DeepEqual(before, store.Baseline(store.States(all))) { + t.Fatal("database failure partially committed content") + } +} diff --git a/store/postgres/editor_browser_test.go b/store/postgres/editor_browser_test.go new file mode 100644 index 0000000..0f06973 --- /dev/null +++ b/store/postgres/editor_browser_test.go @@ -0,0 +1,81 @@ +package postgres + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "strconv" + "testing" + "time" + + "github.com/GoCodeAlone/workflow-plugin-cms/adminui" + cms "github.com/GoCodeAlone/workflow-plugin-cms/internal" + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +// Optional actual browser proof using already installed Playwright/Chrome. This +// fixture has a fresh PG schema and a task-owned ephemeral loopback listener; +// its authority and promotion helper exist only inside this test binary. +func TestPostgresEditorVersionBrowser(t *testing.T) { + if os.Getenv("CMS_TEST_PLAYWRIGHT_PATH") == "" || os.Getenv("CMS_TEST_BROWSER_EXECUTABLE") == "" { + t.Skip("existing browser tools not configured") + } + s := isolatedPostgres(t) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + tenant := &store.Tenant{Slug: "browser-review", Label: "Isolated browser review"} + if s.CreateTenant(ctx, tenant) != nil { + t.Fatal("browser tenant fixture failed") + } + p := &store.Page{Path: "/browser-home", Title: "Browser initial", BodyHTML: "

Initial browser content

", Status: store.StatusDraft} + if s.Create(ctx, tenant.ID, p) != nil { + t.Fatal("browser page fixture failed") + } + api := cms.NewAdminAPI(s, s) + api.RequestAccess = func(*http.Request) bool { return true } + api.TenantAccess = func(_ *http.Request, id int64) bool { return id == tenant.ID } + api.ListTemplates = func(context.Context, int64) ([]string, error) { return []string{}, nil } + mux := http.NewServeMux() + mux.Handle("/api/v1/admin/", api) + mux.Handle("/admin/", http.StripPrefix("/admin", adminui.Handler())) + mux.HandleFunc("/fixture/promote", func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + w.WriteHeader(405) + return + } + all, err := s.List(r.Context(), tenant.ID, "") + if err != nil { + w.WriteHeader(500) + return + } + current, err := s.Get(r.Context(), tenant.ID, p.ID) + if err != nil { + w.WriteHeader(500) + return + } + content := store.ContentOf(current) + content.Title = "Approved fixture promotion" + _, err = s.ApplyPageBatch(r.Context(), tenant.ID, store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "approved-fixture", Kind: "update", TargetID: p.ID, ExpectedVersion: current.Version, Content: &content}}}) + if err != nil { + w.WriteHeader(500) + return + } + _ = json.NewEncoder(w).Encode(map[string]bool{"promoted": true}) + }) + server := httptest.NewServer(mux) + defer server.Close() + command := exec.CommandContext(ctx, "node", "testdata/editor-version.cjs") + command.Env = append(os.Environ(), "CMS_TEST_EDITOR_ORIGIN="+server.URL, "CMS_TEST_TENANT_ID="+strconv.FormatInt(tenant.ID, 10), "CMS_TEST_PAGE_ID="+strconv.FormatInt(p.ID, 10)) + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("isolated editor browser proof failed: %s", output) + } + t.Log(string(output)) + got, err := s.Get(ctx, tenant.ID, p.ID) + if err != nil || got.Title != "Fresh reviewed edit" || got.Version != 5 { + t.Fatal("browser save did not persist across actual API/DB reload") + } +} diff --git a/store/postgres/store.go b/store/postgres/store.go index af37dca..759444b 100644 --- a/store/postgres/store.go +++ b/store/postgres/store.go @@ -235,8 +235,21 @@ func (s *Store) ListDomains(ctx context.Context, tenantID int64) ([]*store.Domai return out, rows.Err() } -// Create inserts a page. +// Create serializes with content batches in this tenant. func (s *Store) Create(ctx context.Context, tenantID int64, p *store.Page) error { + tx, err := s.pageTransaction(ctx, tenantID) + if err != nil { + return err + } + defer tx.Rollback(ctx) + if err = createPage(ctx, tx, tenantID, p); err != nil { + return err + } + return tx.Commit(ctx) +} + +// Create inserts a page. +func createPage(ctx context.Context, q pageQueries, tenantID int64, p *store.Page) error { if p == nil { return errors.New("page: nil") } @@ -245,7 +258,7 @@ func (s *Store) Create(ctx context.Context, tenantID int64, p *store.Page) error if err := p.Validate(); err != nil { return err } - row := s.pool.QueryRow(ctx, ` + row := q.QueryRow(ctx, ` INSERT INTO pages (tenant_id, subsite, path, title, body_html, body_blocks, status, template_id, publish_at, unpublish_at) VALUES ($1, $2, $3, $4, $5, $6, $7, NULLIF($8, ''), $9, $10) RETURNING id, tenant_id, COALESCE(subsite, ''), path, title, COALESCE(body_html, ''), @@ -292,28 +305,44 @@ func (s *Store) GetByPath(ctx context.Context, tenantID int64, subsite, path str return p, nil } -// Update writes a tenant-scoped page. +// Update uses the caller's loaded version, never a newly fetched version. func (s *Store) Update(ctx context.Context, tenantID int64, p *store.Page) error { + tx, err := s.pageTransaction(ctx, tenantID) + if err != nil { + return err + } + defer tx.Rollback(ctx) + if err = updatePage(ctx, tx, tenantID, p); err != nil { + return err + } + return tx.Commit(ctx) +} + +// Update writes a tenant-scoped page. +func updatePage(ctx context.Context, q pageQueries, tenantID int64, p *store.Page) error { if p == nil { return errors.New("page: nil") } + if p.Version <= 0 { + return store.ErrVersionConflict + } p.TenantID = tenantID p.Subsite = normalizeSubsite(p.Subsite) if err := p.Validate(); err != nil { return err } - err := scanPage(s.pool.QueryRow(ctx, ` + err := scanPage(q.QueryRow(ctx, ` UPDATE pages SET subsite = $3, path = $4, title = $5, body_html = $6, body_blocks = $7, status = $8, template_id = NULLIF($9, ''), publish_at = $10, unpublish_at = $11, version = version + 1, updated_at = now() - WHERE tenant_id = $1 AND id = $2 + WHERE tenant_id = $1 AND id = $2 AND version = $12 RETURNING id, tenant_id, COALESCE(subsite, ''), path, title, COALESCE(body_html, ''), body_blocks, status, COALESCE(template_id, ''), publish_at, unpublish_at, version, created_at, updated_at - `, tenantID, p.ID, p.Subsite, p.Path, p.Title, p.BodyHTML, nullableJSON(p.BodyBlocks), p.Status, p.TemplateID, nullableTime(p.PublishAt), nullableTime(p.UnpublishAt)), p) + `, tenantID, p.ID, p.Subsite, p.Path, p.Title, p.BodyHTML, nullableJSON(p.BodyBlocks), p.Status, p.TemplateID, nullableTime(p.PublishAt), nullableTime(p.UnpublishAt), p.Version), p) if errors.Is(err, pgx.ErrNoRows) { - return store.ErrNotFound + return pageMiss(ctx, q, tenantID, p.ID) } if isPGCode(err, pgUniqueViolation) { return store.ErrPathConflict @@ -321,14 +350,29 @@ func (s *Store) Update(ctx context.Context, tenantID int64, p *store.Page) error return err } -// Delete removes a tenant-scoped page. -func (s *Store) Delete(ctx context.Context, tenantID int64, id int64) error { - tag, err := s.pool.Exec(ctx, `DELETE FROM pages WHERE tenant_id = $1 AND id = $2`, tenantID, id) +// Delete requires the caller's loaded version and serializes with batches. +func (s *Store) Delete(ctx context.Context, tenantID int64, id int64, expectedVersion int) error { + tx, err := s.pageTransaction(ctx, tenantID) + if err != nil { + return err + } + defer tx.Rollback(ctx) + if err = deletePage(ctx, tx, tenantID, id, expectedVersion); err != nil { + return err + } + return tx.Commit(ctx) +} + +func deletePage(ctx context.Context, q pageQueries, tenantID, id int64, expectedVersion int) error { + if expectedVersion <= 0 { + return store.ErrVersionConflict + } + tag, err := q.Exec(ctx, `DELETE FROM pages WHERE tenant_id=$1 AND id=$2 AND version=$3`, tenantID, id, expectedVersion) if err != nil { return err } if tag.RowsAffected() == 0 { - return store.ErrNotFound + return pageMiss(ctx, q, tenantID, id) } return nil } diff --git a/store/postgres/testdata/editor-version.cjs b/store/postgres/testdata/editor-version.cjs new file mode 100644 index 0000000..5960da6 --- /dev/null +++ b/store/postgres/testdata/editor-version.cjs @@ -0,0 +1,63 @@ +const assert = require("node:assert/strict"); +const {chromium} = require(process.env.CMS_TEST_PLAYWRIGHT_PATH); + +(async () => { + const origin = process.env.CMS_TEST_EDITOR_ORIGIN; + const tid = process.env.CMS_TEST_TENANT_ID; + const pid = process.env.CMS_TEST_PAGE_ID; + const api = `/api/v1/admin/tenants/${tid}/pages/${pid}`; + const browser = await chromium.launch({ + executablePath: process.env.CMS_TEST_BROWSER_EXECUTABLE, + headless: true, + }); + try { + const context = await browser.newContext({viewport:{width:1100,height:820}}); + const page = await context.newPage(); + page.setDefaultTimeout(10000); + page.on("dialog", dialog => dialog.accept()); + const versions = []; + page.on("request", request => { + if (request.url()===origin+api && ["PUT","DELETE"].includes(request.method())) { + versions.push({method:request.method(),version:request.postDataJSON().expected_version}); + } + }); + await page.goto(`${origin}/admin/tenants/${tid}/pages/${pid}/publishing`); + const title = page.locator('input[name="title"][data-page-field]'); + await title.waitFor({state:"visible"}); + assert.equal(await title.inputValue(),"Browser initial"); + const save = async (value,status) => { + await title.fill(value); + const response = page.waitForResponse(r=>r.url()===origin+api&&r.request().method()==="PUT"); + await page.locator("#btn-save-page").click(); + assert.equal((await response).status(),status); + await page.waitForFunction(()=>!document.querySelector("#btn-save-page").disabled); + }; + await save("First browser save",200); + await save("Second browser save",200); + assert.equal((await page.request.post(origin+"/fixture/promote")).status(),200); + await save("Unsaved browser draft",409); + assert.equal(await title.inputValue(),"Unsaved browser draft"); + assert.match(await page.locator("#save-state").innerText(),/Unsaved/i); + assert.match(await page.locator("#toast").innerText(),/reload/i); + const persisted = await (await page.request.get(origin+api)).json(); + assert.equal(persisted.Title,"Approved fixture promotion"); + assert.equal(persisted.Version,4); + if (process.env.CMS_TEST_SCREENSHOT) await page.screenshot({path:process.env.CMS_TEST_SCREENSHOT,fullPage:true}); + const deletion=page.waitForResponse(r=>r.url()===origin+api&&r.request().method()==="DELETE"); + await page.locator("#btn-delete-page").click(); + assert.equal((await deletion).status(),409); + assert.equal((await page.request.get(origin+api)).status(),200); + await page.reload(); + await title.waitFor({state:"visible"}); + assert.equal(await title.inputValue(),"Approved fixture promotion"); + await save("Fresh reviewed edit",200); + await page.reload(); + await title.waitFor({state:"visible"}); + assert.equal(await title.inputValue(),"Fresh reviewed edit"); + assert.deepEqual(versions,[{method:"PUT",version:1},{method:"PUT",version:2},{method:"PUT",version:3},{method:"DELETE",version:3},{method:"PUT",version:4}]); + assert.equal((await page.request.get(origin+`/api/v1/admin/tenants/${Number(tid)+1}/pages/${pid}`)).status(),403); + console.log(JSON.stringify({browser:"Chrome",store:"isolated Postgres",saveDeleteExpectedVersions:versions,staleSave:409,staleDelete:409,draftRetained:true,reloadPersistence:true,wrongTenant:403})); + } finally { + await browser.close(); + } +})().catch(error => {console.error(error);process.exitCode=1}); diff --git a/store/storetest/batch.go b/store/storetest/batch.go new file mode 100644 index 0000000..ea207e8 --- /dev/null +++ b/store/storetest/batch.go @@ -0,0 +1,190 @@ +// Package storetest provides the same mutation behavior contract for memory and +// real Postgres tests. It writes only the fresh tenant IDs supplied by the test. +package storetest + +import ( + "context" + "errors" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +type BatchStore interface { + store.PageStore + store.PageBatchStore +} + +func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) { + t.Helper() + ctx := context.Background() + create := func(tenant int64, path, title string) *store.Page { + p := &store.Page{Path: path, Title: title, BodyHTML: "

" + title + "

", Status: store.StatusDraft} + if err := s.Create(ctx, tenant, p); err != nil { + t.Fatal(err) + } + return p + } + home := create(targetTenant, "/", "Old home") + keep := create(targetTenant, "/keep", "Keep") + removed := create(targetTenant, "/remove", "Remove") + review := create(sourceTenant, "/", "Approved home") + if review.ID == home.ID { + t.Fatal("source and target must differ") + } + all, err := s.List(ctx, targetTenant, "") + if err != nil { + t.Fatal(err) + } + baseline := store.Baseline(store.States(all)) + content := store.ContentOf(review) + news := store.PageContent{Path: "/news", Title: "New page", BodyHTML: "

News

", Status: store.StatusPublished} + batch := store.PageBatch{Baseline: baseline, Mutations: []store.PageMutation{{Key: "review:home", Kind: "update", TargetID: home.ID, ExpectedVersion: home.Version, Content: &content}, {Key: "review:news", Kind: "create", Content: &news}, {Key: "explicit:remove", Kind: "delete", TargetID: removed.ID, ExpectedVersion: removed.Version}}} + bad := batch + bad.Mutations = append([]store.PageMutation(nil), batch.Mutations...) + invalid := news + invalid.Title = "" + bad.Mutations[1].Content = &invalid + if _, err := s.ApplyPageBatch(ctx, targetTenant, bad); !errors.Is(err, store.ErrBatchInvalid) { + t.Fatalf("invalid item must reject batch: %v", err) + } + unchanged, _ := s.Get(ctx, targetTenant, home.ID) + if unchanged.Version != home.Version || unchanged.Title != home.Title { + t.Fatal("invalid batch partially mutated") + } + if _, err := s.ApplyPageBatch(ctx, sourceTenant, batch); !errors.Is(err, store.ErrVersionConflict) { + t.Fatalf("wrong target accepted: %v", err) + } + receipt, err := s.ApplyPageBatch(ctx, targetTenant, batch) + if err != nil { + t.Fatal(err) + } + got, err := s.Get(ctx, targetTenant, home.ID) + if err != nil || got.Title != review.Title || got.Version != 2 { + t.Fatalf("promoted content did not reload: %v", err) + } + if receipt.Mapping["review:home"] != home.ID || receipt.Mapping["review:news"] == review.ID { + t.Fatal("mapping copied a source identity") + } + kept, _ := s.Get(ctx, targetTenant, keep.ID) + if kept.Version != keep.Version || kept.Title != keep.Title { + t.Fatal("omitted page changed") + } + if err := s.Update(ctx, targetTenant, home); !errors.Is(err, store.ErrVersionConflict) { + t.Fatalf("stale editor overwrote promotion: %v", err) + } + if err := s.Delete(ctx, targetTenant, home.ID, home.Version); !errors.Is(err, store.ErrVersionConflict) { + t.Fatalf("stale delete accepted: %v", err) + } + if err := s.Delete(ctx, sourceTenant, home.ID, got.Version); !errors.Is(err, store.ErrNotFound) { + t.Fatal("cross tenant delete must not leak") + } + if _, err := s.RollbackPageBatch(ctx, targetTenant, receipt); err != nil { + t.Fatal(err) + } + rolled, _ := s.Get(ctx, targetTenant, home.ID) + if rolled.Title != home.Title || rolled.Version <= got.Version { + t.Fatal("rollback did not restore content with monotonic version") + } + restored, _ := s.Get(ctx, targetTenant, removed.ID) + if restored == nil || restored.Version <= removed.Version { + t.Fatal("deleted page not restored with new version") + } + if _, err := s.Get(ctx, targetTenant, receipt.Mapping["review:news"]); !errors.Is(err, store.ErrNotFound) { + t.Fatal("created page survived rollback") + } + all, _ = s.List(ctx, targetTenant, "") + batch.Baseline = store.Baseline(store.States(all)) + batch.Mutations[0].ExpectedVersion = rolled.Version + batch.Mutations[2].ExpectedVersion = restored.Version + receipt, err = s.ApplyPageBatch(ctx, targetTenant, batch) + if err != nil { + t.Fatal(err) + } + kept, _ = s.Get(ctx, targetTenant, keep.ID) + kept.Title = "Later editorial change" + if err := s.Update(ctx, targetTenant, kept); err != nil { + t.Fatal(err) + } + if _, err := s.RollbackPageBatch(ctx, targetTenant, receipt); !errors.Is(err, store.ErrVersionConflict) { + t.Fatalf("rollback overwrote later edit: %v", err) + } + if _, err := s.ApplyPageBatch(ctx, targetTenant, batch); !errors.Is(err, store.ErrVersionConflict) { + t.Fatalf("baseline drift accepted: %v", err) + } + // Simultaneous save/promotion: only one may use the same loaded version. + fresh, _ := s.Get(ctx, targetTenant, home.ID) + all, _ = s.List(ctx, targetTenant, "") + next := store.ContentOf(fresh) + next.Title = "Promotion race winner" + raceBatch := store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "race", Kind: "update", TargetID: fresh.ID, ExpectedVersion: fresh.Version, Content: &next}}} + fresh.Title = "Editor race winner" + start := make(chan struct{}) + results := make(chan error, 2) + go func() { <-start; results <- s.Update(ctx, targetTenant, fresh) }() + go func() { <-start; _, err := s.ApplyPageBatch(ctx, targetTenant, raceBatch); results <- err }() + close(start) + wins := 0 + conflicts := 0 + for i := 0; i < 2; i++ { + err := <-results + if err == nil { + wins++ + } else if errors.Is(err, store.ErrVersionConflict) { + conflicts++ + } else { + t.Fatal(err) + } + } + if wins != 1 || conflicts != 1 { + t.Fatal("racing save and promotion did not conflict") + } + // A delete loaded before promotion also competes on the same version. + fresh, _ = s.Get(ctx, targetTenant, home.ID) + all, _ = s.List(ctx, targetTenant, "") + next = store.ContentOf(fresh) + next.Title = "Delete race promotion" + raceBatch = store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "delete-race", Kind: "update", TargetID: fresh.ID, ExpectedVersion: fresh.Version, Content: &next}}} + start = make(chan struct{}) + go func() { <-start; results <- s.Delete(ctx, targetTenant, fresh.ID, fresh.Version) }() + go func() { <-start; _, err := s.ApplyPageBatch(ctx, targetTenant, raceBatch); results <- err }() + close(start) + wins, conflicts = 0, 0 + for i := 0; i < 2; i++ { + err := <-results + if err == nil { + wins++ + } else if errors.Is(err, store.ErrVersionConflict) { + conflicts++ + } else { + t.Fatal(err) + } + } + if wins != 1 || conflicts != 1 { + t.Fatal("racing delete and promotion did not conflict") + } + // Concurrent creates of one path are serialized too. + start = make(chan struct{}) + for i := 0; i < 2; i++ { + go func() { + <-start + p := &store.Page{Path: "/race-create", Title: "Race", Status: store.StatusDraft} + results <- s.Create(ctx, targetTenant, p) + }() + } + close(start) + wins, conflicts = 0, 0 + for i := 0; i < 2; i++ { + err := <-results + if err == nil { + wins++ + } else if errors.Is(err, store.ErrPathConflict) { + conflicts++ + } else { + t.Fatal(err) + } + } + if wins != 1 || conflicts != 1 { + t.Fatal("duplicate racing create accepted") + } +} From afc9dc9b3a69ee97cf68585fe1e48979f5373220 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 8 Oct 2026 15:12:43 -0400 Subject: [PATCH 02/10] fix: bind rollback to durable revisions and validate reference semantics --- .../2026-10-08-content-promotion-amendment.md | 19 ++++ docs/promotion.md | 13 ++- docs/qa/content-promotion.md | 7 +- internal/page_concurrency_test.go | 4 +- internal/page_document.go | 4 + promotion/bundle.go | 61 ++++++----- promotion/plan.go | 30 +++--- promotion/snapshot_test.go | 38 ++++++- store/batch.go | 39 +++++-- store/batch_memory.go | 16 ++- store/page.go | 24 ++++- store/postgres/batch.go | 78 +++++++++++++- store/postgres/batch_integration_test.go | 63 ++++++++++- store/postgres/editor_browser_test.go | 4 +- .../0001_page_content_revision.up.sql | 18 ++++ store/postgres/store.go | 9 ++ store/storetest/batch.go | 101 ++++++++++++++++-- 17 files changed, 447 insertions(+), 81 deletions(-) create mode 100644 store/postgres/migrations/0001_page_content_revision.up.sql diff --git a/docs/plans/2026-10-08-content-promotion-amendment.md b/docs/plans/2026-10-08-content-promotion-amendment.md index 2399b37..4130ad8 100644 --- a/docs/plans/2026-10-08-content-promotion-amendment.md +++ b/docs/plans/2026-10-08-content-promotion-amendment.md @@ -59,3 +59,22 @@ Use the existing module/build caches, `GOWORK=off`, package-targeted tests with new cache, full local race build, existing-schema mutation or process restart. CI may run its existing full checks plus an isolated Postgres service. Record exact-head checks and obtain independent source review before merge. + +## Independent-review correction: durable mutation generation + +Review of `f7fd874` found a delete/restore/edit/delete ABA hole: surviving page +rows alone cannot detect a changed-then-empty tenant. This additive correction +introduces a durable `cms_page_revisions` ledger with a unique content-scope +identifier and monotonic tenant revision. Every successful ordinary page write, +batch and rollback advances it under the same lock. Atomic `ReadPageState` returns +scope/revision plus rows; plans/receipts bind them and refuse replay or scope drift. +The scope is a content namespace identifier, not an authority grant or credential. + +The explicit SQL migration is a **new required host schema prerequisite**. It +initializes existing tenants and new-tenant ledger rows. No automatic migration, +existing-schema mutation or consumer adoption is authorized by this source PR. +Before adoption, the host must review/apply the migration and fence writes while +replacing every older writer: mixed old/new plugin versions would bypass the +ledger. A new store instance against the same isolated database must retain the +rollback/replay guard. Canonical block links, absolute reference exclusions and +collision-free temporary paths receive regression coverage in the same correction. diff --git a/docs/promotion.md b/docs/promotion.md index fc5f6dd..69afaf2 100644 --- a/docs/promotion.md +++ b/docs/promotion.md @@ -13,7 +13,7 @@ includes source page ID/version keys, content and SHA-256 file inventory. Strict `promotion.Decode` refuses unknown/duplicate JSON keys, trailing documents and oversize input using fixed errors. It never imports tenant settings or authority. -Read the production tenant's pages and inventory its immutable bundle. Review +Read the production tenant atomically using `ReadPageState` and inventory its immutable bundle. The returned durable content scope/revision is part of the batch baseline. Review `promotion.SuggestedMapping` path matches, then supply an explicit mapping for every selected page to `promotion.DryRun`. Target page IDs are distinct from source IDs. A zero target ID explicitly creates; moves require `AllowMove`; deletion @@ -28,7 +28,9 @@ Update requires the caller's loaded `Page.Version`. API PUT and DELETE require `expected_version`; missing preconditions fail and stale versions return 409. The editor keeps an unsaved draft on conflict and refreshes its version after a successful save. Existing external API consumers must upgrade before this plugin -is adopted. `PageStore.Delete` also gains a required expected-version argument. +is adopted. Every successful page write also advances the durable tenant content +revision; receipts bind that revision and the content scope, preventing receipt +replay even after create/delete history leaves the same surviving rows. `PageStore.Delete` also gains a required expected-version argument. Archive the content-only batch receipt before completing host publication. Its before/after rows, explicit mapping and integrity digest support @@ -44,6 +46,13 @@ references and exact hashes. The operator must stage those verified immutable bytes in the target bundle. The library does not configure media storage, copy uploaded objects, fetch third-party embeds or claim rights to external media. +The SQL prerequisite `store/postgres/migrations/0001_page_content_revision.up.sql` +must be reviewed and applied by the host before adoption. It initializes tenant +content scopes/revisions and new-tenant initialization. This library never applies +it automatically. Fence writes while replacing all older writers; mixed versions +bypass the ledger. Preserve ledger history across restart/backup, and never drop +or reset it to force a conflicted rollback. + Live application remains blocked on host integration: current superadmin publication authority, a named approval digest, an exclusive publication/write fence, staged immutable assets, durable content/bundle backups, approved production diff --git a/docs/qa/content-promotion.md b/docs/qa/content-promotion.md index e8ef5cd..76c6ca1 100644 --- a/docs/qa/content-promotion.md +++ b/docs/qa/content-promotion.md @@ -19,6 +19,9 @@ each schema is cleaned by its test. No runtime tables, processes or ports change | Batch → real Postgres → reload | Update/create/delete persist atomically; mapping names target identities; complete page baseline checked | | Ordinary save/delete → batch | Stale loaded versions conflict; racing save/promotion and delete/promotion each admit one winner | | Database failure mid-batch | Isolated trigger refuses the second item; earlier update rolls back and complete baseline remains unchanged | +| Durable revision → rollback replay | Deleted/restore/edit/delete ABA and create/delete history refuse replay; fresh Postgres connections retain the guard | +| Path moves → Postgres | Occupied legacy staging paths do not block title-only updates; two-page path swap persists | +| Canonical blocks → references | Internal/relative/anchor/mail/HTTPS links export through the actual renderer; arbitrary slash text is preserved; absolute/encoded admin/API/media references refuse | | Receipt → rollback | Content restored with increasing versions; created page removed; deleted page restored; later edit refuses whole rollback | | Bundle → validator | Changed bytes, missing assets, static homepage shadow, symlink, admin reference and encoded tenant-upload references refused | | Actual Chrome editor → API → Postgres | Repeated saves send versions 1/2; fixture promotion changes version to 4; stale save/delete send 3 and return 409; unsaved draft retained; reload then version-4 save persists as version 5; wrong tenant returns 403 | @@ -30,7 +33,9 @@ The task evidence directory retains the browser conflict screenshot and test log ## Release limits -`PageStore.Delete` now requires expected version; external API clients must send +The explicit durable-revision SQL migration and a fenced rollout replacing every +older writer are mandatory new host schema prerequisites. No automatic migration +or host schema deployment occurred. `PageStore.Delete` now requires expected version; external API clients must send `expected_version` for PUT/DELETE. The CMS host must consume this change in a separate reviewed release. The historical PR31 worktree and current consumer pin are preserved. Independent review and exact committed-head CI are recorded diff --git a/internal/page_concurrency_test.go b/internal/page_concurrency_test.go index 66ab766..805d2e1 100644 --- a/internal/page_concurrency_test.go +++ b/internal/page_concurrency_test.go @@ -27,8 +27,8 @@ func TestAdminPageSaveDeleteClientPreconditions(t *testing.T) { } content := store.ContentOf(p) content.Title = "Approved promotion" - all, _ := pages.List(context.Background(), 1, "") - _, err := pages.ApplyPageBatch(context.Background(), 1, store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "approved", Kind: "update", TargetID: p.ID, ExpectedVersion: p.Version, Content: &content}}}) + state, _ := pages.ReadPageState(context.Background(), 1) + _, err := pages.ApplyPageBatch(context.Background(), 1, store.PageBatch{TargetScope: state.Scope, BaselineRevision: state.Revision, Baseline: store.Baseline(state.Pages), Mutations: []store.PageMutation{{Key: "approved", Kind: "update", TargetID: p.ID, ExpectedVersion: p.Version, Content: &content}}}) if err != nil { t.Fatal(err) } diff --git a/internal/page_document.go b/internal/page_document.go index d9b0284..8b0972f 100644 --- a/internal/page_document.go +++ b/internal/page_document.go @@ -72,6 +72,10 @@ func renderPageBody(page *store.Page) (string, error) { return page.BodyHTML, nil } +// RenderPageBody shares the actual canonical serve-time renderer with content +// preparation. Link validation follows rendered semantics, not arbitrary text. +func RenderPageBody(page *store.Page) (string, error) { return renderPageBody(page) } + type blockNode struct { Type string `json:"type"` Text string `json:"text"` diff --git a/promotion/bundle.go b/promotion/bundle.go index 136610a..fa0e06b 100644 --- a/promotion/bundle.go +++ b/promotion/bundle.go @@ -14,6 +14,7 @@ import ( "sort" "strings" + cms "github.com/GoCodeAlone/workflow-plugin-cms/internal" "github.com/GoCodeAlone/workflow-plugin-cms/store" "golang.org/x/net/html" ) @@ -147,6 +148,16 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if err != nil || u.User != nil { return ErrBundle } + if strings.ContainsAny(u.Path, "\\\x00") { + return ErrBundle + } + resolved := path.Clean(u.Path) + if u.Scheme == "" && !strings.HasPrefix(u.Path, "/") { + resolved = path.Join(path.Dir(base), u.Path) + } + if forbiddenPath(resolved) { + return ErrBundle + } if u.Scheme != "" { if link && (u.Scheme == "mailto" || u.Scheme == "tel") { return nil @@ -154,24 +165,11 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if u.Scheme != "https" || u.Host == "" || strings.Contains(strings.ToLower(u.Hostname()), "preview.") || strings.Contains(strings.ToLower(u.Hostname()), "-review.") { return ErrBundle } - if strings.HasPrefix(u.Path, "/media/") { - return ErrBundle - } return nil } if u.Host != "" { return ErrBundle } - if strings.Contains(u.Path, "\\") || strings.Contains(u.Path, "\x00") { - return ErrBundle - } - resolved := path.Clean(u.Path) - if !strings.HasPrefix(u.Path, "/") { - resolved = path.Join(path.Dir(base), u.Path) - } - if strings.HasPrefix(resolved, "/media/") || strings.HasPrefix(resolved, "/api/") || strings.HasPrefix(resolved, "/admin") || strings.HasPrefix(resolved, "/cms/") { - return ErrBundle - } if files[resolved] || link && routes[resolved] { return nil } @@ -240,6 +238,13 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten return visit(doc) } for _, p := range pages { + canonical, err := cms.RenderPageBody(p.Page(1, 1, 1)) + if err != nil { + return ErrBundle + } + if err = checkHTML(canonical, p.Path); err != nil { + return err + } if len(p.BodyBlocks) > 0 { var blocks any if json.Unmarshal(p.BodyBlocks, &blocks) != nil { @@ -248,16 +253,6 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten var checkBlock func(any) error checkBlock = func(value any) error { switch v := value.(type) { - case string: - if strings.Contains(v, "/media/") { - return ErrBundle - } - if strings.HasPrefix(v, "/") || strings.HasPrefix(v, "https:") { - return checkRef(v, p.Path, false) - } - if strings.Contains(v, "<") { - return checkHTML(v, p.Path) - } case []any: for _, item := range v { if err := checkBlock(item); err != nil { @@ -265,7 +260,16 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten } } case map[string]any: - for _, item := range v { + for key, item := range v { + if ref, ok := item.(string); ok { + switch key { + case "href", "src", "url", "poster": + if err := checkRef(ref, p.Path, key == "href"); err != nil { + return err + } + } + } + if err := checkBlock(item); err != nil { return err } @@ -312,3 +316,12 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten } return nil } + +func forbiddenPath(p string) bool { + for _, prefix := range []string{"/media", "/api", "/admin", "/cms"} { + if p == prefix || strings.HasPrefix(p, prefix+"/") { + return true + } + } + return false +} diff --git a/promotion/plan.go b/promotion/plan.go index 48433b1..8464493 100644 --- a/promotion/plan.go +++ b/promotion/plan.go @@ -36,16 +36,16 @@ func (p Plan) ContentDigest() string { p.Digest = ""; return sumJSON(p) } // DryRun requires an explicit mapping for every selected page. SuggestedMapping // offers reviewable path matches, but never silently approves a rename/deletion. // The caller must separately verify both on-disk bundles before publication. -func DryRun(s Snapshot, current []*store.Page, mappings []Mapping, deletes []DeleteSelection, targetBundle BundleManifest) (Plan, error) { +func DryRun(s Snapshot, current store.PageSet, mappings []Mapping, deletes []DeleteSelection, targetBundle BundleManifest) (Plan, error) { if err := s.Validate(); err != nil { return Plan{}, err } if targetBundle.Validate() != nil { return Plan{}, ErrBundle } - byID := map[int64]*store.Page{} - for _, p := range current { - if p == nil || p.ID <= 0 || byID[p.ID] != nil { + byID := map[int64]store.PageState{} + for _, p := range current.Pages { + if p.ID <= 0 || byID[p.ID].ID != 0 { return Plan{}, ErrMapping } byID[p.ID] = p @@ -64,7 +64,7 @@ func DryRun(s Snapshot, current []*store.Page, mappings []Mapping, deletes []Del if len(mappings) != len(s.Pages) { return Plan{}, ErrMapping } - plan := Plan{SnapshotDigest: s.Digest, BundleDigest: s.Bundle.Digest, BundleBaselineDigest: targetBundle.Digest, Batch: store.PageBatch{Baseline: store.Baseline(store.States(current))}, Diffs: []Diff{}} + plan := Plan{SnapshotDigest: s.Digest, BundleDigest: s.Bundle.Digest, BundleBaselineDigest: targetBundle.Digest, Batch: store.BatchFor(current, nil), Diffs: []Diff{}} for _, page := range s.Pages { m, ok := byKey[page.Key] if !ok { @@ -78,16 +78,16 @@ func DryRun(s Snapshot, current []*store.Page, mappings []Mapping, deletes []Del mutation.Kind = "create" } else { p := byID[m.TargetID] - if p == nil || targets[p.ID] { + if p.ID == 0 || targets[p.ID] { return Plan{}, ErrMapping } targets[p.ID] = true - if (p.Path != content.Path || p.Subsite != content.Subsite) && !m.AllowMove { + if (p.Content.Path != content.Path || p.Content.Subsite != content.Subsite) && !m.AllowMove { return Plan{}, ErrMapping } diff.Action = "update" - diff.OldPath = p.Path - diff.BeforeDigest = store.ContentOf(p).Digest() + diff.OldPath = p.Content.Path + diff.BeforeDigest = p.Content.Digest() mutation.Kind = "update" mutation.ExpectedVersion = p.Version if diff.BeforeDigest == diff.AfterDigest { @@ -101,15 +101,15 @@ func DryRun(s Snapshot, current []*store.Page, mappings []Mapping, deletes []Del } for _, d := range deletes { p := byID[d.TargetID] - if d.Key == "" || p == nil || targets[p.ID] { + if d.Key == "" || p.ID == 0 || targets[p.ID] { return Plan{}, ErrMapping } targets[p.ID] = true plan.Batch.Mutations = append(plan.Batch.Mutations, store.PageMutation{Key: d.Key, Kind: "delete", TargetID: p.ID, ExpectedVersion: p.Version}) - plan.Diffs = append(plan.Diffs, Diff{Key: d.Key, TargetID: p.ID, Action: "delete", OldPath: p.Path, BeforeDigest: store.ContentOf(p).Digest()}) + plan.Diffs = append(plan.Diffs, Diff{Key: d.Key, TargetID: p.ID, Action: "delete", OldPath: p.Content.Path, BeforeDigest: p.Content.Digest()}) } if len(plan.Batch.Mutations) > 0 { - if err := store.CheckPageBatch(store.States(current), plan.Batch, false); err != nil { + if err := store.CheckPageBatch(current, plan.Batch, false); err != nil { return Plan{}, err } } @@ -117,15 +117,15 @@ func DryRun(s Snapshot, current []*store.Page, mappings []Mapping, deletes []Del return plan, nil } -func SuggestedMapping(s Snapshot, current []*store.Page) ([]Mapping, error) { +func SuggestedMapping(s Snapshot, current store.PageSet) ([]Mapping, error) { if s.Validate() != nil { return nil, ErrInvalid } out := []Mapping{} for _, page := range s.Pages { m := Mapping{Key: page.Key} - for _, p := range current { - if p.Subsite == page.Content.Subsite && p.Path == page.Content.Path { + for _, p := range current.Pages { + if p.Content.Subsite == page.Content.Subsite && p.Content.Path == page.Content.Path { if m.TargetID != 0 { return nil, ErrMapping } diff --git a/promotion/snapshot_test.go b/promotion/snapshot_test.go index 84e1320..1d0b44a 100644 --- a/promotion/snapshot_test.go +++ b/promotion/snapshot_test.go @@ -68,7 +68,7 @@ func TestSnapshotFreezeExplicitMappingAndDryRun(t *testing.T) { if err := pages.Create(context.Background(), 20, keep); err != nil { t.Fatal(err) } - current, _ := pages.List(context.Background(), 20, "") + current, _ := pages.ReadPageState(context.Background(), 20) mappings, err := SuggestedMapping(s, current) if err != nil || mappings[0].TargetID != target.ID || target.ID == s.Pages[0].SourceID { t.Fatal("path suggestion/mapping failed") @@ -139,6 +139,13 @@ func TestBundleHashesReferencesStaticCollisionsAndSymlinks(t *testing.T) { t.Fatal("unsupported reference accepted") } } + for _, ref := range []string{"https://admin.gocodealone.tech/api/v1/admin/tenants/10/pages", "https://example.com/admin", "https://example.com/cms/templates/main.html", "https://example.com/%61pi/private", "https://example.com/assets/../api/private", "https://example.com/assets/%2e%2e/media/10/hash.jpg"} { + p := contents[0] + p.BodyHTML = `Private dependency` + if VerifyBundle(root, manifest, []store.PageContent{p}) != ErrBundle { + t.Fatal("absolute private path accepted") + } + } p := contents[0] p.BodyBlocks = json.RawMessage(`{"type":"doc","url":"\u002fmedia/10/hash.jpg"}`) if VerifyBundle(root, manifest, []store.PageContent{p}) != ErrBundle { @@ -161,3 +168,32 @@ func TestBundleHashesReferencesStaticCollisionsAndSymlinks(t *testing.T) { t.Fatal("bundle symlink accepted") } } + +func TestCanonicalBlockLinksUseActualRendererSemantics(t *testing.T) { + s, root, pages, home := fixture(t) + teaching := &store.Page{Path: "/teaching", Title: "Teaching", Status: store.StatusDraft, TemplateID: "main"} + if pages.Create(context.Background(), 10, teaching) != nil { + t.Fatal("teaching fixture failed") + } + links := []any{} + for _, href := range []string{"/teaching", "teaching", "#voice", "mailto:tina@example.com", "https://example.com/music"} { + links = append(links, map[string]any{"type": "link", "attrs": map[string]any{"href": href}, "content": []any{map[string]any{"type": "text", "text": "Navigation"}}}) + } + links = append(links, map[string]any{"type": "text", "text": "/this is ordinary text, not an asset"}) + home.BodyBlocks, _ = json.Marshal(map[string]any{"type": "doc", "content": links}) + if pages.Update(context.Background(), 10, home) != nil { + t.Fatal("blocks update failed") + } + if _, err := Export(context.Background(), pages, 10, "test-review", []int64{home.ID, teaching.ID}, root, s.CreatedAt); err != nil { + t.Fatalf("canonical link export refused: %v", err) + } + for _, href := range []string{"/media/10/hash.jpg", "https://admin.gocodealone.tech/api/private", "javascript:alert(1)"} { + home.BodyBlocks, _ = json.Marshal(map[string]any{"type": "doc", "content": []any{map[string]any{"type": "link", "attrs": map[string]any{"href": href}}}}) + if pages.Update(context.Background(), 10, home) != nil { + t.Fatal("unsafe fixture failed") + } + if _, err := Export(context.Background(), pages, 10, "test-review", []int64{home.ID, teaching.ID}, root, s.CreatedAt); err != ErrBundle { + t.Fatal("canonical private reference accepted") + } + } +} diff --git a/store/batch.go b/store/batch.go index 81420b4..5810f4c 100644 --- a/store/batch.go +++ b/store/batch.go @@ -125,14 +125,19 @@ type PageMutation struct { Content *PageContent `json:"content,omitempty"` } type PageBatch struct { - Baseline []PageBaseline `json:"baseline"` - Mutations []PageMutation `json:"mutations"` + TargetScope string `json:"target_scope"` + BaselineRevision int64 `json:"baseline_revision"` + Baseline []PageBaseline `json:"baseline"` + Mutations []PageMutation `json:"mutations"` } type PageBatchReceipt struct { - Before []PageState `json:"before"` - After []PageState `json:"after"` - Mapping map[string]int64 `json:"mapping"` - Digest string `json:"digest"` + TargetScope string `json:"target_scope"` + BeforeRevision int64 `json:"before_revision"` + AfterRevision int64 `json:"after_revision"` + Before []PageState `json:"before"` + After []PageState `json:"after"` + Mapping map[string]int64 `json:"mapping"` + Digest string `json:"digest"` } // Seal detects archive corruption. It is not a signature or authorization. @@ -146,15 +151,29 @@ func (r *PageBatchReceipt) Seal() { // PageBatchStore is internal persistence, not an authorized publication API. // The host must provide approval, fencing, verified bundles and durable backups. type PageBatchStore interface { + ReadPageState(context.Context, int64) (PageSet, error) ApplyPageBatch(context.Context, int64, PageBatch) (PageBatchReceipt, error) RollbackPageBatch(context.Context, int64, PageBatchReceipt) (PageBatchReceipt, error) } +// PageSet is read atomically in the invocation's authorized tenant. Scope is a +// durable content namespace, never a permission or copied tenant setting. +type PageSet struct { + Scope string `json:"scope"` + Revision int64 `json:"revision"` + Pages []PageState `json:"pages"` +} + +func BatchFor(state PageSet, mutations []PageMutation) PageBatch { + return PageBatch{TargetScope: state.Scope, BaselineRevision: state.Revision, Baseline: Baseline(state.Pages), Mutations: mutations} +} + // CheckPageBatch compares the entire tenant baseline and checks the final paths // before any mutation. Omitted pages remain untouched. restore is never accepted // by ordinary apply; it is generated only by the guarded rollback implementation. -func CheckPageBatch(current []PageState, batch PageBatch, allowRestore bool) error { - if !reflect.DeepEqual(Baseline(current), batch.Baseline) { +func CheckPageBatch(state PageSet, batch PageBatch, allowRestore bool) error { + current := state.Pages + if state.Scope == "" || state.Scope != batch.TargetScope || state.Revision != batch.BaselineRevision || !reflect.DeepEqual(Baseline(current), batch.Baseline) { return ErrVersionConflict } if len(batch.Mutations) == 0 { @@ -239,7 +258,7 @@ func CheckPageBatch(current []PageState, batch PageBatch, allowRestore bool) err func RollbackBatch(r PageBatchReceipt) (PageBatch, error) { copy := r copy.Seal() - if r.Digest == "" || r.Digest != copy.Digest { + if r.Digest == "" || r.Digest != copy.Digest || r.TargetScope == "" || r.BeforeRevision < 0 || r.AfterRevision <= r.BeforeRevision { return PageBatch{}, ErrBatchInvalid } before := map[int64]PageState{} @@ -262,7 +281,7 @@ func RollbackBatch(r PageBatchReceipt) (PageBatch, error) { } after[p.ID] = p } - b := PageBatch{Baseline: Baseline(r.After)} + b := PageBatch{TargetScope: r.TargetScope, BaselineRevision: r.AfterRevision, Baseline: Baseline(r.After)} for _, p := range r.After { old, ok := before[p.ID] if !ok { diff --git a/store/batch_memory.go b/store/batch_memory.go index 2ec590e..7486816 100644 --- a/store/batch_memory.go +++ b/store/batch_memory.go @@ -15,6 +15,15 @@ func (s *MemoryPageStore) statesLocked(tenantID int64) []PageState { return States(pages) } +func (s *MemoryPageStore) ReadPageState(_ context.Context, tenantID int64) (PageSet, error) { + if tenantID <= 0 { + return PageSet{}, ErrNotFound + } + s.mu.Lock() + defer s.mu.Unlock() + return PageSet{Scope: s.scopeLocked(tenantID), Revision: s.revisions[tenantID], Pages: s.statesLocked(tenantID)}, nil +} + func (s *MemoryPageStore) ApplyPageBatch(_ context.Context, tenantID int64, batch PageBatch) (PageBatchReceipt, error) { s.mu.Lock() defer s.mu.Unlock() @@ -36,10 +45,11 @@ func (s *MemoryPageStore) applyBatchLocked(tenantID int64, batch PageBatch, rest return PageBatchReceipt{}, ErrBatchInvalid } before := s.statesLocked(tenantID) - if err := CheckPageBatch(before, batch, restore); err != nil { + state := PageSet{Scope: s.scopeLocked(tenantID), Revision: s.revisions[tenantID], Pages: before} + if err := CheckPageBatch(state, batch, restore); err != nil { return PageBatchReceipt{}, err } - result := PageBatchReceipt{Before: before, Mapping: map[string]int64{}} + result := PageBatchReceipt{TargetScope: state.Scope, BeforeRevision: state.Revision, Before: before, Mapping: map[string]int64{}} now := time.Now().UTC() for _, m := range batch.Mutations { if m.Kind == "delete" { @@ -69,6 +79,8 @@ func (s *MemoryPageStore) applyBatchLocked(tenantID int64, batch PageBatch, rest result.Mapping[m.Key] = id } result.After = s.statesLocked(tenantID) + s.revisions[tenantID]++ + result.AfterRevision = s.revisions[tenantID] result.Seal() return result, nil } diff --git a/store/page.go b/store/page.go index 9ed3f4a..e3b0e3e 100644 --- a/store/page.go +++ b/store/page.go @@ -15,6 +15,7 @@ package store import ( "context" + "crypto/rand" "encoding/json" "errors" "fmt" @@ -112,14 +113,23 @@ type PageStore interface { // Production uses a postgres-backed implementation that wires the same // interface. type MemoryPageStore struct { - mu sync.RWMutex - nextID int64 - pages map[int64]*Page // by ID + mu sync.RWMutex + nextID int64 + pages map[int64]*Page // by ID + revisions map[int64]int64 + scopes map[int64]string } // NewMemoryPageStore returns an empty in-memory store. func NewMemoryPageStore() *MemoryPageStore { - return &MemoryPageStore{pages: map[int64]*Page{}} + return &MemoryPageStore{pages: map[int64]*Page{}, revisions: map[int64]int64{}, scopes: map[int64]string{}} +} + +func (s *MemoryPageStore) scopeLocked(tenantID int64) string { + if s.scopes[tenantID] == "" { + s.scopes[tenantID] = rand.Text() + } + return s.scopes[tenantID] } // Create inserts the page. Returns ErrPathConflict if (tenant, subsite, @@ -151,6 +161,8 @@ func (s *MemoryPageStore) Create(_ context.Context, tenantID int64, p *Page) err cp.BodyBlocks = append(json.RawMessage(nil), p.BodyBlocks...) cp.PublishAt, cp.UnpublishAt = cloneTime(p.PublishAt), cloneTime(p.UnpublishAt) s.pages[p.ID] = &cp + s.scopeLocked(tenantID) + s.revisions[tenantID]++ return nil } @@ -215,6 +227,8 @@ func (s *MemoryPageStore) Update(_ context.Context, tenantID int64, p *Page) err cp.BodyBlocks = append(json.RawMessage(nil), p.BodyBlocks...) cp.PublishAt, cp.UnpublishAt = cloneTime(p.PublishAt), cloneTime(p.UnpublishAt) s.pages[p.ID] = &cp + s.scopeLocked(tenantID) + s.revisions[tenantID]++ return nil } @@ -230,6 +244,8 @@ func (s *MemoryPageStore) Delete(_ context.Context, tenantID, id int64, expected return ErrVersionConflict } delete(s.pages, id) + s.scopeLocked(tenantID) + s.revisions[tenantID]++ return nil } diff --git a/store/postgres/batch.go b/store/postgres/batch.go index c02719e..05f4e83 100644 --- a/store/postgres/batch.go +++ b/store/postgres/batch.go @@ -2,7 +2,9 @@ package postgres import ( "context" + "crypto/rand" "errors" + "strconv" "github.com/GoCodeAlone/workflow-plugin-cms/store" "github.com/jackc/pgx/v5" @@ -62,6 +64,47 @@ func statesInTransaction(ctx context.Context, tx pgx.Tx, tenantID int64) ([]stor return store.States(pages), nil } +func pageStateInTransaction(ctx context.Context, tx pgx.Tx, tenantID int64) (store.PageSet, error) { + state := store.PageSet{} + err := tx.QueryRow(ctx, `SELECT content_scope,revision FROM cms_page_revisions WHERE tenant_id=$1 FOR UPDATE`, tenantID).Scan(&state.Scope, &state.Revision) + if errors.Is(err, pgx.ErrNoRows) { + return state, store.ErrNotFound + } + if err != nil { + return state, store.ErrBatchInvalid + } + state.Pages, err = statesInTransaction(ctx, tx, tenantID) + return state, err +} + +func advanceRevision(ctx context.Context, tx pgx.Tx, tenantID int64) (int64, error) { + var revision int64 + err := tx.QueryRow(ctx, `UPDATE cms_page_revisions SET revision=revision+1 WHERE tenant_id=$1 RETURNING revision`, tenantID).Scan(&revision) + if errors.Is(err, pgx.ErrNoRows) { + return 0, store.ErrNotFound + } + if err != nil { + return 0, store.ErrBatchInvalid + } + return revision, nil +} + +func (s *Store) ReadPageState(ctx context.Context, tenantID int64) (store.PageSet, error) { + tx, err := s.pageTransaction(ctx, tenantID) + if err != nil { + return store.PageSet{}, err + } + defer tx.Rollback(ctx) + state, err := pageStateInTransaction(ctx, tx, tenantID) + if err != nil { + return state, err + } + if err = tx.Commit(ctx); err != nil { + return store.PageSet{}, store.ErrBatchInvalid + } + return state, nil +} + func (s *Store) ApplyPageBatch(ctx context.Context, tenantID int64, batch store.PageBatch) (store.PageBatchReceipt, error) { return s.applyBatch(ctx, tenantID, batch, false) } @@ -80,19 +123,40 @@ func (s *Store) applyBatch(ctx context.Context, tenantID int64, batch store.Page return store.PageBatchReceipt{}, err } defer tx.Rollback(ctx) - before, err := statesInTransaction(ctx, tx, tenantID) + state, err := pageStateInTransaction(ctx, tx, tenantID) if err != nil { return store.PageBatchReceipt{}, store.ErrBatchInvalid } - if err = store.CheckPageBatch(before, batch, restore); err != nil { + if err = store.CheckPageBatch(state, batch, restore); err != nil { return store.PageBatchReceipt{}, err } - result := store.PageBatchReceipt{Before: before, Mapping: map[string]int64{}} + result := store.PageBatchReceipt{TargetScope: state.Scope, BeforeRevision: state.Revision, Before: state.Pages, Mapping: map[string]int64{}} // Vacate moved paths inside this transaction so swaps work despite immediate // unique constraints. Validated final content replaces every temporary path. + occupied := map[string]bool{} + current := map[int64]store.PageState{} + for _, p := range state.Pages { + occupied[p.Content.Subsite+"\x00"+p.Content.Path] = true + current[p.ID] = p + } for _, m := range batch.Mutations { - if m.Kind == "update" { - if _, err = tx.Exec(ctx, `UPDATE pages SET path='/__cms_batch_internal__/' || id::text WHERE tenant_id=$1 AND id=$2 AND version=$3`, tenantID, m.TargetID, m.ExpectedVersion); err != nil { + if m.Content != nil { + occupied[m.Content.Subsite+"\x00"+m.Content.Path] = true + } + } + for _, m := range batch.Mutations { + if m.Kind == "update" && (m.Content.Path != current[m.TargetID].Content.Path || m.Content.Subsite != current[m.TargetID].Content.Subsite) { + old := current[m.TargetID] + var staging string + for { + staging = "/__cms_batch_stage__/" + rand.Text() + "/" + strconv.FormatInt(m.TargetID, 10) + key := old.Content.Subsite + "\x00" + staging + if !occupied[key] { + occupied[key] = true + break + } + } + if _, err = tx.Exec(ctx, `UPDATE pages SET path=$4 WHERE tenant_id=$1 AND id=$2 AND version=$3`, tenantID, m.TargetID, m.ExpectedVersion, staging); err != nil { return store.PageBatchReceipt{}, store.ErrBatchInvalid } } @@ -127,6 +191,10 @@ func (s *Store) applyBatch(ctx context.Context, tenantID int64, batch store.Page if err != nil { return store.PageBatchReceipt{}, store.ErrBatchInvalid } + result.AfterRevision, err = advanceRevision(ctx, tx, tenantID) + if err != nil { + return store.PageBatchReceipt{}, err + } if err = tx.Commit(ctx); err != nil { return store.PageBatchReceipt{}, store.ErrBatchInvalid } diff --git a/store/postgres/batch_integration_test.go b/store/postgres/batch_integration_test.go index 88d76f6..aa13f1d 100644 --- a/store/postgres/batch_integration_test.go +++ b/store/postgres/batch_integration_test.go @@ -2,6 +2,7 @@ package postgres import ( "context" + "errors" "fmt" "os" "reflect" @@ -52,6 +53,13 @@ func isolatedPostgres(t *testing.T) *Store { if err != nil { t.Fatal("isolated test tables failed") } + migration, err := os.ReadFile("migrations/0001_page_content_revision.up.sql") + if err != nil { + t.Fatal("required test migration unavailable") + } + if _, err = pool.Exec(ctx, string(migration)); err != nil { + t.Fatal("isolated revision migration failed") + } return NewWithPool(pool) } @@ -63,9 +71,19 @@ func TestPostgresPageBatchContract(t *testing.T) { if s.CreateTenant(ctx, target) != nil || s.CreateTenant(ctx, source) != nil { t.Fatal("isolated tenants failed") } - storetest.PageBatches(t, s, target.ID, source.ID) + replay := storetest.PageBatches(t, s, target.ID, source.ID) + // Rebuild with fresh connections: revision history is durable database state. + restartedPool, err := pgxpool.NewWithConfig(ctx, pool.Config()) + if err != nil { + t.Fatal("fresh isolated pool failed") + } + defer restartedPool.Close() + restarted := NewWithPool(restartedPool) + if _, err := restarted.RollbackPageBatch(ctx, source.ID, replay); !errors.Is(err, store.ErrVersionConflict) { + t.Fatal("durable replay guard lost after store reconstruction") + } // A database-side failure after an earlier update must roll back every item. - _, err := pool.Exec(ctx, `CREATE FUNCTION refuse_test_insert() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN IF NEW.title='DB refusal test' THEN RAISE EXCEPTION 'test refusal' USING ERRCODE='23514'; END IF; RETURN NEW; END $$; CREATE TRIGGER refuse_test_insert BEFORE INSERT ON pages FOR EACH ROW EXECUTE FUNCTION refuse_test_insert();`) + _, err = pool.Exec(ctx, `CREATE FUNCTION refuse_test_insert() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN IF NEW.title='DB refusal test' THEN RAISE EXCEPTION 'test refusal' USING ERRCODE='23514'; END IF; RETURN NEW; END $$; CREATE TRIGGER refuse_test_insert BEFORE INSERT ON pages FOR EACH ROW EXECUTE FUNCTION refuse_test_insert();`) if err != nil { t.Fatal("isolated failure trigger failed") } @@ -77,7 +95,11 @@ func TestPostgresPageBatchContract(t *testing.T) { changed := store.ContentOf(all[0]) changed.Title = "Must roll back" refused := store.PageContent{Path: "/database-rejected", Title: "DB refusal test", Status: store.StatusDraft} - batch := store.PageBatch{Baseline: before, Mutations: []store.PageMutation{{Key: "updated-first", Kind: "update", TargetID: all[0].ID, ExpectedVersion: all[0].Version, Content: &changed}, {Key: "refused-second", Kind: "create", Content: &refused}}} + state, err := s.ReadPageState(ctx, target.ID) + if err != nil { + t.Fatal(err) + } + batch := store.PageBatch{TargetScope: state.Scope, BaselineRevision: state.Revision, Baseline: before, Mutations: []store.PageMutation{{Key: "updated-first", Kind: "update", TargetID: all[0].ID, ExpectedVersion: all[0].Version, Content: &changed}, {Key: "refused-second", Kind: "create", Content: &refused}}} if _, err := s.ApplyPageBatch(ctx, target.ID, batch); err != store.ErrBatchInvalid { t.Fatal("database failure was not safely reported") } @@ -86,3 +108,38 @@ func TestPostgresPageBatchContract(t *testing.T) { t.Fatal("database failure partially committed content") } } + +func TestPostgresMissingRevisionSchemaFailsClosed(t *testing.T) { + s := isolatedPostgres(t) + ctx := context.Background() + tenant := &store.Tenant{Slug: "missing-ledger"} + if s.CreateTenant(ctx, tenant) != nil { + t.Fatal("tenant failed") + } + p := &store.Page{Path: "/", Title: "Before", Status: store.StatusDraft} + if s.Create(ctx, tenant.ID, p) != nil { + t.Fatal("page failed") + } + version := p.Version + if _, err := s.pool.Exec(ctx, `DROP TABLE cms_page_revisions CASCADE`); err != nil { + t.Fatal("isolated missing-schema fixture failed") + } + p.Title = "Must not commit" + if s.Update(ctx, tenant.ID, p) != store.ErrBatchInvalid { + t.Fatal("write bypassed missing revision ledger") + } + if s.Delete(ctx, tenant.ID, p.ID, version) != store.ErrBatchInvalid { + t.Fatal("delete bypassed missing revision ledger") + } + create := &store.Page{Path: "/uncommitted", Title: "Must not commit", Status: store.StatusDraft} + if s.Create(ctx, tenant.ID, create) != store.ErrBatchInvalid { + t.Fatal("create bypassed missing revision ledger") + } + got, err := s.Get(ctx, tenant.ID, p.ID) + if err != nil || got.Version != version || got.Title != "Before" { + t.Fatal("missing schema partially committed content") + } + if _, err := s.GetByPath(ctx, tenant.ID, "", "/uncommitted"); !errors.Is(err, store.ErrNotFound) { + t.Fatal("failed create persisted") + } +} diff --git a/store/postgres/editor_browser_test.go b/store/postgres/editor_browser_test.go index 0f06973..7b4f84f 100644 --- a/store/postgres/editor_browser_test.go +++ b/store/postgres/editor_browser_test.go @@ -46,7 +46,7 @@ func TestPostgresEditorVersionBrowser(t *testing.T) { w.WriteHeader(405) return } - all, err := s.List(r.Context(), tenant.ID, "") + state, err := s.ReadPageState(r.Context(), tenant.ID) if err != nil { w.WriteHeader(500) return @@ -58,7 +58,7 @@ func TestPostgresEditorVersionBrowser(t *testing.T) { } content := store.ContentOf(current) content.Title = "Approved fixture promotion" - _, err = s.ApplyPageBatch(r.Context(), tenant.ID, store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "approved-fixture", Kind: "update", TargetID: p.ID, ExpectedVersion: current.Version, Content: &content}}}) + _, err = s.ApplyPageBatch(r.Context(), tenant.ID, store.PageBatch{TargetScope: state.Scope, BaselineRevision: state.Revision, Baseline: store.Baseline(state.Pages), Mutations: []store.PageMutation{{Key: "approved-fixture", Kind: "update", TargetID: p.ID, ExpectedVersion: current.Version, Content: &content}}}) if err != nil { w.WriteHeader(500) return diff --git a/store/postgres/migrations/0001_page_content_revision.up.sql b/store/postgres/migrations/0001_page_content_revision.up.sql new file mode 100644 index 0000000..1b79176 --- /dev/null +++ b/store/postgres/migrations/0001_page_content_revision.up.sql @@ -0,0 +1,18 @@ +-- Required host prerequisite for guarded page batches. Apply under a reviewed +-- write fence before adopting the new CMS store; do not mix old/new writers. +BEGIN; +CREATE TABLE cms_page_revisions ( + tenant_id BIGINT PRIMARY KEY REFERENCES tenants(id) ON DELETE CASCADE, + content_scope TEXT NOT NULL UNIQUE DEFAULT gen_random_uuid()::text, + revision BIGINT NOT NULL DEFAULT 0 CHECK (revision >= 0) +); +INSERT INTO cms_page_revisions (tenant_id) SELECT id FROM tenants; +CREATE FUNCTION cms_initialize_page_revision() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + INSERT INTO cms_page_revisions (tenant_id) VALUES (NEW.id); + RETURN NEW; +END; +$$; +CREATE TRIGGER cms_initialize_page_revision + AFTER INSERT ON tenants FOR EACH ROW EXECUTE FUNCTION cms_initialize_page_revision(); +COMMIT; diff --git a/store/postgres/store.go b/store/postgres/store.go index 759444b..4ceaed0 100644 --- a/store/postgres/store.go +++ b/store/postgres/store.go @@ -245,6 +245,9 @@ func (s *Store) Create(ctx context.Context, tenantID int64, p *store.Page) error if err = createPage(ctx, tx, tenantID, p); err != nil { return err } + if _, err = advanceRevision(ctx, tx, tenantID); err != nil { + return err + } return tx.Commit(ctx) } @@ -315,6 +318,9 @@ func (s *Store) Update(ctx context.Context, tenantID int64, p *store.Page) error if err = updatePage(ctx, tx, tenantID, p); err != nil { return err } + if _, err = advanceRevision(ctx, tx, tenantID); err != nil { + return err + } return tx.Commit(ctx) } @@ -360,6 +366,9 @@ func (s *Store) Delete(ctx context.Context, tenantID int64, id int64, expectedVe if err = deletePage(ctx, tx, tenantID, id, expectedVersion); err != nil { return err } + if _, err = advanceRevision(ctx, tx, tenantID); err != nil { + return err + } return tx.Commit(ctx) } diff --git a/store/storetest/batch.go b/store/storetest/batch.go index ea207e8..273e0bf 100644 --- a/store/storetest/batch.go +++ b/store/storetest/batch.go @@ -5,6 +5,7 @@ package storetest import ( "context" "errors" + "strconv" "testing" "github.com/GoCodeAlone/workflow-plugin-cms/store" @@ -15,7 +16,7 @@ type BatchStore interface { store.PageBatchStore } -func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) { +func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) store.PageBatchReceipt { t.Helper() ctx := context.Background() create := func(tenant int64, path, title string) *store.Page { @@ -32,14 +33,14 @@ func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) { if review.ID == home.ID { t.Fatal("source and target must differ") } - all, err := s.List(ctx, targetTenant, "") + + state, err := s.ReadPageState(ctx, targetTenant) if err != nil { t.Fatal(err) } - baseline := store.Baseline(store.States(all)) content := store.ContentOf(review) news := store.PageContent{Path: "/news", Title: "New page", BodyHTML: "

News

", Status: store.StatusPublished} - batch := store.PageBatch{Baseline: baseline, Mutations: []store.PageMutation{{Key: "review:home", Kind: "update", TargetID: home.ID, ExpectedVersion: home.Version, Content: &content}, {Key: "review:news", Kind: "create", Content: &news}, {Key: "explicit:remove", Kind: "delete", TargetID: removed.ID, ExpectedVersion: removed.Version}}} + batch := store.PageBatch{TargetScope: state.Scope, BaselineRevision: state.Revision, Baseline: store.Baseline(state.Pages), Mutations: []store.PageMutation{{Key: "review:home", Kind: "update", TargetID: home.ID, ExpectedVersion: home.Version, Content: &content}, {Key: "review:news", Kind: "create", Content: &news}, {Key: "explicit:remove", Kind: "delete", TargetID: removed.ID, ExpectedVersion: removed.Version}}} bad := batch bad.Mutations = append([]store.PageMutation(nil), batch.Mutations...) invalid := news @@ -93,8 +94,13 @@ func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) { if _, err := s.Get(ctx, targetTenant, receipt.Mapping["review:news"]); !errors.Is(err, store.ErrNotFound) { t.Fatal("created page survived rollback") } - all, _ = s.List(ctx, targetTenant, "") - batch.Baseline = store.Baseline(store.States(all)) + + state, err = s.ReadPageState(ctx, targetTenant) + if err != nil { + t.Fatal(err) + } + batch.Baseline = store.Baseline(state.Pages) + batch.BaselineRevision = state.Revision batch.Mutations[0].ExpectedVersion = rolled.Version batch.Mutations[2].ExpectedVersion = restored.Version receipt, err = s.ApplyPageBatch(ctx, targetTenant, batch) @@ -114,10 +120,14 @@ func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) { } // Simultaneous save/promotion: only one may use the same loaded version. fresh, _ := s.Get(ctx, targetTenant, home.ID) - all, _ = s.List(ctx, targetTenant, "") + + state, err = s.ReadPageState(ctx, targetTenant) + if err != nil { + t.Fatal(err) + } next := store.ContentOf(fresh) next.Title = "Promotion race winner" - raceBatch := store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "race", Kind: "update", TargetID: fresh.ID, ExpectedVersion: fresh.Version, Content: &next}}} + raceBatch := store.PageBatch{TargetScope: state.Scope, BaselineRevision: state.Revision, Baseline: store.Baseline(state.Pages), Mutations: []store.PageMutation{{Key: "race", Kind: "update", TargetID: fresh.ID, ExpectedVersion: fresh.Version, Content: &next}}} fresh.Title = "Editor race winner" start := make(chan struct{}) results := make(chan error, 2) @@ -141,10 +151,14 @@ func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) { } // A delete loaded before promotion also competes on the same version. fresh, _ = s.Get(ctx, targetTenant, home.ID) - all, _ = s.List(ctx, targetTenant, "") + + state, err = s.ReadPageState(ctx, targetTenant) + if err != nil { + t.Fatal(err) + } next = store.ContentOf(fresh) next.Title = "Delete race promotion" - raceBatch = store.PageBatch{Baseline: store.Baseline(store.States(all)), Mutations: []store.PageMutation{{Key: "delete-race", Kind: "update", TargetID: fresh.ID, ExpectedVersion: fresh.Version, Content: &next}}} + raceBatch = store.PageBatch{TargetScope: state.Scope, BaselineRevision: state.Revision, Baseline: store.Baseline(state.Pages), Mutations: []store.PageMutation{{Key: "delete-race", Kind: "update", TargetID: fresh.ID, ExpectedVersion: fresh.Version, Content: &next}}} start = make(chan struct{}) go func() { <-start; results <- s.Delete(ctx, targetTenant, fresh.ID, fresh.Version) }() go func() { <-start; _, err := s.ApplyPageBatch(ctx, targetTenant, raceBatch); results <- err }() @@ -187,4 +201,71 @@ func PageBatches(t *testing.T, s BatchStore, targetTenant, sourceTenant int64) { if wins != 1 || conflicts != 1 { t.Fatal("duplicate racing create accepted") } + // Occupied legacy staging names must not block unchanged-path updates. + kept, _ = s.Get(ctx, targetTenant, keep.ID) + occupied := create(targetTenant, "/__cms_batch_internal__/"+strconv.FormatInt(kept.ID, 10), "Omitted staging path") + state, err = s.ReadPageState(ctx, targetTenant) + if err != nil { + t.Fatal(err) + } + titleOnly := store.ContentOf(kept) + titleOnly.Title = "Title-only promotion" + if _, err := s.ApplyPageBatch(ctx, targetTenant, store.BatchFor(state, []store.PageMutation{{Key: "title-only", Kind: "update", TargetID: kept.ID, ExpectedVersion: kept.Version, Content: &titleOnly}})); err != nil { + t.Fatalf("occupied staging path blocked title update: %v", err) + } + still, _ := s.Get(ctx, targetTenant, occupied.ID) + if still.Title != occupied.Title { + t.Fatal("omitted staging path changed") + } + a, b := create(targetTenant, "/swap-a", "Swap A"), create(targetTenant, "/swap-b", "Swap B") + state, _ = s.ReadPageState(ctx, targetTenant) + ca, cb := store.ContentOf(a), store.ContentOf(b) + ca.Path, cb.Path = b.Path, a.Path + if _, err := s.ApplyPageBatch(ctx, targetTenant, store.BatchFor(state, []store.PageMutation{{Key: "swap-a", Kind: "update", TargetID: a.ID, ExpectedVersion: a.Version, Content: &ca}, {Key: "swap-b", Kind: "update", TargetID: b.ID, ExpectedVersion: b.Version, Content: &cb}})); err != nil { + t.Fatalf("atomic path swap failed: %v", err) + } + afterA, _ := s.Get(ctx, targetTenant, a.ID) + afterB, _ := s.Get(ctx, targetTenant, b.ID) + if afterA.Path != b.Path || afterB.Path != a.Path { + t.Fatal("path swap not persisted") + } + // Deletion ABA: an old empty post-state can recur after newer mutations. + state, _ = s.ReadPageState(ctx, sourceTenant) + review, _ = s.Get(ctx, sourceTenant, review.ID) + deleted, err := s.ApplyPageBatch(ctx, sourceTenant, store.BatchFor(state, []store.PageMutation{{Key: "aba-delete", Kind: "delete", TargetID: review.ID, ExpectedVersion: review.Version}})) + if err != nil { + t.Fatal(err) + } + if _, err := s.RollbackPageBatch(ctx, sourceTenant, deleted); err != nil { + t.Fatal(err) + } + oldEditor, _ := s.Get(ctx, sourceTenant, review.ID) + fresh, _ = s.Get(ctx, sourceTenant, review.ID) + fresh.Title = "Intervening edit" + if s.Update(ctx, sourceTenant, fresh) != nil || s.Delete(ctx, sourceTenant, fresh.ID, fresh.Version) != nil { + t.Fatal("ABA edit/delete fixture failed") + } + if _, err := s.RollbackPageBatch(ctx, sourceTenant, deleted); !errors.Is(err, store.ErrVersionConflict) { + t.Fatal("old deletion receipt replay accepted changed-then-empty state") + } + if err := s.Update(ctx, sourceTenant, oldEditor); !errors.Is(err, store.ErrNotFound) { + t.Fatal("stale pre-delete editor resurrected") + } + newPage := create(sourceTenant, "/aba-new", "New deletion") + state, _ = s.ReadPageState(ctx, sourceTenant) + deleted, err = s.ApplyPageBatch(ctx, sourceTenant, store.BatchFor(state, []store.PageMutation{{Key: "aba-new-delete", Kind: "delete", TargetID: newPage.ID, ExpectedVersion: newPage.Version}})) + if err != nil { + t.Fatal(err) + } + temporary := create(sourceTenant, "/temporary", "Created and deleted") + if s.Delete(ctx, sourceTenant, temporary.ID, temporary.Version) != nil { + t.Fatal("create/delete fixture failed") + } + if _, err := s.RollbackPageBatch(ctx, sourceTenant, deleted); !errors.Is(err, store.ErrVersionConflict) { + t.Fatal("create/delete history disappeared from rollback guard") + } + if _, err := s.RollbackPageBatch(ctx, targetTenant, deleted); !errors.Is(err, store.ErrVersionConflict) { + t.Fatal("receipt target content scope not bound") + } + return deleted } From 0c2e1a7f511fb718120161066a9e7576c7a8d172 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 8 Oct 2026 15:23:48 -0400 Subject: [PATCH 03/10] Refuse unchecked nested HTML publication resources --- .../2026-10-08-content-promotion-amendment.md | 11 +++ docs/promotion.md | 8 ++ docs/qa/content-promotion.md | 1 + promotion/bundle.go | 36 +++++++- promotion/snapshot_test.go | 82 +++++++++++++++++++ 5 files changed, 134 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-10-08-content-promotion-amendment.md b/docs/plans/2026-10-08-content-promotion-amendment.md index 4130ad8..ec5e2fd 100644 --- a/docs/plans/2026-10-08-content-promotion-amendment.md +++ b/docs/plans/2026-10-08-content-promotion-amendment.md @@ -78,3 +78,14 @@ replacing every older writer: mixed old/new plugin versions would bypass the ledger. A new store instance against the same isolated database must retain the rollback/replay guard. Canonical block links, absolute reference exclusions and collision-free temporary paths receive regression coverage in the same correction. + +## Independent-review correction: nested HTML resources + +The full `afc9dc9` review found unchecked nested/resource HTML attributes. +Preparation now refuses inline nested documents and object/plugin embeddings, +validates form override URLs and the other supported resource attributes, and +refuses unsupported document base/refresh overrides. Regressions exercise page +bodies, selected CMS templates and bundled HTML, including private upload/API +URLs. Public HTTPS iframe sources and validated form overrides remain supported. +This does not add a general active-content sandbox or change the host publication +gates, schema prerequisite, consumer pins or approval boundary. diff --git a/docs/promotion.md b/docs/promotion.md index 69afaf2..eacbb24 100644 --- a/docs/promotion.md +++ b/docs/promotion.md @@ -46,6 +46,14 @@ references and exact hashes. The operator must stage those verified immutable bytes in the target bundle. The library does not configure media storage, copy uploaded objects, fetch third-party embeds or claim rights to external media. +HTML checks cover page bodies, CMS templates and bundled HTML. Inline nested +documents (`srcdoc`), object/plugin embeddings (`object`, `embed`), reference base +overrides and meta refresh are unsupported and refuse validation. Public HTTPS +iframe sources remain supported; form action/override URLs, image source sets +and other resource attributes use the same private-path and asset checks. +This is reference validation, not a JavaScript sandbox. The host must review the +exact approved bundle's scripts and runtime behavior separately. + The SQL prerequisite `store/postgres/migrations/0001_page_content_revision.up.sql` must be reviewed and applied by the host before adoption. It initializes tenant content scopes/revisions and new-tenant initialization. This library never applies diff --git a/docs/qa/content-promotion.md b/docs/qa/content-promotion.md index 76c6ca1..2490662 100644 --- a/docs/qa/content-promotion.md +++ b/docs/qa/content-promotion.md @@ -24,6 +24,7 @@ each schema is cleaned by its test. No runtime tables, processes or ports change | Canonical blocks → references | Internal/relative/anchor/mail/HTTPS links export through the actual renderer; arbitrary slash text is preserved; absolute/encoded admin/API/media references refuse | | Receipt → rollback | Content restored with increasing versions; created page removed; deleted page restored; later edit refuses whole rollback | | Bundle → validator | Changed bytes, missing assets, static homepage shadow, symlink, admin reference and encoded tenant-upload references refused | +| HTML resource surfaces → validator | Page bodies, templates and bundled HTML refuse nested `srcdoc`, object/plugin embeddings and private resource/form overrides; public HTTPS iframe sources and verified local overrides remain usable | | Actual Chrome editor → API → Postgres | Repeated saves send versions 1/2; fixture promotion changes version to 4; stale save/delete send 3 and return 409; unsaved draft retained; reload then version-4 save persists as version 5; wrong tenant returns 403 | The browser test launches an ephemeral loopback test server and a temporary diff --git a/promotion/bundle.go b/promotion/bundle.go index fa0e06b..e2ca59a 100644 --- a/promotion/bundle.go +++ b/promotion/bundle.go @@ -194,16 +194,44 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten } var visit func(*html.Node) error visit = func(n *html.Node) error { - if n.Type == html.ElementNode && n.Data == "base" { - return ErrBundle + if n.Type == html.ElementNode { + switch n.Data { + case "base", "object", "embed": + // These elements can change reference resolution or load + // nested/plugin documents outside the checked HTML tree. + return ErrBundle + } } for _, a := range n.Attr { switch a.Key { - case "href", "src", "poster", "action": + case "srcdoc", "xml:base": + // srcdoc is decoded into an attribute value, not visited + // children. Nested inline documents and XML reference + // base overrides are unsupported. + return ErrBundle + case "base": + if a.Namespace == "xml" { + return ErrBundle + } + case "http-equiv": + if n.Data == "meta" && strings.EqualFold(strings.TrimSpace(a.Val), "refresh") { + return ErrBundle + } + case "href", "src", "poster", "action", "formaction", "background", "manifest": if err := checkRef(a.Val, base, a.Key == "href"); err != nil { return err } - case "srcset": + case "cite", "longdesc": + if err := checkRef(a.Val, base, true); err != nil { + return err + } + case "ping": + for _, ref := range strings.Fields(a.Val) { + if err := checkRef(ref, base, false); err != nil { + return err + } + } + case "srcset", "imagesrcset": for _, ref := range strings.Split(a.Val, ",") { parts := strings.Fields(ref) if len(parts) == 0 { diff --git a/promotion/snapshot_test.go b/promotion/snapshot_test.go index 1d0b44a..3c57c14 100644 --- a/promotion/snapshot_test.go +++ b/promotion/snapshot_test.go @@ -197,3 +197,85 @@ func TestCanonicalBlockLinksUseActualRendererSemantics(t *testing.T) { } } } + +func TestBundleHTMLResourceSurfacesRefusePrivateAndNestedDocuments(t *testing.T) { + cases := []struct { + name string + html string + }{ + {"iframe-srcdoc-upload", ``}, + {"iframe-srcdoc-api", ``}, + {"iframe-srcdoc-nested", ``}, + {"iframe-srcdoc-public-unsupported", ``}, + {"object-upload", ``}, + {"object-api", ``}, + {"object-bundle-unsupported", ``}, + {"embed-admin", ``}, + {"formaction-upload", `
`}, + {"formaction-api", ``}, + {"formaction-admin", ``}, + {"formaction-encoded-api", ``}, + {"formaction-relative-upload", ``}, + {"formaction-missing-local", ``}, + {"meta-refresh-unsupported", ``}, + {"svg-xml-base", ``}, + {"imagesrcset-upload", ``}, + {"ping-api", `Home`}, + {"legacy-background-upload", `
`}, + {"document-manifest-admin", ``}, + {"longdesc-api", ``}, + {"cite-admin", `
Text
`}, + } + for _, surface := range []string{"page-body", "cms-template", "static-html"} { + t.Run(surface, func(t *testing.T) { + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + switch surface { + case "page-body": + p.BodyHTML = tc.html + case "cms-template": + if err := os.WriteFile(filepath.Join(root, "cms/templates/main.html"), []byte(tc.html+`
`), 0600); err != nil { + t.Fatal(err) + } + case "static-html": + if err := os.WriteFile(filepath.Join(root, "resource.html"), []byte(tc.html), 0600); err != nil { + t.Fatal(err) + } + } + manifest, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + if err := VerifyBundle(root, manifest, []store.PageContent{p}); err != ErrBundle { + t.Fatalf("unsupported HTML resource surface accepted: %v", err) + } + }) + } + }) + } +} + +func TestBundleSupportedEmbedsAndResourceOverridesRemainUsable(t *testing.T) { + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + p.BodyHTML = ` + +
+ + + +Home +
Text
` + if err := os.WriteFile(filepath.Join(root, "assets/submit.html"), []byte(`

Submission endpoint fixture

`), 0600); err != nil { + t.Fatal(err) + } + manifest, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + if err = VerifyBundle(root, manifest, []store.PageContent{p}); err != nil { + t.Fatalf("supported public embed or validated resource override refused: %v", err) + } +} From be09cd81ac91a47bf5300aa25a93ec7d2fb8c8f6 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 8 Oct 2026 15:50:46 -0400 Subject: [PATCH 04/10] Allow selected public media links without relaxing upload checks --- .../2026-10-08-content-promotion-amendment.md | 18 ++++ docs/promotion.md | 5 + promotion/bundle.go | 35 +++++-- promotion/snapshot_test.go | 97 +++++++++++++++++++ 4 files changed, 147 insertions(+), 8 deletions(-) diff --git a/docs/plans/2026-10-08-content-promotion-amendment.md b/docs/plans/2026-10-08-content-promotion-amendment.md index ec5e2fd..aaa702a 100644 --- a/docs/plans/2026-10-08-content-promotion-amendment.md +++ b/docs/plans/2026-10-08-content-promotion-amendment.md @@ -89,3 +89,21 @@ bodies, selected CMS templates and bundled HTML, including private upload/API URLs. Public HTTPS iframe sources and validated form overrides remain supported. This does not add a general active-content sandbox or change the host publication gates, schema prerequisite, consumer pins or approval boundary. + +## Consumer-boundary correction: selected public media index + +The unchanged reviewed-source PostgreSQL consumer completed the generic frozen +snapshot → explicit mapping/dryrun → apply → safe rollback path, including +cross-tenant/scope refusals, monotonic versions/revisions and newer-edit refusal. +The optional private six-page bundle then exposed a public `/media` navigation +collision with the upload namespace refusal. The failed optional run and all +completed generic evidence remain preserved outside this public repository. + +Parent authorizes a narrow exception for relative hyperlinks to exactly the +selected public CMS route `/media`. Resource sources, upload children, absolute +private URLs, unselected routes and other private namespaces still refuse. +Acceptance covers body/template/static HTML and canonical block hrefs, encoded +and dot-normalized upload refusals, plus the private bundle consumer in a fresh +PostgreSQL schema. New exact-head tests and full independent source review are +required. No hosted write, migration/adoption, source pin or live apply route +is added; the prior locked plan and release gates stay intact. diff --git a/docs/promotion.md b/docs/promotion.md index eacbb24..85841e6 100644 --- a/docs/promotion.md +++ b/docs/promotion.md @@ -54,6 +54,11 @@ and other resource attributes use the same private-path and asset checks. This is reference validation, not a JavaScript sandbox. The host must review the exact approved bundle's scripts and runtime behavior separately. +The public CMS index route `/media` may be a relative hyperlink only when that +exact route is in the selected snapshot pages. The exception covers navigation, +including canonical block links; it does not permit media resource URLs, upload +children, unselected routes, absolute private URLs or other private namespaces. + The SQL prerequisite `store/postgres/migrations/0001_page_content_revision.up.sql` must be reviewed and applied by the host before adoption. It initializes tenant content scopes/revisions and new-tenant initialization. This library never applies diff --git a/promotion/bundle.go b/promotion/bundle.go index e2ca59a..0190374 100644 --- a/promotion/bundle.go +++ b/promotion/bundle.go @@ -140,7 +140,14 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten return ErrBundle } } - checkRef := func(raw, base string, link bool) error { + type referenceKind uint8 + const ( + resourceReference referenceKind = iota + navigationReference + descriptionReference + ) + checkRef := func(raw, base string, kind referenceKind) error { + link := kind != resourceReference if raw == "" || strings.HasPrefix(raw, "#") { return nil } @@ -155,7 +162,11 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if u.Scheme == "" && !strings.HasPrefix(u.Path, "/") { resolved = path.Join(path.Dir(base), u.Path) } - if forbiddenPath(resolved) { + // /media is also a valid public CMS index route. Only navigation to + // that exact selected route may use it; uploads, resource requests, + // absolute private URLs and other private namespaces remain refused. + selectedMediaLink := kind == navigationReference && u.Scheme == "" && u.Host == "" && resolved == "/media" && routes[resolved] + if forbiddenPath(resolved) && !selectedMediaLink { return ErrBundle } if u.Scheme != "" { @@ -181,7 +192,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if ref == "" { ref = m[2] } - if err := checkRef(ref, base, false); err != nil { + if err := checkRef(ref, base, resourceReference); err != nil { return err } } @@ -218,16 +229,20 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten return ErrBundle } case "href", "src", "poster", "action", "formaction", "background", "manifest": - if err := checkRef(a.Val, base, a.Key == "href"); err != nil { + kind := resourceReference + if a.Key == "href" && (n.Data == "a" || n.Data == "area") { + kind = navigationReference + } + if err := checkRef(a.Val, base, kind); err != nil { return err } case "cite", "longdesc": - if err := checkRef(a.Val, base, true); err != nil { + if err := checkRef(a.Val, base, descriptionReference); err != nil { return err } case "ping": for _, ref := range strings.Fields(a.Val) { - if err := checkRef(ref, base, false); err != nil { + if err := checkRef(ref, base, resourceReference); err != nil { return err } } @@ -237,7 +252,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if len(parts) == 0 { return ErrBundle } - if err := checkRef(parts[0], base, false); err != nil { + if err := checkRef(parts[0], base, resourceReference); err != nil { return err } } @@ -292,7 +307,11 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if ref, ok := item.(string); ok { switch key { case "href", "src", "url", "poster": - if err := checkRef(ref, p.Path, key == "href"); err != nil { + kind := resourceReference + if key == "href" { + kind = navigationReference + } + if err := checkRef(ref, p.Path, kind); err != nil { return err } } diff --git a/promotion/snapshot_test.go b/promotion/snapshot_test.go index 3c57c14..a628c0b 100644 --- a/promotion/snapshot_test.go +++ b/promotion/snapshot_test.go @@ -7,6 +7,7 @@ import ( "errors" "os" "path/filepath" + "strconv" "strings" "testing" "time" @@ -279,3 +280,99 @@ func TestBundleSupportedEmbedsAndResourceOverridesRemainUsable(t *testing.T) { t.Fatalf("supported public embed or validated resource override refused: %v", err) } } + +func TestSelectedMediaRouteLinksPreserveUploadAndPrivateRefusal(t *testing.T) { + cases := []struct { + name string + html string + selected bool + allowed bool + }{ + {"selected-relative-link", `Media`, true, true}, + {"selected-relative-path", `Media`, true, true}, + {"selected-link-query-anchor", `Media`, true, true}, + {"unselected-route-link", `Media`, false, false}, + {"unselected-relative-path", `Media`, false, false}, + {"selected-route-image-source", ``, true, false}, + {"selected-route-stylesheet-resource", ``, true, false}, + {"selected-route-svg-image-resource", ``, true, false}, + {"selected-route-svg-use-resource", ``, true, false}, + {"selected-route-description-reference", ``, true, false}, + {"selected-route-iframe-source", ``, true, false}, + {"selected-route-form-action", `
`, true, false}, + {"selected-route-form-override", ``, true, false}, + {"selected-route-source-set", ``, true, false}, + {"selected-route-style-resource", `

Text

`, true, false}, + {"uploaded-child-link", `Upload`, true, false}, + {"uploaded-child-source", ``, true, false}, + {"encoded-upload-child", `Upload`, true, false}, + {"encoded-upload-separator", `Upload`, true, false}, + {"dot-upload-child", `Upload`, true, false}, + {"encoded-dot-upload-child", `Upload`, true, false}, + {"absolute-media-index", `Private`, true, false}, + {"absolute-media-upload", `Private`, true, false}, + {"protocol-relative-media", `Private`, true, false}, + {"api-namespace-link", `Private`, true, false}, + {"admin-namespace-link", `Private`, true, false}, + {"cms-namespace-link", `Private`, true, false}, + } + for _, surface := range []string{"page-body", "cms-template", "static-html"} { + t.Run(surface, func(t *testing.T) { + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + switch surface { + case "page-body": + p.BodyHTML = tc.html + case "cms-template": + if err := os.WriteFile(filepath.Join(root, "cms/templates/main.html"), []byte(tc.html+`
`), 0600); err != nil { + t.Fatal(err) + } + case "static-html": + if err := os.WriteFile(filepath.Join(root, "resource.html"), []byte(tc.html), 0600); err != nil { + t.Fatal(err) + } + } + contents := []store.PageContent{p} + if tc.selected { + media := s.Pages[0].Content + media.Path = "/media" + media.BodyHTML = "

Public media page.

" + contents = append(contents, media) + } + manifest, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + err = VerifyBundle(root, manifest, contents) + if tc.allowed && err != nil || !tc.allowed && err != ErrBundle { + t.Fatalf("selected media route policy mismatch: %v", err) + } + }) + } + }) + } + // Canonical block hrefs and raw HTML must share the same route policy. + for _, selected := range []bool{false, true} { + for _, href := range []string{"/media", "media", "/media/10/hash.jpg", "/%6dedia/10/hash.jpg", "https://example.com/media", "/api"} { + t.Run("canonical-"+href+"-selected-"+strconv.FormatBool(selected), func(t *testing.T) { + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + p.BodyBlocks, _ = json.Marshal(map[string]any{"type": "doc", "content": []any{map[string]any{"type": "link", "attrs": map[string]any{"href": href}, "content": []any{map[string]any{"type": "text", "text": "Media"}}}}}) + contents := []store.PageContent{p} + if selected { + media := s.Pages[0].Content + media.Path = "/media" + media.BodyHTML = "

Public media page.

" + contents = append(contents, media) + } + allowed := selected && (href == "/media" || href == "media") + err := VerifyBundle(root, s.Bundle, contents) + if allowed && err != nil || !allowed && err != ErrBundle { + t.Fatalf("canonical media route policy mismatch: %v", err) + } + }) + } + } +} From d1755a8df7b94aa7827f1e233f67f0581d42d168 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 8 Oct 2026 15:53:14 -0400 Subject: [PATCH 05/10] Require canonical spelling for the selected media route exception --- docs/plans/2026-10-08-content-promotion-amendment.md | 5 +++++ docs/promotion.md | 2 ++ promotion/bundle.go | 3 ++- promotion/snapshot_test.go | 8 +++++++- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-10-08-content-promotion-amendment.md b/docs/plans/2026-10-08-content-promotion-amendment.md index aaa702a..b80d8ba 100644 --- a/docs/plans/2026-10-08-content-promotion-amendment.md +++ b/docs/plans/2026-10-08-content-promotion-amendment.md @@ -107,3 +107,8 @@ and dot-normalized upload refusals, plus the private bundle consumer in a fresh PostgreSQL schema. New exact-head tests and full independent source review are required. No hosted write, migration/adoption, source pin or live apply route is added; the prior locked plan and release gates stay intact. + +The early source review tightens this exception to canonical raw `/media` or +relative `media` path spelling. Encoded, dot-segment and trailing-slash aliases +remain refused even when normalization would resolve to the selected route. +The same negative cases cover all HTML surfaces and canonical block hrefs. diff --git a/docs/promotion.md b/docs/promotion.md index 85841e6..f9aaf3f 100644 --- a/docs/promotion.md +++ b/docs/promotion.md @@ -58,6 +58,8 @@ The public CMS index route `/media` may be a relative hyperlink only when that exact route is in the selected snapshot pages. The exception covers navigation, including canonical block links; it does not permit media resource URLs, upload children, unselected routes, absolute private URLs or other private namespaces. +Only canonical `/media` or relative `media` path spelling qualifies; encoded, +dot-segment and trailing-slash aliases remain refused. The SQL prerequisite `store/postgres/migrations/0001_page_content_revision.up.sql` must be reviewed and applied by the host before adoption. It initializes tenant diff --git a/promotion/bundle.go b/promotion/bundle.go index 0190374..d2c1b5f 100644 --- a/promotion/bundle.go +++ b/promotion/bundle.go @@ -165,7 +165,8 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten // /media is also a valid public CMS index route. Only navigation to // that exact selected route may use it; uploads, resource requests, // absolute private URLs and other private namespaces remain refused. - selectedMediaLink := kind == navigationReference && u.Scheme == "" && u.Host == "" && resolved == "/media" && routes[resolved] + canonicalMediaPath := u.RawPath == "" && (u.Path == "/media" || u.Path == "media") + selectedMediaLink := kind == navigationReference && u.Scheme == "" && u.Host == "" && canonicalMediaPath && resolved == "/media" && routes[resolved] if forbiddenPath(resolved) && !selectedMediaLink { return ErrBundle } diff --git a/promotion/snapshot_test.go b/promotion/snapshot_test.go index a628c0b..5515772 100644 --- a/promotion/snapshot_test.go +++ b/promotion/snapshot_test.go @@ -293,6 +293,12 @@ func TestSelectedMediaRouteLinksPreserveUploadAndPrivateRefusal(t *testing.T) { {"selected-link-query-anchor", `Media`, true, true}, {"unselected-route-link", `Media`, false, false}, {"unselected-relative-path", `Media`, false, false}, + {"encoded-media-index", `Media`, true, false}, + {"dot-media-index", `Media`, true, false}, + {"encoded-dot-media-index", `Media`, true, false}, + {"root-dot-media-index", `Media`, true, false}, + {"relative-dot-media-index", `Media`, true, false}, + {"trailing-slash-media-index", `Media`, true, false}, {"selected-route-image-source", ``, true, false}, {"selected-route-stylesheet-resource", ``, true, false}, {"selected-route-svg-image-resource", ``, true, false}, @@ -355,7 +361,7 @@ func TestSelectedMediaRouteLinksPreserveUploadAndPrivateRefusal(t *testing.T) { } // Canonical block hrefs and raw HTML must share the same route policy. for _, selected := range []bool{false, true} { - for _, href := range []string{"/media", "media", "/media/10/hash.jpg", "/%6dedia/10/hash.jpg", "https://example.com/media", "/api"} { + for _, href := range []string{"/media", "media", "/%6dedia", "/assets/../media", "/assets/%2e%2e/media", "/./media", "./media", "/media/", "/media/10/hash.jpg", "/%6dedia/10/hash.jpg", "https://example.com/media", "/api"} { t.Run("canonical-"+href+"-selected-"+strconv.FormatBool(selected), func(t *testing.T) { s, root, _, _ := fixture(t) p := s.Pages[0].Content From 5eefa82096757c0807fbb5ff603bda7170793f6e Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 9 Oct 2026 06:06:02 -0400 Subject: [PATCH 06/10] fix(promotion): close bundle admission gaps from review --- docs/promotion.md | 12 +++ host/templates.go | 11 +-- internal/page_document.go | 8 ++ promotion/bundle.go | 64 +++++++----- promotion/bundle_admission_test.go | 151 +++++++++++++++++++++++++++++ promotion/css.go | 151 +++++++++++++++++++++++++++++ promotion/snapshot.go | 2 +- 7 files changed, 367 insertions(+), 32 deletions(-) create mode 100644 promotion/bundle_admission_test.go create mode 100644 promotion/css.go diff --git a/docs/promotion.md b/docs/promotion.md index f9aaf3f..162eb60 100644 --- a/docs/promotion.md +++ b/docs/promotion.md @@ -51,6 +51,18 @@ documents (`srcdoc`), object/plugin embeddings (`object`, `embed`), reference ba overrides and meta refresh are unsupported and refuse validation. Public HTTPS iframe sources remain supported; form action/override URLs, image source sets and other resource attributes use the same private-path and asset checks. +Both scripting modes are inspected, including `noscript` fallback resources. +Stylesheet and HTML extensions are matched without case sensitivity. Every +imported shell is checked, including shells not selected by the snapshot; +unselected shells require root-relative or public external references because +their eventual public page base is unknown. Template identifiers and the 1 MiB +shell limit share the runtime resolver's admission rules. Host-reserved page +routes and encoded path aliases are refused during snapshot validation. +CSS accepts ordinary `url()` and quoted or URL imports. CSS escapes and the +alternative string-image functions `image()`, `image-set()`, +`-webkit-image-set()` and `src()` are unsupported and fail closed. Comments and +ordinary text/font strings remain supported; no new CSS parser dependency is +installed. This is reference validation, not a JavaScript sandbox. The host must review the exact approved bundle's scripts and runtime behavior separately. diff --git a/host/templates.go b/host/templates.go index 7e856c3..99e13c5 100644 --- a/host/templates.go +++ b/host/templates.go @@ -15,14 +15,13 @@ import ( "strings" ) -var templateIDPattern = regexp.MustCompile(`^[a-z][a-z0-9-]{0,63}$`) var tenantSlugPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`) func (s *Server) resolveTenantTemplate(ctx context.Context, id int64, name string) (internal.PageTemplate, error) { if name == "" { return internal.PageTemplate{}, nil } - if !templateIDPattern.MatchString(name) { + if !internal.ValidTenantTemplateID(name) { return internal.PageTemplate{}, errors.New("invalid template") } if s.cfg.TenantsAdmin == nil { @@ -51,8 +50,8 @@ func (s *Server) resolveTenantTemplate(ctx context.Context, id int64, name strin return internal.PageTemplate{}, err } defer file.Close() - data, err := io.ReadAll(io.LimitReader(file, (1<<20)+1)) - if err != nil || len(data) > 1<<20 || strings.Count(string(data), "") != 1 { + data, err := io.ReadAll(io.LimitReader(file, internal.MaxTenantTemplateBytes+1)) + if err != nil || len(data) > internal.MaxTenantTemplateBytes || strings.Count(string(data), "") != 1 { return internal.PageTemplate{}, errors.New("invalid template shell") } return internal.PageTemplate{ID: name, HTML: string(data)}, nil @@ -71,14 +70,14 @@ func (s *Server) listTenantTemplates(ctx context.Context, id int64) ([]string, e } for _, e := range entries { name := strings.TrimSuffix(e.Name(), ".html") - if !e.IsDir() && strings.HasSuffix(e.Name(), ".html") && templateIDPattern.MatchString(name) { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".html") && internal.ValidTenantTemplateID(name) { if _, err := s.resolveTenantTemplate(ctx, id, name); err == nil { names = append(names, name) } } } for name := range s.cfg.PageTemplates { - if templateIDPattern.MatchString(name) { + if internal.ValidTenantTemplateID(name) { names = append(names, name) } } diff --git a/internal/page_document.go b/internal/page_document.go index 8b0972f..8bda8cb 100644 --- a/internal/page_document.go +++ b/internal/page_document.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "html" + "regexp" "strconv" "strings" "time" @@ -11,6 +12,13 @@ import ( "github.com/GoCodeAlone/workflow-plugin-cms/store" ) +// Tenant template admission is shared by runtime resolution and bundle import. +const MaxTenantTemplateBytes = 1 << 20 + +var tenantTemplateID = regexp.MustCompile(`^[a-z][a-z0-9-]{0,63}$`) + +func ValidTenantTemplateID(id string) bool { return tenantTemplateID.MatchString(id) } + const cmsBodySlot = "" // PageTemplate is the shell used to wrap a CMS page body. diff --git a/promotion/bundle.go b/promotion/bundle.go index d2c1b5f..4b8cb6d 100644 --- a/promotion/bundle.go +++ b/promotion/bundle.go @@ -10,7 +10,6 @@ import ( "os" "path" "path/filepath" - "regexp" "sort" "strings" @@ -109,8 +108,6 @@ func InventoryBundle(root string) (BundleManifest, error) { return m, nil } -var cssReferences = regexp.MustCompile(`(?i)url\(\s*["']?([^\s"')]+)["']?\s*\)|@import\s+["']([^"']+)["']`) - // VerifyBundle re-hashes the supplied directory, validates local references and // rejects exact static route shadowing. Uploaded tenant media and admin paths // remain unsupported; copying their URLs would leak review tenant identity. @@ -125,7 +122,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten files["/"+f.Path] = true } for _, p := range pages { - if p.Validate() != nil { + if !publicPageContent(p) { return ErrBundle } routes[p.Path] = true @@ -136,7 +133,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if files["/"+exact] { return ErrBundle } - if p.TemplateID != "" && (!cleanFile(p.TemplateID) || strings.Contains(p.TemplateID, "/") || !files["/cms/templates/"+p.TemplateID+".html"]) { + if p.TemplateID != "" && (!cms.ValidTenantTemplateID(p.TemplateID) || !files["/cms/templates/"+p.TemplateID+".html"]) { return ErrBundle } } @@ -155,6 +152,11 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if err != nil || u.User != nil { return ErrBundle } + // Unselected shells have no public page base yet. Require explicit + // root-relative or external references rather than guessing a base. + if base == "" && u.Scheme == "" && !strings.HasPrefix(u.Path, "/") { + return ErrBundle + } if strings.ContainsAny(u.Path, "\\\x00") { return ErrBundle } @@ -188,22 +190,9 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten return ErrBundle } checkCSS := func(css, base string) error { - for _, m := range cssReferences.FindAllStringSubmatch(css, -1) { - ref := m[1] - if ref == "" { - ref = m[2] - } - if err := checkRef(ref, base, resourceReference); err != nil { - return err - } - } - return nil + return verifyCSSReferences(css, func(ref string) error { return checkRef(ref, base, resourceReference) }) } checkHTML := func(source, base string) error { - doc, err := html.Parse(strings.NewReader(source)) - if err != nil { - return ErrBundle - } var visit func(*html.Node) error visit = func(n *html.Node) error { if n.Type == html.ElementNode { @@ -279,7 +268,18 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten } return nil } - return visit(doc) + // Both browser modes matter: noscript is raw text with scripting + // enabled, and ordinary fallback markup with scripting disabled. + for _, scripting := range []bool{true, false} { + doc, err := html.ParseWithOptions(strings.NewReader(source), html.ParseOptionEnableScripting(scripting)) + if err != nil { + return ErrBundle + } + if err := visit(doc); err != nil { + return err + } + } + return nil } for _, p := range pages { canonical, err := cms.RenderPageBody(p.Page(1, 1, 1)) @@ -334,28 +334,37 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten } } for _, f := range manifest.Files { - if strings.HasSuffix(f.Path, ".css") || strings.HasSuffix(f.Path, ".html") { + ext := strings.ToLower(path.Ext(f.Path)) + if ext == ".css" || ext == ".html" || ext == ".htm" { b, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(f.Path))) if err != nil { return ErrBundle } - if strings.HasSuffix(f.Path, ".css") { + if ext == ".css" { if err := checkCSS(string(b), "/"+f.Path); err != nil { return err } } else { - if strings.HasPrefix(f.Path, "cms/templates/") && strings.Count(string(b), "") != 1 { - return ErrBundle - } // Shell relative references are interpreted at public page paths. if strings.HasPrefix(f.Path, "cms/templates/") { + name := strings.TrimSuffix(path.Base(f.Path), ".html") + if !cms.ValidTenantTemplateID(name) || path.Dir(f.Path) != "cms/templates" || f.Size > cms.MaxTenantTemplateBytes || strings.Count(string(b), "") != 1 { + return ErrBundle + } + selected := false for _, p := range pages { if p.TemplateID+".html" == path.Base(f.Path) { + selected = true if err := checkHTML(string(b), p.Path); err != nil { return err } } } + if !selected { + if err := checkHTML(string(b), ""); err != nil { + return err + } + } } else if err := checkHTML(string(b), "/"+f.Path); err != nil { return err } @@ -373,3 +382,8 @@ func forbiddenPath(p string) bool { } return false } + +func publicPageContent(p store.PageContent) bool { + decoded, err := url.PathUnescape(p.Path) + return p.Validate() == nil && err == nil && decoded == p.Path && p.Path != "/healthz" && !strings.HasPrefix(p.Path, "/admin") && (!forbiddenPath(p.Path) || p.Path == "/media") && (p.TemplateID == "" || cms.ValidTenantTemplateID(p.TemplateID)) +} diff --git a/promotion/bundle_admission_test.go b/promotion/bundle_admission_test.go new file mode 100644 index 0000000..a7e10dc --- /dev/null +++ b/promotion/bundle_admission_test.go @@ -0,0 +1,151 @@ +package promotion + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +func writeAdmissionFile(t *testing.T, root, name, contents string) { + t.Helper() + if err := os.WriteFile(filepath.Join(root, name), []byte(contents), 0600); err != nil { + t.Fatal(err) + } +} + +func TestBundleRefusesAlternateBrowserResourceSyntax(t *testing.T) { + for _, markup := range []string{ + `

Text

`, + ``, + ``, + ``, + ``, + ``, + ``, + } { + for _, surface := range []string{"body", "selected-shell", "unselected-shell", "static.HTML"} { + t.Run(surface+"/"+markup, func(t *testing.T) { + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + switch surface { + case "body": + p.BodyHTML = markup + case "selected-shell": + writeAdmissionFile(t, root, "cms/templates/main.html", markup+``) + case "unselected-shell": + writeAdmissionFile(t, root, "cms/templates/alternate.html", markup+``) + default: + writeAdmissionFile(t, root, surface, markup) + } + m, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + if VerifyBundle(root, m, []store.PageContent{p}) != ErrBundle { + t.Fatal("browser-resolved private resource admitted") + } + }) + } + } +} + +func TestBundleScansUppercaseStylesheetsAndHTM(t *testing.T) { + for name, contents := range map[string]string{ + "assets/site.CSS": `body{background:url(/media/234/photo.jpg)}`, + "fallback.HTM": ``, + } { + t.Run(name, func(t *testing.T) { + s, root, _, _ := fixture(t) + writeAdmissionFile(t, root, name, contents) + m, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + if VerifyBundle(root, m, []store.PageContent{s.Pages[0].Content}) != ErrBundle { + t.Fatal("case or HTML extension bypass admitted") + } + }) + } +} + +func TestBundleTemplateAdmissionMatchesRuntime(t *testing.T) { + for _, name := range []string{"Main", "1main", strings.Repeat("a", 65)} { + t.Run(name, func(t *testing.T) { + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + p.TemplateID = name + writeAdmissionFile(t, root, "cms/templates/"+name+".html", ``) + m, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + if VerifyBundle(root, m, []store.PageContent{p}) != ErrBundle { + t.Fatal("runtime-invalid template admitted") + } + }) + } + for _, selected := range []bool{true, false} { + s, root, _, _ := fixture(t) + name := "alternate" + if selected { + name = "main" + } + writeAdmissionFile(t, root, "cms/templates/"+name+".html", ``+strings.Repeat(" ", 1<<20)) + m, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + if VerifyBundle(root, m, []store.PageContent{s.Pages[0].Content}) != ErrBundle { + t.Fatal("oversized shell admitted") + } + } +} + +func TestBundleUnselectedShellRequiresKnownReferenceBase(t *testing.T) { + for _, ref := range []string{"assets/photo.jpg", "../assets/photo.jpg", "/assets/photo.jpg"} { + s, root, _, _ := fixture(t) + writeAdmissionFile(t, root, "cms/templates/alternate.html", ``) + m, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + err = VerifyBundle(root, m, []store.PageContent{s.Pages[0].Content}) + if (err == nil) != strings.HasPrefix(ref, "/") { + t.Fatalf("unselected shell base admission mismatch: %v", err) + } + } +} + +func TestBundleRefusesHostReservedPageRoutes(t *testing.T) { + for _, route := range []string{"/healthz", "/admin", "/admin/voice", "/administrator", "/%61dmin/voice", "/%68ealthz", "/api/v1/ingest/release", "/cms/templates/main", "/media/234/photo.jpg", "/media", "/public"} { + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + p.Path = route + p.BodyHTML = "

Page.

" + err := VerifyBundle(root, s.Bundle, []store.PageContent{p}) + allowed := route == "/media" || route == "/public" + if (err == nil) != allowed { + t.Fatalf("reserved route admission mismatch for %s: %v", route, err) + } + } +} + +func TestRestrictedCSSSupportsOrdinaryReferences(t *testing.T) { + for _, css := range []string{ + `body{background:URL(" /assets/photo.jpg ");font-family:"A font"}`, + `@import/**/"/assets/site.css"; body{background:url(/assets/photo.jpg)}`, + `/* u\72l(/media/private) is inert */body{color:#fff}`, + } { + if err := verifyCSSReferences(css, func(ref string) error { + if ref != "/assets/photo.jpg" && ref != "/assets/site.css" { + t.Fatal("incorrect resource token") + } + return nil + }); err != nil { + t.Fatalf("supported CSS refused: %v", err) + } + } +} diff --git a/promotion/css.go b/promotion/css.go new file mode 100644 index 0000000..a693c94 --- /dev/null +++ b/promotion/css.go @@ -0,0 +1,151 @@ +package promotion + +import "strings" + +// This is deliberately a restricted CSS boundary, not a general stylesheet +// parser. Escapes and alternative string-image functions are unsupported and +// refused instead of silently missing a browser-resolved resource reference. +func verifyCSSReferences(source string, check func(string) error) error { + space := func(c byte) bool { return c == ' ' || c == '\t' || c == '\r' || c == '\n' || c == '\f' } + ident := func(c byte) bool { + return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-' || c == '_' || c >= 128 + } + // Preserve strings while replacing comments with whitespace. A backslash + // anywhere outside a comment is outside the supported grammar. + var clean strings.Builder + var quote byte + for i := 0; i < len(source); i++ { + c := source[i] + if quote == 0 && c == '/' && i+1 < len(source) && source[i+1] == '*' { + end := strings.Index(source[i+2:], "*/") + if end < 0 { + return ErrBundle + } + i += end + 3 + clean.WriteByte(' ') + continue + } + if c == '\\' || c == 0 { + return ErrBundle + } + if quote != 0 { + if c == quote { + quote = 0 + } else if c == '\r' || c == '\n' || c == '\f' { + return ErrBundle + } + } else if c == '\'' || c == '"' { + quote = c + } + clean.WriteByte(c) + } + if quote != 0 { + return ErrBundle + } + css := clean.String() + skip := func(i int) int { + for i < len(css) && space(css[i]) { + i++ + } + return i + } + quoted := func(i int) (string, int, error) { + q := css[i] + start := i + 1 + i = start + for i < len(css) && css[i] != q { + i++ + } + if i == len(css) { + return "", i, ErrBundle + } + return css[start:i], i + 1, nil + } + for i := 0; i < len(css); { + if css[i] == '\'' || css[i] == '"' { + _, next, err := quoted(i) + if err != nil { + return err + } + i = next + continue + } + at := css[i] == '@' + if at { + i++ + if i == len(css) { + return ErrBundle + } + } + if !ident(css[i]) { + i++ + continue + } + start := i + for i < len(css) && ident(css[i]) { + i++ + } + name := strings.ToLower(css[start:i]) + next := skip(i) + if at && name == "import" { + if next == len(css) { + return ErrBundle + } + if css[next] == '\'' || css[next] == '"' { + ref, end, err := quoted(next) + if err != nil { + return err + } + if err = check(strings.TrimSpace(ref)); err != nil { + return err + } + i = end + continue + } + // Only a URL function may supply the other supported import form. + if !strings.HasPrefix(strings.ToLower(css[next:]), "url(") { + return ErrBundle + } + i = next + continue + } + if next >= len(css) || css[next] != '(' { + continue + } + switch name { + case "image", "image-set", "-webkit-image-set", "src": + return ErrBundle + case "url": + j := skip(next + 1) + if j == len(css) { + return ErrBundle + } + var ref string + if css[j] == '\'' || css[j] == '"' { + var err error + ref, j, err = quoted(j) + if err != nil { + return err + } + j = skip(j) + } else { + start := j + for j < len(css) && css[j] != ')' { + j++ + } + ref = strings.TrimSpace(css[start:j]) + if strings.ContainsAny(ref, "('\" \t\r\n\f") { + return ErrBundle + } + } + if j == len(css) || css[j] != ')' { + return ErrBundle + } + if err := check(strings.TrimSpace(ref)); err != nil { + return err + } + i = j + 1 + } + } + return nil +} diff --git a/promotion/snapshot.go b/promotion/snapshot.go index 5c9f178..ab12d41 100644 --- a/promotion/snapshot.go +++ b/promotion/snapshot.go @@ -61,7 +61,7 @@ func (s Snapshot) Validate() error { return ErrInvalid } keys[p.Key] = true - if err := p.Content.Validate(); err != nil { + if !publicPageContent(p.Content) { return ErrInvalid } key := p.Content.Subsite + "\x00" + p.Content.Path From 163de284b744629e14e146d53660209371105761 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 9 Oct 2026 06:13:25 -0400 Subject: [PATCH 07/10] fix(promotion): match served MIME and subsite routing --- docs/promotion.md | 6 +++ promotion/bundle.go | 73 ++++++++++++++++++++++-------- promotion/bundle_admission_test.go | 66 +++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 19 deletions(-) diff --git a/docs/promotion.md b/docs/promotion.md index 162eb60..81fa703 100644 --- a/docs/promotion.md +++ b/docs/promotion.md @@ -63,6 +63,12 @@ alternative string-image functions `image()`, `image-set()`, `-webkit-image-set()` and `src()` are unsupported and fail closed. Comments and ordinary text/font strings remain supported; no new CSS parser dependency is installed. +Unknown file extensions are classified using the host's `ServeContent` MIME +sniffing rules; HTML documents receive the same reference checks. SVG, XHTML +and other XML documents are unsupported and refused pending a reviewed parser. +Internal links require a selected page in the source page's own subsite or the +root fallback, matching host routing. A selected sibling-subsite page cannot +make an otherwise unreachable link valid, including the `/media` exception. This is reference validation, not a JavaScript sandbox. The host must review the exact approved bundle's scripts and runtime behavior separately. diff --git a/promotion/bundle.go b/promotion/bundle.go index 4b8cb6d..74f3368 100644 --- a/promotion/bundle.go +++ b/promotion/bundle.go @@ -6,6 +6,8 @@ import ( "encoding/json" "io" "io/fs" + "mime" + "net/http" "net/url" "os" "path" @@ -125,7 +127,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if !publicPageContent(p) { return ErrBundle } - routes[p.Path] = true + routes[p.Subsite+"\x00"+p.Path] = true exact := strings.TrimPrefix(p.Path, "/") if exact == "" { exact = "index.html" @@ -143,7 +145,8 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten navigationReference descriptionReference ) - checkRef := func(raw, base string, kind referenceKind) error { + hasRoute := func(subsite, route string) bool { return routes[subsite+"\x00"+route] || routes["\x00"+route] } + checkRef := func(raw, base, subsite string, kind referenceKind) error { link := kind != resourceReference if raw == "" || strings.HasPrefix(raw, "#") { return nil @@ -168,7 +171,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten // that exact selected route may use it; uploads, resource requests, // absolute private URLs and other private namespaces remain refused. canonicalMediaPath := u.RawPath == "" && (u.Path == "/media" || u.Path == "media") - selectedMediaLink := kind == navigationReference && u.Scheme == "" && u.Host == "" && canonicalMediaPath && resolved == "/media" && routes[resolved] + selectedMediaLink := kind == navigationReference && u.Scheme == "" && u.Host == "" && canonicalMediaPath && resolved == "/media" && hasRoute(subsite, resolved) if forbiddenPath(resolved) && !selectedMediaLink { return ErrBundle } @@ -184,15 +187,15 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if u.Host != "" { return ErrBundle } - if files[resolved] || link && routes[resolved] { + if files[resolved] || link && hasRoute(subsite, resolved) { return nil } return ErrBundle } checkCSS := func(css, base string) error { - return verifyCSSReferences(css, func(ref string) error { return checkRef(ref, base, resourceReference) }) + return verifyCSSReferences(css, func(ref string) error { return checkRef(ref, base, "", resourceReference) }) } - checkHTML := func(source, base string) error { + checkHTML := func(source, base, subsite string) error { var visit func(*html.Node) error visit = func(n *html.Node) error { if n.Type == html.ElementNode { @@ -223,16 +226,16 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if a.Key == "href" && (n.Data == "a" || n.Data == "area") { kind = navigationReference } - if err := checkRef(a.Val, base, kind); err != nil { + if err := checkRef(a.Val, base, subsite, kind); err != nil { return err } case "cite", "longdesc": - if err := checkRef(a.Val, base, descriptionReference); err != nil { + if err := checkRef(a.Val, base, subsite, descriptionReference); err != nil { return err } case "ping": for _, ref := range strings.Fields(a.Val) { - if err := checkRef(ref, base, resourceReference); err != nil { + if err := checkRef(ref, base, subsite, resourceReference); err != nil { return err } } @@ -242,7 +245,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if len(parts) == 0 { return ErrBundle } - if err := checkRef(parts[0], base, resourceReference); err != nil { + if err := checkRef(parts[0], base, subsite, resourceReference); err != nil { return err } } @@ -286,7 +289,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if err != nil { return ErrBundle } - if err = checkHTML(canonical, p.Path); err != nil { + if err = checkHTML(canonical, p.Path, p.Subsite); err != nil { return err } if len(p.BodyBlocks) > 0 { @@ -312,7 +315,7 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten if key == "href" { kind = navigationReference } - if err := checkRef(ref, p.Path, kind); err != nil { + if err := checkRef(ref, p.Path, p.Subsite, kind); err != nil { return err } } @@ -329,18 +332,23 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten return err } } - if err := checkHTML(p.BodyHTML, p.Path); err != nil { + if err := checkHTML(p.BodyHTML, p.Path, p.Subsite); err != nil { return err } } for _, f := range manifest.Files { - ext := strings.ToLower(path.Ext(f.Path)) - if ext == ".css" || ext == ".html" || ext == ".htm" { + contentType, err := servedBundleContentType(root, f.Path) + if err != nil || strings.HasSuffix(contentType, "+xml") || contentType == "text/xml" || contentType == "application/xml" { + // SVG/XHTML/XML can contain nested document resources. They need + // their own reviewed parser; silently skipping them is unsafe. + return ErrBundle + } + if contentType == "text/css" || contentType == "text/html" { b, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(f.Path))) if err != nil { return ErrBundle } - if ext == ".css" { + if contentType == "text/css" { if err := checkCSS(string(b), "/"+f.Path); err != nil { return err } @@ -355,17 +363,17 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten for _, p := range pages { if p.TemplateID+".html" == path.Base(f.Path) { selected = true - if err := checkHTML(string(b), p.Path); err != nil { + if err := checkHTML(string(b), p.Path, p.Subsite); err != nil { return err } } } if !selected { - if err := checkHTML(string(b), ""); err != nil { + if err := checkHTML(string(b), "", ""); err != nil { return err } } - } else if err := checkHTML(string(b), "/"+f.Path); err != nil { + } else if err := checkHTML(string(b), "/"+f.Path, ""); err != nil { return err } } @@ -374,6 +382,33 @@ func VerifyBundle(root string, manifest BundleManifest, pages []store.PageConten return nil } +// Match the actual host: .html is explicitly HTML; ServeContent otherwise +// uses TypeByExtension and then sniffs the first 512 bytes for unknown types. +func servedBundleContentType(root, name string) (string, error) { + value := mime.TypeByExtension(path.Ext(name)) + if strings.EqualFold(path.Ext(name), ".html") { + value = "text/html" + } + if value == "" { + f, err := os.Open(filepath.Join(root, filepath.FromSlash(name))) + if err != nil { + return "", ErrBundle + } + prefix := make([]byte, 512) + n, readErr := io.ReadFull(f, prefix) + closeErr := f.Close() + if readErr != nil && readErr != io.EOF && readErr != io.ErrUnexpectedEOF || closeErr != nil { + return "", ErrBundle + } + value = http.DetectContentType(prefix[:n]) + } + contentType, _, err := mime.ParseMediaType(value) + if err != nil { + return "", ErrBundle + } + return contentType, nil +} + func forbiddenPath(p string) bool { for _, prefix := range []string{"/media", "/api", "/admin", "/cms"} { if p == prefix || strings.HasPrefix(p, prefix+"/") { diff --git a/promotion/bundle_admission_test.go b/promotion/bundle_admission_test.go index a7e10dc..5e164be 100644 --- a/promotion/bundle_admission_test.go +++ b/promotion/bundle_admission_test.go @@ -1,14 +1,80 @@ package promotion import ( + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" "testing" + "time" "github.com/GoCodeAlone/workflow-plugin-cms/store" ) +func TestBundleInspectsDocumentsAsActuallyServed(t *testing.T) { + for _, tc := range []struct{ name, content, mediaType string }{ + {"resource", ``, "text/html"}, + {"resource.unknown-extension", ``, "text/html"}, + {"resource.svg", ``, "image/svg+xml"}, + {"resource.xhtml", ``, "application/xhtml+xml"}, + {"resource-xml", ``, "text/xml"}, + } { + t.Run(tc.name, func(t *testing.T) { + response := httptest.NewRecorder() + http.ServeContent(response, httptest.NewRequest("GET", "/"+tc.name, nil), tc.name, time.Time{}, strings.NewReader(tc.content)) + if !strings.HasPrefix(response.Header().Get("Content-Type"), tc.mediaType) { + t.Fatal("expected served document classification missing") + } + s, root, _, _ := fixture(t) + p := s.Pages[0].Content + p.BodyHTML = `` + writeAdmissionFile(t, root, tc.name, tc.content) + m, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + if VerifyBundle(root, m, []store.PageContent{p}) != ErrBundle { + t.Fatal("unchecked served document admitted") + } + }) + } +} + +func TestBundleLinksRespectSubsiteAndRootFallback(t *testing.T) { + for _, destination := range []string{"/beta-only", "/media"} { + for _, surface := range []string{"body", "blocks", "template"} { + for _, targetSubsite := range []string{"beta", "alpha", ""} { + t.Run(destination+"/"+surface+"/"+targetSubsite, func(t *testing.T) { + s, root, _, _ := fixture(t) + source := s.Pages[0].Content + source.Path, source.Subsite, source.BodyHTML = "/alpha-home", "alpha", "

Alpha.

" + link := `Destination` + target := source + target.Path, target.Subsite = destination, targetSubsite + target.TemplateID = "" + switch surface { + case "body": + source.BodyHTML = link + case "blocks": + source.BodyBlocks = []byte(`{"type":"doc","content":[{"type":"paragraph","content":[{"type":"link","attrs":{"href":"` + destination + `"},"content":[{"type":"text","text":"Destination"}]}]}]}`) + case "template": + writeAdmissionFile(t, root, "cms/templates/main.html", link+``) + } + m, err := InventoryBundle(root) + if err != nil { + t.Fatal(err) + } + err = VerifyBundle(root, m, []store.PageContent{source, target}) + if (err == nil) != (targetSubsite != "beta") { + t.Fatalf("cross-subsite link admission mismatch: %v", err) + } + }) + } + } + } +} + func writeAdmissionFile(t *testing.T, root, name, contents string) { t.Helper() if err := os.WriteFile(filepath.Join(root, name), []byte(contents), 0600); err != nil { From b7aa3336830232c68167134f0391e7fb9c4d928c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 9 Oct 2026 12:19:00 -0400 Subject: [PATCH 08/10] feat(cms): persist saved page history with trusted actor and CAS readback --- README.md | 5 + docs/saved-page-history.md | 91 +++++++ internal/admin_contribution.go | 11 +- internal/admin_http.go | 7 +- internal/page_history.go | 61 +++++ internal/page_history_test.go | 112 ++++++++ store/batch_memory.go | 16 +- store/history.go | 170 ++++++++++++ store/history_test.go | 38 +++ store/page.go | 12 +- store/postgres/batch.go | 8 + store/postgres/batch_integration_test.go | 13 + store/postgres/history.go | 112 ++++++++ store/postgres/history_integration_test.go | 246 ++++++++++++++++++ .../migrations/0002_page_saved_history.up.sql | 27 ++ store/postgres/store.go | 29 ++- store/storetest/history.go | 126 +++++++++ 17 files changed, 1066 insertions(+), 18 deletions(-) create mode 100644 docs/saved-page-history.md create mode 100644 internal/page_history.go create mode 100644 internal/page_history_test.go create mode 100644 store/history.go create mode 100644 store/history_test.go create mode 100644 store/postgres/history.go create mode 100644 store/postgres/history_integration_test.go create mode 100644 store/postgres/migrations/0002_page_saved_history.up.sql create mode 100644 store/storetest/history.go diff --git a/README.md b/README.md index 83c43bf..40b0ea3 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,11 @@ including `body_blocks`, `template_id`, `publish_at`, and `unpublish_at`. `body_blocks` is the canonical editor document when present; `body_html` remains the backward-compatible fallback for older pages. +Saved write history and optimistic save/delete preconditions are documented in +[Saved page history](docs/saved-page-history.md). Adoption requires both explicit +plugin SQL prerequisites and matched host schema/authorization support. It does +not reconstruct earlier edits or unsaved browser drafts. + ## Install ```yaml diff --git a/docs/saved-page-history.md b/docs/saved-page-history.md new file mode 100644 index 0000000..b627557 --- /dev/null +++ b/docs/saved-page-history.md @@ -0,0 +1,91 @@ +# Saved page history and concurrent editing + +This is an additive dependency on the content-promotion implementation. It is +source support, not an applied host migration or runtime adoption. + +Every successful managed page create, update, delete, batch apply and batch +rollback appends one saved history event in the same transaction as the page +mutation and monotonic tenant revision. Each event records the changed pages' +before/after versions and full content (including blocks, HTML, schedules, +template and status), content scope, operation, UTC saved time and actor label. +Deletes retain their former saved content. Unchanged pages are omitted from +batch events. A saved version-only edit is still recorded. + +The host supplies an authenticated stable subject through its existing +AuditActor callback. Direct store clients use WithPageWriteActor only after +their own authentication and authorization. JSON, arbitrary headers and +unverified token claims must never set that context value. Missing or invalid +labels are recorded as unattributed. This metadata grants no permissions. +Digests detect content corruption; they are not signatures or proof of the +actor's authority. The existing audit logger remains separate. + +## Read saved state + +GET /api/v1/admin/tenants/:tid/pages/history accepts canonical after_revision +(exclusive, default 0) and limit (1–100, default 20). Both request authorization +and current tenant access callbacks are required, including direct AdminAPI +use. Configure the host to authorize this route with the same tenant-scoped +page-read authority as ordinary private page reads. Responses are private, +no-store and include the content scope, adoption marker, current tenant +revision, immutable events, next cursor and has_more. They include drafts, so +they must never be exposed on a public content route. + +Use the saved page's ID, Version and content digest to compare a fresh page read +with its latest applicable history event. Read more pages of history until +has_more is false; current_revision is an atomic head for each response. Repeat +from that head if new saved writes occur during pagination. PostgreSQL uses a +read-only repeatable-read transaction; reconnecting does not lose the history. + +PUT and DELETE continue to require the editor's loaded expected_version. +Missing preconditions fail; stale versions return 409 without a page write, +revision increment or history event. The caller must retain the draft and +review a fresh read. Never retry a stale save with a substituted fresh version. + +## Explicit adoption and retention + +1. Review and install the first content-revision SQL prerequisite, then the + additional store/postgres/migrations/0002_page_saved_history.up.sql under + the same exclusive write fence. The second prerequisite records each + tenant's current revision as history_start_revision. It creates no past + actor claims or content snapshots. The first event is the next revision. +2. Ship matched plugin and host authorization/callers. Replace every old page + writer before releasing the fence. Missing history schema or an unrecorded + cooperative-writer revision gap fails closed and rolls back the entire + managed write. Reconcile gaps from trustworthy evidence; never reset the + marker to disguise missing events. +3. Include pages, cms_page_revisions and cms_page_history in database backups. + Verify content scope, start marker, saved head and history on restore. + History is append-only through database UPDATE/DELETE/TRUNCATE triggers. + Tenant deletion is blocked while retained history references its ledger; + privileged retention or erasure requires a separate reviewed procedure. +4. Extend the host's versioned schema and auth-continuity admission explicitly + for this second prerequisite. The existing clean10→clean11 admission + prepared for the first revision ledger does not cover it. No migration, + plugin version pin, release, auth-baseline rewrite or deployment is performed + by this library or change. + +All participating writers must use this store; arbitrary external SQL page +edits are not covered by the cooperative store transaction contract. +Schema owners can bypass triggers, so this is not a tamper-proof audit service. +There is no public restore endpoint or automatic retention deletion. + +## Unsaved drafts + +Database history verifies committed saves after adoption. It cannot reveal +text that exists only in a browser tab, reconstruct pre-adoption editing +history, or guarantee that an owner is not currently typing. An independent +unsaved-draft check needs an explicit editor autosave/presence protocol with +separate draft state, identity, expiry and conflict handling. It must not treat +an empty presence record as permission to overwrite saved content. Current +CAS protects the next saved write regardless of such presence. + +## Validation + +The shared memory/PostgreSQL contract covers snapshots, trusted actor labels, +deletion, pagination, deep-copy isolation, stale-save refusal, batch/rollback +history and replay refusal. Actual PostgreSQL fixtures use unique task-owned +schemas and check reconnect durability, adoption without fabricated backfill, +append-only triggers, missing-schema/mixed-writer fail-closed behavior, +concurrent save winners and transaction rollback after a late history INSERT +failure. HTTP fixtures cover protected readback, tenant denial, unconfigured +auth refusal, canonical queries/routes and ignored arbitrary actor headers. diff --git a/internal/admin_contribution.go b/internal/admin_contribution.go index 0fcdd3e..f7417aa 100644 --- a/internal/admin_contribution.go +++ b/internal/admin_contribution.go @@ -47,10 +47,11 @@ func CMSAdminContributionMetadata(authorized bool) map[string]string { return map[string]string{} } return map[string]string{ - "sites_path": "/api/v1/admin/tenants", - "pages_path": "/api/v1/admin/tenants/{tenant_id}/pages", - "templates_path": "/api/v1/admin/tenants/{tenant_id}/templates", - "overlays_path": "/api/v1/admin/tenants/{tenant_id}/overlays", - "launch_edit_path": "/api/v1/admin/tenants/{tenant_id}/edit-session", + "sites_path": "/api/v1/admin/tenants", + "pages_path": "/api/v1/admin/tenants/{tenant_id}/pages", + "page_history_path": "/api/v1/admin/tenants/{tenant_id}/pages/history", + "templates_path": "/api/v1/admin/tenants/{tenant_id}/templates", + "overlays_path": "/api/v1/admin/tenants/{tenant_id}/overlays", + "launch_edit_path": "/api/v1/admin/tenants/{tenant_id}/edit-session", } } diff --git a/internal/admin_http.go b/internal/admin_http.go index bf6bc5b..809678d 100644 --- a/internal/admin_http.go +++ b/internal/admin_http.go @@ -79,6 +79,9 @@ func (a *AdminAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { writeJSONError(w, 403, "forbidden", "tenant access denied") return } + if a.AuditActor != nil && (r.Method == http.MethodPost || r.Method == http.MethodPut || r.Method == http.MethodDelete) { + r = r.WithContext(store.WithPageWriteActor(r.Context(), a.AuditActor(r))) + } switch { case route == "/reload" && r.Method == "POST": a.reload(w, r) @@ -108,6 +111,8 @@ func (a *AdminAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { a.pageTemplates(w, r, tid) case route == "/pages/permissions" && r.Method == "GET": a.pagePermissions(w, r, tid) + case route == "/pages/history" && r.Method == "GET": + a.pageHistory(w, r, tid) case route == "/overlays/clone" && r.Method == "POST": a.cloneOverlay(w, r, tid) case route == "/overlays/publish" && r.Method == "PUT": @@ -151,7 +156,7 @@ func parseAdminRoute(path string) (route string, tid, rid int64) { } route = "/" + parts[2] + "/" + parts[3] switch route { - case "/pages/preview", "/pages/templates", "/pages/permissions", "/overlays/clone", "/overlays/publish", "/overlays/disable", "/nav/published", "/widgets/render", "/media/validate": + case "/pages/preview", "/pages/templates", "/pages/permissions", "/pages/history", "/overlays/clone", "/overlays/publish", "/overlays/disable", "/nav/published", "/widgets/render", "/media/validate": return route, tid, 0 } if parts[2] != "pages" && parts[2] != "domains" { diff --git a/internal/page_history.go b/internal/page_history.go new file mode 100644 index 0000000..757fa31 --- /dev/null +++ b/internal/page_history.go @@ -0,0 +1,61 @@ +package internal + +import ( + "errors" + "net/http" + "net/url" + "strconv" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +func (a *AdminAPI) pageHistory(w http.ResponseWriter, r *http.Request, tenantID int64) { + w.Header().Set("Cache-Control", "private, no-store") + // Historical drafts contain private content. A new read endpoint must not + // inherit the legacy direct-handler assumption that an outer host gates it. + if a.RequestAccess == nil || a.TenantAccess == nil { + writeJSONError(w, 503, "unavailable", "history authorization not configured") + return + } + history, ok := a.pages.(store.PageHistoryStore) + if !ok { + writeJSONError(w, 503, "unavailable", "page history not configured") + return + } + q := store.PageHistoryQuery{Limit: 20} + query, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + writeJSONError(w, 400, "bad_request", "canonical history query required") + return + } + for key, values := range query { + if len(values) != 1 || (key != "after_revision" && key != "limit") { + writeJSONError(w, 400, "bad_request", "canonical history query required") + return + } + n, err := strconv.ParseInt(values[0], 10, 64) + if err != nil || strconv.FormatInt(n, 10) != values[0] || n < 0 { + writeJSONError(w, 400, "bad_request", "canonical history query required") + return + } + if key == "after_revision" { + q.AfterRevision = n + } else { + if n < 1 || n > 100 { + writeJSONError(w, 400, "bad_request", "history limit must be between 1 and 100") + return + } + q.Limit = int(n) + } + } + h, err := history.ReadPageHistory(r.Context(), tenantID, q) + if errors.Is(err, store.ErrNotFound) { + writeJSONError(w, 404, "not_found", "page history not found") + return + } + if err != nil { + writeJSONError(w, 503, "unavailable", "page history unavailable") + return + } + writeJSON(w, 200, h) +} diff --git a/internal/page_history_test.go b/internal/page_history_test.go new file mode 100644 index 0000000..36abbdd --- /dev/null +++ b/internal/page_history_test.go @@ -0,0 +1,112 @@ +package internal + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +func historyAPI() *AdminAPI { + api := newAdminTestAPI() + api.RequestAccess = func(*http.Request) bool { return true } + api.TenantAccess = func(_ *http.Request, id int64) bool { return id == 234 } + api.AuditActor = func(*http.Request) string { return "user:authenticated-owner" } + return api +} + +func historyResponse(t *testing.T, api *AdminAPI, path string) (*httptest.ResponseRecorder, store.PageHistory) { + t.Helper() + r := httptest.NewRequest(http.MethodGet, path, nil) + w := httptest.NewRecorder() + api.ServeHTTP(w, r) + var h store.PageHistory + if w.Code == 200 && json.Unmarshal(w.Body.Bytes(), &h) != nil { + t.Fatal("invalid history JSON") + } + return w, h +} + +func TestAdminSavedPageHistoryAndTrustedActor(t *testing.T) { + api := historyAPI() + base := "/api/v1/admin/tenants/234/pages" + w, p := doJSON(t, api, http.MethodPost, base, map[string]any{"path": "/rock-metal", "title": "Rock", "body_html": "

Saved

", "status": "draft"}) + if w.Code != 201 { + t.Fatal(w.Code) + } + w, _ = doJSON(t, api, http.MethodPut, base+"/1", map[string]any{"expected_version": p["Version"], "title": "Updated"}) + if w.Code != 200 { + t.Fatal(w.Code) + } + w, h := historyResponse(t, api, base+"/history?limit=1") + if w.Code != 200 || w.Header().Get("Cache-Control") != "private, no-store" || len(h.Entries) != 1 || !h.HasMore || h.CurrentRevision != 2 || h.Entries[0].Actor != "user:authenticated-owner" || h.Entries[0].After[0].Content.BodyHTML != "

Saved

" { + t.Fatal("protected saved history incorrect") + } + w, h = historyResponse(t, api, base+"/history?after_revision=1&limit=1") + if w.Code != 200 || len(h.Entries) != 1 || h.Entries[0].Before[0].Content.Title != "Rock" || h.Entries[0].After[0].Content.Title != "Updated" { + t.Fatal("saved update readback incorrect") + } + // Neither JSON nor an arbitrary actor header can supply trusted identity. + w, _ = doJSON(t, api, http.MethodPut, base+"/1", map[string]any{"expected_version": 2, "title": "Forged", "actor": "superadmin"}) + if w.Code != 400 { + t.Fatal("JSON actor accepted") + } + api.AuditActor = nil + r := httptest.NewRequest(http.MethodPut, base+"/1", strings.NewReader("{\"expected_version\":2,\"title\":\"Header ignored\"}")) + r.Header.Set("X-Actor", "superadmin") + w = httptest.NewRecorder() + api.ServeHTTP(w, r) + if w.Code != 200 { + t.Fatal(w.Code) + } + w, h = historyResponse(t, api, base+"/history?after_revision=2") + if w.Code != 200 || len(h.Entries) != 1 || h.Entries[0].Actor != "unattributed" { + t.Fatal("untrusted header fabricated identity") + } + w, _ = doJSON(t, api, http.MethodDelete, base+"/1", map[string]any{"expected_version": 3}) + if w.Code != 204 { + t.Fatal(w.Code) + } + w, h = historyResponse(t, api, base+"/history?after_revision=3") + if w.Code != 200 || len(h.Entries) != 1 || h.Entries[0].Operation != "delete" || h.Entries[0].Before[0].Content.Title != "Header ignored" { + t.Fatal("deleted page history missing") + } +} + +func TestAdminHistoryFailsClosedAndCanonicalRoutes(t *testing.T) { + path := "/api/v1/admin/tenants/234/pages/history" + for _, missing := range []string{"request", "tenant"} { + api := historyAPI() + if missing == "request" { + api.RequestAccess = nil + } else { + api.TenantAccess = nil + } + w, _ := historyResponse(t, api, path) + if w.Code != 503 { + t.Fatal("unconfigured history authority allowed") + } + } + api := historyAPI() + api.RequestAccess = func(*http.Request) bool { return false } + if w, _ := historyResponse(t, api, path); w.Code != 403 { + t.Fatal("denied request allowed") + } + api = historyAPI() + if w, _ := historyResponse(t, api, "/api/v1/admin/tenants/235/pages/history"); w.Code != 403 { + t.Fatal("foreign tenant history allowed") + } + for _, query := range []string{"?limit=0", "?limit=101", "?limit=01", "?after_revision=-1", "?after_revision=01", "?limit=1&limit=2", "?actor=x", "?limit=1;bad", "?limit=%zz"} { + if w, _ := historyResponse(t, api, path+query); w.Code != 400 { + t.Fatalf("noncanonical query allowed: %s", query) + } + } + for _, bad := range []string{"/api/v1/admin/tenants/0234/pages/history", path + "/1", path + "/history"} { + if w, _ := historyResponse(t, api, bad); w.Code != 404 { + t.Fatal("noncanonical history route allowed") + } + } +} diff --git a/store/batch_memory.go b/store/batch_memory.go index 7486816..b954294 100644 --- a/store/batch_memory.go +++ b/store/batch_memory.go @@ -24,23 +24,23 @@ func (s *MemoryPageStore) ReadPageState(_ context.Context, tenantID int64) (Page return PageSet{Scope: s.scopeLocked(tenantID), Revision: s.revisions[tenantID], Pages: s.statesLocked(tenantID)}, nil } -func (s *MemoryPageStore) ApplyPageBatch(_ context.Context, tenantID int64, batch PageBatch) (PageBatchReceipt, error) { +func (s *MemoryPageStore) ApplyPageBatch(ctx context.Context, tenantID int64, batch PageBatch) (PageBatchReceipt, error) { s.mu.Lock() defer s.mu.Unlock() - return s.applyBatchLocked(tenantID, batch, false) + return s.applyBatchLocked(ctx, tenantID, batch, false) } -func (s *MemoryPageStore) RollbackPageBatch(_ context.Context, tenantID int64, receipt PageBatchReceipt) (PageBatchReceipt, error) { +func (s *MemoryPageStore) RollbackPageBatch(ctx context.Context, tenantID int64, receipt PageBatchReceipt) (PageBatchReceipt, error) { batch, err := RollbackBatch(receipt) if err != nil { return PageBatchReceipt{}, err } s.mu.Lock() defer s.mu.Unlock() - return s.applyBatchLocked(tenantID, batch, true) + return s.applyBatchLocked(ctx, tenantID, batch, true) } -func (s *MemoryPageStore) applyBatchLocked(tenantID int64, batch PageBatch, restore bool) (PageBatchReceipt, error) { +func (s *MemoryPageStore) applyBatchLocked(ctx context.Context, tenantID int64, batch PageBatch, restore bool) (PageBatchReceipt, error) { if tenantID <= 0 { return PageBatchReceipt{}, ErrBatchInvalid } @@ -81,6 +81,12 @@ func (s *MemoryPageStore) applyBatchLocked(tenantID int64, batch PageBatch, rest result.After = s.statesLocked(tenantID) s.revisions[tenantID]++ result.AfterRevision = s.revisions[tenantID] + old, saved := ChangedPageStates(result.Before, result.After) + operation := "batch.apply" + if restore { + operation = "batch.rollback" + } + s.recordHistoryLocked(ctx, tenantID, operation, old, saved) result.Seal() return result, nil } diff --git a/store/history.go b/store/history.go new file mode 100644 index 0000000..b389f76 --- /dev/null +++ b/store/history.go @@ -0,0 +1,170 @@ +package store + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "strings" + "time" + "unicode" +) + +var ErrHistoryUnavailable = errors.New("page history unavailable; no write committed") +var ErrHistoryQuery = errors.New("invalid page history query") + +type pageWriteActorKey struct{} + +// WithPageWriteActor carries the host's authenticated stable identity into a +// store transaction. It grants no authority. Never populate it from a request +// body, arbitrary header, or unverified token claim. +func WithPageWriteActor(ctx context.Context, actor string) context.Context { + actor = strings.TrimSpace(actor) + if actor == "" || len(actor) > 256 || strings.ContainsFunc(actor, unicode.IsControl) { + actor = "unattributed" + } + return context.WithValue(ctx, pageWriteActorKey{}, actor) +} + +func pageWriteActor(ctx context.Context) string { + if actor, ok := ctx.Value(pageWriteActorKey{}).(string); ok { + return actor + } + return "unattributed" +} + +// PageHistoryEntry records only pages touched by one committed write. Deleted +// pages remain in Before. Versions, blocks, schedules and templates are saved +// content; Scope and Revision identify the durable tenant content namespace. +// Digest detects corruption; it is not a signature or proof of authorization. +type PageHistoryEntry struct { + Scope string `json:"scope"` + Revision int64 `json:"revision"` + CommittedAt time.Time `json:"committed_at"` + Actor string `json:"actor"` + Operation string `json:"operation"` + Before []PageState `json:"before"` + After []PageState `json:"after"` + Digest string `json:"digest"` +} + +func (e PageHistoryEntry) ContentDigest() string { + // Bind semantic content digests so JSONB key ordering does not break reads. + v := struct { + Scope string `json:"scope"` + Revision int64 `json:"revision"` + CommittedAt time.Time `json:"committed_at"` + Actor string `json:"actor"` + Operation string `json:"operation"` + Before []PageBaseline `json:"before"` + After []PageBaseline `json:"after"` + }{e.Scope, e.Revision, e.CommittedAt.UTC(), e.Actor, e.Operation, Baseline(e.Before), Baseline(e.After)} + b, _ := json.Marshal(v) + h := sha256.Sum256(b) + return hex.EncodeToString(h[:]) +} + +func NewPageHistoryEntry(ctx context.Context, scope string, revision int64, operation string, before, after []PageState, now time.Time) PageHistoryEntry { + e := PageHistoryEntry{scope, revision, now.UTC().Truncate(time.Microsecond), pageWriteActor(ctx), operation, cloneStates(before), cloneStates(after), ""} + e.Digest = e.ContentDigest() + return e +} + +func cloneStates(states []PageState) []PageState { + out := make([]PageState, 0, len(states)) + for _, p := range states { + out = append(out, PageState{p.ID, p.Version, ContentOf(p.Content.Page(1, p.ID, p.Version))}) + } + return out +} + +// ChangedPageStates includes version-only saves and excludes untouched pages. +func ChangedPageStates(before, after []PageState) ([]PageState, []PageState) { + b, a := map[int64]PageState{}, map[int64]PageState{} + for _, p := range before { + b[p.ID] = p + } + for _, p := range after { + a[p.ID] = p + } + changed := func(p PageState, other map[int64]PageState) bool { + o, ok := other[p.ID] + return !ok || p.Version != o.Version || p.Content.Digest() != o.Content.Digest() + } + var old, saved []PageState + for _, p := range before { + if changed(p, a) { + old = append(old, p) + } + } + for _, p := range after { + if changed(p, b) { + saved = append(saved, p) + } + } + return cloneStates(old), cloneStates(saved) +} + +type PageHistoryQuery struct { + AfterRevision int64 + Limit int +} + +func (q PageHistoryQuery) Validate() error { + if q.AfterRevision < 0 || q.Limit < 1 || q.Limit > 100 { + return ErrHistoryQuery + } + return nil +} + +// HistoryStartRevision is the head at adoption. No entries before it are +// claimed or fabricated. CurrentRevision is read atomically with this page of +// history. NextRevision is an exclusive cursor; HasMore signals another page. +type PageHistory struct { + Scope string `json:"scope"` + HistoryStartRevision int64 `json:"history_start_revision"` + CurrentRevision int64 `json:"current_revision"` + Entries []PageHistoryEntry `json:"entries"` + NextRevision int64 `json:"next_revision"` + HasMore bool `json:"has_more"` +} + +type PageHistoryStore interface { + ReadPageHistory(context.Context, int64, PageHistoryQuery) (PageHistory, error) +} + +func (s *MemoryPageStore) recordHistoryLocked(ctx context.Context, tenantID int64, operation string, before, after []PageState) { + s.history[tenantID] = append(s.history[tenantID], NewPageHistoryEntry(ctx, s.scopeLocked(tenantID), s.revisions[tenantID], operation, before, after, time.Now())) +} + +func (s *MemoryPageStore) ReadPageHistory(_ context.Context, tenantID int64, q PageHistoryQuery) (PageHistory, error) { + if tenantID <= 0 { + return PageHistory{}, ErrNotFound + } + if err := q.Validate(); err != nil { + return PageHistory{}, err + } + s.mu.RLock() + defer s.mu.RUnlock() + if s.scopes[tenantID] == "" { + return PageHistory{}, ErrNotFound + } + h := PageHistory{Scope: s.scopes[tenantID], CurrentRevision: s.revisions[tenantID], Entries: []PageHistoryEntry{}, NextRevision: q.AfterRevision} + for _, entry := range s.history[tenantID] { + if entry.Revision <= q.AfterRevision { + continue + } + if len(h.Entries) == q.Limit { + h.HasMore = true + break + } + e := entry + e.Before, e.After = cloneStates(e.Before), cloneStates(e.After) + h.Entries = append(h.Entries, e) + h.NextRevision = e.Revision + } + return h, nil +} + +var _ PageHistoryStore = (*MemoryPageStore)(nil) diff --git a/store/history_test.go b/store/history_test.go new file mode 100644 index 0000000..d0d96a8 --- /dev/null +++ b/store/history_test.go @@ -0,0 +1,38 @@ +package store_test + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" + "github.com/GoCodeAlone/workflow-plugin-cms/store/storetest" +) + +func TestMemorySavedPageHistory(t *testing.T) { + storetest.PageHistory(t, store.NewMemoryPageStore(), 1, 2) +} + +func TestHistoryActorAndSemanticDigest(t *testing.T) { + for _, actor := range []string{"", "\n", "user\x00injected", strings.Repeat("x", 257)} { + e := store.NewPageHistoryEntry(store.WithPageWriteActor(context.Background(), actor), "scope", 1, "create", nil, nil, time.Now()) + if e.Actor != "unattributed" { + t.Fatal("invalid actor accepted") + } + } + e := store.NewPageHistoryEntry(context.Background(), "scope", 1, "create", nil, nil, time.Now()) + if e.Actor != "unattributed" { + t.Fatal("missing identity fabricated") + } + changed := e + changed.Actor = "other" + if e.ContentDigest() == changed.ContentDigest() { + t.Fatal("actor not bound by digest") + } + changed = e + changed.Revision++ + if e.ContentDigest() == changed.ContentDigest() { + t.Fatal("revision not bound by digest") + } +} diff --git a/store/page.go b/store/page.go index e3b0e3e..6a42c20 100644 --- a/store/page.go +++ b/store/page.go @@ -118,11 +118,12 @@ type MemoryPageStore struct { pages map[int64]*Page // by ID revisions map[int64]int64 scopes map[int64]string + history map[int64][]PageHistoryEntry } // NewMemoryPageStore returns an empty in-memory store. func NewMemoryPageStore() *MemoryPageStore { - return &MemoryPageStore{pages: map[int64]*Page{}, revisions: map[int64]int64{}, scopes: map[int64]string{}} + return &MemoryPageStore{pages: map[int64]*Page{}, revisions: map[int64]int64{}, scopes: map[int64]string{}, history: map[int64][]PageHistoryEntry{}} } func (s *MemoryPageStore) scopeLocked(tenantID int64) string { @@ -134,7 +135,7 @@ func (s *MemoryPageStore) scopeLocked(tenantID int64) string { // Create inserts the page. Returns ErrPathConflict if (tenant, subsite, // path) is already taken. -func (s *MemoryPageStore) Create(_ context.Context, tenantID int64, p *Page) error { +func (s *MemoryPageStore) Create(ctx context.Context, tenantID int64, p *Page) error { if p == nil { return errors.New("page: nil") } @@ -163,6 +164,7 @@ func (s *MemoryPageStore) Create(_ context.Context, tenantID int64, p *Page) err s.pages[p.ID] = &cp s.scopeLocked(tenantID) s.revisions[tenantID]++ + s.recordHistoryLocked(ctx, tenantID, "create", nil, States([]*Page{&cp})) return nil } @@ -198,7 +200,7 @@ func (s *MemoryPageStore) GetByPath(_ context.Context, tenantID int64, subsite, // Update writes the page. Returns ErrNotFound if no record matches // (tenant_id, id). Bumps Version + UpdatedAt. -func (s *MemoryPageStore) Update(_ context.Context, tenantID int64, p *Page) error { +func (s *MemoryPageStore) Update(ctx context.Context, tenantID int64, p *Page) error { if p == nil { return errors.New("page: nil") } @@ -229,11 +231,12 @@ func (s *MemoryPageStore) Update(_ context.Context, tenantID int64, p *Page) err s.pages[p.ID] = &cp s.scopeLocked(tenantID) s.revisions[tenantID]++ + s.recordHistoryLocked(ctx, tenantID, "update", States([]*Page{existing}), States([]*Page{&cp})) return nil } // Delete removes the page. ErrNotFound on miss / wrong tenant. -func (s *MemoryPageStore) Delete(_ context.Context, tenantID, id int64, expectedVersion int) error { +func (s *MemoryPageStore) Delete(ctx context.Context, tenantID, id int64, expectedVersion int) error { s.mu.Lock() defer s.mu.Unlock() existing, ok := s.pages[id] @@ -246,6 +249,7 @@ func (s *MemoryPageStore) Delete(_ context.Context, tenantID, id int64, expected delete(s.pages, id) s.scopeLocked(tenantID) s.revisions[tenantID]++ + s.recordHistoryLocked(ctx, tenantID, "delete", States([]*Page{existing}), nil) return nil } diff --git a/store/postgres/batch.go b/store/postgres/batch.go index 05f4e83..600a517 100644 --- a/store/postgres/batch.go +++ b/store/postgres/batch.go @@ -195,6 +195,14 @@ func (s *Store) applyBatch(ctx context.Context, tenantID int64, batch store.Page if err != nil { return store.PageBatchReceipt{}, err } + old, saved := store.ChangedPageStates(result.Before, result.After) + operation := "batch.apply" + if restore { + operation = "batch.rollback" + } + if err := appendPageHistory(ctx, tx, tenantID, result.AfterRevision, operation, old, saved); err != nil { + return store.PageBatchReceipt{}, err + } if err = tx.Commit(ctx); err != nil { return store.PageBatchReceipt{}, store.ErrBatchInvalid } diff --git a/store/postgres/batch_integration_test.go b/store/postgres/batch_integration_test.go index aa13f1d..b88abe7 100644 --- a/store/postgres/batch_integration_test.go +++ b/store/postgres/batch_integration_test.go @@ -17,6 +17,10 @@ import ( // Uses a uniquely named schema and drops only that schema. Never uses the // application's search_path/tables. Connection values are not printed. func isolatedPostgres(t *testing.T) *Store { + return isolatedPostgresHistory(t, true) +} + +func isolatedPostgresHistory(t *testing.T, includeHistory bool) *Store { t.Helper() uri := os.Getenv("CMS_TEST_DATABASE_URL") if uri == "" { @@ -60,6 +64,15 @@ func isolatedPostgres(t *testing.T) *Store { if _, err = pool.Exec(ctx, string(migration)); err != nil { t.Fatal("isolated revision migration failed") } + if includeHistory { + historyMigration, err := os.ReadFile("migrations/0002_page_saved_history.up.sql") + if err != nil { + t.Fatal("required history test migration unavailable") + } + if _, err = pool.Exec(ctx, string(historyMigration)); err != nil { + t.Fatal("isolated history migration failed") + } + } return NewWithPool(pool) } diff --git a/store/postgres/history.go b/store/postgres/history.go new file mode 100644 index 0000000..d2dd627 --- /dev/null +++ b/store/postgres/history.go @@ -0,0 +1,112 @@ +package postgres + +import ( + "context" + "encoding/json" + "errors" + "time" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" + "github.com/jackc/pgx/v5" +) + +func pageBefore(ctx context.Context, tx pgx.Tx, tenantID, pageID int64) ([]store.PageState, error) { + p := &store.Page{} + err := scanPage(tx.QueryRow(ctx, pageSelectSQL()+` WHERE tenant_id=$1 AND id=$2 FOR UPDATE`, tenantID, pageID), p) + if errors.Is(err, pgx.ErrNoRows) { + return nil, store.ErrNotFound + } + if err != nil { + return nil, err + } + return store.States([]*store.Page{p}), nil +} + +func appendPageHistory(ctx context.Context, tx pgx.Tx, tenantID, revision int64, operation string, before, after []store.PageState) error { + var scope string + if err := tx.QueryRow(ctx, `SELECT content_scope FROM cms_page_revisions WHERE tenant_id=$1`, tenantID).Scan(&scope); err != nil { + return store.ErrHistoryUnavailable + } + e := store.NewPageHistoryEntry(ctx, scope, revision, operation, before, after, time.Now()) + old, err := json.Marshal(e.Before) + if err != nil { + return store.ErrHistoryUnavailable + } + saved, err := json.Marshal(e.After) + if err != nil { + return store.ErrHistoryUnavailable + } + // Refuse gaps caused by mixed old/new cooperative writers. Adoption starts + // at the explicit marker; later appends require the previous saved event. + tag, err := tx.Exec(ctx, `INSERT INTO cms_page_history (tenant_id,content_scope,revision,committed_at,actor,operation,before_pages,after_pages,digest) + SELECT tenant_id,content_scope,$2,$3,$4,$5,$6,$7,$8 FROM cms_page_revisions + WHERE tenant_id=$1 AND revision=$2 AND + (history_start_revision=$2-1 OR EXISTS(SELECT 1 FROM cms_page_history h WHERE h.tenant_id=$1 AND h.content_scope=cms_page_revisions.content_scope AND h.revision=$2-1))`, tenantID, revision, e.CommittedAt, e.Actor, e.Operation, old, saved, e.Digest) + if err != nil || tag.RowsAffected() != 1 { + return store.ErrHistoryUnavailable + } + return nil +} + +func (s *Store) ReadPageHistory(ctx context.Context, tenantID int64, q store.PageHistoryQuery) (store.PageHistory, error) { + if tenantID <= 0 { + return store.PageHistory{}, store.ErrNotFound + } + if err := q.Validate(); err != nil { + return store.PageHistory{}, err + } + // A read-only consistent snapshot does not take the page writer lock. + tx, err := s.pool.BeginTx(ctx, pgx.TxOptions{IsoLevel: pgx.RepeatableRead, AccessMode: pgx.ReadOnly}) + if err != nil { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + defer tx.Rollback(ctx) + h := store.PageHistory{Entries: []store.PageHistoryEntry{}, NextRevision: q.AfterRevision} + err = tx.QueryRow(ctx, `SELECT content_scope,history_start_revision,revision FROM cms_page_revisions WHERE tenant_id=$1`, tenantID).Scan(&h.Scope, &h.HistoryStartRevision, &h.CurrentRevision) + if errors.Is(err, pgx.ErrNoRows) { + return store.PageHistory{}, store.ErrNotFound + } + if err != nil || h.HistoryStartRevision > h.CurrentRevision { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + var head int64 + if err := tx.QueryRow(ctx, `SELECT COALESCE(MAX(revision),$2) FROM cms_page_history WHERE tenant_id=$1 AND content_scope=$3`, tenantID, h.HistoryStartRevision, h.Scope).Scan(&head); err != nil || head != h.CurrentRevision { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + rows, err := tx.Query(ctx, `SELECT content_scope,revision,committed_at,actor,operation,before_pages,after_pages,digest FROM cms_page_history WHERE tenant_id=$1 AND revision>$2 ORDER BY revision LIMIT $3`, tenantID, q.AfterRevision, q.Limit+1) + if err != nil { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + defer rows.Close() + previous := q.AfterRevision + if previous < h.HistoryStartRevision { + previous = h.HistoryStartRevision + } + for rows.Next() { + var e store.PageHistoryEntry + var before, after []byte + if err := rows.Scan(&e.Scope, &e.Revision, &e.CommittedAt, &e.Actor, &e.Operation, &before, &after, &e.Digest); err != nil || json.Unmarshal(before, &e.Before) != nil || json.Unmarshal(after, &e.After) != nil { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + if e.Scope != h.Scope || e.Revision != previous+1 || e.Digest != e.ContentDigest() { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + previous = e.Revision + if len(h.Entries) == q.Limit { + h.HasMore = true + break + } + h.Entries = append(h.Entries, e) + h.NextRevision = e.Revision + } + if rows.Err() != nil { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + rows.Close() + if err := tx.Commit(ctx); err != nil { + return store.PageHistory{}, store.ErrHistoryUnavailable + } + return h, nil +} + +var _ store.PageHistoryStore = (*Store)(nil) diff --git a/store/postgres/history_integration_test.go b/store/postgres/history_integration_test.go new file mode 100644 index 0000000..925d1aa --- /dev/null +++ b/store/postgres/history_integration_test.go @@ -0,0 +1,246 @@ +package postgres + +import ( + "context" + "errors" + "os" + "reflect" + "sync" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" + "github.com/GoCodeAlone/workflow-plugin-cms/store/storetest" + "github.com/jackc/pgx/v5/pgxpool" +) + +func historyTenant(t *testing.T, s *Store, slug string) int64 { + t.Helper() + tenant := &store.Tenant{Slug: slug} + if err := s.CreateTenant(context.Background(), tenant); err != nil { + t.Fatal("isolated tenant failed") + } + return tenant.ID +} + +func TestPostgresSavedPageHistory(t *testing.T) { + s := isolatedPostgres(t) + tenantID := historyTenant(t, s, "saved-history") + otherID := historyTenant(t, s, "other-history") + storetest.PageHistory(t, s, tenantID, otherID) + before, err := s.ReadPageHistory(context.Background(), tenantID, store.PageHistoryQuery{Limit: 100}) + if err != nil { + t.Fatal(err) + } + // Reconnect to the same task-owned schema: deleted snapshots and actor + // history must survive loss of all in-memory store state. + pool, err := pgxpool.NewWithConfig(context.Background(), s.pool.Config()) + if err != nil { + t.Fatal("fresh isolated pool failed") + } + defer pool.Close() + after, err := NewWithPool(pool).ReadPageHistory(context.Background(), tenantID, store.PageHistoryQuery{Limit: 100}) + if err != nil || !reflect.DeepEqual(before, after) { + t.Fatal("saved history lost after reconnect") + } + for _, query := range []string{ + "UPDATE cms_page_history SET actor='rewritten'", + "DELETE FROM cms_page_history", + "TRUNCATE cms_page_history", + } { + if _, err := s.pool.Exec(context.Background(), query); err == nil { + t.Fatal("append-only history rewritable") + } + } +} + +func TestPostgresHistoryAdoptionDoesNotInventPast(t *testing.T) { + s := isolatedPostgresHistory(t, false) + ctx := context.Background() + tenantID := historyTenant(t, s, "existing-before-history") + // Simulate the previous released store before history is installed. + var pageID int64 + if err := s.pool.QueryRow(ctx, "INSERT INTO pages(tenant_id,path,title,status) VALUES($1,'/existing','Existing','draft') RETURNING id", tenantID).Scan(&pageID); err != nil { + t.Fatal("prehistory fixture failed") + } + if _, err := s.pool.Exec(ctx, "UPDATE cms_page_revisions SET revision=7 WHERE tenant_id=$1", tenantID); err != nil { + t.Fatal("prehistory ledger failed") + } + migration, err := os.ReadFile("migrations/0002_page_saved_history.up.sql") + if err != nil { + t.Fatal(err) + } + if _, err := s.pool.Exec(ctx, string(migration)); err != nil { + t.Fatal("isolated history adoption failed") + } + h, err := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 20}) + if err != nil || h.HistoryStartRevision != 7 || h.CurrentRevision != 7 || len(h.Entries) != 0 { + t.Fatal("historical actors or snapshots fabricated") + } + p, err := s.Get(ctx, tenantID, pageID) + if err != nil { + t.Fatal(err) + } + p.Title = "First recorded save" + if err := s.Update(store.WithPageWriteActor(ctx, "user:first-recorded"), tenantID, p); err != nil { + t.Fatal(err) + } + h, err = s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 20}) + if err != nil || len(h.Entries) != 1 || h.Entries[0].Revision != 8 || h.Entries[0].Before[0].Content.Title != "Existing" || h.Entries[0].Actor != "user:first-recorded" { + t.Fatal("adoption marker or first saved event incorrect") + } +} + +func TestPostgresHistoryFailureRollsBackEveryWrite(t *testing.T) { + s := isolatedPostgres(t) + ctx := context.Background() + tenantID := historyTenant(t, s, "history-failure") + p := &store.Page{Path: "/", Title: "Before", Status: store.StatusDraft} + if err := s.Create(ctx, tenantID, p); err != nil { + t.Fatal(err) + } + state, err := s.ReadPageState(ctx, tenantID) + if err != nil { + t.Fatal(err) + } + content := store.PageContent{Path: "/batch", Title: "Batch", Status: store.StatusDraft} + receipt, err := s.ApplyPageBatch(ctx, tenantID, store.BatchFor(state, []store.PageMutation{{Key: "created", Kind: "create", Content: &content}})) + if err != nil { + t.Fatal(err) + } + before, err := s.ReadPageState(ctx, tenantID) + if err != nil { + t.Fatal(err) + } + if _, err := s.pool.Exec(ctx, "DROP TABLE cms_page_history"); err != nil { + t.Fatal("isolated missing-history fixture failed") + } + p.Title = "Must not save" + if err := s.Update(ctx, tenantID, p); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("update bypassed missing history") + } + if err := s.Delete(ctx, tenantID, p.ID, 1); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("delete bypassed missing history") + } + if err := s.Create(ctx, tenantID, &store.Page{Path: "/new", Title: "Must not save", Status: store.StatusDraft}); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("create bypassed missing history") + } + content.Path = "/must-not-save" + if _, err := s.ApplyPageBatch(ctx, tenantID, store.BatchFor(before, []store.PageMutation{{Key: "refused", Kind: "create", Content: &content}})); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("batch bypassed missing history") + } + if _, err := s.RollbackPageBatch(ctx, tenantID, receipt); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("rollback bypassed missing history") + } + after, err := s.ReadPageState(ctx, tenantID) + if err != nil || !reflect.DeepEqual(before, after) { + t.Fatal("history failure partly committed page or revision") + } + if _, err := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 20}); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("missing history reported as empty complete history") + } +} + +func TestPostgresMixedWriterGapRefuses(t *testing.T) { + s := isolatedPostgres(t) + ctx := context.Background() + tenantID := historyTenant(t, s, "mixed-writer") + p := &store.Page{Path: "/", Title: "Before", Status: store.StatusDraft} + if err := s.Create(ctx, tenantID, p); err != nil { + t.Fatal(err) + } + // A superseded writer advanced only the prior revision ledger. + if _, err := s.pool.Exec(ctx, "UPDATE cms_page_revisions SET revision=revision+1 WHERE tenant_id=$1", tenantID); err != nil { + t.Fatal("isolated mixed-writer fixture failed") + } + p.Title = "Must refuse" + if err := s.Update(ctx, tenantID, p); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("unrecorded revision gap accepted") + } + got, err := s.Get(ctx, tenantID, p.ID) + if err != nil || got.Title != "Before" || got.Version != 1 { + t.Fatal("gap refusal committed content") + } + if _, err := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 20}); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("gap concealed by history reader") + } +} + +func TestPostgresConcurrentSaveRecordsOneWinner(t *testing.T) { + s := isolatedPostgres(t) + ctx := context.Background() + tenantID := historyTenant(t, s, "concurrent-history") + p := &store.Page{Path: "/", Title: "Loaded", Status: store.StatusDraft} + if err := s.Create(ctx, tenantID, p); err != nil { + t.Fatal(err) + } + ready := make(chan struct{}) + results := make(chan error, 2) + var group sync.WaitGroup + for _, title := range []string{"Owner save", "Agent save"} { + loaded, err := s.Get(ctx, tenantID, p.ID) + if err != nil { + t.Fatal(err) + } + loaded.Title = title + group.Add(1) + go func(page *store.Page) { + defer group.Done() + <-ready + results <- s.Update(store.WithPageWriteActor(ctx, page.Title), tenantID, page) + }(loaded) + } + close(ready) + group.Wait() + close(results) + saved, refused := 0, 0 + for err := range results { + if err == nil { + saved++ + } else if errors.Is(err, store.ErrVersionConflict) { + refused++ + } else { + t.Fatal(err) + } + } + h, err := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 20}) + got, getErr := s.Get(ctx, tenantID, p.ID) + if saved != 1 || refused != 1 || err != nil || getErr != nil || len(h.Entries) != 2 || h.CurrentRevision != 2 || h.Entries[1].After[0].Content.Title != got.Title || h.Entries[1].Actor != got.Title { + t.Fatal("concurrent save overwrote winner or recorded a refused write") + } +} + +func TestPostgresHistoryInsertFailureIsAtomic(t *testing.T) { + s := isolatedPostgres(t) + ctx := context.Background() + tenantID := historyTenant(t, s, "insert-history-failure") + p := &store.Page{Path: "/", Title: "Before", Status: store.StatusDraft} + if err := s.Create(ctx, tenantID, p); err != nil { + t.Fatal(err) + } + before, err := s.ReadPageState(ctx, tenantID) + if err != nil { + t.Fatal(err) + } + historyBefore, err := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 20}) + if err != nil { + t.Fatal(err) + } + // Inject a late failure at the actual history INSERT boundary after all + // batch page writes and the tenant revision increment have executed. + _, err = s.pool.Exec(ctx, "CREATE FUNCTION refuse_history_insert() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RAISE EXCEPTION 'fixture refusal'; END $$; CREATE TRIGGER refuse_history_insert BEFORE INSERT ON cms_page_history FOR EACH ROW EXECUTE FUNCTION refuse_history_insert();") + if err != nil { + t.Fatal("isolated history failure trigger failed") + } + update := store.ContentOf(p) + update.Title = "Must roll back" + create := store.PageContent{Path: "/new", Title: "Must also roll back", Status: store.StatusDraft} + batch := store.BatchFor(before, []store.PageMutation{{Key: "update", Kind: "update", TargetID: p.ID, ExpectedVersion: p.Version, Content: &update}, {Key: "create", Kind: "create", Content: &create}}) + if _, err := s.ApplyPageBatch(ctx, tenantID, batch); !errors.Is(err, store.ErrHistoryUnavailable) { + t.Fatal("late history failure not refused") + } + after, err := s.ReadPageState(ctx, tenantID) + historyAfter, historyErr := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 20}) + if err != nil || historyErr != nil || !reflect.DeepEqual(before, after) || !reflect.DeepEqual(historyBefore, historyAfter) { + t.Fatal("late history failure partially committed") + } +} diff --git a/store/postgres/migrations/0002_page_saved_history.up.sql b/store/postgres/migrations/0002_page_saved_history.up.sql new file mode 100644 index 0000000..c3a19d7 --- /dev/null +++ b/store/postgres/migrations/0002_page_saved_history.up.sql @@ -0,0 +1,27 @@ +-- Explicit additional host prerequisite, after 0001 and under a write fence. +-- No fabricated backfill: history begins after each current tenant revision. +BEGIN; +ALTER TABLE cms_page_revisions ADD COLUMN history_start_revision BIGINT NOT NULL DEFAULT 0 CHECK (history_start_revision >= 0 AND history_start_revision <= revision); +UPDATE cms_page_revisions SET history_start_revision = revision; +CREATE TABLE cms_page_history ( + tenant_id BIGINT NOT NULL REFERENCES cms_page_revisions(tenant_id), + content_scope TEXT NOT NULL, + revision BIGINT NOT NULL CHECK (revision > 0), + committed_at TIMESTAMPTZ NOT NULL, + actor TEXT NOT NULL CHECK (length(actor) BETWEEN 1 AND 256), + operation TEXT NOT NULL CHECK (operation IN ('create','update','delete','batch.apply','batch.rollback')), + before_pages JSONB NOT NULL CHECK (jsonb_typeof(before_pages) = 'array'), + after_pages JSONB NOT NULL CHECK (jsonb_typeof(after_pages) = 'array'), + digest TEXT NOT NULL CHECK (digest ~ '^[0-9a-f]{64}$'), + PRIMARY KEY (tenant_id, revision) +); +CREATE FUNCTION cms_refuse_history_rewrite() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + RAISE EXCEPTION 'saved page history is append-only'; +END; +$$; +CREATE TRIGGER cms_refuse_history_rewrite BEFORE UPDATE OR DELETE ON cms_page_history + FOR EACH ROW EXECUTE FUNCTION cms_refuse_history_rewrite(); +CREATE TRIGGER cms_refuse_history_truncate BEFORE TRUNCATE ON cms_page_history + FOR EACH STATEMENT EXECUTE FUNCTION cms_refuse_history_rewrite(); +COMMIT; diff --git a/store/postgres/store.go b/store/postgres/store.go index 4ceaed0..4d25840 100644 --- a/store/postgres/store.go +++ b/store/postgres/store.go @@ -245,7 +245,11 @@ func (s *Store) Create(ctx context.Context, tenantID int64, p *store.Page) error if err = createPage(ctx, tx, tenantID, p); err != nil { return err } - if _, err = advanceRevision(ctx, tx, tenantID); err != nil { + revision, err := advanceRevision(ctx, tx, tenantID) + if err != nil { + return err + } + if err := appendPageHistory(ctx, tx, tenantID, revision, "create", nil, store.States([]*store.Page{p})); err != nil { return err } return tx.Commit(ctx) @@ -310,15 +314,26 @@ func (s *Store) GetByPath(ctx context.Context, tenantID int64, subsite, path str // Update uses the caller's loaded version, never a newly fetched version. func (s *Store) Update(ctx context.Context, tenantID int64, p *store.Page) error { + if p == nil { + return errors.New("page: nil") + } tx, err := s.pageTransaction(ctx, tenantID) if err != nil { return err } defer tx.Rollback(ctx) + before, err := pageBefore(ctx, tx, tenantID, p.ID) + if err != nil { + return err + } if err = updatePage(ctx, tx, tenantID, p); err != nil { return err } - if _, err = advanceRevision(ctx, tx, tenantID); err != nil { + revision, err := advanceRevision(ctx, tx, tenantID) + if err != nil { + return err + } + if err := appendPageHistory(ctx, tx, tenantID, revision, "update", before, store.States([]*store.Page{p})); err != nil { return err } return tx.Commit(ctx) @@ -363,10 +378,18 @@ func (s *Store) Delete(ctx context.Context, tenantID int64, id int64, expectedVe return err } defer tx.Rollback(ctx) + before, err := pageBefore(ctx, tx, tenantID, id) + if err != nil { + return err + } if err = deletePage(ctx, tx, tenantID, id, expectedVersion); err != nil { return err } - if _, err = advanceRevision(ctx, tx, tenantID); err != nil { + revision, err := advanceRevision(ctx, tx, tenantID) + if err != nil { + return err + } + if err := appendPageHistory(ctx, tx, tenantID, revision, "delete", before, nil); err != nil { return err } return tx.Commit(ctx) diff --git a/store/storetest/history.go b/store/storetest/history.go new file mode 100644 index 0000000..a97a606 --- /dev/null +++ b/store/storetest/history.go @@ -0,0 +1,126 @@ +package storetest + +import ( + "context" + "encoding/json" + "errors" + "reflect" + "testing" + "time" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +type HistoryStore interface { + store.PageStore + store.PageBatchStore + store.PageHistoryStore +} + +// PageHistory exercises the same saved-content contract in memory and actual +// PostgreSQL. Only committed writes create history; stale saves never retry. +func PageHistory(t *testing.T, s HistoryStore, tenantID, otherTenantID int64) { + t.Helper() + ctx := store.WithPageWriteActor(context.Background(), "user:owner-1") + scheduled := time.Date(2030, 11, 3, 6, 30, 0, 0, time.UTC) + p := &store.Page{Path: "/history", Title: "Original", BodyHTML: "

Original

", BodyBlocks: json.RawMessage("{\"z\":[],\"a\":\"blocks\"}"), TemplateID: "studio", Status: store.StatusScheduled, PublishAt: &scheduled} + if err := s.Create(ctx, tenantID, p); err != nil { + t.Fatal(err) + } + original := store.ContentOf(p) + q := store.PageHistoryQuery{Limit: 100} + h, err := s.ReadPageHistory(ctx, tenantID, q) + if err != nil || len(h.Entries) != 1 { + t.Fatalf("create history: %v %#v", err, h) + } + e := h.Entries[0] + if e.Actor != "user:owner-1" || e.Operation != "create" || e.Revision != 1 || h.CurrentRevision != 1 || h.HistoryStartRevision != 0 || e.Scope != h.Scope || len(e.Before) != 0 || len(e.After) != 1 || e.After[0].Content.Digest() != original.Digest() || e.Digest != e.ContentDigest() || e.CommittedAt.IsZero() { + t.Fatal("create did not bind saved content, trusted actor and revision") + } + // Returned values and caller pointers must not rewrite stored snapshots. + e.After[0].Content.BodyBlocks[0] = ' ' + e.After[0].Content.PublishAt = nil + p.BodyBlocks = nil + p.PublishAt = nil + p.Title = "Saved change" + if err := s.Update(store.WithPageWriteActor(ctx, "agent:review-1"), tenantID, p); err != nil { + t.Fatal(err) + } + saved := store.ContentOf(p) + stale := saved.Page(tenantID, p.ID, 1) + stale.Title = "Stale must not save" + if err := s.Update(ctx, tenantID, stale); !errors.Is(err, store.ErrVersionConflict) { + t.Fatal("stale save accepted") + } + if err := s.Delete(ctx, tenantID, p.ID, 1); !errors.Is(err, store.ErrVersionConflict) { + t.Fatal("stale delete accepted") + } + h, err = s.ReadPageHistory(ctx, tenantID, q) + if err != nil || len(h.Entries) != 2 || h.CurrentRevision != 2 || h.Entries[0].After[0].Content.Digest() != original.Digest() || h.Entries[1].Before[0].Content.Digest() != original.Digest() || h.Entries[1].After[0].Content.Digest() != saved.Digest() || h.Entries[1].Actor != "agent:review-1" || h.Entries[1].After[0].Version != 2 { + t.Fatal("update history changed earlier snapshot or recorded failed writes") + } + if err := s.Delete(context.Background(), tenantID, p.ID, p.Version); err != nil { + t.Fatal(err) + } + if _, err := s.Get(ctx, tenantID, p.ID); !errors.Is(err, store.ErrNotFound) { + t.Fatal("deleted page still present") + } + h, err = s.ReadPageHistory(ctx, tenantID, q) + if err != nil || len(h.Entries) != 3 || h.Entries[2].Operation != "delete" || h.Entries[2].Actor != "unattributed" || len(h.Entries[2].After) != 0 || h.Entries[2].Before[0].Content.Digest() != saved.Digest() { + t.Fatal("deleted saved content not retained") + } + if other, err := s.ReadPageHistory(ctx, otherTenantID, q); err != nil && !errors.Is(err, store.ErrNotFound) || len(other.Entries) != 0 { + t.Fatal("history leaked cross-tenant") + } + first, err := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{Limit: 2}) + if err != nil || len(first.Entries) != 2 || !first.HasMore || first.NextRevision != 2 { + t.Fatal("history first page incorrect") + } + last, err := s.ReadPageHistory(ctx, tenantID, store.PageHistoryQuery{AfterRevision: first.NextRevision, Limit: 2}) + if err != nil || len(last.Entries) != 1 || last.HasMore || last.NextRevision != 3 { + t.Fatal("history cursor replayed or omitted entries") + } + for _, query := range []store.PageHistoryQuery{{Limit: 0}, {Limit: 101}, {AfterRevision: -1, Limit: 1}} { + if _, err := s.ReadPageHistory(ctx, tenantID, query); !errors.Is(err, store.ErrHistoryQuery) { + t.Fatal("invalid history query accepted") + } + } + if _, err := s.ReadPageHistory(ctx, 0, q); !errors.Is(err, store.ErrNotFound) { + t.Fatal("unscoped history read accepted") + } + // A batch event retains only its changed pages and rollback gets a new + // tenant revision; receipt sealing and replay prevention stay authoritative. + keep := &store.Page{Path: "/keep", Title: "Untouched", Status: store.StatusDraft} + if err := s.Create(ctx, tenantID, keep); err != nil { + t.Fatal(err) + } + state, err := s.ReadPageState(ctx, tenantID) + if err != nil { + t.Fatal(err) + } + content := store.PageContent{Path: "/batch", Title: "Batch", Status: store.StatusDraft} + receipt, err := s.ApplyPageBatch(ctx, tenantID, store.BatchFor(state, []store.PageMutation{{Key: "create", Kind: "create", Content: &content}})) + if err != nil { + t.Fatal(err) + } + if _, err := s.RollbackPageBatch(ctx, tenantID, receipt); err != nil { + t.Fatal(err) + } + h, err = s.ReadPageHistory(ctx, tenantID, q) + if err != nil || len(h.Entries) != 6 || h.Entries[4].Operation != "batch.apply" || len(h.Entries[4].Before) != 0 || len(h.Entries[4].After) != 1 || h.Entries[5].Operation != "batch.rollback" || len(h.Entries[5].Before) != 1 || len(h.Entries[5].After) != 0 || h.Entries[4].After[0].ID == keep.ID { + t.Fatal("batch/rollback history omitted changes or included untouched pages") + } + before := h + if _, err := s.RollbackPageBatch(ctx, tenantID, receipt); !errors.Is(err, store.ErrVersionConflict) { + t.Fatal("rollback replay accepted") + } + after, err := s.ReadPageHistory(ctx, tenantID, q) + if err != nil || !reflect.DeepEqual(before, after) { + t.Fatal("failed rollback wrote history") + } + for _, entry := range after.Entries { + if entry.Digest != entry.ContentDigest() { + t.Fatal("history digest mismatch") + } + } +} From 1b875a3813fb812ce156429a4fbccd067d3234f4 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 9 Oct 2026 12:22:04 -0400 Subject: [PATCH 09/10] fix(cms): preserve verified history actor without optional audit signer --- host/page_history_test.go | 36 ++++++++++++++++++++++++++++++++++++ host/server.go | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 host/page_history_test.go diff --git a/host/page_history_test.go b/host/page_history_test.go new file mode 100644 index 0000000..95399bf --- /dev/null +++ b/host/page_history_test.go @@ -0,0 +1,36 @@ +package host + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-cms/store" +) + +func TestServerSavedHistoryActorWithoutOptionalAuditLogger(t *testing.T) { + s := New(Config{ + Pages: store.NewMemoryPageStore(), + AdminAuth: func(*http.Request) bool { return true }, + AdminTenantAccess: func(_ *http.Request, tid int64) bool { return tid == 234 }, + AuditActor: func(*http.Request) string { return "user:verified-owner" }, + }) + if s.Audit() != nil { + t.Fatal("fixture unexpectedly enabled signed logger") + } + base := "/api/v1/admin/tenants/234/pages" + r := httptest.NewRequest(http.MethodPost, base, strings.NewReader("{\"path\":\"/saved\",\"title\":\"Saved\",\"status\":\"draft\"}")) + w := httptest.NewRecorder() + s.ServeHTTP(w, r) + if w.Code != 201 { + t.Fatalf("actual host create: %d", w.Code) + } + w = httptest.NewRecorder() + s.ServeHTTP(w, httptest.NewRequest(http.MethodGet, base+"/history", nil)) + var h store.PageHistory + if w.Code != 200 || json.Unmarshal(w.Body.Bytes(), &h) != nil || len(h.Entries) != 1 || h.Entries[0].Actor != "user:verified-owner" { + t.Fatal("host lost verified history actor without separate signed logger") + } +} diff --git a/host/server.go b/host/server.go index e56a2fd..be81482 100644 --- a/host/server.go +++ b/host/server.go @@ -151,6 +151,7 @@ func New(cfg Config) *Server { api.TenantAccess = cfg.AdminTenantAccess api.PlatformAccess = cfg.AdminPlatformAccess api.RequestAccess = cfg.AdminAuth + api.AuditActor = cfg.AuditActor api.ResolveTemplate = s.resolveTenantTemplate api.ListTemplates = s.listTenantTemplates s.admin = api @@ -169,7 +170,6 @@ func New(cfg Config) *Server { if cfg.AuditSignKey != "" { s.audit = audit.New(cfg.AuditSignKey, cfg.AuditSink) s.admin.Audit = s.audit - s.admin.AuditActor = cfg.AuditActor } s.adminRoot = adminui.Handler() s.adminUI = http.StripPrefix("/admin", s.adminRoot) From 9d81e7f0d2ac72f38f388c679a226d79add96d69 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 9 Oct 2026 19:18:33 -0400 Subject: [PATCH 10/10] docs(cms): describe saved page history in changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d809ae8..f21068e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Content-only frozen snapshots, explicit target mappings and dry-run diffs with verified bundle references. - Atomic baseline/revision-guarded page batches and rollback that preserve newer edits; requires the explicit host content-revision migration. +- Transactional saved page history retains changed-page before/after content and trusted host-supplied actors for page writes, batches and rollback; requires the explicit history migration and coordinated host adoption. +- Tenant-authorized history readback provides bounded cursors and an adoption-start marker. Refused stale writes append no history; pre-adoption edits and unsaved drafts are not reconstructed. ### Changed