Cheldrop Docs

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, <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.

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).

August 19, 2026 Raw file Download