[Plugin] Reject invalid product-data input at content:beforeSave, not silently after the save - #102
[Plugin] Reject invalid product-data input at content:beforeSave, not silently after the save#102vedanshujain wants to merge 4 commits into
Conversation
…nput before the CMS save Records the trade behind the incoming `content:beforeSave` price guard: em-dash requires `content:write` to REGISTER that hook at all (and silently skips it otherwise), but the capability is far broader than what we use — in trusted mode it is hook-free transactional write access to every collection. States the surface honestly, plus the bounds (compile-time `PluginContext` with no `content` member, the depcruise sandbox-clean rule, and the `collection !== "products"` early return), the CI-uncoverable capability trap, the strip-not-sentinel invariant, the accepted silence degradation, and the publish-brick vector with its mandatory staging assertion. 0010 and 0011 are reserved by the in-flight service-hardening and entitlements lanes, so this record takes 0012. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Fkmqxb8d6hh4ETNNz43eZ
… silently after the save
A decimal ("24.99") or negative price typed into the "Product data" widget's
minor-units Price field used to save cleanly into the CMS while the commerce
service kept the old amount — green "Saved" toast, no error anywhere. The
validation was correct but lived strictly DOWNSTREAM of the CMS write, in a
fire-and-forget `content:afterSave` hook with no channel back to the editor.
Adds a rejection point upstream of the write:
- `product-commerce/commerce-save-blockers.ts` — a NEW, deliberately narrow
predicate that blocks only PRESENT-AND-WRONG values. Absent / cleared / ""
are always clean, so unpriced products stay saveable and clearing the price
is always a way back. `parseCommerceFields` (a derive guard that legitimately
errors on absence) is neither reused nor modified.
- `product-commerce/commerce-rejection-message.ts` — the sanitized sentinel key
that carries the merchant-facing message, plus the always-on
`scrubStaleSentinels` that guarantees a stale sentinel can never block a save.
- `sync/before-save.ts` — the handler. Rejection is RETURN-based, never a throw:
em-dash's sandboxed dispatcher swallows throws and proceeds with the bad
payload. The load-bearing invariant is the STRIP — a blocked payload carries
no `commerce` key, so the last-good bag survives on both write paths. The
`collection !== "products"` early return is a security control (ADR-0012).
Wiring: `content:beforeSave` in plugin.ts + types.ts, `content:write` in
manifest.ts (mandatory — em-dash silently skips the hook without it), with the
CI-uncoverable capability-trap comment at both sites. The Price label now says
"whole number, no decimals" explicitly, since BlockKitFieldWidget drops any
min/step an element declares.
34 tests: 16 pure unit (predicate + message hygiene), 15 workerd sandbox
(repro, invariant, scrub, create path, non-products, ctx shape), 2 widget /
capability guards, 1 staging site-config. `sync/hooks.ts` and
`parse-commerce-fields.ts` are untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Fkmqxb8d6hh4ETNNz43eZ
…e 3-item list `test/account-routes.sandbox.test.ts:142` still pinned the old two-capability array. It sits behind `describe.skipIf(PG_CONNECTION_STRING === undefined)`, so a non-PG run skipped it entirely and the manifest widening in the previous commit went unnoticed there — it fails against a real Postgres. Now asserts ["content:read", "content:write", "network:request"] with the same ADR-0012 rationale comment carried by the other capability guards. Verified with PG_CONNECTION_STRING set: the whole plugin project is 397/397, 0 skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Fkmqxb8d6hh4ETNNz43eZ
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Staging Playwright DoD — PR #102 (plan §6.7 a–d)Own instance from the worktree ( Confirmed the hook actually registers: this is the first end-to-end proof that Results
Key observations — Step 2 (the core repro)
Step 5 — draft/autosave sentinel check (how it was verified)Typed an invalid decimal ( Verified via the local D1 sqlite file in the worktree ( SELECT count(*) FROM revisions WHERE entry_id='01KYF4DW9XDE4P6ZAD5X0S38AH' AND data LIKE '%Urumi —%'; -- 0
SELECT count(*) FROM ec_products WHERE id='01KYF4DW9XDE4P6ZAD5X0S38AH' AND commerce LIKE '%Urumi —%'; -- 0Both zero. Stronger than "sentinel present but scrubbed on next read": the blocked autosave request never produced a new Defects / notes
Screenshots (stored on VM, not committed — paths under the QA work dir)
No credential/token values appear in any screenshot or in this comment. Instance stopped after the run ( |
CI evidence (GitHub Actions did not dispatch a run for this branch)GitHub created zero workflow runs for The CI job was therefore reproduced locally on this exact head commit (
Combined with the two independent code approvals and the staging DoD above (all steps PASS, sentinel-in-storage check verified by direct local-D1 SQL), this PR meets the merge gate. 🤖 Generated with Claude Code |
|
Closing: superseded by work order 02 (plans/work-order-02-fold-service-into-plugin.md). Once the plugin owns commerce truth in-process the CMS-saved-price-vs-service divergence this PR guards against disappears structurally, and the sandboxed beforeSave hook cannot veto a save in any case. The branch also conflicts with main, its ADR number collides with the accepted ADR-0012, and it adds a content:write capability the descriptor is pinned to exclude. |
1 participant
What
From the 2026-07-26 E2E QA (P1): typing a decimal like
24.99into the minor-units price field showed a "Saved" toast and persisted in the CMS while the commerce service silently kept the old price — the admin UI and the live storefront disagreed with no error anywhere.content:beforeSavehandler (plugin gainscontent:write— ADR-0012): invalid commerce input can never reach the CMS write. The strip is the load-bearing invariant (per-key column writes keep stored data at last-good, so silent divergence is structurally impossible); a sanitized sentinel key surfaces a red "Failed to save: Urumi — Not saved. Price "24.99" must be whole minor units (2499 = $24.99)…" toast as best-effort UX.commerceSaveBlockersblocks only present-and-wrong values (decimal/negative/NaN price, whitespace-only or malformed SKU, placeholder currency/kind with price present); absent/cleared fields are clean no-ops — unpriced products stay saveable and clearing a price is the escape hatch. Price0is valid.content:writegrants, the CI-uncoverable capability trap (our sandbox has no capability gate — comments at both wiring sites), and the degradation modes.sync/hooks.tsandparse-commerce-fields.tsuntouched (concurrent-lane isolation, since landed as #101's surface).Verification
dataadversarial bag, oversized/newline SKU), sentinel scrub ×multi-key, isNew create path, ×2 real-workerd bootsChangeset
@urumi/pluginminor.🤖 Generated with Claude Code
https://claude.ai/code/session_013Fkmqxb8d6hh4ETNNz43eZ