Refresh checkpoint: profile metadata publishing and pictures
This commit is contained in:
parent
a6329d5640
commit
8dd367805a
1 changed files with 87 additions and 167 deletions
|
|
@ -1,4 +1,4 @@
|
|||
# Checkpoint — All audit items closed (2026-08-21)
|
||||
# Checkpoint — Profile metadata publishing + profile pictures (2026-08-22)
|
||||
|
||||
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,177 +6,97 @@ verified green at the moment this file was written.
|
|||
## Where things are
|
||||
|
||||
- Project: `/home/avi/Projects/0_Nostr`
|
||||
- Git repo: `master` @ `f7db29e` ("Add SSRF guard, signer queue cap, secret echo, request
|
||||
timeout"). Before it: `130d7e2` (zeroization), `4461307` (owner-only writes), `4bd7660`
|
||||
(legacy vault perms), `4d4dfde` (CSP + navigation guards), `6e627a3` (upload tokens),
|
||||
`4bde395` (IPC allowlist).
|
||||
- Git repo: `master` @ `a6329d5` ("Publish profile metadata (name + picture) so external
|
||||
clients show it"). Before it: `db81f8d` (profile deletion with undo), `9a8f334`
|
||||
(audit checkpoint refresh), and the 2026-08-21 audit-fix commits (`f7db29e`, `d90b6e5`,
|
||||
`130d7e2`).
|
||||
- Working tree is **clean** apart from this checkpoint update, which is committed right after.
|
||||
|
||||
## What was completed: the full 2026-08-21 security audit remediation
|
||||
## What was completed
|
||||
|
||||
All ten findings from the 2026-08-21 security audit are fixed.
|
||||
**Problem:** profiles created in the app never published a Nostr kind 0 metadata event,
|
||||
so other clients showed generated petnames ("evil iguana", "homeless leech") or a
|
||||
truncated npub instead of the user's chosen name.
|
||||
|
||||
**#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`).
|
||||
1. **Automatic metadata on creation** — `create_profile` now publishes a kind 0 event
|
||||
with the label as `name`/`display_name` to all enabled relays (best-effort; relay
|
||||
failures never block creation).
|
||||
2. **"Publish name" action for existing profiles** — new button on every Profiles-screen
|
||||
card plus CLI `publish-name <npub>`. Returns a per-relay report shown in the UI.
|
||||
3. **Profile pictures end-to-end** —
|
||||
- Vault: optional `picture: Option<String>` per profile (serde default → old vaults
|
||||
load unchanged).
|
||||
- Backend: `set_profile_picture` validates http(s) URLs only, stores the URL, and
|
||||
publishes kind 0 including `picture`; clearing supported (`None`).
|
||||
- GUI: "Picture" button opens a modal — paste a URL, upload a file via the existing
|
||||
nostr.build pipeline, or remove; avatar shows the picture everywhere in-app.
|
||||
- CLI: `set-picture <npub> <url>`.
|
||||
4. **Nested-runtime safety** — metadata publishing runs on a dedicated OS thread with its
|
||||
own tokio runtime, so both sync (CLI) and async (IPC server) callers are safe.
|
||||
5. **Finished prior session's delete/undo work** — exposed `undo_history` in
|
||||
`AppStateView`, fixed invalid Button variants / missing icon / null-safety errors so
|
||||
the frontend typechecks again.
|
||||
6. **Test hygiene fix** — `Settings::default()` points at real relays and tests were
|
||||
silently publishing events to them (one got rate-limited by damus.io). All test suites
|
||||
now use offline settings; test time dropped from ~126 s to ~3 s.
|
||||
|
||||
**#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 added in this session
|
||||
|
||||
**#3 Header-based CSP + navigation/window guards (`4d4dfde`):**
|
||||
- The static CSP meta tag was **removed** from `frontend/index.html` and replaced with
|
||||
response headers stamped by `main.ts` (`onHeadersReceived`, mainFrame only):
|
||||
- `CSP_PROD` for `app://` pages: `script-src 'self'` — **no `'unsafe-inline'`**, so an
|
||||
injected `<script>` cannot execute. `connect-src 'self'` (all real network goes through
|
||||
the backend/main process); img-src keeps `https:` for remote feed/preview images.
|
||||
- `CSP_DEV` when `NOSTR_GUI_DEV_URL` matches: keeps `'unsafe-inline'` (Vite's React-refresh
|
||||
preamble is an inline script) but pins `connect-src` to localhost instead of the old
|
||||
wide-open `ws:` — HMR still works.
|
||||
- The meta had to go because it would also have blocked the dev preamble; headers let us
|
||||
ship strict-prod / workable-dev from one HTML file.
|
||||
- `will-navigate`: any navigation away from app:// or the dev origin is blocked; http(s)
|
||||
targets open in the system browser via `shell.openExternal`.
|
||||
- `setWindowOpenHandler`: all `target="_blank"` popups are denied; external links go to the
|
||||
system browser. Unknown schemes are denied without opening anything.
|
||||
- `a6329d5` Publish profile metadata (name + picture) so external clients show it
|
||||
|
||||
**#4 Legacy vault permissions (`4bd7660`):**
|
||||
- The original CLI left `profiles_vault.json` files with default (often group-readable)
|
||||
permissions containing plaintext keys. Now:
|
||||
- On **every** startup, `load_vault()` calls the new `harden_stray_legacy_vaults()`
|
||||
(`src/vault.rs`): each known legacy location is checked, and any file that parses as a
|
||||
populated vault is chmod'd to 0600 — whether or not migration ever runs.
|
||||
- During migration itself, the legacy file is tightened before the backup copy is made.
|
||||
- Hardening is best-effort (`let _ =`) so odd filesystems can never break vault loading;
|
||||
unparsable and keyless files are deliberately not touched.
|
||||
- One-time local cleanup done by hand in this session: repo-root `profiles_vault.json`
|
||||
was chmod'd from 0664 → 0600.
|
||||
- New tests: recognised legacy vault tightened to 0600; unparsable file ignored; empty /
|
||||
encrypted-but-populated vaults classified correctly.
|
||||
## Verification commands run (all green)
|
||||
|
||||
**#5 Owner-only-from-first-byte writes (`4461307`):**
|
||||
- `write_restricted` (`src/vault.rs`): the tmp file is now created with `.mode(0o600)`
|
||||
(OpenOptionsExt), so it never exists under default umask permissions. The explicit
|
||||
`set_permissions` stays, covering a tmp file left over by a crashed earlier run (where
|
||||
`mode` would not apply to an existing file).
|
||||
- `backup_file`: replaced `fs::copy` — which gives the new file the *source's* permission
|
||||
bits, so backing up a group-readable legacy vault briefly produced a fully readable copy —
|
||||
with a manual open-at-0600 + `io::copy`, making backups owner-only from their first byte.
|
||||
- New test: backup of a 0664 source comes out 0600 with identical content.
|
||||
|
||||
**#6 Key material zeroization (`130d7e2`, adds `zeroize = "1"`):**
|
||||
Decrypted secrets and derived keys no longer linger in unscrubbed heap memory:
|
||||
- `crypto.rs`: `decrypt_secret` returns `Zeroizing<String>` (self-shredding on drop); the
|
||||
UTF-8 error path wipes the raw bytes too; a failed Argon2 derivation wipes its key buffer.
|
||||
- `profiles.rs`: `resolve_secret_key` / `resolve_active_secret_key` now return
|
||||
`Zeroizing<String>`, so every transient plaintext key flowing to publish/upload/signer is
|
||||
wiped when its scope ends. Newly generated keys in `create_profile` are wrapped the same
|
||||
way. `reveal_secret_key` still hands a display copy to the UI by design.
|
||||
- `app.rs`: `lock()` and a new `Drop for App` zeroize the session vault key; wrong-password
|
||||
derivations wipe their throwaway keys; password change wipes the previous vault key and
|
||||
the old unlock key; `remove_password` moves decrypted values into storage via
|
||||
`std::mem::take` without extra copies (plaintext-at-rest is that feature's purpose).
|
||||
- Callers in `publish.rs`, `uploads.rs`, `signer.rs` needed no changes (deref coercion).
|
||||
|
||||
**Smaller items (`f7db29e`, main.ts + signer.rs):**
|
||||
- **SSRF guard**: `fetchLinkPreview` resolves the URL's host and refuses loopback/private/
|
||||
link-local targets — literal IPs, DNS answers, and localhost/.local names all checked
|
||||
(`ipv4IsPrivate`/`ipv6IsPrivate`/`resolvesToPrivateAddress`). Crafted note links can no
|
||||
longer make the app probe localhost or the LAN.
|
||||
- **Signer queue cap** (`MAX_PENDING_APPROVALS = 20`): a relay flooding `sign_event`
|
||||
requests cannot grow the approval queue unboundedly or bury a genuine prompt; overflow
|
||||
requests get the standard "no decision" error.
|
||||
- **NIP-46 secret echo**: when the nostrconnect:// link carried a secret, the client's
|
||||
`connect` request must echo it back or it is refused ("did not include the expected
|
||||
secret"), proving the link arrived unmodified.
|
||||
- **Backend request timeout**: `backendRequest` reaps any round-trip after 120s (generous
|
||||
enough for multi-relay publishes), deleting its pending entry so a hung backend cannot
|
||||
leak promises.
|
||||
|
||||
## Commits
|
||||
|
||||
- `f7db29e` "Add SSRF guard, signer queue cap, secret echo, request timeout" — main.ts +
|
||||
signer.rs (+204/−3).
|
||||
- `130d7e2` "Zeroize transient secret key material in memory" — Cargo.toml, crypto.rs,
|
||||
profiles.rs, app.rs (+77/−23).
|
||||
- `4461307` "Create vault files owner-only from the first byte" — src/vault.rs (+48/−2).
|
||||
- `4bd7660` "Tighten permissions on leftover legacy vault files" — src/vault.rs (+79/−3).
|
||||
- `4d4dfde` "Enforce header-based CSP and block window open/navigation" — main.ts + index.html.
|
||||
- `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)
|
||||
Rust (repo root):
|
||||
|
||||
```
|
||||
cargo test # 84 passed
|
||||
cargo clippy --all-targets # clean
|
||||
cargo test # 96 passed; 0 failed
|
||||
cargo clippy --all-targets # 0 errors/warnings from session code
|
||||
cargo fmt --check # clean
|
||||
cargo build --release # ok
|
||||
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
|
||||
npm run build # rebuilds the React bundle (dist/)
|
||||
cargo build --release # success
|
||||
```
|
||||
|
||||
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.
|
||||
- DevTools console: `document.cookie` / injected `<script>` does not run; Application tab
|
||||
shows the CSP header on the app:// document (#3). Link previews open in the system browser.
|
||||
Frontend (`frontend/`):
|
||||
|
||||
## How to resume
|
||||
```
|
||||
npm test # 14 files, 78 tests passed
|
||||
npm run typecheck # clean
|
||||
npm run lint # 0 errors
|
||||
npm run format:check # clean
|
||||
npm run build # vite build success
|
||||
npm run electron:build # tsc electron main success
|
||||
```
|
||||
|
||||
1. Open the repo: `cd /home/avi/Projects/0_Nostr`
|
||||
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. Re-run verification with the commands above.
|
||||
## How to use / reproduce
|
||||
|
||||
## Outstanding / next steps (if you continue)
|
||||
GUI:
|
||||
|
||||
Nothing outstanding from the audit — all ten findings are closed:
|
||||
```bash
|
||||
cd ~/Projects/0_Nostr/frontend && npm start
|
||||
```
|
||||
|
||||
| # | Finding | Fix commit |
|
||||
|---|---------|-----------|
|
||||
| 1 | Unrestricted renderer→backend IPC | `4bde395` |
|
||||
| 2 | Arbitrary file upload paths | `6e627a3` |
|
||||
| 3 | CSP `unsafe-inline`, no nav guards | `4d4dfde` |
|
||||
| 4 | Legacy vault world-readable | `4bd7660` |
|
||||
| 5 | Permission race windows on write | `4461307` |
|
||||
| 6 | Key material not zeroized | `130d7e2` |
|
||||
| 7 | Link-preview SSRF | `f7db29e` |
|
||||
| 8 | Signer queue flooding | `f7db29e` |
|
||||
| 9 | NIP-46 secret not verified | `f7db29e` |
|
||||
| 10 | No backend request timeout | `f7db29e` |
|
||||
- New profiles publish their name automatically on creation.
|
||||
- Existing profiles: **Profiles → Publish name** button, or **Picture** button to set a
|
||||
photo (URL paste or file upload) which publishes immediately.
|
||||
|
||||
**Profile deletion with undo functionality** (2026-08-22):
|
||||
- Users can delete a profile via `nostr-manager-backend delete-profile <npub>`, which moves the profile to an in-memory undo stack rather than permanently removing it
|
||||
- Profiles can be restored with `nostr-manager-backend undo-delete`, which pops the last deleted profile from the undo stack and re-adds it to the vault (becoming active if no other profile exists)
|
||||
- The undo stack is in-memory only (lost on process exit); a persistent implementation would require vault metadata changes
|
||||
- Vault must be unlocked or unencrypted for deletion to be permitted
|
||||
- Existing profile data and vault integrity are preserved
|
||||
- CLI commands: `delete-profile <npub>` and `undo-delete`
|
||||
- Rust changes verified: `cargo test` (91 tests), `cargo clippy --all-targets` clean, `cargo fmt --check` clean, `cargo build --release` successful
|
||||
CLI:
|
||||
|
||||
Possible future work (not audit items): DNS-rebinding TOCTOU in the SSRF guard, OS keyring integration for the vault password, an automated dependency-audit CI job (`cargo audit`, `npm audit`), GUI integration for profile deletion/undo in the ProfilesScreen.
|
||||
```bash
|
||||
B=~/Projects/0_Nostr/target/release/nostr-manager-backend
|
||||
$B list # show profiles
|
||||
$B relays enable wss://nos.lol # enable at least one relay first
|
||||
$B publish-name <npub> # republish stored name
|
||||
$B set-picture <npub> https://…/img.png # set + publish picture
|
||||
```
|
||||
|
||||
Verified live during the session: kind 0 events confirmed present on nos.lol,
|
||||
relay.primal.net, relay.damus.io and the user's own wss://nostr.l484.com via direct
|
||||
websocket queries; Iris shows the stored names after publish.
|
||||
|
||||
## Notes & next steps
|
||||
|
||||
- Nostr has no relay-to-relay sync: names/pictures are only visible on relays they were
|
||||
published to. Keep major relays enabled so clients that don't read your own relay can
|
||||
see the profile.
|
||||
- Clients cache profiles; hard-refresh (Ctrl+Shift+R) after republishing.
|
||||
- Possible follow-ups: rename profiles (edit label + republish), NIP-05 identifiers,
|
||||
banner/about fields in the edit UI.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue