From aba0d187d72eb6d650a2019b071c144e4e61743b Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 9 Aug 2026 10:30:20 -0700 Subject: [PATCH] fix(site-builder): make Reset Entire Site's guard load-bearing in the handler Two Important review findings on the previous commit (1a01834): 1. handleResetSite ran unconditionally -- the confirm button's `disabled` attribute was the only thing standing between a mismatched/empty typed value and a full site wipe. Extracted the match check into a single exported pure predicate, siteResetConfirmMatches(typed, domain), used for the button's disabled/cursor/opacity (previously three duplicated inline comparisons) AND as the first line of handleResetSite itself, which now returns early if it doesn't hold. An empty domain is rejected outright (`!!domain &&` short-circuits) so the guard holds even if the handler were ever reached with no configured domain, independent of the entry point being hidden. 2. The dialog said "design tokens" but resetToDefaults() also wipes headCode (analytics/search-console/third-party scripts) and favicon -- neither is one of the 17 documented design properties, so a user had no reason to read them as included. Copy now names both explicitly. Re-verified every remaining claim in the paragraph against what the handler actually does (page/header/footer replacement, no undo, no publish call, images untouched, 30000ms auto-save) -- all still hold. Verified load-bearing by temporarily reverting each guard in place (no git stash -- shared across worktrees/sessions per review feedback) and confirming the corresponding test fails: dropping the !!domain check broke the empty-domain unit test; removing the handleResetSite check broke a new test that invokes the confirm button's React onClick directly (bypassing both the disabled attribute and react-dom's own disabled-click suppression, which independent investigation confirmed blocks a plain DOM `.disabled = false; .click()`/dispatchEvent bypass -- pulling onClick off the element's stashed __reactProps$ key was the only way to actually exercise the handler's own guard). Co-Authored-By: Claude Opus 5 (1M context) --- .../right/SiteDesignPanel.reset.test.tsx | 77 ++++++++++++++++++- craft/src/panels/right/SiteDesignPanel.tsx | 42 ++++++++-- 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/craft/src/panels/right/SiteDesignPanel.reset.test.tsx b/craft/src/panels/right/SiteDesignPanel.reset.test.tsx index 1e7aec5..7a3b185 100644 --- a/craft/src/panels/right/SiteDesignPanel.reset.test.tsx +++ b/craft/src/panels/right/SiteDesignPanel.reset.test.tsx @@ -33,7 +33,7 @@ vi.mock('../../state/EditorConfigContext', () => ({ useEditorConfig: () => ({ whpConfig: mockSiteDomain ? { siteDomain: mockSiteDomain } : null, isWHP: !!mockSiteDomain }), })); -import { SiteDesignPanel } from './SiteDesignPanel'; +import { SiteDesignPanel, siteResetConfirmMatches } from './SiteDesignPanel'; let container: HTMLDivElement; let root: Root; @@ -63,6 +63,50 @@ function setInputValue(input: HTMLInputElement, value: string) { input.dispatchEvent(new Event('input', { bubbles: true })); } +/** + * Reach past React's OWN disabled-button protection to grab the `onClick` + * function it attached to a DOM node, and call it directly. + * + * Verified empirically (see task-15 follow-up investigation) that neither + * `el.disabled = false; el.click()` nor a raw `dispatchEvent(new + * MouseEvent('click', ...))` actually invokes a React `onClick` handler once + * React has rendered the element with `disabled` truthy: react-dom's event + * system special-cases form controls and refuses to dispatch synthetic + * click/similar events against its own last-rendered `disabled` prop, + * regardless of what the live DOM property says. That's a second, redundant + * safety net on top of the native browser behavior -- but it also means a + * pure DOM-level "disabled bypass" can't reach `handleResetSite` at all + * through the button, and so can't exercise the handler's OWN guard the way + * this test needs to. Pulling `onClick` off the fiber's stashed props + * (`__reactProps$...`, the same object React itself calls into) and + * invoking it directly is what actually reaches `handleResetSite` -- the + * same code path a differently-wired future trigger (a keyboard shortcut, a + * second button, a copy-paste bug) would also go through without passing + * back through the `disabled` gate at all. + */ +function invokeReactOnClick(el: HTMLElement): void { + const propsKey = Object.keys(el).find((k) => k.startsWith('__reactProps$')); + const onClick = propsKey ? (el as unknown as Record void }>)[propsKey].onClick : undefined; + if (!onClick) throw new Error('no React onClick prop found on element'); + onClick(); +} + +describe('siteResetConfirmMatches -- the guard predicate itself', () => { + test('matches only an exact (trimmed) domain match', () => { + expect(siteResetConfirmMatches('example.com', 'example.com')).toBe(true); + expect(siteResetConfirmMatches('example.co', 'example.com')).toBe(false); + expect(siteResetConfirmMatches('', 'example.com')).toBe(false); + expect(siteResetConfirmMatches(' example.com ', 'example.com')).toBe(true); + }); + + test('an empty domain never matches, even against an empty typed value -- ' + + 'this is the guard that must hold even if the (currently hidden) entry ' + + 'point were ever reachable in standalone mode', () => { + expect(siteResetConfirmMatches('', '')).toBe(false); + expect(siteResetConfirmMatches(' ', '')).toBe(false); + }); +}); + describe('Reset Entire Site -- guard + wiring (mocked hooks)', () => { test('the confirm button is disabled until the domain is typed exactly', () => { mockSiteDomain = 'example.com'; @@ -117,6 +161,37 @@ describe('Reset Entire Site -- guard + wiring (mocked hooks)', () => { unmount(); }); + test('the handler itself refuses a non-matching value, even when its onClick is invoked ' + + 'directly (bypassing `disabled` and React\'s own disabled-click suppression) -- proves ' + + 'the guard is load-bearing in handleResetSite, not just a UI affordance a different ' + + 'trigger path could route around', () => { + mockSiteDomain = 'example.com'; + replaceAllPages.mockClear(); + setHeader.mockClear(); + setFooter.mockClear(); + resetToDefaults.mockClear(); + + render(); + act(() => { (container.querySelector('[data-action="open-site-reset"]') as HTMLButtonElement).click(); }); + const input = container.querySelector('[data-testid="site-reset-domain"]') as HTMLInputElement; + act(() => { setInputValue(input, 'not-the-domain'); }); + + const confirm = container.querySelector('[data-action="confirm-site-reset"]') as HTMLButtonElement; + expect(confirm.disabled).toBe(true); // sanity: the UI affordance is still doing its job too + + act(() => { invokeReactOnClick(confirm); }); + + expect(replaceAllPages).not.toHaveBeenCalled(); + expect(setHeader).not.toHaveBeenCalled(); + expect(setFooter).not.toHaveBeenCalled(); + expect(resetToDefaults).not.toHaveBeenCalled(); + // The dialog is still open -- handleResetSite returned early before + // reaching its own close-the-dialog cleanup. + expect(container.querySelector('[data-action="confirm-site-reset"]')).toBeTruthy(); + + unmount(); + }); + test('standalone mode (no WHP_CONFIG, siteDomain "") hides the entry point entirely, ' + 'while non-standalone mode shows it -- an empty typed input must never trivially ' + 'satisfy the guard. Asserted as a contrast within one test (not "absence" alone) ' + diff --git a/craft/src/panels/right/SiteDesignPanel.tsx b/craft/src/panels/right/SiteDesignPanel.tsx index 922cf28..6b84f70 100644 --- a/craft/src/panels/right/SiteDesignPanel.tsx +++ b/craft/src/panels/right/SiteDesignPanel.tsx @@ -193,6 +193,31 @@ const BLANK_FOOTER_TREE: SerializedTreeNode = { nodes: [], }; +/** + * Single source of truth for "has the user typed enough to arm the confirm + * button" -- used for the `disabled` attribute/cursor/opacity AND, more + * importantly, inside `handleResetSite` itself. `disabled` is a UI + * affordance, not a safety mechanism (it only stops a plain mouse click); + * for the one action in this app that irreversibly wipes a user's whole + * site draft, the real guard has to live in the handler, checked against + * this exact same predicate rather than a second hand-rolled comparison + * that could drift out of sync with it. + * + * An empty/falsy `domain` always returns `false`, even if `typed` is also + * empty -- `''.trim() === ''.trim()` would otherwise "match" trivially. + * Standalone mode (no `WHP_CONFIG`) has `siteDomain === ''`, and while the + * entry point that would let a user reach this code is hidden in that case + * (see the `siteDomain &&` guard around the whole danger zone below), this + * function must not depend on that -- it has to fail safe on its own if + * ever reached with no configured domain. + * + * Exported so it's directly unit-testable without needing a way to render + * the (deliberately unreachable-when-`siteDomain`-is-empty) confirm UI. + */ +export function siteResetConfirmMatches(typed: string, domain: string): boolean { + return !!domain && typed.trim() === domain.trim(); +} + /* ---------- Main SiteDesignPanel ---------- */ export const SiteDesignPanel: React.FC = () => { @@ -209,6 +234,12 @@ export const SiteDesignPanel: React.FC = () => { const [tab, setTab] = useState('basic'); const handleResetSite = (): void => { + // Load-bearing guard -- see `siteResetConfirmMatches`'s docstring. Not + // just a UI nicety: this must hold even if the confirm button's + // `disabled` attribute were ever bypassed (a synthetic click, or a + // future refactor that drops it). + if (!siteResetConfirmMatches(siteResetTyped, siteDomain)) return; + replaceAllPages([{ name: 'Home', tree: BLANK_PAGE_TREE }]); setHeader(BLANK_HEADER_TREE); setFooter(BLANK_FOOTER_TREE); @@ -457,8 +488,9 @@ export const SiteDesignPanel: React.FC = () => { ) : ( <>

- This blanks every page, the header, the footer and all - design tokens, leaving one empty Home page. Uploaded images are kept. + This blanks every page, the header, the footer, all + design tokens (colors, fonts, radii), and your custom head code and + favicon, leaving one empty Home page. Uploaded images are kept. This cannot be undone. Your published site stays as it is until you publish again — but the editor auto-saves roughly every 30 seconds, so the blank version becomes your saved draft shortly after. @@ -476,14 +508,14 @@ export const SiteDesignPanel: React.FC = () => {