Skip to content

Commit 5e0bb31

Browse files
committed
fix(cli): harden stash env — all 10 findings from the branch review
An 8-angle adversarial review of this branch confirmed 10 defects (plus one skill nit); this commit fixes all of them: 1. --write's overwrite decision (refusal AND interactive confirm) now runs BEFORE minting — a declined overwrite previously discarded a live, shown-exactly-once access key, contradicting the file's own contract. 2. Stdout is now actually pipe-clean: every clack call routes chrome to stderr via { output: process.stderr }, so `stash env > prod.env` and pipes into dotenv consumers capture only the block. (clack defaults everything to stdout — the old comment's claim was wrong.) 3. This also fixes prompts rendering into redirected stdout (isInteractive is stdin-only): prompts on stderr stay visible on the terminal. 4. `--write <path>` is now a feature, not a parse trap: parseArgs put the path in values.write leaving flags.write undefined, silently printing secrets to stdout. main.ts passes values.write ?? flags.write and the command accepts a target path. 5. `--name` followed by another flag gets its own name_requires_value diagnostic instead of a false missing_name. 6. `stash env <positional>` is rejected with did-you-mean --name guidance instead of silently vanishing into the subcommand slot. 7. --json + --write now compose: the file is written and the JSON confirmation ({ status: 'written' }) is deliberately secret-free. 8. 0600 is enforced on overwrite too (writeFileSync's mode only applies on creation) via an explicit chmod. 9. All three API responses are zod-validated; non-base64 client_key is refused before Node's lenient decoder can transcode garbage; a service shape change can no longer print undefined into a credentials file. 10. The member role is pinned in the request (wire value verified lowercase in cts-common) AND asserted on the response — a non-member key is refused with revocation guidance, so the documented 'cannot mint admin keys' invariant is enforced, not assumed. 11. All exits go through CliExit (never deep process.exit), so run() records env outcomes for telemetry; cancel paths use CliExit(0). The three divergent dead-code exit trailers are gone. 12. skills/stash-cli: the bare --env-file mention is attributed to `supabase functions serve`, per the manifest-resolution rule. Tests: 21 env unit tests (preflight-before-mint asserted via zero fetch calls; chrome-on-stderr; role pinning; validation refusals; json+write secret-free event; chmod-on-overwrite) and 4 e2e cases (all credential-free pre-mint failures, including asserting error chrome lands on stderr, not stdout). Full suite: 534 unit / 62 e2e green, code:check exit 0. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
1 parent 3b83a37 commit 5e0bb31

8 files changed

Lines changed: 525 additions & 154 deletions

File tree

.changeset/stash-env-mint-credentials.md

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,24 @@
55
`stash env` now works: it mints deployment credentials from your device-code
66
session and prints them as env vars — no dashboard copy-paste. The command
77
creates a fresh ZeroKMS client and a member-role CipherStash access key (named
8-
via `--name`), then emits `CS_WORKSPACE_CRN`, `CS_CLIENT_ID`, `CS_CLIENT_KEY`,
9-
and `CS_CLIENT_ACCESS_KEY` to stdout, `--write` (`.env.production.local`, mode
10-
0600), or `--json`. Creating access keys requires the admin role in the
11-
workspace; the minted key itself is always member — the CLI deliberately
12-
cannot mint admin keys.
8+
via `--name`; the role is pinned in the request and verified on the response —
9+
the CLI deliberately cannot mint admin keys), then emits `CS_WORKSPACE_CRN`,
10+
`CS_CLIENT_ID`, `CS_CLIENT_KEY`, and `CS_CLIENT_ACCESS_KEY`.
11+
12+
Output goes to stdout by default — and stdout is pipe-clean (progress UI is on
13+
stderr), so `stash env --name x > prod.env` and pipes into secret stores are
14+
safe. `--write [path]` writes a file instead (default `.env.production.local`,
15+
enforced mode 0600 even when overwriting), confirming before overwriting and
16+
refusing non-interactively — always *before* anything is minted, so a refusal
17+
never discards the shown-exactly-once access key. `--json` emits NDJSON; with
18+
`--write` the confirmation event is deliberately secret-free. API responses
19+
are schema-validated so a service change can never print `undefined` into a
20+
credentials file. Creating access keys requires the admin role in the
21+
workspace.
1322

1423
This is also the supported credential path for WASM/edge local development
1524
(Supabase Edge Functions, Cloudflare Workers, Deno), where the runtime cannot
1625
read the `~/.cipherstash` device profile: mint a key and feed it via
17-
`--env-file` or the platform's secret store.
26+
`supabase functions serve --env-file` or the platform's secret store.
1827

1928
The `STASH_EXPERIMENTAL_ENV_CMD` gate is removed.

packages/cli/src/bin/main.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,9 +533,17 @@ async function dispatch(
533533
break
534534
case 'env':
535535
await envCommand({
536-
write: flags.write,
536+
// parseArgs puts `--write path/x` in values and bare `--write` in
537+
// flags — accept both so a path after --write targets that file
538+
// instead of silently printing secrets to stdout.
539+
write: values.write ?? flags.write,
537540
json: flags.json,
538541
name: values.name,
542+
// `--name` followed by another flag (or nothing) is booleanised by
543+
// parseArgs; surface it as its own error instead of missing_name.
544+
nameMissingValue: flags.name === true,
545+
// `stash env my-app` would otherwise vanish into `subcommand`.
546+
unexpectedArg: subcommand ?? commandArgs[0],
539547
})
540548
break
541549
case 'manifest':

packages/cli/src/cli/registry.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,10 +545,15 @@ export const registry: CommandGroup[] = [
545545
'admin keys) and is shown exactly once — pipe the output into your',
546546
'deployment secret store. Creating access keys requires your user to',
547547
'have the admin role in the workspace.',
548+
'',
549+
'Stdout carries only the dotenv block (or the --json events);',
550+
'progress UI goes to stderr, so `stash env --name x > prod.env`',
551+
'and pipes into secret stores are safe.',
548552
].join('\n'),
549553
examples: [
550554
'env --name my-app-prod',
551555
'env --name my-app-prod --write',
556+
'env --name staging --write .env.staging.local',
552557
'env --name edge-dev --json',
553558
],
554559
flags: [
@@ -560,13 +565,14 @@ export const registry: CommandGroup[] = [
560565
},
561566
{
562567
name: '--write',
568+
value: '[path]',
563569
description:
564-
'Write the vars to .env.production.local (mode 0600) instead of printing them.',
570+
'Write the vars to a file (default .env.production.local, mode 0600) instead of printing them. An existing file prompts before overwriting — and is refused non-interactively — before anything is minted.',
565571
},
566572
{
567573
name: '--json',
568574
description:
569-
'Emit a single machine-readable JSON object (or a { status: "error" } envelope) instead of a dotenv block. Implies no prompts.',
575+
'Emit machine-readable NDJSON (a { status: "minted" } object, or { status: "written" } with --write — deliberately secret-free since the secrets are in the file; failures are { status: "error" }). Implies no prompts.',
570576
},
571577
],
572578
},

0 commit comments

Comments
 (0)