`, `<h1>`, `<header>`, `<p>`. A file named `x" onfocus=alert(1) autofocus q.md` produces: <a class="cheldrop-btn" href="x" onfocus=alert(1) autofocus q.md" download> The handler then runs for every visitor to the public site. Confirmed by evaluating `toolbarHtml` directly. Reachable without the owner typing the name: site folders are expected to live in Dropbox and iCloud (that is what `CLOUD_PATH_MARKERS` in `sitefolder.js` is for), and the MCP server lets an agent write files into them. **Fix:** `esc()` already exists at `main/renderer.js:598` and is used correctly by the header/footer/sidebar code at 740–869. Apply it to every interpolation in the wrapper templates. Start with `toolbarHtml` (152, 160), then work down the list above. **Fixed** in `c33a2cf`: `esc()` moved above its first use and applied to every interpolation of a name, label or title — the toolbar, all five wrappers, the site map entries and both gallery grids. Two neighbours in the same blast radius went with it: the gallery's `{{IMAGES_JSON}}` now escapes `<` and U+2028/9 rather than trusting `JSON.stringify` inside a `<script>`, and template fills take a function replacer, because `String.replace` reads `$&` and `` $` `` in a replacement string — a file named `$&x.md` was a substitution, not a name. `test/escaping.test.js` covers the injection cases and asserts plain names still render byte-identical. --- ### 2. The R2 publish empties the bucket before it uploads — FIXED 18 August 2026 `main/cloudflare.js:719` Step 2 purges every object, step 3 uploads. Any failure in between leaves the live site serving whatever partial set made it. A Live R2 site that loses its connection after 3 of 200 files is a three-file site, and live sync's backoff ladder (30s → 600s) means a persistent cause leaves it broken indefinitely with nobody watching. **Fix:** the Workers path in the same function already does it safely — upload everything, then deploy the version referencing the new manifest atomically. Give R2 the same ordering: upload under new keys, then swap, then purge what is no longer referenced. At minimum, do not purge until the upload has succeeded. **Fixed:** the publish now uploads first, then calls `pruneR2Bucket` to delete only the keys the new publish does not contain. Same-named objects are replaced in place, so the live site is never missing content, and an upload that fails part-way deletes nothing. `purgeR2Bucket` remains for `deleteSite`. --- ### 3. `set_password` can strand a site in an unpublishable state — FIXED 18 August 2026 `main/mcp-server.js:943` It writes `password_protected: true` into `cheldrop.yaml` *before* publishing, and does not revert when the publish fails (line 960). Live sync then republishes from that yaml. `performPublish` reads `passwordProtected: true` on an R2 backend, but the unattended path carries no password and the site entry still says unprotected — so `cloudflare.js:703` returns "Password protection is on but no password is set". Every retry hits the same branch forever, asking someone to type a password who is not there. **Fix:** write the yaml only after a successful publish, the way `performPublish` already does at `ipc.js:389`. Or roll it back on failure. **Fixed:** the pre-publish `writeSiteConfig` is gone. `set_password` now passes `passwordEnabled` to `manualPublish` and nothing else — `performPublish` already merges that into the config it publishes with, and writes the merged yaml back only after the publish succeeds (`ipc.js:389`). A failed publish now leaves `cheldrop.yaml` exactly as it was, so live sync has nothing unpublishable to retry. --- ### 4. The two-publish guard gives up after 30 seconds — FIXED 18 August 2026 `main/livesync.js:366` `manualPublish` awaits `livesync.pause()` so a watcher publish and a button publish cannot collide. The wait has a deadline and no way to report reaching it: const deadline = Date.now() + 30000 while (Date.now() < deadline) { if (![...watched.values()].some(s => s.publishing)) return // only success exit await new Promise(r => setTimeout(r, 150)) } // falls through after 30s — caller cannot tell this from success Two `replaceEverything: true` publishes then race, and the one that lands last wins — which can be the older snapshot, since the live publish started first. The user sees an ordinary success. **Fix:** return `false` on timeout; have `manualPublish` surface it rather than proceeding. 30s is not generous by this module's own standards — `RETRY_MS` opens at 30s assuming network stalls are common. **Fixed:** `pause()` now returns `true` when the field is clear and `false` when the deadline passes with a publish still running (the wait is `PAUSE_DRAIN_MS`, exported, and overridable per call so tests need not sit out 30 s). `manualPublish` checks that return and refuses — `{ ok: false, error }`, the shape every caller already handles — instead of starting a second full replacement. The watchers are still held on the refusal path, so `resume()` in the `finally` stays correct. `test/livesync-pause.test.js` drives a real watcher with a publish held open and asserts the false, the true once it lands, and that the caller's guard sits before the publish. --- ## Fix soon — degrades the product ### 5. A dead bucket-wipe endpoint still ships on every R2 site — FIXED 18 August 2026 `main/r2-worker.js:218` Every deployed R2 router Worker exposes `POST /__cheldrop_admin__/purge`, which deletes every object in the bucket. Nothing calls it any more — `cloudflare.js` purges through the R2 API instead and says so at line 545. The guarding secret is stored unencrypted in `sites.json` as `adminSecret`, a few fields from `cfToken`, which *is* safeStorage-encrypted. Anyone who reads that file can destroy the site's contents with no other credential. The check at line 219 is a plain `!==`, not the constant-time `safeEqual` used at line 197. **Fix:** remove the endpoint from the generated Worker. If it must stay, use `safeEqual` and encrypt `adminSecret` at rest. **Fixed:** the endpoint is gone from `r2-worker.js`, and with it the reason `adminSecret` existed — `deployR2Worker` no longer generates one, no publish result carries one, and `getDb()` deletes the field from records written before this, so the one plaintext secret in `sites.json` leaves disk. The deployed Worker now has no write path to the bucket at all; the app does everything through the R2 API with the token it already holds. Deleting the source only helps sites published afterwards, so the second half is a version gate: `R2_WORKER_VERSION` is stored per site as `workerVersion`, and a publish redeploys the Worker when the deployed script is behind it. That redeploy at first covered only unprotected sites — a deploy replaces the bindings wholesale, and a protected site redeployed without a fresh auth record (which needs the password; live sync has none) would come back public — which left protected sites on the old Worker, endpoint and all, until the next time the password was set. **Now closed:** the upload inherits the binding instead of rebuilding it, with `keep_bindings: ['secret_text']`, so the password crosses the redeploy without Cheldrop ever holding it. Since a lost binding would fail silently and publicly, the inherit path is bracketed by `checkAuthBinding`, which distinguishes *present*, *absent* and *unknown* — the endpoint is `GET /workers/scripts/{name}/secrets`, which returns binding names and never values. Absent or unknown beforehand means there is nothing safe to inherit, so the old Worker stays. Anything but present afterwards means the site may be answering without a password, so it is re-deployed with an auth record built from 32 random bytes nobody holds — locked, not public — and the publish fails asking for the password to be set again. The same check also stops a site whose entry says public but whose Worker holds an auth binding from being quietly unprotected by a router update. `test/r2-worker.test.js` asserts the script carries no admin endpoint and no delete call, and models the API's binding semantics — write-only secret values, `keep_bindings` inheritance, and a knob that makes that inheritance fail — to cover all seven redeploy decisions, including the lock. --- ### 6. Folder walking follows symlinks with no cycle guard — FIXED 18 August 2026 `main/sitefolder.js:757` `readFolderFiles` uses `fs.statSync`, which resolves symlinks, with neither loop detection nor a containment check against the folder root. `ln -s .. loop` inside a site folder makes `walk()` recurse until stack exhaustion or `ENAMETOOLONG`, during a publish or live sync, with no useful error. `ln -s ~/Documents refs` publishes that whole tree to the public URL. **Fix:** `fs.lstatSync`, and skip symlinks (or resolve and check containment). **Fixed:** the walk resolves and checks containment rather than skipping links outright, so a link used the way people use them — one file, one folder, inside the site — still publishes. `lstat` identifies the link, `realpath` resolves it, and anything landing outside the folder's own real path is skipped. Both sides of that test are real paths: a site folder in Dropbox or iCloud is itself reached through a link, and comparing the paths as written makes a folder fail its own containment test. Cycles are cut by the chain of directories open above the current one, not by every directory seen. The difference showed up in the tests: dedup on "seen" makes `sub/` and a `also-sub -> sub` link race, and whichever `readdir` returns second is dropped — so the real folder can lose to the link. On the open chain, `ln -s .. loop` and `ln -s . self` terminate while a linked sibling publishes under both names, which is what the folder actually holds. Dangling links are skipped rather than thrown on — on macOS the old code did not recurse forever, it aborted the publish with an uncaught `ELOOP` or `ENOENT` from deep inside the walk. A missing folder still throws, as before. `test/folder-walk.test.js` builds real folders and real links: the two exploits, both cycles, the linked sibling, the file link, the dangling link, and a site folder reached through a link. Six of its ten cases fail against the old walk. --- ### 7. Extensionless URLs work on the free tier and 404 after upgrading — FIXED 18 August 2026 `main/r2-worker.js:253` The R2 router resolves `/about` to `about/index.html`, then falls back to `about` — never `about.html`. The Workers backend is deployed with `html_handling: 'auto-trailing-slash'` (`cloudflare.js:373`), which does serve it. So a folder holding `about.html` answers `/about` on the free tier and 404s once the same site moves to `backend: r2`. **Fix:** add the `<path>.html` lookup to the router's fallback chain. **Fixed:** the chain is now `<path>/index.html`, `<path>.html`, `<path>`, and `/about/` falls back to `about.html` as well. The candidate list is a superset of the old one in the same order, so no address that resolved before resolves to something else now — the folder index deliberately keeps precedence over the sibling page, since that is what a live site is already serving. The only behaviour that moves is a site holding both `about.html` and a bare `about`, where the `.html` file now wins; that is the answer the free tier gives. Two neighbours went with it. The dot test that decides "this address names a file" read the whole path, so a directory called `v1.2` stopped index resolution for everything beneath it — it reads the last segment now. And `R2_WORKER_VERSION` is bumped to 3, which is what carries the routing fix to sites already live; with finding 5 closed, that now includes protected ones. `test/r2-routing.test.js` evaluates the generated Worker and drives it with real `Request` objects against a fake bucket that records every key looked for. Five of its eleven cases fail against the old router. --- ### 8. Deleting a site can leave its hostname attached — FIXED 18 August 2026 `main/cloudflare.js:808` (and `:645`) `deleteWorkerAndDomain` reads `/workers/domains` with no pagination and treats the first page as the whole account. Past that page it finds no match, skips the detach, and deletes the script anyway — leaving the hostname bound to a missing Worker and its DNS record behind. Re-publishing that subdomain later fails with "That subdomain already has a DNS record", the error `friendlyError` special-cases at line 51. This is a known bug class here: the comment at line 184 records an account whose 19 Pages projects showed as 10. `listR2Buckets` paginates by cursor; only these two domain lookups do not. **Fix:** paginate, the way `listPaged` and `listR2Buckets` already do. The exact account size that triggers it depends on Cloudflare's default page size for this endpoint — unverified, no account was available. **Fixed:** both lookups now go through `findWorkerDomain`, which asks for the one hostname it wants — `?hostname=<fqdn>`, the way `deleteCnameRecord` already queries DNS — so the answer no longer depends on how many domains the account holds. The filter is not trusted on its own: the match is confirmed here, and a list that comes back holding *other* hostnames means the filter was ignored, which falls back to a full paged sweep in `listWorkerDomains`. That sweep is deliberately careful, because this endpoint's paging is undocumented and the account to verify it against still was not available. It sends `page` and not `per_page` — `listPagesProjects` records an endpoint that rejects `per_page` with a 400, and a lookup whose failure mode is a stranded hostname is the wrong place to risk it. Termination follows what the response says rather than what it is assumed to do: no `result_info` means the whole list arrived at once, `total_pages` is followed when given, a short page ends the sweep when only `per_page` is given, and a page identical to page one means `page` was ignored and stops it. Every path is bounded at 50 pages. The attach side went with it, since it reads the same list: a re-publish on a large account could not see the domain it already had, so it re-attached and Cloudflare answered with the DNS-record clash `friendlyError` special-cases — turning an ordinary republish into that error. The delete side is unchanged in shape: still best effort, still deletes the script if the lookup itself fails. `test/worker-domains.test.js` runs each case against three fake servers — one honouring the hostname filter, one ignoring it and paging, one ignoring both and answering with everything — because the fix has to be right whichever this endpoint turns out to be. Four of its eighteen cases fail against the old lookup, and the two publish-side failures fail with the DNS-record error verbatim. --- ### 9. A file that disappears mid-publish freezes the UI — FIXED 18 August 2026 `renderer/src/pages/Publish.jsx:500` The `FileReader` promise inside `handlePublish` can reject and nothing catches it, so the function exits before `cleanup()` and before any error state is set. `status` stays `'publishing'` with no message and no way back but restarting, and the `onPublishProgress` listener from line 461 leaks. Every other failure path in the function calls `cleanup()` first. **Fix:** wrap the read in try/catch, call `cleanup()`, set the error state. **Fixed:** the read is wrapped, and the catch takes the same three steps every other failure path in the function already takes — `cleanup()`, `setStatus('error')`, a message — so a file that vanished between being picked and being published ends as an ordinary error the user can act on, rather than a stuck spinner. The message names the file and says to add the files again, since a `File` handle whose backing file has gone will not read on a retry either. --- ### 10. Quitting does not wait for a publish in flight — FIXED 18 August 2026 `main/index.js:114` `before-quit` calls `livesync.stopAll()`, which clears timers, closes watchers and sets status `off` — then returns. The in-flight `performPublish` is a floating promise nobody awaits, so the process dies mid-upload. A Live site's debounce fires, the publish starts, the user presses ⌘Q two seconds later. On R2 that lands inside the purge-then-upload window of finding 2, so the live site can be left empty. `livesync.pause()` already polls `s.publishing` for exactly this; `stopAll()` just does not use it. `tray.js:133` avoids `role: 'quit'` with the comment "quitting has to stop the watchers first" — which implies more safety than `stopAll()` delivers. And `updater.js` sets `autoInstallOnAppQuit = true`, so a quietly downloaded update turns every ordinary quit into this path without the user choosing it. **Fix:** `preventDefault()` in `before-quit`, await a drain (reuse `pause()`), then quit. Cap the wait so a hung publish cannot block quit forever. **Fixed:** `before-quit` now holds the quit, calls `livesync.pause()` — the same drain a manual publish uses, so the watchers are held and whatever is uploading gets to land — and only then runs the teardown it always ran, before quitting for real. The cap is 15 s rather than livesync's 30 s: this wait is in front of a user who just pressed ⌘Q, and a hung upload must not make quit look broken. When the deadline passes the app quits anyway, having tried. A second ⌘Q during the drain is held too — impatience is not permission to die mid-upload, and the cap is what bounds the wait. `test/quit-drain.test.js` (`npm run test:quitdrain`) drives the real handler with a publish held open and asserts the order: nothing is torn down while the drain runs, and the quit lands after it either way. --- ### 11. One downloaded update disables checking for later ones — FIXED 18 August 2026 `main/updater.js:150` `update-downloaded` sets `downloadedVersion`, and nothing ever clears it. From then on `checkForUpdatesManually` returns early for the rest of the process lifetime, re-offering the version already on disk. Download 1.1.0, pick "Later". Two weeks on, 1.2.0 and 1.3.0 have shipped; "Check for Updates…" still says "Cheldrop 1.1.0 is ready to install", with no hint newer versions exist and no way to look. The only escape is quitting (installing the stale 1.1.0) or restarting. This matters here for the reason the file's own header gives: Phase 4.3 makes Cheldrop a permanently-running login-item agent, so process lifetime is weeks. **Fix:** re-check anyway, and replace the pending download when the feed offers something newer. **Fixed:** the early return is gone — "Check for Updates…" always reaches the feed, and the handlers decide what to do with what came back. `autoDownload` is off so the decision is ours: a version already on disk is kept and offered for install rather than fetched a second time, and anything newer supersedes it — cancelling an in-flight download through its `CancellationToken` when one is running. The downloaded file stays installable on quit until its replacement lands, so a supersede that never finishes still leaves the user an upgrade. Ordering is a local semver compare rather than the transitively-hoisted `semver`: 1.10.0 has to read as newer than 1.9.0, and a prerelease as older than the release it leads to. Two dialogs stopped lying on the way: a pulled release now offers the ready update instead of "you are up to date", and a failed check offers it instead of leaving the user an error with no way to install what is already downloaded. `test/updater-recheck.test.js` (`npm run test:updaterecheck`) drives the module as one session — download, "Later", weeks of newer releases — with a fake feed, and asserts the check happens every time, that nothing is downloaded twice, and that the supersede cancels. --- ## Minor ### 12. A comment promises a hash the code never applies — FIXED 18 August 2026 `main/ipc.js:441` // Use a hash of the name to avoid collisions across concurrent publishes const safeName = name.replace(/[^a-zA-Z0-9._-]/g, '_') Sanitising blocks traversal (separators become `_`, a bare `..` fails as `EISDIR`), so the security half is sound. It does nothing about collisions: a manual publish and an MCP `publish_now` staging `index.html` at the same moment write the same path, and one site publishes the other's file. **Fix:** hash as the comment says, or correct the comment. **Fixed:** a hash of the name would not have helped — the collision is two publishes staging the *same* name, which hashes the same. Each call now names its file after the call: a pid/counter/random stamp in front of the sanitised name, so concurrent publishes cannot meet. The stamp goes in front because the extension decides content type and the Workers asset hash, and the publish payload carries the real name beside the path, so no downstream reader loses anything. Sanitising stays and keeps doing the job it was actually doing — containing the write — with a fallback name so a name that sanitises to nothing cannot address the directory itself. Unique names never overwrite, so the staging directory is swept of day-old files on each publish; publishes take seconds, so anything older is litter. `test/temp-stage.test.js` (`npm run test:tempstage`) stages `index.html` twice and asserts each publish keeps its own bytes, and that traversal still does not survive. ### 13. The tray menu rebuilds once per uploaded file — FIXED 18 August 2026 `main/tray.js:201` The `livesync.onStatus` handler calls `refreshSites()` on every status event, and progress events fire per file (`cloudflare.js:512`). Each one runs `buildTrayMenu()` → `Menu.buildFromTemplate`, `setContextMenu`, `setToolTip`, and `trayIcon()`, which constructs a fresh `nativeImage` from the PNG path. A 200-file R2 publish does all of that 200 times in a few seconds. The visible symptom is the tray menu churning while the user has it open — replacing a macOS context menu mid-display disrupts it. The db read is not the cost (`getDb()` memoises); the native menu construction and image loading are. **Fix:** rebuild on state transitions only, or coalesce on a short timer. **Fixed:** both, split by what the event actually changes. A site that starts, lands or fails changes the glyph, the icon and the tooltip, so a state transition still rebuilds at once — the menu bar must not lag the thing it reports. Progress inside a publish changes one line of text and arrives once per file, so those coalesce onto a 1 s trailing timer: the detail still lands, the last one wins, and a 200-file publish costs one rebuild instead of 200. The timer is cleared in `destroyTray()`, so a quit mid-publish cannot fire a rebuild at a destroyed tray a second later. `test/tray-menu.test.js` (`npm run test:traymenu`) drives a 200-file publish through the real handler and asserts the collapse, that the menu ends on the last progress line, and that landing still draws immediately. --- ## Checked and found sound Recorded so this is not read as a fault list only. Each is a place a specific bug was looked for and the handling was already correct. - Electron hardening — `contextIsolation: true`, `nodeIntegration: false`, a fixed verb list in preload, no `dangerouslySetInnerHTML` anywhere. - Token storage — safeStorage round-trip, prefix-marked ciphertext, in-place plaintext migration, and the encryption-unavailable fallback. - MCP transport — 127.0.0.1 only, `crypto.timingSafeEqual` with a length pre-check, Origin allowlist against DNS rebinding, 1 MB body cap. - Licence gates — free-tier and R2 gates enforced in `performPublish`, so a hand-edited `cheldrop.yaml` cannot walk past them. - Licence resilience — inconclusive checks keep the activation inside a 7-day grace window; only a definitive rejection revokes. - Reconciliation — one unreadable account makes a kind *unknown*, not *missing*. - `safeHref` — correctly rejects `javascript:`, including newline/tab variants. - Path containment — `readTheme`, `readBundledTheme`, `materializeSiteFolder`. - `writeSiteConfig` skips identical writes, which is what stops a Live folder publishing itself in a loop. - js-yaml 5.2.3 — `load` is safe by default. --- ## Scope — read this before trusting a gap Not every file got the same depth. **Read line by line (~6,100 lines):** `preload.js`, `index.js`, `ipc.js`, `livesync.js`, `cloudflare.js`, `r2-worker.js`, `sitefolder.js`, `reconcile.js`, `menu.js`, `mcp-server.js`, `tray.js`, `updater.js`; `renderer.js` lines 1–200; `license.js` lines 1–80; `Publish.jsx` around the publish handler. **Targeted search only** — greps for specific bug classes, not a full read: `renderer.js` lines 200–1265, the rest of `license.js`, `Sites.jsx`, `Settings.jsx`, `Docs.jsx`, `QuickStart.jsx`. **Not opened:** `test/` (798, read for coverage only), `build/` scripts, bundled templates. No finding was verified against a live Cloudflare account. The separate review of commit `6d923a4` (the 16px mark fallback) carries two findings of its own: a favicon ring that still renders as a sub-pixel hairline (`build/brand/mark-dot-light.svg:5`), and tray geometry that has drifted from the SVG both files cite (`build/tray/make-tray-icons.py:53`).

# Full-app review — 18 August 2026 Reviewed at `main @ 6d923a4`. Thirteen findings, ordered by whether they can break or expose a published site. Each entry names the file, the line, what goes wrong, and the shape of the fix. Fixed entries are marked; everything unmarked still stands. Report (same content, rendered): https://claude.ai/code/artifact/879bdcb0-e26b-4d29-bc6a-487a4dcf8f4f --- ## Fix first — can break or expose a live site ### 1. Filenames become script on the published site — FIXED 18 August 2026 `main/renderer.js:152` (and 160, 231, 241, 242, 262, 271, 294, 305, 307, 330, 342, 344, 346, 389, 400, 1045, 1127) Filenames are interpolated into generated HTML with no escaping across the file-wrapper templates — `href`, `src`, `alt`, `title`, ``, `<h1>`, `<header>`, `<p>`. A file named `x" onfocus=alert(1) autofocus q.md` produces: <a class="cheldrop-btn" href="x" onfocus=alert(1) autofocus q.md" download> The handler then runs for every visitor to the public site. Confirmed by evaluating `toolbarHtml` directly. Reachable without the owner typing the name: site folders are expected to live in Dropbox and iCloud (that is what `CLOUD_PATH_MARKERS` in `sitefolder.js` is for), and the MCP server lets an agent write files into them. **Fix:** `esc()` already exists at `main/renderer.js:598` and is used correctly by the header/footer/sidebar code at 740–869. Apply it to every interpolation in the wrapper templates. Start with `toolbarHtml` (152, 160), then work down the list above. **Fixed** in `c33a2cf`: `esc()` moved above its first use and applied to every interpolation of a name, label or title — the toolbar, all five wrappers, the site map entries and both gallery grids. Two neighbours in the same blast radius went with it: the gallery's `{{IMAGES_JSON}}` now escapes `<` and U+2028/9 rather than trusting `JSON.stringify` inside a `<script>`, and template fills take a function replacer, because `String.replace` reads `$&` and `` $` `` in a replacement string — a file named `$&x.md` was a substitution, not a name. `test/escaping.test.js` covers the injection cases and asserts plain names still render byte-identical. --- ### 2. The R2 publish empties the bucket before it uploads — FIXED 18 August 2026 `main/cloudflare.js:719` Step 2 purges every object, step 3 uploads. Any failure in between leaves the live site serving whatever partial set made it. A Live R2 site that loses its connection after 3 of 200 files is a three-file site, and live sync's backoff ladder (30s → 600s) means a persistent cause leaves it broken indefinitely with nobody watching. **Fix:** the Workers path in the same function already does it safely — upload everything, then deploy the version referencing the new manifest atomically. Give R2 the same ordering: upload under new keys, then swap, then purge what is no longer referenced. At minimum, do not purge until the upload has succeeded. **Fixed:** the publish now uploads first, then calls `pruneR2Bucket` to delete only the keys the new publish does not contain. Same-named objects are replaced in place, so the live site is never missing content, and an upload that fails part-way deletes nothing. `purgeR2Bucket` remains for `deleteSite`. --- ### 3. `set_password` can strand a site in an unpublishable state — FIXED 18 August 2026 `main/mcp-server.js:943` It writes `password_protected: true` into `cheldrop.yaml` *before* publishing, and does not revert when the publish fails (line 960). Live sync then republishes from that yaml. `performPublish` reads `passwordProtected: true` on an R2 backend, but the unattended path carries no password and the site entry still says unprotected — so `cloudflare.js:703` returns "Password protection is on but no password is set". Every retry hits the same branch forever, asking someone to type a password who is not there. **Fix:** write the yaml only after a successful publish, the way `performPublish` already does at `ipc.js:389`. Or roll it back on failure. **Fixed:** the pre-publish `writeSiteConfig` is gone. `set_password` now passes `passwordEnabled` to `manualPublish` and nothing else — `performPublish` already merges that into the config it publishes with, and writes the merged yaml back only after the publish succeeds (`ipc.js:389`). A failed publish now leaves `cheldrop.yaml` exactly as it was, so live sync has nothing unpublishable to retry. --- ### 4. The two-publish guard gives up after 30 seconds — FIXED 18 August 2026 `main/livesync.js:366` `manualPublish` awaits `livesync.pause()` so a watcher publish and a button publish cannot collide. The wait has a deadline and no way to report reaching it: const deadline = Date.now() + 30000 while (Date.now() < deadline) { if (![...watched.values()].some(s => s.publishing)) return // only success exit await new Promise(r => setTimeout(r, 150)) } // falls through after 30s — caller cannot tell this from success Two `replaceEverything: true` publishes then race, and the one that lands last wins — which can be the older snapshot, since the live publish started first. The user sees an ordinary success. **Fix:** return `false` on timeout; have `manualPublish` surface it rather than proceeding. 30s is not generous by this module's own standards — `RETRY_MS` opens at 30s assuming network stalls are common. **Fixed:** `pause()` now returns `true` when the field is clear and `false` when the deadline passes with a publish still running (the wait is `PAUSE_DRAIN_MS`, exported, and overridable per call so tests need not sit out 30 s). `manualPublish` checks that return and refuses — `{ ok: false, error }`, the shape every caller already handles — instead of starting a second full replacement. The watchers are still held on the refusal path, so `resume()` in the `finally` stays correct. `test/livesync-pause.test.js` drives a real watcher with a publish held open and asserts the false, the true once it lands, and that the caller's guard sits before the publish. --- ## Fix soon — degrades the product ### 5. A dead bucket-wipe endpoint still ships on every R2 site — FIXED 18 August 2026 `main/r2-worker.js:218` Every deployed R2 router Worker exposes `POST /__cheldrop_admin__/purge`, which deletes every object in the bucket. Nothing calls it any more — `cloudflare.js` purges through the R2 API instead and says so at line 545. The guarding secret is stored unencrypted in `sites.json` as `adminSecret`, a few fields from `cfToken`, which *is* safeStorage-encrypted. Anyone who reads that file can destroy the site's contents with no other credential. The check at line 219 is a plain `!==`, not the constant-time `safeEqual` used at line 197. **Fix:** remove the endpoint from the generated Worker. If it must stay, use `safeEqual` and encrypt `adminSecret` at rest. **Fixed:** the endpoint is gone from `r2-worker.js`, and with it the reason `adminSecret` existed — `deployR2Worker` no longer generates one, no publish result carries one, and `getDb()` deletes the field from records written before this, so the one plaintext secret in `sites.json` leaves disk. The deployed Worker now has no write path to the bucket at all; the app does everything through the R2 API with the token it already holds. Deleting the source only helps sites published afterwards, so the second half is a version gate: `R2_WORKER_VERSION` is stored per site as `workerVersion`, and a publish redeploys the Worker when the deployed script is behind it. That redeploy at first covered only unprotected sites — a deploy replaces the bindings wholesale, and a protected site redeployed without a fresh auth record (which needs the password; live sync has none) would come back public — which left protected sites on the old Worker, endpoint and all, until the next time the password was set. **Now closed:** the upload inherits the binding instead of rebuilding it, with `keep_bindings: ['secret_text']`, so the password crosses the redeploy without Cheldrop ever holding it. Since a lost binding would fail silently and publicly, the inherit path is bracketed by `checkAuthBinding`, which distinguishes *present*, *absent* and *unknown* — the endpoint is `GET /workers/scripts/{name}/secrets`, which returns binding names and never values. Absent or unknown beforehand means there is nothing safe to inherit, so the old Worker stays. Anything but present afterwards means the site may be answering without a password, so it is re-deployed with an auth record built from 32 random bytes nobody holds — locked, not public — and the publish fails asking for the password to be set again. The same check also stops a site whose entry says public but whose Worker holds an auth binding from being quietly unprotected by a router update. `test/r2-worker.test.js` asserts the script carries no admin endpoint and no delete call, and models the API's binding semantics — write-only secret values, `keep_bindings` inheritance, and a knob that makes that inheritance fail — to cover all seven redeploy decisions, including the lock. --- ### 6. Folder walking follows symlinks with no cycle guard — FIXED 18 August 2026 `main/sitefolder.js:757` `readFolderFiles` uses `fs.statSync`, which resolves symlinks, with neither loop detection nor a containment check against the folder root. `ln -s .. loop` inside a site folder makes `walk()` recurse until stack exhaustion or `ENAMETOOLONG`, during a publish or live sync, with no useful error. `ln -s ~/Documents refs` publishes that whole tree to the public URL. **Fix:** `fs.lstatSync`, and skip symlinks (or resolve and check containment). **Fixed:** the walk resolves and checks containment rather than skipping links outright, so a link used the way people use them — one file, one folder, inside the site — still publishes. `lstat` identifies the link, `realpath` resolves it, and anything landing outside the folder's own real path is skipped. Both sides of that test are real paths: a site folder in Dropbox or iCloud is itself reached through a link, and comparing the paths as written makes a folder fail its own containment test. Cycles are cut by the chain of directories open above the current one, not by every directory seen. The difference showed up in the tests: dedup on "seen" makes `sub/` and a `also-sub -> sub` link race, and whichever `readdir` returns second is dropped — so the real folder can lose to the link. On the open chain, `ln -s .. loop` and `ln -s . self` terminate while a linked sibling publishes under both names, which is what the folder actually holds. Dangling links are skipped rather than thrown on — on macOS the old code did not recurse forever, it aborted the publish with an uncaught `ELOOP` or `ENOENT` from deep inside the walk. A missing folder still throws, as before. `test/folder-walk.test.js` builds real folders and real links: the two exploits, both cycles, the linked sibling, the file link, the dangling link, and a site folder reached through a link. Six of its ten cases fail against the old walk. --- ### 7. Extensionless URLs work on the free tier and 404 after upgrading — FIXED 18 August 2026 `main/r2-worker.js:253` The R2 router resolves `/about` to `about/index.html`, then falls back to `about` — never `about.html`. The Workers backend is deployed with `html_handling: 'auto-trailing-slash'` (`cloudflare.js:373`), which does serve it. So a folder holding `about.html` answers `/about` on the free tier and 404s once the same site moves to `backend: r2`. **Fix:** add the `<path>.html` lookup to the router's fallback chain. **Fixed:** the chain is now `<path>/index.html`, `<path>.html`, `<path>`, and `/about/` falls back to `about.html` as well. The candidate list is a superset of the old one in the same order, so no address that resolved before resolves to something else now — the folder index deliberately keeps precedence over the sibling page, since that is what a live site is already serving. The only behaviour that moves is a site holding both `about.html` and a bare `about`, where the `.html` file now wins; that is the answer the free tier gives. Two neighbours went with it. The dot test that decides "this address names a file" read the whole path, so a directory called `v1.2` stopped index resolution for everything beneath it — it reads the last segment now. And `R2_WORKER_VERSION` is bumped to 3, which is what carries the routing fix to sites already live; with finding 5 closed, that now includes protected ones. `test/r2-routing.test.js` evaluates the generated Worker and drives it with real `Request` objects against a fake bucket that records every key looked for. Five of its eleven cases fail against the old router. --- ### 8. Deleting a site can leave its hostname attached — FIXED 18 August 2026 `main/cloudflare.js:808` (and `:645`) `deleteWorkerAndDomain` reads `/workers/domains` with no pagination and treats the first page as the whole account. Past that page it finds no match, skips the detach, and deletes the script anyway — leaving the hostname bound to a missing Worker and its DNS record behind. Re-publishing that subdomain later fails with "That subdomain already has a DNS record", the error `friendlyError` special-cases at line 51. This is a known bug class here: the comment at line 184 records an account whose 19 Pages projects showed as 10. `listR2Buckets` paginates by cursor; only these two domain lookups do not. **Fix:** paginate, the way `listPaged` and `listR2Buckets` already do. The exact account size that triggers it depends on Cloudflare's default page size for this endpoint — unverified, no account was available. **Fixed:** both lookups now go through `findWorkerDomain`, which asks for the one hostname it wants — `?hostname=<fqdn>`, the way `deleteCnameRecord` already queries DNS — so the answer no longer depends on how many domains the account holds. The filter is not trusted on its own: the match is confirmed here, and a list that comes back holding *other* hostnames means the filter was ignored, which falls back to a full paged sweep in `listWorkerDomains`. That sweep is deliberately careful, because this endpoint's paging is undocumented and the account to verify it against still was not available. It sends `page` and not `per_page` — `listPagesProjects` records an endpoint that rejects `per_page` with a 400, and a lookup whose failure mode is a stranded hostname is the wrong place to risk it. Termination follows what the response says rather than what it is assumed to do: no `result_info` means the whole list arrived at once, `total_pages` is followed when given, a short page ends the sweep when only `per_page` is given, and a page identical to page one means `page` was ignored and stops it. Every path is bounded at 50 pages. The attach side went with it, since it reads the same list: a re-publish on a large account could not see the domain it already had, so it re-attached and Cloudflare answered with the DNS-record clash `friendlyError` special-cases — turning an ordinary republish into that error. The delete side is unchanged in shape: still best effort, still deletes the script if the lookup itself fails. `test/worker-domains.test.js` runs each case against three fake servers — one honouring the hostname filter, one ignoring it and paging, one ignoring both and answering with everything — because the fix has to be right whichever this endpoint turns out to be. Four of its eighteen cases fail against the old lookup, and the two publish-side failures fail with the DNS-record error verbatim. --- ### 9. A file that disappears mid-publish freezes the UI — FIXED 18 August 2026 `renderer/src/pages/Publish.jsx:500` The `FileReader` promise inside `handlePublish` can reject and nothing catches it, so the function exits before `cleanup()` and before any error state is set. `status` stays `'publishing'` with no message and no way back but restarting, and the `onPublishProgress` listener from line 461 leaks. Every other failure path in the function calls `cleanup()` first. **Fix:** wrap the read in try/catch, call `cleanup()`, set the error state. **Fixed:** the read is wrapped, and the catch takes the same three steps every other failure path in the function already takes — `cleanup()`, `setStatus('error')`, a message — so a file that vanished between being picked and being published ends as an ordinary error the user can act on, rather than a stuck spinner. The message names the file and says to add the files again, since a `File` handle whose backing file has gone will not read on a retry either. --- ### 10. Quitting does not wait for a publish in flight — FIXED 18 August 2026 `main/index.js:114` `before-quit` calls `livesync.stopAll()`, which clears timers, closes watchers and sets status `off` — then returns. The in-flight `performPublish` is a floating promise nobody awaits, so the process dies mid-upload. A Live site's debounce fires, the publish starts, the user presses ⌘Q two seconds later. On R2 that lands inside the purge-then-upload window of finding 2, so the live site can be left empty. `livesync.pause()` already polls `s.publishing` for exactly this; `stopAll()` just does not use it. `tray.js:133` avoids `role: 'quit'` with the comment "quitting has to stop the watchers first" — which implies more safety than `stopAll()` delivers. And `updater.js` sets `autoInstallOnAppQuit = true`, so a quietly downloaded update turns every ordinary quit into this path without the user choosing it. **Fix:** `preventDefault()` in `before-quit`, await a drain (reuse `pause()`), then quit. Cap the wait so a hung publish cannot block quit forever. **Fixed:** `before-quit` now holds the quit, calls `livesync.pause()` — the same drain a manual publish uses, so the watchers are held and whatever is uploading gets to land — and only then runs the teardown it always ran, before quitting for real. The cap is 15 s rather than livesync's 30 s: this wait is in front of a user who just pressed ⌘Q, and a hung upload must not make quit look broken. When the deadline passes the app quits anyway, having tried. A second ⌘Q during the drain is held too — impatience is not permission to die mid-upload, and the cap is what bounds the wait. `test/quit-drain.test.js` (`npm run test:quitdrain`) drives the real handler with a publish held open and asserts the order: nothing is torn down while the drain runs, and the quit lands after it either way. --- ### 11. One downloaded update disables checking for later ones — FIXED 18 August 2026 `main/updater.js:150` `update-downloaded` sets `downloadedVersion`, and nothing ever clears it. From then on `checkForUpdatesManually` returns early for the rest of the process lifetime, re-offering the version already on disk. Download 1.1.0, pick "Later". Two weeks on, 1.2.0 and 1.3.0 have shipped; "Check for Updates…" still says "Cheldrop 1.1.0 is ready to install", with no hint newer versions exist and no way to look. The only escape is quitting (installing the stale 1.1.0) or restarting. This matters here for the reason the file's own header gives: Phase 4.3 makes Cheldrop a permanently-running login-item agent, so process lifetime is weeks. **Fix:** re-check anyway, and replace the pending download when the feed offers something newer. **Fixed:** the early return is gone — "Check for Updates…" always reaches the feed, and the handlers decide what to do with what came back. `autoDownload` is off so the decision is ours: a version already on disk is kept and offered for install rather than fetched a second time, and anything newer supersedes it — cancelling an in-flight download through its `CancellationToken` when one is running. The downloaded file stays installable on quit until its replacement lands, so a supersede that never finishes still leaves the user an upgrade. Ordering is a local semver compare rather than the transitively-hoisted `semver`: 1.10.0 has to read as newer than 1.9.0, and a prerelease as older than the release it leads to. Two dialogs stopped lying on the way: a pulled release now offers the ready update instead of "you are up to date", and a failed check offers it instead of leaving the user an error with no way to install what is already downloaded. `test/updater-recheck.test.js` (`npm run test:updaterecheck`) drives the module as one session — download, "Later", weeks of newer releases — with a fake feed, and asserts the check happens every time, that nothing is downloaded twice, and that the supersede cancels. --- ## Minor ### 12. A comment promises a hash the code never applies — FIXED 18 August 2026 `main/ipc.js:441` // Use a hash of the name to avoid collisions across concurrent publishes const safeName = name.replace(/[^a-zA-Z0-9._-]/g, '_') Sanitising blocks traversal (separators become `_`, a bare `..` fails as `EISDIR`), so the security half is sound. It does nothing about collisions: a manual publish and an MCP `publish_now` staging `index.html` at the same moment write the same path, and one site publishes the other's file. **Fix:** hash as the comment says, or correct the comment. **Fixed:** a hash of the name would not have helped — the collision is two publishes staging the *same* name, which hashes the same. Each call now names its file after the call: a pid/counter/random stamp in front of the sanitised name, so concurrent publishes cannot meet. The stamp goes in front because the extension decides content type and the Workers asset hash, and the publish payload carries the real name beside the path, so no downstream reader loses anything. Sanitising stays and keeps doing the job it was actually doing — containing the write — with a fallback name so a name that sanitises to nothing cannot address the directory itself. Unique names never overwrite, so the staging directory is swept of day-old files on each publish; publishes take seconds, so anything older is litter. `test/temp-stage.test.js` (`npm run test:tempstage`) stages `index.html` twice and asserts each publish keeps its own bytes, and that traversal still does not survive. ### 13. The tray menu rebuilds once per uploaded file — FIXED 18 August 2026 `main/tray.js:201` The `livesync.onStatus` handler calls `refreshSites()` on every status event, and progress events fire per file (`cloudflare.js:512`). Each one runs `buildTrayMenu()` → `Menu.buildFromTemplate`, `setContextMenu`, `setToolTip`, and `trayIcon()`, which constructs a fresh `nativeImage` from the PNG path. A 200-file R2 publish does all of that 200 times in a few seconds. The visible symptom is the tray menu churning while the user has it open — replacing a macOS context menu mid-display disrupts it. The db read is not the cost (`getDb()` memoises); the native menu construction and image loading are. **Fix:** rebuild on state transitions only, or coalesce on a short timer. **Fixed:** both, split by what the event actually changes. A site that starts, lands or fails changes the glyph, the icon and the tooltip, so a state transition still rebuilds at once — the menu bar must not lag the thing it reports. Progress inside a publish changes one line of text and arrives once per file, so those coalesce onto a 1 s trailing timer: the detail still lands, the last one wins, and a 200-file publish costs one rebuild instead of 200. The timer is cleared in `destroyTray()`, so a quit mid-publish cannot fire a rebuild at a destroyed tray a second later. `test/tray-menu.test.js` (`npm run test:traymenu`) drives a 200-file publish through the real handler and asserts the collapse, that the menu ends on the last progress line, and that landing still draws immediately. --- ## Checked and found sound Recorded so this is not read as a fault list only. Each is a place a specific bug was looked for and the handling was already correct. - Electron hardening — `contextIsolation: true`, `nodeIntegration: false`, a fixed verb list in preload, no `dangerouslySetInnerHTML` anywhere. - Token storage — safeStorage round-trip, prefix-marked ciphertext, in-place plaintext migration, and the encryption-unavailable fallback. - MCP transport — 127.0.0.1 only, `crypto.timingSafeEqual` with a length pre-check, Origin allowlist against DNS rebinding, 1 MB body cap. - Licence gates — free-tier and R2 gates enforced in `performPublish`, so a hand-edited `cheldrop.yaml` cannot walk past them. - Licence resilience — inconclusive checks keep the activation inside a 7-day grace window; only a definitive rejection revokes. - Reconciliation — one unreadable account makes a kind *unknown*, not *missing*. - `safeHref` — correctly rejects `javascript:`, including newline/tab variants. - Path containment — `readTheme`, `readBundledTheme`, `materializeSiteFolder`. - `writeSiteConfig` skips identical writes, which is what stops a Live folder publishing itself in a loop. - js-yaml 5.2.3 — `load` is safe by default. --- ## Scope — read this before trusting a gap Not every file got the same depth. **Read line by line (~6,100 lines):** `preload.js`, `index.js`, `ipc.js`, `livesync.js`, `cloudflare.js`, `r2-worker.js`, `sitefolder.js`, `reconcile.js`, `menu.js`, `mcp-server.js`, `tray.js`, `updater.js`; `renderer.js` lines 1–200; `license.js` lines 1–80; `Publish.jsx` around the publish handler. **Targeted search only** — greps for specific bug classes, not a full read: `renderer.js` lines 200–1265, the rest of `license.js`, `Sites.jsx`, `Settings.jsx`, `Docs.jsx`, `QuickStart.jsx`. **Not opened:** `test/` (798, read for coverage only), `build/` scripts, bundled templates. No finding was verified against a live Cloudflare account. The separate review of commit `6d923a4` (the 16px mark fallback) carries two findings of its own: a favicon ring that still renders as a sub-pixel hairline (`build/brand/mark-dot-light.svg:5`), and tray geometry that has drifted from the SVG both files cite (`build/tray/make-tray-icons.py:53`).