mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-28 08:22:03 +00:00
fix: four real bugs found by end-to-end manual testing
Ran the full CLI surface against a freshly generated throwaway seed (profile -> agent unlock -> all 8 entry kinds -> reveal/sinks -> tokens -> audit -> backup -> relay sync -> seed-only recovery). Four defects that no unit test had caught: - clipboard sink hung the CLI forever. xclip/wl-copy fork a resident process to own the X/Wayland selection; it inherited our stdio, so any reader of the CLI's stdout blocked indefinitely, and waiting for process exit never returned. Now detached with ignored stdio, success declared once stdin is flushed. - 'vault import' read the mnemonic straight from the env, so it failed whenever the vault was unlocked through the agent -- the normal path. - 'vault import' never wrote anything; it only printed a summary. It now restores into the active vault, refuses to overwrite a non-empty vault without --yes, and keeps the old behavior behind --inspect. - A corrupt/foreign vault file surfaced the raw codec error "invalid token encoding". It now names the actual likely causes. Also documented the variadic-option limitation that a trailing flag is lost (commander stops collecting at the next dash-prefixed token): a quoted command spec now splits on whitespace, no shell involved. Regression tests for all four. 151 core + 44 CLI tests green. Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
1 parent
274bc0e1b8
commit
dcb8fc9219
6 files changed
+195
-24
No files matched your search
@@ -50,7 +50,7 @@ until a later milestone.
|
||||
|
||||
| Environment | Status |
|
||||
|---|---|
|
||||
| Node 22 (vitest) | green — 151/151 core + 41 CLI |
|
||||
| Node 22 (vitest) | green — 151/151 core + 44 CLI |
|
||||
| jsdom browser-like env | green — 148/148 core (3 transport tests Node-only) |
|
||||
| Real Chromium/Firefox (vitest browser mode) | todo — lands with Milestone 6 web app CI |
|
||||
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
import { register } from "tsx/esm/api";
|
||||
register();
|
||||
const { MockRelay } = await import(
|
||||
"/home/user/Documents/GitHub/SeedPass/js/packages/core/test/mockRelay.ts"
|
||||
);
|
||||
const relay = new MockRelay();
|
||||
const url = await relay.start();
|
||||
console.log(url);
|
||||
process.on("SIGTERM", () => relay.stop().then(() => process.exit(0)));
|
||||
await new Promise(() => {});
|
||||
@@ -11,7 +11,7 @@ import { Command } from "commander";
|
||||
import process from "node:process";
|
||||
import { entryMetadata, resolveEntry } from "./refs.js";
|
||||
import { materializeSecret } from "./secrets.js";
|
||||
import { clipboardSink, execSink, stdinSink } from "./sinks.js";
|
||||
import { clipboardSink, execSink, parseCommandSpec, stdinSink } from "./sinks.js";
|
||||
import { capabilities } from "./capabilities.js";
|
||||
import { openVault, saveVault, type OpenedVault } from "./vaultFile.js";
|
||||
import {
|
||||
@@ -51,6 +51,7 @@ import {
|
||||
addNostrKeyEntry,
|
||||
} from "@seedpass/core";
|
||||
import { readFile, writeFile } from "node:fs/promises";
|
||||
import { existsSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { AppDir, resolveAppDir, INDEX_FILENAME } from "./appDir.js";
|
||||
import { loadConfig, saveConfig } from "./configFile.js";
|
||||
@@ -538,8 +539,14 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
|
||||
.command("use <refOrQuery>")
|
||||
.description("deliver a secret to a sink without printing it")
|
||||
.option("--clipboard", "copy to the system clipboard")
|
||||
.option("--exec <cmd...>", "run a command with SEEDPASS_SECRET in its env")
|
||||
.option("--stdin-to <cmd...>", "pipe the secret to a command's stdin")
|
||||
.option(
|
||||
"--exec <cmd...>",
|
||||
'run a command with SEEDPASS_SECRET in its env (quote it to include flags: --exec "cmd -f")',
|
||||
)
|
||||
.option(
|
||||
"--stdin-to <cmd...>",
|
||||
'pipe the secret to a command\'s stdin (quote it to include flags: --stdin-to "wc -c")',
|
||||
)
|
||||
.option("--at <timestamp>", "TOTP: unix time to compute the code at")
|
||||
.action(
|
||||
async (
|
||||
@@ -564,11 +571,11 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
|
||||
if (cmdOpts.clipboard) {
|
||||
result = await clipboardSink(secret.value);
|
||||
} else if (cmdOpts.exec) {
|
||||
const [cmd, ...args] = cmdOpts.exec;
|
||||
result = await execSink(secret.value, cmd!, args);
|
||||
const [cmd, args] = parseCommandSpec(cmdOpts.exec);
|
||||
result = await execSink(secret.value, cmd, args);
|
||||
} else {
|
||||
const [cmd, ...args] = cmdOpts.stdinTo!;
|
||||
result = await stdinSink(secret.value, cmd!, args);
|
||||
const [cmd, args] = parseCommandSpec(cmdOpts.stdinTo!);
|
||||
result = await stdinSink(secret.value, cmd, args);
|
||||
}
|
||||
io.out(
|
||||
JSON.stringify({
|
||||
@@ -1030,19 +1037,54 @@ export function buildProgram(io: ProgramIo = defaultIo): Command {
|
||||
|
||||
vaultCmd
|
||||
.command("import <srcFile>")
|
||||
.description("read a portable backup and print its reference summary")
|
||||
.action(async (srcFile: string) => {
|
||||
.description("restore a portable backup into the active vault")
|
||||
.option("--inspect", "verify and summarize without writing")
|
||||
.option("--yes", "confirm replacing a vault that already has entries")
|
||||
.action(async (srcFile: string, o: { inspect?: boolean; yes?: boolean }) => {
|
||||
const opts = program.opts() as GlobalOpts;
|
||||
const app = new AppDir(resolveAppDir(opts.appDir));
|
||||
const raw = await readFile(srcFile);
|
||||
const mnemonic = process.env["SEEDPASS_MNEMONIC"];
|
||||
const index = await importBackup(new Uint8Array(raw), {
|
||||
...(mnemonic && { mnemonic }),
|
||||
// Encrypted backups need the seed; plaintext ones do not, so only
|
||||
// resolve when required (and use the agent, not just the env).
|
||||
const isEncrypted = !/"encryption_mode"\s*:\s*"none"/.test(raw.toString("utf8"));
|
||||
const mnemonic = isEncrypted ? await resolveMnemonic(app, opts) : undefined;
|
||||
const parsed = await importBackup(new Uint8Array(raw), {
|
||||
...(mnemonic !== undefined && { mnemonic }),
|
||||
});
|
||||
const entries = (index["entries"] ?? {}) as Record<string, { label?: string }>;
|
||||
const index = parseVaultIndex(parsed);
|
||||
const entryCount = Object.keys(index.entries).length;
|
||||
|
||||
if (o.inspect) {
|
||||
io.out(
|
||||
JSON.stringify({
|
||||
inspected: srcFile,
|
||||
schema_version: index.schema_version,
|
||||
entry_count: entryCount,
|
||||
written: false,
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const targetMnemonic = mnemonic ?? (await resolveMnemonic(app, opts));
|
||||
const fp = await currentFingerprint(app, opts);
|
||||
const vaultPath = opts.vault ?? join(app.profileDir(fp), INDEX_FILENAME);
|
||||
if (existsSync(vaultPath) && !o.yes) {
|
||||
const current = await openVault(vaultPath, targetMnemonic);
|
||||
if (Object.keys(current.index.entries).length > 0) {
|
||||
throw new Error(
|
||||
"refusing to replace a non-empty vault without --yes " +
|
||||
"(use --inspect to check the backup first)",
|
||||
);
|
||||
}
|
||||
}
|
||||
await saveVault({ index, mnemonic: targetMnemonic, path: vaultPath });
|
||||
io.out(
|
||||
JSON.stringify({
|
||||
imported: srcFile,
|
||||
schema_version: index["schema_version"],
|
||||
entry_count: Object.keys(entries).length,
|
||||
into: vaultPath,
|
||||
schema_version: index.schema_version,
|
||||
entry_count: entryCount,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -16,6 +16,23 @@ export interface SinkResult {
|
||||
/** Env var the --exec sink injects into the child process. */
|
||||
export const EXEC_ENV_VAR = "SEEDPASS_SECRET";
|
||||
|
||||
/**
|
||||
* Split a sink command spec into [command, ...args].
|
||||
*
|
||||
* Commander's variadic options stop collecting at the next token starting
|
||||
* with "-", so `--exec wc -c` loses the flag. Passing the whole command as
|
||||
* one quoted string (`--exec "wc -c"`) is the documented way to include
|
||||
* flags; split it here. Splitting is on whitespace only — no shell is
|
||||
* involved, so quoting/expansion never happens.
|
||||
*/
|
||||
export function parseCommandSpec(spec: string[]): [string, string[]] {
|
||||
const parts =
|
||||
spec.length === 1 && /\s/.test(spec[0]!) ? spec[0]!.trim().split(/\s+/) : spec;
|
||||
const [cmd, ...args] = parts;
|
||||
if (!cmd) throw new Error("empty command");
|
||||
return [cmd, args];
|
||||
}
|
||||
|
||||
/** Run a command with the secret injected as an env var (never on argv). */
|
||||
export async function execSink(
|
||||
secret: string,
|
||||
@@ -66,16 +83,53 @@ const CLIPBOARD_COMMANDS: Array<{ cmd: string; args: string[] }> = [
|
||||
{ cmd: "clip.exe", args: [] },
|
||||
];
|
||||
|
||||
/**
|
||||
* Feed a secret to one clipboard tool.
|
||||
*
|
||||
* X11/Wayland clipboard tools (xclip, wl-copy) fork a resident process that
|
||||
* owns the selection until another client takes it. So we must NOT wait for
|
||||
* the process to exit, and must NOT let it inherit our stdio — a resident
|
||||
* child holding our stdout blocks every reader of this CLI's output. We
|
||||
* detach it, ignore its stdio, and treat "stdin accepted and flushed"
|
||||
* as success.
|
||||
*/
|
||||
function feedClipboardTool(secret: string, cmd: string, args: string[]): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const child = spawn(cmd, args, { stdio: ["pipe", "ignore", "ignore"], detached: true });
|
||||
let settled = false;
|
||||
const fail = (e: Error) => {
|
||||
if (!settled) {
|
||||
settled = true;
|
||||
reject(e);
|
||||
}
|
||||
};
|
||||
child.on("error", fail);
|
||||
// An immediate non-zero exit means the tool rejected the input
|
||||
// (e.g. no display); a tool that stays resident never fires this.
|
||||
child.on("exit", (code) => {
|
||||
if (code !== 0) fail(new Error(`${cmd} exited ${code}`));
|
||||
});
|
||||
child.stdin.on("error", fail);
|
||||
child.stdin.end(secret, () => {
|
||||
// Give a failing tool a moment to report before declaring success.
|
||||
setTimeout(() => {
|
||||
if (!settled) {
|
||||
settled = true;
|
||||
child.unref();
|
||||
resolve();
|
||||
}
|
||||
}, 150);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/** Copy the secret to the system clipboard via the first available tool. */
|
||||
export async function clipboardSink(secret: string): Promise<SinkResult> {
|
||||
let lastError: unknown = new Error("no clipboard tool found");
|
||||
for (const { cmd, args } of CLIPBOARD_COMMANDS) {
|
||||
try {
|
||||
const result = await stdinSink(secret, cmd, args);
|
||||
if (result.exitCode === 0) {
|
||||
return { sink: "clipboard", detail: `copied via ${cmd}` };
|
||||
}
|
||||
lastError = new Error(`${cmd} exited ${result.exitCode}`);
|
||||
await feedClipboardTool(secret, cmd, args);
|
||||
return { sink: "clipboard", detail: `copied via ${cmd}` };
|
||||
} catch (e) {
|
||||
lastError = e;
|
||||
}
|
||||
|
||||
@@ -24,7 +24,19 @@ export async function openVault(path: string, mnemonic: string): Promise<OpenedV
|
||||
const blob = new Uint8Array(await readFile(path));
|
||||
const { ciphertext } = parseEncryptedFile(blob);
|
||||
const key = deriveIndexKeyBytes(mnemonic);
|
||||
const plaintext = await decryptPayload(key, ciphertext);
|
||||
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 };
|
||||
}
|
||||
|
||||
@@ -240,6 +240,23 @@ describe("sink delivery keeps secrets out of CLI output", () => {
|
||||
expect(r.stdout).not.toContain("abc123");
|
||||
});
|
||||
|
||||
it("accepts a quoted command spec so flags survive option parsing", async () => {
|
||||
// Regression: commander's variadic options stop at the next "-token",
|
||||
// so `--stdin-to wc -c` lost the flag. A quoted spec is the documented
|
||||
// way to include flags.
|
||||
const dir = await mkdtemp(join(tmpdir(), "seedpass-spec-"));
|
||||
const capture = join(dir, "captured.txt");
|
||||
const script = join(dir, "cap.sh");
|
||||
await writeFile(script, `#!/bin/sh\nprintf '%s %s' "$1" "$(cat)" > "${capture}"\n`, {
|
||||
mode: 0o755,
|
||||
});
|
||||
const r = await run(
|
||||
"--vault", vaultPath, "use", "api-token", "--stdin-to", `${script} -flagged`,
|
||||
);
|
||||
expect(JSON.parse(r.stdout).sink).toBe("stdin");
|
||||
expect(await readFile(capture, "utf8")).toBe("-flagged abc123");
|
||||
});
|
||||
|
||||
it("use refuses ambiguous sink selection", async () => {
|
||||
const r = await run(
|
||||
"--vault", vaultPath, "use", "api-token", "--clipboard", "--stdin-to", "cat",
|
||||
@@ -333,15 +350,51 @@ describe("util generate-password", () => {
|
||||
});
|
||||
|
||||
describe("vault export/import", () => {
|
||||
it("exports an encrypted portable backup the importer accepts", async () => {
|
||||
it("exports an encrypted portable backup and inspects it without writing", async () => {
|
||||
const dir = await mkdtemp(join(tmpdir(), "seedpass-export-"));
|
||||
const dest = join(dir, "backup.json");
|
||||
const exportResult = await run("--vault", vaultPath, "vault", "export", dest);
|
||||
expect(JSON.parse(exportResult.stdout).encrypted).toBe(true);
|
||||
|
||||
const importResult = await run("vault", "import", dest);
|
||||
const importResult = await run("vault", "import", dest, "--inspect");
|
||||
const summary = JSON.parse(importResult.stdout);
|
||||
expect(summary.schema_version).toBe(4);
|
||||
expect(summary.entry_count).toBe(10);
|
||||
expect(summary.written).toBe(false);
|
||||
});
|
||||
|
||||
it("import actually restores into a vault and guards non-empty targets", async () => {
|
||||
const dir = await mkdtemp(join(tmpdir(), "seedpass-import-"));
|
||||
const backup = join(dir, "backup.json");
|
||||
await run("--vault", vaultPath, "vault", "export", backup);
|
||||
|
||||
// Fresh empty vault: import writes without --yes
|
||||
const target = join(dir, "target.enc");
|
||||
await writeFile(
|
||||
target,
|
||||
await encryptV3(deriveIndexKeyBytes(MNEMONIC), utf8(JSON.stringify({ schema_version: 4, entries: {} }))),
|
||||
);
|
||||
const first = await run("--vault", target, "vault", "import", backup);
|
||||
expect(JSON.parse(first.stdout).entry_count).toBe(10);
|
||||
const listed = JSON.parse((await run("--vault", target, "entry", "list")).stdout);
|
||||
expect(listed).toHaveLength(10);
|
||||
|
||||
// Now non-empty: refuses without --yes, proceeds with it
|
||||
const guarded = await run("--vault", target, "vault", "import", backup);
|
||||
expect(String((guarded.error as Error).message)).toContain("--yes");
|
||||
const forced = await run("--vault", target, "vault", "import", backup, "--yes");
|
||||
expect(JSON.parse(forced.stdout).entry_count).toBe(10);
|
||||
});
|
||||
});
|
||||
|
||||
describe("error messages", () => {
|
||||
it("explains a corrupt or foreign vault file instead of leaking a codec error", async () => {
|
||||
const dir = await mkdtemp(join(tmpdir(), "seedpass-corrupt-"));
|
||||
const broken = join(dir, "broken.enc");
|
||||
await writeFile(broken, "not a seedpass vault");
|
||||
const r = await run("--vault", broken, "entry", "list");
|
||||
const msg = String((r.error as Error).message);
|
||||
expect(msg).toContain("could not decrypt vault");
|
||||
expect(msg).toContain("different seed");
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user