mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-11-03 19:34:45 +00:00
Four independent reviewers went over the branch. The worst findings were in
code I had just written and reviewed myself, which is the argument for
having done this.
CRITICAL
- A mnemonic was validated in one form and derived from in another. @scure's
normalize only counts words, so a phrase with a trailing newline passed
validation and then derived a completely different seed, while Python
canonicalizes and derives the correct one. The result was a vault encrypted
under a key that neither implementation -- nor the user's written-down
phrase -- could ever reproduce. Mnemonics are now canonicalized (NFKD,
lowercase, single-space) once, and every derivation site uses that form:
Bip85.fromMnemonic, deriveIndexKeyBytes, deriveTotpSecret, deriveKeyIndex
and generateFingerprint.
- atomicWrite ignored short writes. `handle.write()` may write fewer bytes
than asked (ENOSPC, RLIMIT_FSIZE, EINTR); the truncated temp file was then
fsynced and renamed over the only copy of the vault, with exit code 0 and
no stderr -- inside the function whose comment promised crash safety. It
now loops until every byte lands, fails loudly otherwise, and removes the
temp file on any failure.
- `fingerprint add` overwrote an existing profile. It consulted only
fingerprints.json, which readFingerprints returns empty for whenever the
file is corrupt or unreadable, then rewrote parent_seed.enc and the index.
Restoring a profile directory from backup and re-registering it -- the
canonical recovery gesture -- destroyed the vault. It now refuses if the
seed or index file already exists.
- Entry-id allocation could collide. A non-numeric or above-2^53 key (which
can arrive through import or a merge) made nextIndex return NaN or stop
incrementing, and insert overwrote blindly, so each new entry destroyed the
previous one. Both are now rejected, and insert refuses to overwrite.
HIGH
- RelayPool accepted any event a relay chose to send: verifyEvent only proves
an event is internally consistent, never that it came from the pubkey,
kind, or id we asked for. Events are now structurally validated (including
created_at being a finite safe integer -- "1e999" parsed to Infinity and
sorted ahead of everything forever) and matched against the filter.
- Snapshot handling had no resource budget: a decompression bomb turned 20 KB
of relay data into 20 MB, unbounded. Chunk count, chunk size, total
compressed and total decompressed bytes are now capped, the gunzip is
aborted mid-stream when it exceeds the budget, and each chunk must match
the size its signed manifest declares.
- Manifest tie-breaking used relay arrival order, letting a relay pick which
of two same-second snapshots won; it now breaks ties on event id, which no
party can steer. Falling back to an older manifest is recorded so callers
can surface a downgrade.
- Argon2/PBKDF2 parameters were read unbounded from the kdf block, which sits
outside the AEAD and is writable by anyone with directory access: 1e9 time
cost hung indefinitely, and this core targets browsers too. Now clamped.
- Config and fingerprint-registry writes held no lock at all, losing 4 of 6
concurrent updates in testing. Both now use a read-modify-write lock, and
the vault lock itself only breaks a lock whose owner pid is gone, steals it
atomically by rename, and releases only if it still owns it.
- Plaintext egress (document export, vault export) used writeFile's mode,
which applies only on creation, and followed symlinks -- an existing
world-readable file or a planted symlink received the secret. Both now go
through atomicWrite, which creates a fresh 0600 inode.
MEDIUM
- canonicalJson did not match Python. Exponent padding (1e-7 vs 1e-07), the
1e16 threshold, -0, and key ordering (UTF-16 code units vs code points) all
differed, and the code comment claiming they agreed was false. Since entry
hashes decide merge winners, two clients could keep different versions of
an entry forever. Now matched, with 24 differential fixtures generated from
CPython, and values above 2^53 refused rather than guessed at.
- "__proto__" as an entry or tombstone key silently vanished and polluted the
prototype; containers are now null-prototype. Python truthiness differs
from JS on empty containers, so `"_deleted": []` tombstoned an entry in JS
and kept it in Python; now matched.
- Policy blocks with JSON null produced the literal string "null" as the
special-character alphabet. TOTP secrets that pyotp accepts (lowercase,
unpadded) were rejected. Both fixed.
- hexToBytes accepted malformed input ("1z" -> 0x01, "-1" -> 0xff) where
Python raises; relay-supplied ids and signatures reach it.
- Password/fingerprint normalization used JS trim(), which strips a different
set than Python's strip(), changing derived keys; now uses Python's set.
Also removes a scratch file a reviewer's session left committed.
220 core + 71 CLI tests green; 36/36 cross-implementation checks green.
Co-Authored-By: Claude Fable 5 <[email protected]>
217 lines
7.5 KiB
TypeScript
217 lines
7.5 KiB
TypeScript
/**
|
|
* Vault file access for the CLI: an encrypted index file (V3 payload, with
|
|
* or without the JSON kdf/ct wrapper) unlocked by the parent seed mnemonic.
|
|
*/
|
|
|
|
import { readFile, open, rename, rm, stat } from "node:fs/promises";
|
|
import { basename, dirname, join } from "node:path";
|
|
import { randomBytes } from "node:crypto";
|
|
import process from "node:process";
|
|
import {
|
|
decryptPayload,
|
|
parseEncryptedFile,
|
|
deriveIndexKeyBytes,
|
|
encryptV3,
|
|
parseVaultIndex,
|
|
utf8,
|
|
type VaultIndex,
|
|
} from "@seedpass/core";
|
|
|
|
export interface OpenedVault {
|
|
index: VaultIndex;
|
|
mnemonic: string;
|
|
path: string;
|
|
}
|
|
|
|
export async function openVault(path: string, mnemonic: string): Promise<OpenedVault> {
|
|
const blob = new Uint8Array(await readFile(path));
|
|
const { ciphertext } = parseEncryptedFile(blob);
|
|
const key = deriveIndexKeyBytes(mnemonic);
|
|
let plaintext: Uint8Array;
|
|
try {
|
|
plaintext = await decryptPayload(key, ciphertext);
|
|
} catch (cause) {
|
|
// Raw decrypt errors ("invalid token encoding") tell the user nothing
|
|
// about which of the two likely causes they are looking at.
|
|
throw new Error(
|
|
`could not decrypt vault at ${path}: the file is corrupt, is not a ` +
|
|
`SeedPass vault, or belongs to a different seed ` +
|
|
`(restore with 'nostr restore' or 'vault import')`,
|
|
{ cause },
|
|
);
|
|
}
|
|
const index = parseVaultIndex(JSON.parse(new TextDecoder().decode(plaintext)));
|
|
return { index, mnemonic, path };
|
|
}
|
|
|
|
/**
|
|
* Persist the vault.
|
|
*
|
|
* Writes must survive a crash and must not let two concurrent commands
|
|
* silently discard each other's changes, so this:
|
|
* 1. validates the index before anything touches disk,
|
|
* 2. takes an exclusive lock for the read-modify-write window,
|
|
* 3. writes a temp file in the same directory, fsyncs it, and renames
|
|
* over the target (atomic within a filesystem),
|
|
* 4. fsyncs the directory so the rename itself is durable.
|
|
*/
|
|
export async function saveVault(vault: OpenedVault): Promise<void> {
|
|
await withVaultLock(vault.path, () => saveVaultHoldingLock(vault));
|
|
}
|
|
|
|
/**
|
|
* Save without taking the lock — for callers already inside `withVaultLock`.
|
|
*
|
|
* Serializing only the write is not enough: two commands that each read,
|
|
* mutate and write will still lose one set of changes. The lock has to span
|
|
* the whole read-modify-write cycle, which is what `mutateVault` does.
|
|
*/
|
|
export async function saveVaultHoldingLock(vault: OpenedVault): Promise<void> {
|
|
// A malformed index must never reach disk: a bad write here is what makes
|
|
// a vault unopenable afterwards.
|
|
parseVaultIndex(JSON.parse(JSON.stringify(vault.index)), { migrate: false });
|
|
|
|
const key = deriveIndexKeyBytes(vault.mnemonic);
|
|
const payload = await encryptV3(key, utf8(JSON.stringify(vault.index)));
|
|
await atomicWrite(vault.path, payload);
|
|
}
|
|
|
|
/** Write `data` to `path` atomically, with 0600 and durability. */
|
|
export async function atomicWrite(path: string, data: Uint8Array): Promise<void> {
|
|
const dir = dirname(path);
|
|
const tmp = join(dir, `.${basename(path)}.${process.pid}.${randomBytes(6).toString("hex")}.tmp`);
|
|
const handle = await open(tmp, "wx", 0o600);
|
|
let renamed = false;
|
|
try {
|
|
// A short write is not an error in POSIX, and ignoring it is how a
|
|
// truncated file gets fsynced and renamed over the only good copy —
|
|
// silently, with a success exit code. Loop until everything lands and
|
|
// fail loudly if it cannot.
|
|
let written = 0;
|
|
while (written < data.length) {
|
|
const { bytesWritten } = await handle.write(data, written, data.length - written);
|
|
if (bytesWritten <= 0) {
|
|
throw new Error(
|
|
`short write to ${tmp}: wrote ${written} of ${data.length} bytes`,
|
|
);
|
|
}
|
|
written += bytesWritten;
|
|
}
|
|
if (written !== data.length) {
|
|
throw new Error(`short write to ${tmp}: wrote ${written} of ${data.length} bytes`);
|
|
}
|
|
// fchmod on the handle, not the path: a path-based chmod after the file
|
|
// is visible can be redirected through a symlink.
|
|
await handle.chmod(0o600);
|
|
await handle.sync();
|
|
await handle.close();
|
|
await rename(tmp, path);
|
|
renamed = true;
|
|
} finally {
|
|
if (!renamed) {
|
|
// Never leave a partial ciphertext copy behind.
|
|
await handle.close().catch(() => {});
|
|
await rm(tmp, { force: true }).catch(() => {});
|
|
}
|
|
}
|
|
const dirHandle = await open(dir, "r");
|
|
try {
|
|
await dirHandle.sync();
|
|
} catch {
|
|
// Directory fsync is not supported everywhere; the rename still lands.
|
|
} finally {
|
|
await dirHandle.close();
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Hold an exclusive lock for the duration of `fn`.
|
|
*
|
|
* `wx` on a sidecar file is the portable primitive here: creation is atomic,
|
|
* so exactly one holder wins. A stale lock (crashed holder) is broken after
|
|
* LOCK_STALE_MS rather than wedging the CLI forever.
|
|
*/
|
|
const LOCK_STALE_MS = 30_000;
|
|
|
|
/** Is a pid still running (and therefore still holding its lock)? */
|
|
function pidAlive(pid: number): boolean {
|
|
if (!Number.isInteger(pid) || pid <= 0) return false;
|
|
try {
|
|
process.kill(pid, 0);
|
|
return true;
|
|
} catch (e) {
|
|
// EPERM means it exists but belongs to someone else — still alive.
|
|
return (e as NodeJS.ErrnoException).code === "EPERM";
|
|
}
|
|
}
|
|
|
|
export async function withVaultLock<T>(path: string, fn: () => Promise<T>): Promise<T> {
|
|
const lockPath = `${path}.lock`;
|
|
// A token unique to this acquisition. Releasing or breaking a lock without
|
|
// checking it is how two processes end up both believing they hold it:
|
|
// rm-then-create is not atomic, so a slow holder's lock can be deleted by
|
|
// a racer that already decided it was stale.
|
|
const token = `${process.pid}:${randomBytes(12).toString("hex")}`;
|
|
const deadline = Date.now() + LOCK_STALE_MS;
|
|
|
|
for (;;) {
|
|
try {
|
|
const handle = await open(lockPath, "wx", 0o600);
|
|
try {
|
|
await handle.write(token);
|
|
await handle.sync();
|
|
} finally {
|
|
await handle.close();
|
|
}
|
|
break;
|
|
} catch (e) {
|
|
if ((e as NodeJS.ErrnoException).code !== "EEXIST") throw e;
|
|
|
|
let holder = "";
|
|
let age = 0;
|
|
try {
|
|
holder = await readFile(lockPath, "utf8");
|
|
age = Date.now() - (await stat(lockPath)).mtimeMs;
|
|
} catch {
|
|
continue; // released between our attempts
|
|
}
|
|
const holderPid = Number.parseInt(holder.split(":")[0] ?? "", 10);
|
|
|
|
// Only break a lock whose owner is demonstrably gone. Age alone is a
|
|
// guess — a slow disk or a suspended laptop is not a crash.
|
|
if (!pidAlive(holderPid) && age > LOCK_STALE_MS) {
|
|
// Steal atomically: move the stale file aside and only proceed if
|
|
// this process is the one that managed to move it.
|
|
const claim = `${lockPath}.claim.${randomBytes(8).toString("hex")}`;
|
|
try {
|
|
await rename(lockPath, claim);
|
|
await rm(claim, { force: true });
|
|
} catch {
|
|
// Another process won the steal; fall through and retry.
|
|
}
|
|
continue;
|
|
}
|
|
if (Date.now() > deadline) {
|
|
throw new Error(
|
|
`another SeedPass process (pid ${holderPid || "unknown"}) is holding ` +
|
|
`${lockPath}; retry, or remove it if no such process is running`,
|
|
);
|
|
}
|
|
await new Promise((r) => setTimeout(r, 25));
|
|
}
|
|
}
|
|
|
|
try {
|
|
return await fn();
|
|
} finally {
|
|
// Release only if we still own it: a lock broken out from under us
|
|
// belongs to someone else now, and deleting it would evict them.
|
|
try {
|
|
const current = await readFile(lockPath, "utf8");
|
|
if (current === token) await rm(lockPath, { force: true });
|
|
} catch {
|
|
// Already gone.
|
|
}
|
|
}
|
|
}
|