diff --git a/CHECKPOINT-encryption.md b/CHECKPOINT-encryption.md index 8fa618a..d6fd71d 100644 --- a/CHECKPOINT-encryption.md +++ b/CHECKPOINT-encryption.md @@ -1,4 +1,4 @@ -# Checkpoint — Renderer IPC method allowlist (2026-08-21) +# Checkpoint — Upload pick tokens (2026-08-21) A stopping point you can return to if this session is closed. Everything below was verified green at the moment this file was written. @@ -6,35 +6,45 @@ verified green at the moment this file was written. ## Where things are - Project: `/home/avi/Projects/0_Nostr` -- Git repo: `master` @ `4bde395` ("Restrict renderer IPC to an explicit method allowlist"). +- Git repo: `master` @ `6e627a3` ("Replace upload file paths with single-use pick tokens"). + Before it: `4bde395` ("Restrict renderer IPC to an explicit method allowlist"), then + `dafed33` (checkpoint refresh), on top of contact-aware feed (`a1445d1`) etc. - Working tree is **clean** apart from this checkpoint update, which is committed right after. -- Underneath sit the contact-aware feed (`a1445d1`), feed aggregation (`37a9d94`) and the - NIP-46 signer work, all committed. -## What was completed: security hardening item #1 (IPC allowlist) +## What was completed: security hardening items #1 and #2 -First fix from the 2026-08-21 security audit. The Electron main process now enforces an -explicit allowlist of renderer-callable methods, so a compromised web page inside the app -can no longer invent arbitrary backend calls (e.g. unknown/future methods or typos that -might alias onto sensitive ones). +Fixes from the 2026-08-21 security audit. -- **Electron** (`frontend/electron/main.ts`): - - New `RENDERER_METHODS` set: the three native methods (`pick_image`, `link_preview`, - `upload_image`) plus every Rust-backend method actually used by `frontend/src/lib/api.ts` - (init/get_state, profiles, publish, feed, relays, settings, vault password/unlock, - reveal_secret_key, signer control/approve). - - `isAllowedMethod()` gate at the top of the `backend:request` handler; anything not in - the list returns `{status:'error', code:'unknown_method'}` and is logged to stderr — - it never reaches the backend process. - - No behaviour change for the real UI: `window.backend.request` is only ever called from - `api.ts`, whose full surface is covered by the allowlist (confirmed by grep + tests). -- Remaining audit items (not yet done): token-based upload path validation (#2), CSP - `'unsafe-inline'` removal + window-open deny (#3), legacy-vault permissions (#4), - permission race windows (#5), key zeroization (#6). +**#1 IPC method allowlist (`4bde395`, `main.ts` only):** +- `RENDERER_METHODS` set in the Electron main process: the three native methods + (`pick_image`, `link_preview`, `upload_image`) plus every Rust-backend method used by + `frontend/src/lib/api.ts`. +- `isAllowedMethod()` gate at the top of the `backend:request` handler; unknown methods get + `{status:'error', code:'unknown_method'}`, are logged, and never reach the backend. +- No behaviour change for the real UI (only `api.ts` calls `window.backend.request`). + +**#2 Upload pick tokens (`6e627a3`):** +The renderer used to send raw filesystem paths back to main for upload, so a compromised +page could read+publish arbitrary local files to nostr.build. Now: +- `pickImage()` (`main.ts`) mints a random 32-hex-char token per picked file via + `crypto.randomBytes`; tokens map to the file record in `pickedTokens` (main-process memory + only). The renderer receives `{token, name, mime}` — it never sees any path. +- `upload_image` accepts only a valid `token`; the token is consumed (deleted) before the + upload starts, so each pick authorises exactly one upload. Unknown/used/expired tokens + return `{status:'error', code:'unknown_token', message:'That image selection has expired. + Please attach it again.'}` — the user just re-picks the image. +- Renderer updates: `PickedImage.path` → `PickedImage.token` (`types.ts`), `api.uploadImage(token)` + (`api.ts`), context type + callback (`AppProvider.tsx`), `ComposeScreen.onAttach` uses + `image.token`. +- Fake backend mirrors the contract: `upload_image` without a non-empty `token` throws an + `unknown_token` error, so tests exercise the same protocol rule. ## Commits -- `4bde395` "Restrict renderer IPC to an explicit method allowlist" — `main.ts` only (+49 lines). +- `6e627a3` "Replace upload file paths with single-use pick tokens" — main.ts, api.ts, + types.ts, AppProvider.tsx, ComposeScreen.tsx, fakeBackend.ts (+62/−27). +- `4bde395` "Restrict renderer IPC to an explicit method allowlist" — main.ts (+49 lines). +- `dafed33` "Refresh checkpoint with IPC allowlist hardening". ## How it was verified (all green) @@ -47,31 +57,34 @@ npm run typecheck # clean (frontend/) npm run lint # clean (pre-existing module-type warning only) npm run format:check # clean npm test # 78 passed (14 files) -npm run electron:build # compiles the Electron main process with the new gate +npm run electron:build # compiles the Electron main process npm run build # rebuilds the React bundle (dist/) ``` +Manual protocol checks worth doing once: +- DevTools console: `window.backend.request('not_a_method')` → `unknown_method` envelope (#1). +- DevTools console: `window.backend.request('upload_image', {path:'/etc/passwd'})` → + `unknown_token` error; no file is read (#2). Real attachment flow works unchanged. + ## How to resume 1. Open the repo: `cd /home/avi/Projects/0_Nostr` -2. State is committed: `git status` should be clean; `git log --oneline -3` shows `4bde395` +2. State is committed: `git status` should be clean; `git log --oneline -4` shows `6e627a3` on top. 3. Launch as usual: `cd frontend && npm start` (backend already built in `target/release/`). -4. To see the gate working: from the DevTools console run - `window.backend.request('not_a_method')` → resolves to a `unknown_method` error envelope; - normal screens keep working unchanged. -5. Re-run verification with the commands above. +4. Re-run verification with the commands above. ## Outstanding / next steps (if you continue) -Security-audit follow-ups in priority order: +Remaining audit items in priority order: -1. **Upload path tokens** — `upload_image` currently reads any renderer-supplied file path; - issue random tokens per picked file and accept only those (`main.ts`). -2. **CSP** — drop `'unsafe-inline'` from `script-src` in `frontend/index.html`; add - `setWindowOpenHandler` deny + `shell.openExternal` for external links in `main.ts`. -3. **Legacy vault perms** — repo-root `profiles_vault.json` is group-readable (0664); - chmod 600 / delete after confirming migration; also auto-tighten during migration in - `src/vault.rs`. -4. **Write-race fixes** — create files with mode 0600 at creation time in `vault.rs`. -5. **Zeroize** — wipe decrypted key material via the `zeroize` crate. +1. **CSP** — drop `'unsafe-inline'` from `script-src` in `frontend/index.html`; add + `setWindowOpenHandler(() => ({action:'deny'}))` + route external links through + `shell.openExternal` in `main.ts`. +2. **Legacy vault perms** — repo-root `profiles_vault.json` is group-readable (0664); + chmod 600 / delete after confirming migration; auto-tighten during migration in `src/vault.rs`. +3. **Write-race fixes** — create files with mode 0600 at creation time in `vault.rs` + (`write_restricted`, `backup_file`). +4. **Zeroize** — wipe decrypted key material via the `zeroize` crate. +5. Smaller: link-preview SSRF guard (block loopback/private IPs), signer pending-cap, + NIP-46 secret echo check, backend request timeout.