Refresh checkpoint with upload pick tokens
This commit is contained in:
parent
6e627a3341
commit
4fb789de37
1 changed files with 56 additions and 43 deletions
|
|
@ -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
|
A stopping point you can return to if this session is closed. Everything below was
|
||||||
verified green at the moment this file was written.
|
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
|
## Where things are
|
||||||
|
|
||||||
- Project: `/home/avi/Projects/0_Nostr`
|
- 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.
|
- 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
|
Fixes from the 2026-08-21 security audit.
|
||||||
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).
|
|
||||||
|
|
||||||
- **Electron** (`frontend/electron/main.ts`):
|
**#1 IPC method allowlist (`4bde395`, `main.ts` only):**
|
||||||
- New `RENDERER_METHODS` set: the three native methods (`pick_image`, `link_preview`,
|
- `RENDERER_METHODS` set in the Electron main process: the three native methods
|
||||||
`upload_image`) plus every Rust-backend method actually used by `frontend/src/lib/api.ts`
|
(`pick_image`, `link_preview`, `upload_image`) plus every Rust-backend method used by
|
||||||
(init/get_state, profiles, publish, feed, relays, settings, vault password/unlock,
|
`frontend/src/lib/api.ts`.
|
||||||
reveal_secret_key, signer control/approve).
|
- `isAllowedMethod()` gate at the top of the `backend:request` handler; unknown methods get
|
||||||
- `isAllowedMethod()` gate at the top of the `backend:request` handler; anything not in
|
`{status:'error', code:'unknown_method'}`, are logged, and never reach the backend.
|
||||||
the list returns `{status:'error', code:'unknown_method'}` and is logged to stderr —
|
- No behaviour change for the real UI (only `api.ts` calls `window.backend.request`).
|
||||||
it never reaches the backend process.
|
|
||||||
- No behaviour change for the real UI: `window.backend.request` is only ever called from
|
**#2 Upload pick tokens (`6e627a3`):**
|
||||||
`api.ts`, whose full surface is covered by the allowlist (confirmed by grep + tests).
|
The renderer used to send raw filesystem paths back to main for upload, so a compromised
|
||||||
- Remaining audit items (not yet done): token-based upload path validation (#2), CSP
|
page could read+publish arbitrary local files to nostr.build. Now:
|
||||||
`'unsafe-inline'` removal + window-open deny (#3), legacy-vault permissions (#4),
|
- `pickImage()` (`main.ts`) mints a random 32-hex-char token per picked file via
|
||||||
permission race windows (#5), key zeroization (#6).
|
`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
|
## 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)
|
## 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 lint # clean (pre-existing module-type warning only)
|
||||||
npm run format:check # clean
|
npm run format:check # clean
|
||||||
npm test # 78 passed (14 files)
|
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/)
|
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
|
## How to resume
|
||||||
|
|
||||||
1. Open the repo: `cd /home/avi/Projects/0_Nostr`
|
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.
|
on top.
|
||||||
3. Launch as usual: `cd frontend && npm start` (backend already built in `target/release/`).
|
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
|
4. Re-run verification with the commands above.
|
||||||
`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.
|
|
||||||
|
|
||||||
## Outstanding / next steps (if you continue)
|
## 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;
|
1. **CSP** — drop `'unsafe-inline'` from `script-src` in `frontend/index.html`; add
|
||||||
issue random tokens per picked file and accept only those (`main.ts`).
|
`setWindowOpenHandler(() => ({action:'deny'}))` + route external links through
|
||||||
2. **CSP** — drop `'unsafe-inline'` from `script-src` in `frontend/index.html`; add
|
`shell.openExternal` in `main.ts`.
|
||||||
`setWindowOpenHandler` deny + `shell.openExternal` for external links in `main.ts`.
|
2. **Legacy vault perms** — repo-root `profiles_vault.json` is group-readable (0664);
|
||||||
3. **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`.
|
||||||
chmod 600 / delete after confirming migration; also auto-tighten during migration in
|
3. **Write-race fixes** — create files with mode 0600 at creation time in `vault.rs`
|
||||||
`src/vault.rs`.
|
(`write_restricted`, `backup_file`).
|
||||||
4. **Write-race fixes** — create files with mode 0600 at creation time in `vault.rs`.
|
4. **Zeroize** — wipe decrypted key material via the `zeroize` crate.
|
||||||
5. **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.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue