Security hardening (P0 RCE + audit follow-through), web macro editor, and redesign #6
Reference in New Issue
Block a user
Delete Branch "security/p0-rce-hardening"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Closes the unauthenticated remote-code-execution chain found in a security audit and continues through the rest of the audit's findings, adding web-based macro authoring and a visual redesign. 7 commits, grouped by concern.
Security
/api/*routes and the/wshandshake (timing-safe compare); PWA shell stays public to bootstrap. Macroappcommands no longer run through a shell (shell=Trueremoved;shlex-tokenized, exec'd directly) — kills;/&&/|/$()/redirection injection. LAN binding is configurable (web.allow_lan). Image path-traversal in/api/imagefixed. Anti-clickjacking response headers./healthno longer leaks session IDs; credentials removed from URLs (header-based); sessions bounded + TTL-pruned; 12-char session IDs with rejection sampling; atomic/debounced session store; helmet CSP, trust proxy, restricted CORS; sane.env.example.innerHTML), strict CSP, no inline handlers,JSON.parseguarded.Robustness
macros.jsonwrites, execution moved outside the lock, debouncedlast_used, key/wait validation./api/executeoffloaded off the event loop, image upload hardened (size cap, Pillow validation, server-generated name, relative path only).minimize_to_trayhonored.Features & design
Verification
End-to-end against the real FastAPI server in a headless browser: token auth (401/200), WebSocket connect, macro created through the web editor (correct schema, atomic persist, live refresh), and the shell-injection RCE confirmed closed (a redirection payload did not execute). Relay TypeScript type-checks clean; Python compiles; concurrent macro-store writes verified deadlock-free.
Review round (included in this branch)
An automated completeness review of the branch confirmed all intended changes are present and the P0 RCE chain is genuinely closed, and caught two Major issues that were then fixed here:
sessionIdalone and shared between the web/desktop paths, letting anyone with the share link lock out the owner and block the desktop. Now namespaced per-path and per-IP.macropad-relay/public/app.html. Now rebuilt with safe DOM construction, externalized scripts, andscript-src 'self'.All minor findings were also addressed (credentials out of local image URLs, local auth-failure state, tightened
connect-src, relay enumeration-oracle removal, atomic session cap, backoff-after-auth,ALLOWED_ORIGINSdeploy note). The relay web client is intentionally kept execute-only (documented in the README).Not run interactively
The PySide6 desktop GUI and the live relay were reviewed + compile/type-checked but not exercised at runtime (no desktop session / relay deployment here) — worth a manual smoke test.
🤖 Generated with Claude Code
The local FastAPI server previously exposed macro create/execute over the network with no authentication while bound to 0.0.0.0, and macro `app` commands ran via subprocess with shell=True — an unauthenticated RCE for anyone on the LAN (and, via the relay, the internet). - Mandatory per-install auth token (secrets.token_urlsafe) required on all /api/* routes and the /ws handshake; timing-safe hmac.compare_digest. PWA shell + /static assets remain public to bootstrap. - Token auto-generated and persisted in settings.json (now chmod 0600). - Bind host is configurable (web.allow_lan); token is the security boundary. - macro `app` launch no longer uses shell=True — commands are tokenized with shlex and exec'd directly, killing ;/&&/|/$() shell chaining. - Relay client injects the local token and only forwards /api/* paths, and no longer forwards attacker-supplied headers to the local server. - Fixed path traversal in /api/image/{path} (realpath-confined to macro_images). - GUI embeds the token in the LAN URL/QR; PWA reads it and sends it on API, WebSocket, and image requests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Security: - Eliminate DOM-based XSS: renderMacros/renderTabs rebuilt with createElement/textContent/setAttribute + addEventListener. No macro name, id, image_path, or category ever reaches innerHTML. - Remove all inline on*= handlers; add a strict Content-Security-Policy meta (script-src 'self', object-src 'none', base-uri 'none', frame-ancestors 'none'). - Guard JSON.parse on WebSocket frames. Robustness: - Service worker: network-first for the app shell + JS/CSS so fixes actually ship; synthetic 503 for offline API calls; drop no-op /ws intercept; bump cache version. - WebSocket reconnect: exponential backoff (1s→30s), tear down old socket before reconnect, stop on pagehide/beforeunload. - Tab-switch race fixed with a request-id guard; executing state cleared in finally. Accessibility: - Macro cards and tabs are real buttons with aria-labels/roles; toast + connection status are aria-live; removed user-scalable=no. Feature — web-based macro creation/editing: - Floating "+" button + modal command-sequence builder (text/key/hotkey/ wait/app) with reorder/remove; create via POST, edit via PUT (prefilled from GET /api/macro/{id}), delete with a named confirm. Works in both local and relay auth modes. Empty state now offers "Create your first macro". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>