diff --git a/.codex/skills/hack-cli/SKILL.md b/.codex/skills/hack-cli/SKILL.md index 28776a4d0..232d368e6 100644 --- a/.codex/skills/hack-cli/SKILL.md +++ b/.codex/skills/hack-cli/SKILL.md @@ -14,7 +14,7 @@ Use `hack` as the primary interface for local-first development. - These instructions were generated by hack CLI v4.2.1; use the content revision and command behavior to assess freshness. - Check guidance only when Hack is relevant and there is evidence of drift; use targeted `hack setup` checks for a read-only inventory. Routine tasks do not require a startup audit. -- Content revision: `51163ae32e56` (version alone is not a freshness guarantee). +- Content revision: `c3c646a036aa` (version alone is not a freshness guarantee). ## Product boundary @@ -151,6 +151,7 @@ Use `hack` as the primary interface for local-first development. - Layer order: `.hack/hack.env.default.yaml`, the selected `.hack/hack.env..yaml`, then worktree-local `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml`; service values override `global`, and host commands apply `host` values last. - Omit `--env` to use the configured default overlay; `--env base` (or `default`/`none`) selects base layers only, the same on every command. An overlay name grants no production authority and no network or instance isolation. - Use `hack env explain --env --service ` for provenance without the value; use `hack env get ` only when a plaintext value is explicitly needed, and treat its output as secret. +- Store a secret by piping it: ` | hack env add --service --secret --value-stdin` (or the hidden prompt in a terminal); never pass secret values as command arguments, where shell history and process listings expose them. - Running containers keep the env they were created with; after an env change, refresh with `hack env apply --service ` or `hack restart`. - Accepted flags, injection timing and refresh path per command: the command and delivery matrix in Hack's env guide (`docs/env.md` in the Hack repository). diff --git a/.cursor/rules/hack.mdc b/.cursor/rules/hack.mdc index 2dcc4e8f2..c682bd513 100644 --- a/.cursor/rules/hack.mdc +++ b/.cursor/rules/hack.mdc @@ -9,7 +9,7 @@ Prefer `hack` when shell access is available. Use MCP only when shell access is - These instructions were generated by hack CLI v4.2.1; use the content revision and command behavior to assess freshness. - Check guidance only when Hack is relevant and there is evidence of drift; use targeted `hack setup` checks for a read-only inventory. Routine tasks do not require a startup audit. -- Content revision: `51163ae32e56` (version alone is not a freshness guarantee). +- Content revision: `c3c646a036aa` (version alone is not a freshness guarantee). ## Product boundary @@ -70,6 +70,7 @@ Prefer `hack` when shell access is available. Use MCP only when shell access is - Layer order: `.hack/hack.env.default.yaml`, the selected `.hack/hack.env..yaml`, then worktree-local `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml`; service values override `global`, and host commands apply `host` values last. - Omit `--env` to use the configured default overlay; `--env base` (or `default`/`none`) selects base layers only, the same on every command. An overlay name grants no production authority and no network or instance isolation. - Use `hack env explain --env --service ` for provenance without the value; use `hack env get ` only when a plaintext value is explicitly needed, and treat its output as secret. +- Store a secret by piping it: ` | hack env add --service --secret --value-stdin` (or the hidden prompt in a terminal); never pass secret values as command arguments, where shell history and process listings expose them. - Running containers keep the env they were created with; after an env change, refresh with `hack env apply --service ` or `hack restart`. - Accepted flags, injection timing and refresh path per command: the command and delivery matrix in Hack's env guide (`docs/env.md` in the Hack repository). diff --git a/docs/env.md b/docs/env.md index f2063f356..0729a7fb6 100644 --- a/docs/env.md +++ b/docs/env.md @@ -139,8 +139,11 @@ Projects can set a default overlay in `.hack/hack.config.json`: Use `--env=base` to bypass that default and read only `.hack/hack.env.default.yaml` (plus local overrides). `default` and `none` are aliases for `base`, in any case, on every command that accepts `--env`; an overlay file named after one of those spellings is never -selected by name. Other names are normalized (for example ` QA ` selects `qa`), and a -non-empty value with no usable name characters is refused with `Invalid --env value.` +selected by name. Any other value must already be a canonical overlay name: lowercase +letters and digits joined by single hyphens (`^[a-z0-9]+(-[a-z0-9]+)*$`, for example +`qa` or `qa-2`); surrounding whitespace is ignored. Hack never rewrites a requested name, +so `QA`, `q_a` or `X=1` is refused with a fixed `Invalid --env value.` message that does +not repeat the argument, before any project file is read or any child process starts. See the [command and delivery matrix](#command-and-delivery-matrix) for what each command does with the selection. @@ -226,9 +229,9 @@ For example, `hack host exec --scope api -- bun db:migrate` still runs on the ho Every command that accepts `--env` resolves it with the same rule: omitted selects the configured default overlay (`env.defaultOverlay`); `base`, `default` or `none` -selects base layers only; any other value selects that normalized overlay name. The -table lists every such command; `tests/env-command-matrix.test.ts` fails when a -command gains or loses `--env` without a matching row, or when a row's selectors +selects base layers only; any other value must be a canonical overlay name and selects +that overlay. The table lists every such command; `tests/env-command-matrix.test.ts` +fails when a command gains or loses `--env` without a matching row, or when a row's selectors drift from the CLI. Commands not listed here refuse `--env` as an option that is not valid for them (arguments after a standalone `--` belong to the child command). @@ -350,13 +353,37 @@ Add or update values: ```bash hack env add API_BASE_URL https://api.example.com -hack env add SERVICE_TOKEN abc123 --service api --secret hack env add --env qa API_BASE_URL https://qa.example.com hack env add API_BASE_URL https://api.example.com --local ``` `hack env set` still works as a compatibility alias for `hack env add`, but `add` is the primary UX now. +### Entering secret values + +Never pass a secret value as a command argument: arguments land in shell history and are +visible to other local users and tools through process listings. Use one of these instead: + +```bash +# Pipe the value from a secret manager or file; it never appears in argv. + | hack env add SERVICE_TOKEN --service api --secret --value-stdin +hack env add SERVICE_TOKEN --service api --secret --value-stdin < token.txt + +# In an interactive terminal, omit the value to use the hidden prompt. +hack env add SERVICE_TOKEN --service api --secret +``` + +`--value-stdin` works on `hack env add` and `hack env set`, with or without `--secret`. +It reads all of stdin, up to 64 KiB, and strips exactly one trailing newline (`\n` or +`\r\n`), so `printf '%s\n' "$value"` and `printf '%s' "$value"` store the same value; any +further trailing newlines are kept. It refuses, with a fixed message that never repeats the +input and without writing anything: + +- an interactive terminal on stdin (pipe the value, or drop `--value-stdin` for the hidden prompt) +- a value argument alongside the flag, including `KEY=value` +- a missing key argument +- empty input, input over 64 KiB, invalid UTF-8, or a NUL byte + `--local` writes to the worktree-local override file (`.hack/hack.env.local.yaml` or `.hack/hack.env..local.yaml`) instead of the shared repo env file — use it for per-checkout values you don't want to commit. @@ -364,7 +391,8 @@ per-checkout values you don't want to commit. If you omit `key`, `value`, or `--service`, `hack env add` prompts for them interactively. Pass `--no-interactive` (or set `HACK_NO_INTERACTIVE=1`) for scripted/agent usage: the command applies documented defaults or fails fast with `E_INTERACTIVE_REQUIRED` instead of -blocking on a prompt. `hack env unset` and `hack env set` accept `--local` and +blocking on a prompt. In scripts, supply secret values with `--value-stdin` rather than as +arguments. `hack env unset` and `hack env set` accept `--local` and `--no-interactive` too. Remove a value: @@ -603,6 +631,7 @@ If you are writing new docs or new project setup flows, document the YAML overla - Commit `.hack/hack.env..yaml` when the overlay is team-shared - Never commit `.hack.secret.key` - Prefer `hack env add` over hand-editing encrypted values +- Enter secrets with `--value-stdin` or the hidden prompt, never as command arguments - Prefer direct runtime injection over materializing `.hack/.env` - Use `hack host exec` or env-aware sessions for host-side scripts like migrations, generators, and admin tasks diff --git a/docs/reference/cli.md b/docs/reference/cli.md index dddd00ecb..5fe0d6314 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -2334,6 +2334,8 @@ Add or update an env value hack env add [key] [value] [options] ``` +Non-secret values may be passed as arguments. For secrets, pipe the value with --value-stdin (for example ` \| hack env add API_TOKEN --secret --value-stdin`) or omit the value in a terminal to use the hidden prompt, so the value never appears in argv, shell history, or process listings. + ### Arguments | Arg | Description | @@ -2349,6 +2351,7 @@ hack env add [key] [value] [options] | `--project ` | Target a registered project by name (from ~/.hack/projects.json) | | `--env ` | Select an env overlay by name; omit for the configured default overlay, or use 'base' or 'default' for base layers only | | `--secret` | Store value as an encrypted secure entry | +| `--value-stdin` | Read the value from piped stdin (max 64 KiB, one trailing newline stripped) so it never appears in argv or shell history | | `--local` | Write to the worktree-local env override file instead of the shared repo env file | | `--service ` | Target scope (global or a discovered service name) | | `--no-interactive` | Never prompt: apply documented defaults or fail with E_INTERACTIVE_REQUIRED (also via HACK_NO_INTERACTIVE=1) | @@ -2365,6 +2368,8 @@ Alias for env add hack env set [key] [value] [options] ``` +Same value rules as `hack env add`: pipe secrets with --value-stdin or use the hidden prompt instead of passing them as arguments. + ### Arguments | Arg | Description | @@ -2380,6 +2385,7 @@ hack env set [key] [value] [options] | `--project ` | Target a registered project by name (from ~/.hack/projects.json) | | `--env ` | Select an env overlay by name; omit for the configured default overlay, or use 'base' or 'default' for base layers only | | `--secret` | Store value as an encrypted secure entry | +| `--value-stdin` | Read the value from piped stdin (max 64 KiB, one trailing newline stripped) so it never appears in argv or shell history | | `--local` | Write to the worktree-local env override file instead of the shared repo env file | | `--service ` | Target scope (global or a discovered service name) | | `--no-interactive` | Never prompt: apply documented defaults or fail with E_INTERACTIVE_REQUIRED (also via HACK_NO_INTERACTIVE=1) | diff --git a/examples/basic/AGENTS.md b/examples/basic/AGENTS.md index c5fb011c0..c430447b4 100644 --- a/examples/basic/AGENTS.md +++ b/examples/basic/AGENTS.md @@ -6,7 +6,7 @@ Use `hack` as the single interface for local-first runtime orchestration (compos Integration freshness: - These instructions were generated by hack CLI v4.2.1; use the content revision and command behavior to assess freshness. - Check guidance only when Hack is relevant and there is evidence of drift; use targeted `hack setup` checks for a read-only inventory. Routine tasks do not require a startup audit. -- Content revision: `51163ae32e56` (version alone is not a freshness guarantee). +- Content revision: `c3c646a036aa` (version alone is not a freshness guarantee). Product boundary: - Supported CLI surface: project init, up/down/restart, open, logs, env, host exec/shell, sessions, doctor, and daemon. @@ -128,6 +128,7 @@ Env composition: - Layer order: `.hack/hack.env.default.yaml`, the selected `.hack/hack.env..yaml`, then worktree-local `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml`; service values override `global`, and host commands apply `host` values last. - Omit `--env` to use the configured default overlay; `--env base` (or `default`/`none`) selects base layers only, the same on every command. An overlay name grants no production authority and no network or instance isolation. - Use `hack env explain --env --service ` for provenance without the value; use `hack env get ` only when a plaintext value is explicitly needed, and treat its output as secret. +- Store a secret by piping it: ` | hack env add --service --secret --value-stdin` (or the hidden prompt in a terminal); never pass secret values as command arguments, where shell history and process listings expose them. - Running containers keep the env they were created with; after an env change, refresh with `hack env apply --service ` or `hack restart`. - Accepted flags, injection timing and refresh path per command: the command and delivery matrix in Hack's env guide (`docs/env.md` in the Hack repository). diff --git a/examples/basic/CLAUDE.md b/examples/basic/CLAUDE.md index c5fb011c0..c430447b4 100644 --- a/examples/basic/CLAUDE.md +++ b/examples/basic/CLAUDE.md @@ -6,7 +6,7 @@ Use `hack` as the single interface for local-first runtime orchestration (compos Integration freshness: - These instructions were generated by hack CLI v4.2.1; use the content revision and command behavior to assess freshness. - Check guidance only when Hack is relevant and there is evidence of drift; use targeted `hack setup` checks for a read-only inventory. Routine tasks do not require a startup audit. -- Content revision: `51163ae32e56` (version alone is not a freshness guarantee). +- Content revision: `c3c646a036aa` (version alone is not a freshness guarantee). Product boundary: - Supported CLI surface: project init, up/down/restart, open, logs, env, host exec/shell, sessions, doctor, and daemon. @@ -128,6 +128,7 @@ Env composition: - Layer order: `.hack/hack.env.default.yaml`, the selected `.hack/hack.env..yaml`, then worktree-local `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml`; service values override `global`, and host commands apply `host` values last. - Omit `--env` to use the configured default overlay; `--env base` (or `default`/`none`) selects base layers only, the same on every command. An overlay name grants no production authority and no network or instance isolation. - Use `hack env explain --env --service ` for provenance without the value; use `hack env get ` only when a plaintext value is explicitly needed, and treat its output as secret. +- Store a secret by piping it: ` | hack env add --service --secret --value-stdin` (or the hidden prompt in a terminal); never pass secret values as command arguments, where shell history and process listings expose them. - Running containers keep the env they were created with; after an env change, refresh with `hack env apply --service ` or `hack restart`. - Accepted flags, injection timing and refresh path per command: the command and delivery matrix in Hack's env guide (`docs/env.md` in the Hack repository). diff --git a/plugins/hack/rules/hack.mdc b/plugins/hack/rules/hack.mdc index e184f0ef9..7e8faaf7c 100644 --- a/plugins/hack/rules/hack.mdc +++ b/plugins/hack/rules/hack.mdc @@ -14,7 +14,7 @@ Prefer `hack` when shell access is available. Use MCP only when shell access is - These instructions were generated by hack CLI v4.2.1; use the content revision and command behavior to assess freshness. - Check guidance only when Hack is relevant and there is evidence of drift; use targeted `hack setup` checks for a read-only inventory. Routine tasks do not require a startup audit. -- Content revision: `51163ae32e56` (version alone is not a freshness guarantee). +- Content revision: `c3c646a036aa` (version alone is not a freshness guarantee). ## Product boundary @@ -75,6 +75,7 @@ Prefer `hack` when shell access is available. Use MCP only when shell access is - Layer order: `.hack/hack.env.default.yaml`, the selected `.hack/hack.env..yaml`, then worktree-local `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml`; service values override `global`, and host commands apply `host` values last. - Omit `--env` to use the configured default overlay; `--env base` (or `default`/`none`) selects base layers only, the same on every command. An overlay name grants no production authority and no network or instance isolation. - Use `hack env explain --env --service ` for provenance without the value; use `hack env get ` only when a plaintext value is explicitly needed, and treat its output as secret. +- Store a secret by piping it: ` | hack env add --service --secret --value-stdin` (or the hidden prompt in a terminal); never pass secret values as command arguments, where shell history and process listings expose them. - Running containers keep the env they were created with; after an env change, refresh with `hack env apply --service ` or `hack restart`. - Accepted flags, injection timing and refresh path per command: the command and delivery matrix in Hack's env guide (`docs/env.md` in the Hack repository). diff --git a/plugins/hack/skills/hack-cli/SKILL.md b/plugins/hack/skills/hack-cli/SKILL.md index bbf2ff1ba..315545a30 100644 --- a/plugins/hack/skills/hack-cli/SKILL.md +++ b/plugins/hack/skills/hack-cli/SKILL.md @@ -14,7 +14,7 @@ Use `hack` as the primary interface for local-first development. - These instructions were generated by hack CLI v4.2.1; use the content revision and command behavior to assess freshness. - Check guidance only when Hack is relevant and there is evidence of drift; use targeted `hack setup` checks for a read-only inventory. Routine tasks do not require a startup audit. -- Content revision: `51163ae32e56` (version alone is not a freshness guarantee). +- Content revision: `c3c646a036aa` (version alone is not a freshness guarantee). ## Product boundary @@ -151,6 +151,7 @@ Use `hack` as the primary interface for local-first development. - Layer order: `.hack/hack.env.default.yaml`, the selected `.hack/hack.env..yaml`, then worktree-local `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml`; service values override `global`, and host commands apply `host` values last. - Omit `--env` to use the configured default overlay; `--env base` (or `default`/`none`) selects base layers only, the same on every command. An overlay name grants no production authority and no network or instance isolation. - Use `hack env explain --env --service ` for provenance without the value; use `hack env get ` only when a plaintext value is explicitly needed, and treat its output as secret. +- Store a secret by piping it: ` | hack env add --service --secret --value-stdin` (or the hidden prompt in a terminal); never pass secret values as command arguments, where shell history and process listings expose them. - Running containers keep the env they were created with; after an env change, refresh with `hack env apply --service ` or `hack restart`. - Accepted flags, injection timing and refresh path per command: the command and delivery matrix in Hack's env guide (`docs/env.md` in the Hack repository). diff --git a/src/agents/instruction-source.ts b/src/agents/instruction-source.ts index 25a1cb569..13f09d3ec 100644 --- a/src/agents/instruction-source.ts +++ b/src/agents/instruction-source.ts @@ -240,6 +240,7 @@ export const INSTRUCTION_SECTIONS: readonly InstructionSection[] = [ "Layer order: `.hack/hack.env.default.yaml`, the selected `.hack/hack.env..yaml`, then worktree-local `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml`; service values override `global`, and host commands apply `host` values last.", "Omit `--env` to use the configured default overlay; `--env base` (or `default`/`none`) selects base layers only, the same on every command. An overlay name grants no production authority and no network or instance isolation.", "Use `hack env explain --env --service ` for provenance without the value; use `hack env get ` only when a plaintext value is explicitly needed, and treat its output as secret.", + "Store a secret by piping it: ` | hack env add --service --secret --value-stdin` (or the hidden prompt in a terminal); never pass secret values as command arguments, where shell history and process listings expose them.", "Running containers keep the env they were created with; after an env change, refresh with `hack env apply --service ` or `hack restart`.", "Accepted flags, injection timing and refresh path per command: the command and delivery matrix in Hack's env guide (`docs/env.md` in the Hack repository).", ], diff --git a/src/agents/integration-ownership.ts b/src/agents/integration-ownership.ts index 4156a6251..cc5da86a9 100644 --- a/src/agents/integration-ownership.ts +++ b/src/agents/integration-ownership.ts @@ -106,6 +106,7 @@ const KNOWN_RENDER_FINGERPRINTS: Readonly< "121d8f26374d24f2a09890a3", // v5.0.0-next.3..next "6449442a8f230412d4e5f9e7", // v5.0.0-next.4..next "d83da046cf7541ba30950880", // v5.0.0-next.4..next (offline env-matrix pointer) + "c75c5f5eb9fdcf6842ed1eda", // v5.0.0-next.4..next (--value-stdin secret entry) ]), "hack-init-skill": new Set([ "cd15fb6d8ed8e25071d05e75", // v3.1.0..v4.0.3 @@ -133,6 +134,7 @@ const KNOWN_RENDER_FINGERPRINTS: Readonly< "06cf6d0920afc1780b9946ba", // v5.0.0-next.3..next "cf0a31e197f9034485413674", // v5.0.0-next.4..next "3a99c9097855f93808e482cb", // v5.0.0-next.4..next (offline env-matrix pointer) + "29499060cad876931810b458", // v5.0.0-next.4..next (--value-stdin secret entry) ]), }; diff --git a/src/agents/integration-revision.ts b/src/agents/integration-revision.ts index d418edef4..2b49586e7 100644 --- a/src/agents/integration-revision.ts +++ b/src/agents/integration-revision.ts @@ -3,4 +3,4 @@ * source test recomputes this value and fails whenever guidance changes * without a revision update. */ -export const HACK_AGENT_INTEGRATION_CONTENT_REVISION = "51163ae32e56"; +export const HACK_AGENT_INTEGRATION_CONTENT_REVISION = "c3c646a036aa"; diff --git a/src/agents/onboarding-prompt.ts b/src/agents/onboarding-prompt.ts index 7636acbf8..93217cef6 100644 --- a/src/agents/onboarding-prompt.ts +++ b/src/agents/onboarding-prompt.ts @@ -143,7 +143,7 @@ function renderSetupPhase(opts: { "- Check for reuse before authoring new services: if the repo already containerizes for prod (existing Dockerfiles/compose with build contexts, runtime-config injection points like an nginx-served runtime-config.js), prefer reusing those images/contexts with minimal glue over hand-authoring new dev-mode services from scratch.", "- Define one compose service per process chosen for containerization in `.hack/docker-compose.yml`. HTTP services need the Caddy labels (`caddy`, `caddy.reverse_proxy`, `caddy.tls=internal`) and the `hack-dev` network to be routable.", `- Design hostnames: the primary app answers on dev_host (${devHostExample}); every other routable service gets a subdomain (\`api.${devHostExample}\`, \`grafana.${devHostExample}\`, ...). Set dev_host in \`.hack/hack.config.json\`.`, - "- Migrate only the env entries required for the authorized setup. Use `hack env add ` for non-secret values. For secrets, use an authorized secret-manager/injection path or let the user enter them through `hack env add --secret`; never put secret values in an agent prompt, command argument, log, or source control.", + "- Migrate only the env entries required for the authorized setup. Use `hack env add ` for non-secret values. For secrets, pipe the value from an authorized secret manager with ` | hack env add --secret --value-stdin`, or let the user enter it at the hidden prompt of `hack env add --secret`; never put secret values in an agent prompt, command argument, log, or source control.", "- Verify container env injection and host commands through `hack host exec` before treating any old env file as superseded. Preserve files still needed by other workflows; deletion or broader migration requires authorization.", "- Encode required host-side setup (tunnels, proxies, credential bootstrap) in `.hack/hack.config.json` under `startup`/`lifecycle` — not in ad-hoc terminal instructions.", ]; diff --git a/src/cli/env-arguments.ts b/src/cli/env-arguments.ts new file mode 100644 index 000000000..a761b54eb --- /dev/null +++ b/src/cli/env-arguments.ts @@ -0,0 +1,128 @@ +import { + assertValidProjectEnvScopeName, + ENV_KEY_ASSIGNMENT_MESSAGE, + ENV_KEY_INVALID_MESSAGE, + ENV_SCOPE_INVALID_MESSAGE, +} from "../lib/project-env-config.ts"; +import { + normalizeProjectName, + PROJECT_NAME_PATTERN, + PROJECT_SELECTOR_PATTERN, +} from "../lib/project-name.ts"; +import { CliUsageError } from "./command.ts"; + +const ENV_KEY_PATTERN = /^[A-Z_][A-Z0-9_]*$/; +const SCOPE_CONFLICT_MESSAGE = "Use either --scope or --service, not both."; +const APPLY_SERVICE_REQUIRED_MESSAGE = + "--service is required."; +export const INVALID_PROJECT_OPTION_MESSAGE = + "Invalid --project value. Project names use ASCII letters, digits, hyphens, underscores, spaces or slashes; run 'hack projects' to see registered projects."; + +/** + * Parsers for the env-scope and env-key arguments of `hack env` and + * `hack host` commands. + * + * Invariant: every refusal is fixed text. A scope or key argument is often a + * mistyped `KEY=value` or a value pasted into the wrong position, so no error + * raised here may contain any part of the argument. Callers must parse + * user-supplied scopes here before resolving env files, reading stdin, or + * spawning a child process, so a refused argument never reaches another + * component that might echo it. + */ + +/** Normalize a `--service`/`--scope` value; empty means `global`. */ +export function parseEnvScopeOption(input: { + readonly scopeName: string | undefined; +}): string { + try { + return assertValidProjectEnvScopeName({ scopeName: input.scopeName }); + } catch { + throw new CliUsageError(ENV_SCOPE_INVALID_MESSAGE); + } +} + +/** + * Resolve the `--scope`/`--service` pair of host execution commands. + * Returns `null` when neither names a scope (the global scope). + */ +export function parseExecutionScopeOptions(input: { + readonly scopeName: string | undefined; + readonly serviceName: string | undefined; +}): string | null { + const scopeName = input.scopeName?.trim() ?? ""; + const serviceName = input.serviceName?.trim() ?? ""; + if (scopeName.length > 0 && serviceName.length > 0) { + throw new CliUsageError(SCOPE_CONFLICT_MESSAGE); + } + const requested = scopeName || serviceName; + if (requested.length === 0) { + return null; + } + return parseEnvScopeOption({ scopeName: requested }); +} + +/** `hack env materialize --service`: omitted or empty is `null` (global). */ +export function parseMaterializeScopeOption(input: { + readonly serviceName: string | undefined; +}): string | null { + if ((input.serviceName?.trim() ?? "").length === 0) { + return null; + } + return parseEnvScopeOption({ scopeName: input.serviceName }); +} + +/** `hack env apply --service`: a single non-global service scope. */ +export function parseApplyServiceOption(input: { + readonly serviceName: string | undefined; +}): string { + if ((input.serviceName?.trim() ?? "").length === 0) { + throw new CliUsageError(APPLY_SERVICE_REQUIRED_MESSAGE); + } + const scope = parseEnvScopeOption({ scopeName: input.serviceName }); + if (scope === "global") { + throw new CliUsageError(APPLY_SERVICE_REQUIRED_MESSAGE); + } + return scope; +} + +/** + * `hack env get [--service ]`. Validated here, before any project + * or backend access. The command also replaces every failure with one generic + * message, but these refusals stay value-free on their own so that removing + * that wrapper cannot reintroduce an echo. + */ +export function parseEnvGetArguments(input: { + readonly key: string; + readonly serviceName: string | undefined; +}): { readonly key: string; readonly scope: string } { + if (input.key.includes("=")) { + throw new CliUsageError(ENV_KEY_ASSIGNMENT_MESSAGE); + } + if (!ENV_KEY_PATTERN.test(input.key)) { + throw new CliUsageError(ENV_KEY_INVALID_MESSAGE); + } + return { + key: input.key, + scope: parseEnvScopeOption({ scopeName: input.serviceName }), + }; +} + +/** + * Parse `--project` for `hack env` and `hack host` commands. An argument + * containing `=` or any character outside `PROJECT_SELECTOR_PATTERN` may be a + * pasted value, so it is refused with fixed text before the registry is read + * and is never slugged into a name. `canonical` reports whether the argument + * was typed in canonical form, the only form an error may echo. + */ +export function parseProjectSelectorOption(input: { + readonly projectOpt: string; +}): { readonly name: string; readonly canonical: boolean } { + const trimmed = input.projectOpt.trim(); + const name = PROJECT_SELECTOR_PATTERN.test(trimmed) + ? normalizeProjectName(trimmed) + : null; + if (!name) { + throw new CliUsageError(INVALID_PROJECT_OPTION_MESSAGE); + } + return { name, canonical: PROJECT_NAME_PATTERN.test(trimmed) }; +} diff --git a/src/cli/env-selection.ts b/src/cli/env-selection.ts index e45b9d08b..c88abf402 100644 --- a/src/cli/env-selection.ts +++ b/src/cli/env-selection.ts @@ -1,10 +1,13 @@ -import { - ENV_BASE_SELECTION_ALIASES, - parseEnvConfigSelection, -} from "../lib/project.ts"; +import { parseEnvConfigSelection } from "../lib/project.ts"; import { CliUsageError } from "./command.ts"; -const INVALID_ENV_SELECTION_MESSAGE = "Invalid --env value."; +/** + * Fixed refusal for an unusable `--env` value. It names the accepted grammar + * and never the argument, which may be a mistyped `KEY=value` or a pasted + * value. + */ +export const INVALID_ENV_SELECTION_MESSAGE = + "Invalid --env value. Overlay names must match ^[a-z0-9]+(-[a-z0-9]+)*$; use base, default or none for base layers only."; /** * Resolve a CLI `--env` value through the one shared selection grammar. @@ -12,30 +15,28 @@ const INVALID_ENV_SELECTION_MESSAGE = "Invalid --env value."; * - `undefined` (option omitted): callers apply the configured default overlay. * - `null`: base layers only. `base`, `default` and `none` (any case) and an * empty value bypass the configured default overlay. - * - `string`: the canonical overlay name (`normalizeEnvConfigName`). + * - `string`: the overlay name, which must already be canonical + * (`ENV_CONFIG_NAME_PATTERN`); surrounding whitespace is ignored. * - * A non-empty value with no usable overlay characters is refused with a fixed, - * value-free usage error instead of silently selecting the base layers. - * Every CLI command that accepts `--env` must resolve it here so that + * Anything else, including any value containing `=`, is refused with a fixed, + * value-free usage error. A requested name is never slugged: slugging + * `X=secret` to `xsecret` would print part of the value as the selection and + * write it into an overlay file name. + * + * `runCli` resolves `--env` once before any handler runs, so a refusal + * precedes project discovery, migration, file access and child processes. + * Every CLI command that accepts `--env` must also resolve it here so that * `default`/`base` mean the same layers everywhere. */ export function resolveRequestedEnvName(input: { readonly envOption: string | undefined; }): string | null | undefined { - const selection = parseEnvConfigSelection(input.envOption); if (input.envOption === undefined) { - return selection; + return undefined; } + const selection = parseEnvConfigSelection(input.envOption); if (selection === undefined) { throw new CliUsageError(INVALID_ENV_SELECTION_MESSAGE); } - const trimmed = input.envOption.trim(); - if ( - selection === null && - trimmed.length > 0 && - !ENV_BASE_SELECTION_ALIASES.has(trimmed.toLowerCase()) - ) { - throw new CliUsageError(INVALID_ENV_SELECTION_MESSAGE); - } return selection; } diff --git a/src/cli/run.ts b/src/cli/run.ts index 101c63e82..e7fdd7145 100644 --- a/src/cli/run.ts +++ b/src/cli/run.ts @@ -18,7 +18,9 @@ import { parsePositionalsForCommand, resolveCommand, } from "./command.ts"; +import { resolveRequestedEnvName } from "./env-selection.ts"; import { printHelpForPath } from "./help.ts"; +import { optEnv } from "./options.ts"; import { CLI_SPEC } from "./spec.ts"; function isTruthyEnv(value: string | undefined): boolean { @@ -80,6 +82,10 @@ export async function runCli(argv: readonly string[]): Promise { resolved.command.options, parsed.values ); + validateSharedEnvSelection({ + command: resolved.command, + parsedValues: parsed.values, + }); const pos = parsePositionalsForCommand( resolved.command.positionals, resolved.remainingPositionals @@ -260,6 +266,24 @@ function validateResolvedCommandOptions(opts: { } } +/** + * Refuse an unusable shared `--env` value before the handler runs, so no + * command discovers a project, migrates or reads files, or starts a child + * process with it. The refusal is fixed text that never includes the value. + */ +function validateSharedEnvSelection(opts: { + readonly command: AnyCommandSpec; + readonly parsedValues: Record; +}): void { + if (!opts.command.options.includes(optEnv)) { + return; + } + const value = opts.parsedValues[optEnv.name]; + if (typeof value === "string") { + resolveRequestedEnvName({ envOption: value }); + } +} + function allowsUnknownOptions(opts: { readonly command: AnyCommandSpec | null; }): boolean { diff --git a/src/commands/env.ts b/src/commands/env.ts index 3e9e3a23f..68a8b46b9 100644 --- a/src/commands/env.ts +++ b/src/commands/env.ts @@ -9,11 +9,23 @@ import { defineOption, withHandler, } from "../cli/command.ts"; +import { + parseApplyServiceOption, + parseEnvGetArguments, + parseEnvScopeOption, + parseExecutionScopeOptions, + parseMaterializeScopeOption, + parseProjectSelectorOption, +} from "../cli/env-arguments.ts"; import { resolveRequestedEnvName } from "../cli/env-selection.ts"; import { optEnv, optJson, optPath, optProject } from "../cli/options.ts"; import { readControlPlaneConfig } from "../control-plane/sdk/config.ts"; import { updateGlobalConfig } from "../lib/config.ts"; import { resolveGlobalConfigPath } from "../lib/config-paths.ts"; +import { + assertEnvValueStdinUsage, + readEnvValueFromStdin, +} from "../lib/env-value-stdin.ts"; import { readTextFile } from "../lib/fs.ts"; import { isRecord } from "../lib/guards.ts"; import { resolveHackInvocation } from "../lib/hack-cli.ts"; @@ -55,8 +67,8 @@ import { sanitizeProjectSlug, } from "../lib/project.ts"; import { - assertValidProjectEnvScopeName, discoverComposeServiceNames, + ENV_KEY_INVALID_MESSAGE, materializeProjectEnv, migrateLegacyProjectEnv, parseProjectEnvTarget, @@ -69,7 +81,6 @@ import { setProjectEnvValue, unsetProjectEnvValue, } from "../lib/project-env-config.ts"; -import { normalizeProjectName } from "../lib/project-name.ts"; import { resolveRegisteredProjectByName } from "../lib/projects-registry.ts"; import { formatSecretStoreDescriptor, @@ -95,6 +106,14 @@ const optSecret = defineOption({ description: "Store value as an encrypted secure entry", } as const); +const optValueStdin = defineOption({ + name: "valueStdin", + type: "boolean", + long: "--value-stdin", + description: + "Read the value from piped stdin (max 64 KiB, one trailing newline stripped) so it never appears in argv or shell history", +} as const); + const optLocal = defineOption({ name: "local", type: "boolean", @@ -230,8 +249,18 @@ const applySpec = defineCommand({ const addSpec = defineCommand({ name: "add", summary: "Add or update an env value", + description: + "Non-secret values may be passed as arguments. For secrets, pipe the value with --value-stdin (for example ` | hack env add API_TOKEN --secret --value-stdin`) or omit the value in a terminal to use the hidden prompt, so the value never appears in argv, shell history, or process listings.", group: "Project", - options: [optPath, optProject, optEnv, optSecret, optLocal, optService], + options: [ + optPath, + optProject, + optEnv, + optSecret, + optValueStdin, + optLocal, + optService, + ], positionals: [ { name: "key", required: false }, { name: "value", required: false }, @@ -242,8 +271,18 @@ const addSpec = defineCommand({ const setSpec = defineCommand({ name: "set", summary: "Alias for env add", + description: + "Same value rules as `hack env add`: pipe secrets with --value-stdin or use the hidden prompt instead of passing them as arguments.", group: "Project", - options: [optPath, optProject, optEnv, optSecret, optLocal, optService], + options: [ + optPath, + optProject, + optEnv, + optSecret, + optValueStdin, + optLocal, + optService, + ], positionals: [ { name: "key", required: false }, { name: "value", required: false }, @@ -588,6 +627,9 @@ function normalizeGlobalEncryptedFileConfigPath(input: { return resolvedPath; } +const UNKNOWN_PROJECT_HINT = + "Run 'hack init' in that repo (or run 'hack projects' to see registered projects)."; + async function resolveProjectForEnv(opts: { readonly ctx: CliContext; readonly pathOpt: string | undefined; @@ -598,14 +640,19 @@ async function resolveProjectForEnv(opts: { } if (opts.projectOpt !== undefined) { - const name = normalizeProjectName(opts.projectOpt); - if (!name) { - throw new CliUsageError("Invalid --project value."); - } - const project = await resolveRegisteredProjectByName({ name }); + const selector = parseProjectSelectorOption({ + projectOpt: opts.projectOpt, + }); + const project = await resolveRegisteredProjectByName({ + name: selector.name, + }); if (!project) { + // Echo only a name the user typed in canonical form; never a slug + // derived from their input. throw new CliUsageError( - `Unknown project "${name}". Run 'hack init' in that repo (or run 'hack projects' to see registered projects).` + selector.canonical + ? `Unknown project "${selector.name}". ${UNKNOWN_PROJECT_HINT}` + : `Unknown project. ${UNKNOWN_PROJECT_HINT}` ); } return project; @@ -736,8 +783,9 @@ function resolveHostEnvTarget(input: { ) { return target as (typeof HOST_ENV_TARGET_VALUES)[number]; } + // Fixed text: the rejected argument may be a pasted value. throw new CliUsageError( - `Invalid --target "${target}". Expected one of: ${HOST_ENV_TARGET_VALUES.join(", ")}` + `Invalid --target value. Expected one of: ${HOST_ENV_TARGET_VALUES.join(", ")}.` ); } @@ -901,7 +949,7 @@ async function promptForEnvAddInput(input: { if (!canPrompt()) { requireInteractive({ what: "hack env add needs a key and value", - hint: "Pass them as arguments: `hack env add / ` (or `hack env add KEY VALUE --service `, with --secret for secrets).", + hint: "Pass the key as an argument and pipe the value on stdin: ` | hack env add / --value-stdin` (add --secret for secrets). Non-secret values may also be passed as `hack env add / `; never pass secret values as arguments.", }); } @@ -984,12 +1032,9 @@ const handleEnvGet: CommandHandlerFor = async ({ ctx, args, }) => { - const key = args.positionals.key; - if (!ENV_KEY_PATTERN.test(key)) { - throw new CliUsageError("Invalid env key"); - } - const scope = assertValidProjectEnvScopeName({ - scopeName: args.options.service, + const { key, scope } = parseEnvGetArguments({ + key: args.positionals.key, + serviceName: args.options.service, }); const project = await resolveProjectForEnv({ ctx, @@ -1051,17 +1096,9 @@ const handleEnvList: CommandHandlerFor = async ({ envName, }); if (modern) { - const selectedScope = (() => { - try { - return assertValidProjectEnvScopeName({ - scopeName: args.options.service, - }); - } catch (error: unknown) { - throw new CliUsageError( - error instanceof Error ? error.message : "Invalid env scope." - ); - } - })(); + const selectedScope = parseEnvScopeOption({ + scopeName: args.options.service, + }); const envValues = selectProjectEnvValues({ resolved: modern, scopeName: selectedScope, @@ -1265,18 +1302,19 @@ const handleEnvExplain: CommandHandlerFor = async ({ ctx, args, }): Promise => { - const project = await resolveProjectForEnv({ - ctx, - pathOpt: args.options.path, - projectOpt: args.options.project, - }); - const projectName = await resolveProjectName(project); + // Refuse malformed arguments with fixed text before any project file is read. const envName = resolveRequestedEnvName({ envOption: args.options.env }); const parsed = parseProjectEnvTarget({ keyOrPath: args.positionals.key, scopeOverride: args.options.service, }); const target = resolveHostEnvTarget({ targetOption: args.options.target }); + const project = await resolveProjectForEnv({ + ctx, + pathOpt: args.options.path, + projectOpt: args.options.project, + }); + const projectName = await resolveProjectName(project); const modern = await loadModernProjectEnv({ project, envName }); if (modern) { renderEnvExplain( @@ -1334,10 +1372,10 @@ const handleEnvApply: CommandHandlerFor = async ({ ctx, args, }): Promise => { - const service = args.options.service?.trim() ?? ""; - if (service.length === 0 || service === "global") { - throw new CliUsageError("--service is required."); - } + const service = parseApplyServiceOption({ + serviceName: args.options.service, + }); + const envName = resolveRequestedEnvName({ envOption: args.options.env }); const project = await resolveProjectForEnv({ ctx, pathOpt: args.options.path, @@ -1352,8 +1390,9 @@ const handleEnvApply: CommandHandlerFor = async ({ "--path", project.projectRoot, ]; - if (args.options.env) { - command.push("--env", args.options.env); + // Forward only the validated selection, never the raw argument. + if (envName !== undefined) { + command.push("--env", envName ?? "base"); } if (args.options.json) { command.push("--json"); @@ -1699,24 +1738,44 @@ const handleEnvAdd: CommandHandlerFor = async ({ ctx, args, }): Promise => { + const keyArg = args.positionals.key?.trim(); + const valueArg = args.positionals.value; + const valueStdin = args.options.valueStdin === true; + // --value-stdin: refuse bad combinations and read the bounded value before + // any project file is migrated or written, so a refusal stores nothing. + if (valueStdin) { + assertEnvValueStdinUsage({ + hasKey: Boolean(keyArg), + hasPositionalValue: + valueArg !== undefined || keyArg?.includes("=") === true, + }); + } + // Validate a key argument with fixed, value-free refusals before stdin is + // read or any project file is migrated or written. + const interactive = !(valueStdin || keyArg || valueArg); + const argTarget = interactive + ? null + : parseProjectEnvTarget({ + keyOrPath: keyArg ?? "", + scopeOverride: args.options.service, + }); + const envName = resolveRequestedEnvName({ + envOption: args.options.env, + }); + const stdinValue = valueStdin ? await readEnvValueFromStdin() : null; + const project = await resolveProjectForEnv({ ctx, pathOpt: args.options.path, projectOpt: args.options.project, }); const projectName = await resolveProjectName(project); - const envName = resolveRequestedEnvName({ - envOption: args.options.env, - }); await maybeMigrateLegacyProjectEnv({ project, projectName, reason: "mutation", }); - const keyArg = args.positionals.key?.trim(); - const valueArg = args.positionals.value; - const interactive = !(keyArg || valueArg); const prompted = interactive ? await promptForEnvAddInput({ project, @@ -1726,17 +1785,18 @@ const handleEnvAdd: CommandHandlerFor = async ({ const parsedTarget = prompted ? { scope: prompted.scope, key: prompted.key } - : parseProjectEnvTarget({ - keyOrPath: keyArg ?? "", - scopeOverride: args.options.service, - }); - const value = prompted - ? prompted.value - : await resolveEnvValue({ - key: parsedTarget.key, - value: valueArg ?? null, - secret: args.options.secret === true, - }); + : argTarget; + if (!parsedTarget) { + throw new CliUsageError("Env key is required."); + } + const value = + prompted?.value ?? + stdinValue ?? + (await resolveEnvValue({ + key: parsedTarget.key, + value: valueArg ?? null, + secret: args.options.secret === true, + })); const secret = prompted ? prompted.secret : args.options.secret === true; const result = await setProjectEnvValue({ @@ -1769,6 +1829,19 @@ const handleEnvSet: CommandHandlerFor = async ({ ctx, args, }): Promise => { + const rawKey = (args.positionals.key ?? "").trim(); + const rawValue = args.positionals.value; + const [keyFromSpec, valueFromSpec] = parseKeyValueSpec( + rawValue === undefined ? rawKey : `${rawKey}=${rawValue}` + ); + const valueFromStdin = args.options.valueStdin === true; + if (valueFromStdin) { + assertEnvValueStdinUsage({ + hasKey: keyFromSpec.length > 0, + hasPositionalValue: valueFromSpec !== null, + }); + } + const project = await resolveProjectForEnv({ ctx, pathOpt: args.options.path, @@ -1799,11 +1872,6 @@ const handleEnvSet: CommandHandlerFor = async ({ const runtimeConfig = await readHackEnvRuntimeConfig({ projectDir: project.projectDir, }); - const rawKey = (args.positionals.key ?? "").trim(); - const rawValue = args.positionals.value; - const [keyFromSpec, valueFromSpec] = parseKeyValueSpec( - rawValue === undefined ? rawKey : `${rawKey}=${rawValue}` - ); const key = await resolveEnvKey({ key: keyFromSpec }); const mutationSource = storeSecret ? "keychain" : "plain_env"; const sourceValidation = await validateHackEnvMutationSource({ @@ -1815,11 +1883,13 @@ const handleEnvSet: CommandHandlerFor = async ({ logger.error({ message: sourceValidation.message }); return 1; } - const value = await resolveEnvValue({ - key, - value: valueFromSpec, - secret: storeSecret, - }); + const value = valueFromStdin + ? await readEnvValueFromStdin() + : await resolveEnvValue({ + key, + value: valueFromSpec, + secret: storeSecret, + }); if (storeSecret) { const store = await resolveConfiguredSecretStore({ project, projectName }); @@ -1870,6 +1940,10 @@ const handleEnvMaterialize: CommandHandlerFor = async ({ ctx, args, }): Promise => { + // Validate before migration or file access so a refused scope changes nothing. + const serviceName = parseMaterializeScopeOption({ + serviceName: args.options.service, + }); const project = await resolveProjectForEnv({ ctx, pathOpt: args.options.path, @@ -1891,7 +1965,7 @@ const handleEnvMaterialize: CommandHandlerFor = async ({ projectRoot: project.projectRoot, projectDir: project.projectDir, envName, - serviceName: args.options.service?.trim() || null, + serviceName, serviceNames, }); logger.success({ @@ -1911,18 +1985,6 @@ function resolveShellCommandCommand(input: { return ["/bin/sh", "-lc", input.command]; } -function resolveExecutionScopeName(input: { - readonly scopeName?: string; - readonly serviceName?: string; -}): string | null { - const scopeName = input.scopeName?.trim(); - const serviceName = input.serviceName?.trim(); - if (scopeName && serviceName) { - throw new CliUsageError("Use either --scope or --service, not both."); - } - return scopeName || serviceName || null; -} - async function runHostCommandWithInjectedEnv(input: { readonly ctx: CliContext; readonly pathOpt: string | undefined; @@ -1936,6 +1998,10 @@ async function runHostCommandWithInjectedEnv(input: { readonly timeout?: number; readonly lifetime?: string; }): Promise { + const executionScope = parseExecutionScopeOptions({ + scopeName: input.scopeName, + serviceName: input.serviceName, + }); if ( input.timeout !== undefined && (!Number.isFinite(input.timeout) || @@ -1962,18 +2028,18 @@ async function runHostCommandWithInjectedEnv(input: { "--timeout requires non-TTY stdin; pipe input or redirect from /dev/null for a finite job." ); } - const project = await resolveProjectForEnv({ - ctx: input.ctx, - pathOpt: input.pathOpt, - projectOpt: input.projectOpt, - }); - const projectName = await resolveProjectName(project); const envName = resolveRequestedEnvName({ envOption: input.envOpt, }); const target = resolveHostEnvTarget({ targetOption: input.targetOpt, }); + const project = await resolveProjectForEnv({ + ctx: input.ctx, + pathOpt: input.pathOpt, + projectOpt: input.projectOpt, + }); + const projectName = await resolveProjectName(project); const shellCommand = input.shellCommandOpt?.trim(); const positionalCommand = input.command; if (shellCommand && positionalCommand.length > 0) { @@ -1987,10 +2053,7 @@ async function runHostCommandWithInjectedEnv(input: { project, projectName, envName, - serviceName: resolveExecutionScopeName({ - scopeName: input.scopeName, - serviceName: input.serviceName, - }), + serviceName: executionScope, target, }); const declaredLifetime = @@ -2021,26 +2084,27 @@ async function openHostShellWithInjectedEnv(input: { readonly serviceName?: string; readonly targetOpt: string | undefined; }): Promise { - const project = await resolveProjectForEnv({ - ctx: input.ctx, - pathOpt: input.pathOpt, - projectOpt: input.projectOpt, + const executionScope = parseExecutionScopeOptions({ + scopeName: input.scopeName, + serviceName: input.serviceName, }); - const projectName = await resolveProjectName(project); const envName = resolveRequestedEnvName({ envOption: input.envOpt, }); const target = resolveHostEnvTarget({ targetOption: input.targetOpt, }); + const project = await resolveProjectForEnv({ + ctx: input.ctx, + pathOpt: input.pathOpt, + projectOpt: input.projectOpt, + }); + const projectName = await resolveProjectName(project); const envState = await resolveEnvInjection({ project, projectName, envName, - serviceName: resolveExecutionScopeName({ - scopeName: input.scopeName, - serviceName: input.serviceName, - }), + serviceName: executionScope, target, }); @@ -2133,10 +2197,12 @@ const handleEnvUnset: CommandHandlerFor = async ({ projectDir: project.projectDir, }); if (!modernExists) { - const store = await resolveConfiguredSecretStore({ project, projectName }); + // Validate the key before resolving a secret store, so a refused key + // never reaches a backend. const key = await resolveEnvKey({ key: (args.positionals.key ?? "").trim(), }); + const store = await resolveConfiguredSecretStore({ project, projectName }); const okLegacy = await confirmSafe({ message: `Unset "${key}" from ${resolveEnvFilePath({ projectDir: project.projectDir, envName })} and ${formatSecretStoreDescriptor({ descriptor: store.descriptor })}${envName ? ` (env ${envName})` : ""}?`, @@ -2378,7 +2444,7 @@ async function resolveEnvKey(opts: { readonly key: string }): Promise { const fromPos = opts.key.trim(); if (fromPos.length > 0) { if (!ENV_KEY_PATTERN.test(fromPos)) { - throw new CliUsageError(`Invalid env key: ${fromPos}`); + throw new CliUsageError(ENV_KEY_INVALID_MESSAGE); } return fromPos; } @@ -2422,7 +2488,7 @@ async function resolveEnvValue(opts: { if (!canPrompt()) { requireInteractive({ what: `a value for "${opts.key}" is required`, - hint: "Pass the value as an argument (e.g. `hack env set KEY value` or `hack env add KEY value --secret`).", + hint: "Pipe the value on stdin with --value-stdin (e.g. ` | hack env add KEY --secret --value-stdin`), or run in a terminal to use the hidden prompt. Never pass secret values as arguments.", }); } diff --git a/src/lib/env-value-stdin.ts b/src/lib/env-value-stdin.ts new file mode 100644 index 000000000..b43a8939a --- /dev/null +++ b/src/lib/env-value-stdin.ts @@ -0,0 +1,202 @@ +import { CliUsageError } from "../cli/command.ts"; + +/** + * Non-argv value input for `hack env add|set --value-stdin`. + * + * Contract: + * - The value is read only from a non-TTY stdin (a pipe, file, or here-doc), + * so it never appears in argv, shell history, or process listings. A TTY + * stdin is refused before any byte is read; the hidden prompt is the + * interactive path. + * - Input is bounded by {@link ENV_VALUE_STDIN_MAX_BYTES}. Reading stops and + * the stream is canceled as soon as the bound is exceeded, so an endless + * producer cannot exhaust memory. + * - Exactly one trailing newline (`\n`, or `\r\n` treated as one newline) is + * stripped, so `printf '%s\n' "$v"` and `printf '%s' "$v"` store the same + * value. Any further trailing newlines are preserved as part of the value. + * - Empty input (after that strip), invalid UTF-8, and NUL bytes are refused. + * + * Every refusal is a closed, value-free {@link EnvValueStdinError}: messages + * are fixed strings and never include caller input, byte content, or the + * underlying read error. + */ +export const ENV_VALUE_STDIN_MAX_BYTES = 64 * 1024; + +export const ENV_VALUE_STDIN_REFUSAL_REASONS = [ + "interactive_tty", + "oversize", + "empty", + "invalid_utf8", + "nul_byte", + "read_failed", + "positional_value", + "missing_key", +] as const; + +export type EnvValueStdinRefusalReason = + (typeof ENV_VALUE_STDIN_REFUSAL_REASONS)[number]; + +const REFUSAL_MESSAGES: Readonly> = { + interactive_tty: + "--value-stdin needs piped input, but stdin is an interactive terminal. Pipe the value from a secret manager or file, or omit --value-stdin to use the hidden prompt. Nothing was stored.", + oversize: `--value-stdin input exceeds the ${ENV_VALUE_STDIN_MAX_BYTES}-byte limit. Nothing was stored.`, + empty: "--value-stdin received an empty value. Nothing was stored.", + invalid_utf8: "--value-stdin input is not valid UTF-8. Nothing was stored.", + nul_byte: + "--value-stdin input contains a NUL byte, which env values cannot carry. Nothing was stored.", + read_failed: "--value-stdin could not read stdin. Nothing was stored.", + positional_value: + "--value-stdin cannot be combined with a value argument (including KEY=value). Pass only the key and pipe the value. Nothing was stored.", + missing_key: + "--value-stdin requires the key as an argument, for example `hack env add API_TOKEN --secret --value-stdin`. Nothing was stored.", +}; + +/** Closed refusal for `--value-stdin`; the message never carries input. */ +export class EnvValueStdinError extends CliUsageError { + readonly reason: EnvValueStdinRefusalReason; + + constructor(reason: EnvValueStdinRefusalReason) { + super(REFUSAL_MESSAGES[reason]); + this.name = "EnvValueStdinError"; + this.reason = reason; + } +} + +/** Where `--value-stdin` reads from. Injected by tests; defaults to process stdin. */ +export type EnvValueStdinSource = { + readonly isTTY: boolean; + readonly open: () => ReadableStream; +}; + +function processStdinSource(): EnvValueStdinSource { + return { + isTTY: process.stdin.isTTY === true, + open: () => Bun.stdin.stream(), + }; +} + +/** + * Refuse `--value-stdin` argument combinations before reading anything. + * + * @param input.hasKey - Whether a key argument was supplied. + * @param input.hasPositionalValue - Whether a value argument (or a + * `KEY=value` spec) was supplied alongside the flag. + */ +export function assertEnvValueStdinUsage(input: { + readonly hasKey: boolean; + readonly hasPositionalValue: boolean; +}): void { + if (input.hasPositionalValue) { + throw new EnvValueStdinError("positional_value"); + } + if (!input.hasKey) { + throw new EnvValueStdinError("missing_key"); + } +} + +/** + * Read one env value from a non-TTY stdin under the module contract. + * + * @param input.source - Stdin source; defaults to the process stdin. + * @param input.maxBytes - Byte bound; defaults to {@link ENV_VALUE_STDIN_MAX_BYTES}. + * @returns The decoded value with exactly one trailing newline removed. + * @throws EnvValueStdinError for every refusal (closed, value-free). + */ +export async function readEnvValueFromStdin( + input: { + readonly source?: EnvValueStdinSource; + readonly maxBytes?: number; + } = {} +): Promise { + const source = input.source ?? processStdinSource(); + const maxBytes = input.maxBytes ?? ENV_VALUE_STDIN_MAX_BYTES; + if (source.isTTY) { + throw new EnvValueStdinError("interactive_tty"); + } + + const bytes = await readBounded({ source, maxBytes }); + const decoded = decodeUtf8(bytes); + bytes.fill(0); + if (decoded.includes("\0")) { + throw new EnvValueStdinError("nul_byte"); + } + const value = stripOneTrailingNewline(decoded); + if (value.length === 0) { + throw new EnvValueStdinError("empty"); + } + return value; +} + +async function readBounded(input: { + readonly source: EnvValueStdinSource; + readonly maxBytes: number; +}): Promise { + const chunks: Uint8Array[] = []; + let total = 0; + let reader: ReadableStreamDefaultReader; + try { + reader = input.source.open().getReader(); + } catch { + throw new EnvValueStdinError("read_failed"); + } + + const wipeChunks = (): void => { + for (const chunk of chunks) { + chunk.fill(0); + } + }; + + try { + while (true) { + let result: Awaited>; + try { + result = await reader.read(); + } catch { + wipeChunks(); + throw new EnvValueStdinError("read_failed"); + } + if (result.done) { + break; + } + total += result.value.byteLength; + if (total > input.maxBytes) { + result.value.fill(0); + wipeChunks(); + await reader.cancel().catch(() => undefined); + throw new EnvValueStdinError("oversize"); + } + chunks.push(result.value); + } + } finally { + reader.releaseLock(); + } + + const out = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + out.set(chunk, offset); + offset += chunk.byteLength; + } + wipeChunks(); + return out; +} + +function decodeUtf8(bytes: Uint8Array): string { + try { + return new TextDecoder("utf-8", { fatal: true }).decode(bytes); + } catch { + bytes.fill(0); + throw new EnvValueStdinError("invalid_utf8"); + } +} + +/** Remove exactly one trailing `\n` or `\r\n`; leave everything else intact. */ +export function stripOneTrailingNewline(value: string): string { + if (value.endsWith("\r\n")) { + return value.slice(0, -2); + } + if (value.endsWith("\n")) { + return value.slice(0, -1); + } + return value; +} diff --git a/src/lib/project-env-config.ts b/src/lib/project-env-config.ts index b4d0166e1..3a18af37c 100644 --- a/src/lib/project-env-config.ts +++ b/src/lib/project-env-config.ts @@ -81,6 +81,24 @@ const NATIVE_ENV_WORKLOAD_PATTERN = /^[a-z0-9][a-z0-9._-]{0,62}$/; const NATIVE_ENV_OVERLAY_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; const PROJECT_ENV_KEY_PATTERN = /^[A-Z_][A-Z0-9_]*$/; const PROJECT_ENV_SCOPE_PATTERN = /^[a-z0-9][a-z0-9._-]*$/; + +/** + * Fixed refusals for env key and scope arguments. + * + * Invariant: these messages never interpolate the rejected argument. A key + * argument is often a mistyped `KEY=value` or a value pasted into the key + * position, so echoing any part of it could disclose a secret in terminal + * output, logs, or CI transcripts. + */ +export const ENV_KEY_INVALID_MESSAGE = + "Invalid env key: keys must match ^[A-Z_][A-Z0-9_]*$ (for example API_TOKEN). The argument is not shown because it may contain a value."; +export const ENV_KEY_ASSIGNMENT_MESSAGE = + 'Invalid env key: the key argument cannot contain "=". Pass only the key (matching ^[A-Z_][A-Z0-9_]*$) and supply secret values with --value-stdin or the hidden prompt. The argument is not shown because it may contain a value.'; +export const ENV_SCOPE_INVALID_MESSAGE = + 'Invalid env scope: scopes must be "global" or match ^[a-z0-9][a-z0-9._-]*$ (usually a compose service name). The argument is not shown because it may contain a value.'; +export const ENV_SCOPE_UNKNOWN_MESSAGE = + "Unknown env scope: the requested scope is not global, a compose service, or a scope in the project env files. The argument is not shown because it may contain a value."; +const UNKNOWN_SCOPE_LIST_LIMIT = 20; const PROJECT_ENV_SECRET_PREFIX = "v1" as const; const PROJECT_ENV_ALGORITHM = "aes-256-gcm"; const PROJECT_ENV_IV_BYTES = 12; @@ -171,14 +189,42 @@ export function selectProjectEnvValues(opts: { return { ...opts.resolved.globalEnv }; } - const scoped = opts.resolved.serviceEnv[scopeName]; + const scoped = Object.hasOwn(opts.resolved.serviceEnv, scopeName) + ? opts.resolved.serviceEnv[scopeName] + : undefined; if (!scoped) { - throw new Error(`Unknown env scope: ${scopeName}`); + throw new Error( + unknownProjectEnvScopeMessage({ + configuredScopes: Object.keys(opts.resolved.serviceEnv), + }) + ); } return { ...scoped }; } +/** + * Fixed refusal for a scope that is not configured. It may list scope names + * taken from the project's compose and env files, but never the requested + * argument: a mistyped `--service KEY=value` must not be echoed. + */ +export function unknownProjectEnvScopeMessage(opts: { + readonly configuredScopes: readonly string[]; +}): string { + const configured = [ + "global", + ...[...new Set(opts.configuredScopes)] + .filter((scope) => scope !== "global") + .sort(), + ]; + const shown = configured.slice(0, UNKNOWN_SCOPE_LIST_LIMIT); + const more = + configured.length > shown.length + ? ` (and ${configured.length - shown.length} more)` + : ""; + return `${ENV_SCOPE_UNKNOWN_MESSAGE} Configured scopes: ${shown.join(", ")}${more}.`; +} + export function selectProjectEnvValuesForExecutionTarget(opts: { readonly resolved: ProjectEnvResolvedConfig; readonly scopeName?: string | null; @@ -194,7 +240,10 @@ export function selectProjectEnvValuesForExecutionTarget(opts: { const scopeName = normalizeProjectEnvScopeName({ scopeName: opts.scopeName, }); - return { ...(opts.resolved.hostTargetEnv[scopeName] ?? selected) }; + const hostTarget = Object.hasOwn(opts.resolved.hostTargetEnv, scopeName) + ? opts.resolved.hostTargetEnv[scopeName] + : undefined; + return { ...(hostTarget ?? selected) }; } export function isValidProjectEnvScopeName(opts: { @@ -220,7 +269,7 @@ export function assertValidProjectEnvScopeName(opts: { scopeName: opts.scopeName, }); if (!isValidProjectEnvScopeName({ scopeName })) { - throw new Error(`Invalid env scope: ${scopeName}`); + throw new Error(ENV_SCOPE_INVALID_MESSAGE); } return scopeName; } @@ -691,7 +740,9 @@ function parseProjectEnvConfig(opts: { if (!PROJECT_ENV_KEY_PATTERN.test(key)) { return { ok: false, - error: `Invalid env key "${key}" in scope "${scope}".`, + // The scope is validated above; the key is not echoed because a + // malformed entry may hold a pasted value. + error: `Invalid env key in scope "${scope}"; keys must match ^[A-Z_][A-Z0-9_]*$.`, }; } const normalizedValue = parseProjectEnvStoredValue({ valueRaw }); @@ -931,7 +982,14 @@ export async function resolveProjectEnvValue(opts: { !opts.serviceNames.includes(scope) && !Object.hasOwn(layers.merged.values, scope) ) { - throw new Error("Unknown env scope"); + throw new Error( + unknownProjectEnvScopeMessage({ + configuredScopes: [ + ...opts.serviceNames, + ...Object.keys(layers.merged.values), + ], + }) + ); } const entries = resolveEffectiveStoredEntries({ layers: layers.envLayers, @@ -2214,7 +2272,7 @@ export async function setProjectEnvValue(opts: { await assertLegacyProjectDirectory({ projectDir: opts.projectDir }); await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot }); if (!PROJECT_ENV_KEY_PATTERN.test(opts.key)) { - throw new Error(`Invalid env key: ${opts.key}`); + throw new Error(ENV_KEY_INVALID_MESSAGE); } const scope = assertValidProjectEnvScopeName({ scopeName: opts.scope, @@ -4078,6 +4136,11 @@ export function parseProjectEnvTarget(input: { if (trimmed.length === 0) { throw new Error("Env key is required."); } + // Refuse `KEY=value` before any dotted-scope split, so no fragment of the + // value can reach a scope or key error. Every refusal below is fixed text. + if (trimmed.includes("=")) { + throw new Error(ENV_KEY_ASSIGNMENT_MESSAGE); + } const dotIndex = trimmed.lastIndexOf("."); if (dotIndex > 0) { @@ -4088,13 +4151,13 @@ export function parseProjectEnvTarget(input: { const key = trimmed.slice(dotIndex + 1).trim(); assertValidProjectEnvScopeName({ scopeName: scope }); if (!PROJECT_ENV_KEY_PATTERN.test(key)) { - throw new Error(`Invalid env key: ${key}`); + throw new Error(ENV_KEY_INVALID_MESSAGE); } return { scope, key }; } if (!PROJECT_ENV_KEY_PATTERN.test(trimmed)) { - throw new Error(`Invalid env key: ${trimmed}`); + throw new Error(ENV_KEY_INVALID_MESSAGE); } return { scope: assertValidProjectEnvScopeName({ diff --git a/src/lib/project-name.ts b/src/lib/project-name.ts index 819a990b9..ce7a89ede 100644 --- a/src/lib/project-name.ts +++ b/src/lib/project-name.ts @@ -16,6 +16,17 @@ export function normalizeProjectName(input: string): string | null { return name.length > 0 ? name : null; } +/** A canonical project name: the fixed point of `normalizeProjectName`. */ +export const PROJECT_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; + +/** + * A project selector a user may type: a canonical name or a legacy alias + * spelling (ASCII letters, digits, `-`, `_`, space and `/`). Anything else, + * including any argument containing `=`, may be a pasted value and must be + * refused with text that does not include it. + */ +export const PROJECT_SELECTOR_PATTERN = /^[A-Za-z0-9_ /-]+$/; + export class AmbiguousProjectNameError extends Error { constructor(name: string) { super( diff --git a/src/lib/project.ts b/src/lib/project.ts index 7a54cea5d..c11630d32 100644 --- a/src/lib/project.ts +++ b/src/lib/project.ts @@ -133,6 +133,24 @@ export const ENV_BASE_SELECTION_ALIASES: ReadonlySet = new Set([ "none", ]); +/** + * Canonical overlay name: the fixed point of `normalizeEnvConfigName` + * (lowercase letters and digits joined by single hyphens). + */ +export const ENV_CONFIG_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; + +/** + * Parse a requested overlay selection (CLI `--env`, daemon `env`). + * + * - `undefined` input: no selection. + * - `null`: base layers only (empty, or a base alias in any case). + * - `string`: the overlay name, exactly as given apart from surrounding + * whitespace. + * - `undefined` for a non-empty input: refused. A requested name is never + * slugged, so `X=secret` or `QA` cannot silently become `xsecret` or `qa` + * and then be printed or written into an overlay file name. Callers must + * refuse with fixed text that does not include the input. + */ export function parseEnvConfigSelection( input: string | undefined ): string | null | undefined { @@ -149,7 +167,7 @@ export function parseEnvConfigSelection( return null; } - return normalizeEnvConfigName(trimmed); + return ENV_CONFIG_NAME_PATTERN.test(trimmed) ? trimmed : undefined; } export function sanitizeBranchSlug(input: string): string { diff --git a/tests/env-command-modern.test.ts b/tests/env-command-modern.test.ts index 61956952a..f7d3e5058 100644 --- a/tests/env-command-modern.test.ts +++ b/tests/env-command-modern.test.ts @@ -88,7 +88,9 @@ test("modern env list rejects unknown service scopes", async () => { ]); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Unknown env scope: typo_service"); + expect(result.stderr).toContain("Unknown env scope:"); + expect(result.stderr).toContain("Configured scopes: global, api"); + expect(result.stderr).not.toContain("typo_service"); }); test("env explain reports precedence and delivery without revealing values", async () => { diff --git a/tests/env-key-argument-redaction-cli.test.ts b/tests/env-key-argument-redaction-cli.test.ts new file mode 100644 index 000000000..8bc3e16c2 --- /dev/null +++ b/tests/env-key-argument-redaction-cli.test.ts @@ -0,0 +1,896 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + mkdir, + mkdtemp, + readdir, + readFile, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { type AnyCommandSpec, CliUsageError } from "../src/cli/command.ts"; +import { + INVALID_PROJECT_OPTION_MESSAGE, + parseEnvGetArguments, + parseExecutionScopeOptions, + parseProjectSelectorOption, +} from "../src/cli/env-arguments.ts"; +import { INVALID_ENV_SELECTION_MESSAGE } from "../src/cli/env-selection.ts"; +import { optEnv, optProject } from "../src/cli/options.ts"; +import { CLI_SPEC } from "../src/cli/spec.ts"; +import { parseEnvConfigSelection } from "../src/lib/project.ts"; +import { + ENV_KEY_ASSIGNMENT_MESSAGE, + ENV_KEY_INVALID_MESSAGE, + ENV_SCOPE_INVALID_MESSAGE, +} from "../src/lib/project-env-config.ts"; + +/** + * Key- and scope-argument refusals in `hack env` and `hack host` must never + * echo the rejected argument. + * + * A key or `--service`/`--scope` argument is often a mistyped `KEY=value` or + * a value pasted into the wrong position. Each case runs the real CLI in a fresh subprocess with a + * distinctive canary inside the argument and proves the canary is absent from + * stdout, stderr, and the argv/env of every child process the CLI spawns. + * + * Isolation: temp HOME and global config, a temp project, the project-key + * YAML store or an injected `encrypted_file` backend with an env-provided key. + * No keychain, 1Password, Docker, or agent client config is reached. + */ + +const REPO_ROOT = resolve(import.meta.dir, ".."); +const ENTRYPOINT = resolve(REPO_ROOT, "index.ts"); +const RECORDER = resolve(import.meta.dir, "fixtures/spawn-argv-recorder.ts"); +/** Lowercase canary: valid as a scope fragment, invalid as an env key. */ +const CANARY = "echo-canary-c1-9d4f7a"; +/** Uppercase-bearing canary: invalid as a scope name. */ +const SCOPE_CANARY = "Echo-Scope-Canary-C1-3b6e"; +const SECRETS_FILE_KEY = "env-key-redaction-test-key"; + +const INVALID_KEY = "Invalid env key: keys must match ^[A-Z_][A-Z0-9_]*$"; +const ASSIGNMENT_KEY = 'the key argument cannot contain "="'; +const INVALID_SCOPE = 'Invalid env scope: scopes must be "global"'; +const GET_REFUSAL = "Unable to read env value."; + +type RunResult = { + readonly exitCode: number; + readonly stdout: string; + readonly stderr: string; + readonly spawnLog: string; +}; + +let root: string | null = null; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "hack-env-key-redaction-")); + await mkdir(join(root, "home"), { recursive: true }); +}); + +afterEach(async () => { + if (root) { + await rm(root, { recursive: true, force: true }); + root = null; + } +}); + +function requireRoot(): string { + if (!root) { + throw new Error("Missing temp root"); + } + return root; +} + +async function runHack(input: { + readonly args: readonly string[]; + readonly cwd?: string; + readonly stdin?: string; +}): Promise { + const base = requireRoot(); + const spawnLog = join(base, `spawn-${crypto.randomUUID()}.jsonl`); + const proc = Bun.spawn( + [process.execPath, "--preload", RECORDER, ENTRYPOINT, ...input.args], + { + cwd: input.cwd ?? REPO_ROOT, + env: { + PATH: process.env.PATH ?? "/usr/bin:/bin", + HOME: join(base, "home"), + TMPDIR: join(base, "home"), + HACK_GLOBAL_CONFIG_PATH: join(base, "home", "hack-config.json"), + HACK_LOGGER: "console", + HACK_SECRETS_FILE_KEY: SECRETS_FILE_KEY, + HACK_TEST_SPAWN_ARGV_LOG: spawnLog, + }, + stdin: input.stdin === undefined ? "ignore" : "pipe", + stdout: "pipe", + stderr: "pipe", + } + ); + if (input.stdin !== undefined && proc.stdin) { + proc.stdin.write(input.stdin); + await proc.stdin.end(); + } + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + const spawnLogText = await readFile(spawnLog, "utf8").catch(() => ""); + return { exitCode, stdout, stderr, spawnLog: spawnLogText }; +} + +function expectRefusedWithoutEcho(input: { + readonly result: RunResult; + readonly message: string; + readonly canary: string; + readonly rawArgument: string; +}): void { + const { result } = input; + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain(input.message); + for (const stream of [result.stdout, result.stderr, result.spawnLog]) { + expect(stream).not.toContain(input.canary); + expect(stream).not.toContain(input.rawArgument); + // A slugged echo (case-folded, punctuation stripped) is still an echo. + expect(stream.toLowerCase()).not.toContain(input.canary.toLowerCase()); + } +} + +async function createModernProject(): Promise { + const projectRoot = join(requireRoot(), "modern"); + const projectDir = join(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + join(projectDir, "docker-compose.yml"), + "services:\n api:\n image: alpine:3.20\n" + ); + await writeFile( + join(projectDir, "hack.config.json"), + `${JSON.stringify({ name: "key-redaction-modern", dev_host: "key-redaction-modern.hack" }, null, 2)}\n` + ); + await writeFile( + join(projectDir, "hack.env.default.yaml"), + 'version: 1\nenvironment: default\nsecretsprovider: project_key\nvalues:\n global:\n GLOBAL_FLAG: "1"\n' + ); + return projectRoot; +} + +async function createLegacyProject(): Promise<{ + readonly projectRoot: string; + readonly storePath: string; +}> { + const base = requireRoot(); + const projectRoot = join(base, "legacy"); + const projectDir = join(projectRoot, ".hack"); + const storePath = join(base, "secrets.enc.json"); + await mkdir(projectDir, { recursive: true }); + await writeFile(join(projectDir, "docker-compose.yml"), "services: {}\n"); + await writeFile( + join(projectDir, "hack.config.json"), + `${JSON.stringify( + { + name: "key-redaction-legacy", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: storePath }, + }, + }, + }, + null, + 2 + )}\n` + ); + await writeFile( + join(projectDir, "hack.env.json"), + `${JSON.stringify( + { + version: 1, + vars: [{ key: "API_KEY", required: false, source: "keychain" }], + }, + null, + 2 + )}\n` + ); + return { projectRoot, storePath }; +} + +async function readTree(paths: readonly string[]): Promise { + const contents = await Promise.all( + paths.map((path) => readFile(path, "utf8").catch(() => "")) + ); + return contents.join("\n--\n"); +} + +function modernFiles(projectRoot: string): readonly string[] { + return [ + "hack.env.default.yaml", + "hack.env.local.yaml", + ".env", + "hack.config.json", + ].map((file) => join(projectRoot, ".hack", file)); +} + +test( + "modern env add/set/unset/explain refuse KEY=value without echoing any of it", + async () => { + const projectRoot = await createModernProject(); + const before = await readTree(modernFiles(projectRoot)); + const assignment = `API_TOKEN=${CANARY}`; + // A dotted value would previously split into a scope that carried the + // value prefix into "Invalid env scope: API_TOKEN=...". + const dottedAssignment = `API_TOKEN=${CANARY}.tail`; + const cases: readonly (readonly string[])[] = [ + ["env", "add", assignment], + ["env", "add", assignment, "--secret"], + ["env", "add", dottedAssignment], + ["env", "add", assignment, "--service", "api"], + ["env", "set", assignment], + ["env", "unset", assignment], + ["env", "unset", dottedAssignment], + ["env", "explain", assignment], + ]; + for (const args of cases) { + const raw = args[2] ?? ""; + const result = await runHack({ args: [...args, "--path", projectRoot] }); + expectRefusedWithoutEcho({ + result, + message: ASSIGNMENT_KEY, + canary: CANARY, + rawArgument: raw, + }); + } + expect(await readTree(modernFiles(projectRoot))).toBe(before); + }, + { timeout: 90_000 } +); + +test( + "modern env commands refuse a value pasted as the key without echoing it", + async () => { + const projectRoot = await createModernProject(); + const before = await readTree(modernFiles(projectRoot)); + const cases: readonly { + readonly args: readonly string[]; + readonly message: string; + readonly canary: string; + }[] = [ + { args: ["env", "add", CANARY], message: INVALID_KEY, canary: CANARY }, + { + args: ["env", "add", CANARY, "positional-value"], + message: INVALID_KEY, + canary: CANARY, + }, + { + args: ["env", "add", `api.${CANARY}`], + message: INVALID_KEY, + canary: CANARY, + }, + { + args: ["env", "add", `${SCOPE_CANARY}.API_TOKEN`, "value"], + message: INVALID_SCOPE, + canary: SCOPE_CANARY, + }, + { + args: ["env", "add", "API_TOKEN", "value", "--service", SCOPE_CANARY], + message: INVALID_SCOPE, + canary: SCOPE_CANARY, + }, + { args: ["env", "set", CANARY], message: INVALID_KEY, canary: CANARY }, + { args: ["env", "unset", CANARY], message: INVALID_KEY, canary: CANARY }, + // `env get` replaces every failure with one generic message. + { args: ["env", "get", CANARY], message: GET_REFUSAL, canary: CANARY }, + { + args: ["env", "get", "API_TOKEN", "--service", SCOPE_CANARY], + message: GET_REFUSAL, + canary: SCOPE_CANARY, + }, + { + args: ["env", "explain", CANARY], + message: INVALID_KEY, + canary: CANARY, + }, + ]; + for (const entry of cases) { + const result = await runHack({ + args: [...entry.args, "--path", projectRoot], + }); + expectRefusedWithoutEcho({ + result, + message: entry.message, + canary: entry.canary, + rawArgument: entry.args[2] ?? "", + }); + } + expect(await readTree(modernFiles(projectRoot))).toBe(before); + }, + { timeout: 120_000 } +); + +test( + "a refused key argument is rejected before stdin is read or files change", + async () => { + const projectRoot = await createModernProject(); + const before = await readTree(modernFiles(projectRoot)); + // Empty stdin would be refused as "empty value" if it were read first, so + // seeing the key refusal proves the key is validated before stdin. + const result = await runHack({ + args: ["env", "add", CANARY, "--secret", "--value-stdin"], + stdin: "", + cwd: projectRoot, + }); + expectRefusedWithoutEcho({ + result, + message: INVALID_KEY, + canary: CANARY, + rawArgument: CANARY, + }); + expect(result.stderr).not.toContain( + "--value-stdin received an empty value" + ); + expect(await readTree(modernFiles(projectRoot))).toBe(before); + }, + { timeout: 30_000 } +); + +test( + "legacy env set and unset refuse an invalid key without echoing it or touching the store", + async () => { + const { projectRoot, storePath } = await createLegacyProject(); + const files = [ + storePath, + join(projectRoot, ".hack", ".env"), + join(projectRoot, ".hack", "hack.env.json"), + ]; + const before = await readTree(files); + const cases: readonly (readonly string[])[] = [ + ["env", "set", CANARY], + ["env", "set", CANARY, "positional-value"], + ["env", "set", `${CANARY}=positional-value`], + ["env", "set", CANARY, "--secret"], + ["env", "unset", CANARY], + ]; + for (const args of cases) { + const result = await runHack({ args, cwd: projectRoot }); + expectRefusedWithoutEcho({ + result, + message: INVALID_KEY, + canary: CANARY, + rawArgument: args[2] ?? "", + }); + } + expect(await readTree(files)).toBe(before); + }, + { timeout: 90_000 } +); + +test( + "a malformed key in an env config file is reported without echoing the key", + async () => { + const projectRoot = await createModernProject(); + await writeFile( + join(projectRoot, ".hack", "hack.env.default.yaml"), + `version: 1\nenvironment: default\nsecretsprovider: project_key\nvalues:\n global:\n "API_TOKEN=${CANARY}": "1"\n` + ); + const result = await runHack({ + args: ["env", "list", "--path", projectRoot], + }); + expectRefusedWithoutEcho({ + result, + message: 'Invalid env key in scope "global"', + canary: CANARY, + rawArgument: `API_TOKEN=${CANARY}`, + }); + }, + { timeout: 30_000 } +); + +const UNKNOWN_SCOPE = "Unknown env scope: the requested scope is not global"; + +/** Program names (argv[0]) of every recorded child process. */ +function spawnedPrograms(spawnLog: string): readonly string[] { + return spawnLog + .split("\n") + .filter((line) => line.trim().length > 0) + .map((line) => { + const record: unknown = JSON.parse(line); + const argv = + typeof record === "object" && record !== null && "argv" in record + ? record.argv + : null; + return Array.isArray(argv) ? String(argv[0]) : ""; + }); +} + +/** + * Every command that accepts a scope through --service or --scope. Options + * precede `--`, after which everything is the host command's own argv. + */ +function scopeOptionCases(input: { + readonly scope: string; + readonly projectRoot: string; +}): readonly (readonly string[])[] { + const path = ["--path", input.projectRoot]; + return [ + ["env", "materialize", "--service", input.scope, ...path], + ["env", "exec", "--service", input.scope, ...path, "--", "true"], + ["env", "shell", "--service", input.scope, ...path], + ["env", "list", "--service", input.scope, ...path], + ["env", "apply", "--service", input.scope, ...path], + ["host", "exec", "--scope", input.scope, ...path, "--", "true"], + ["host", "shell", "--scope", input.scope, ...path], + ]; +} + +test( + "scope options refuse a malformed scope without echoing it or touching files", + async () => { + const projectRoot = await createModernProject(); + const before = await readTree(modernFiles(projectRoot)); + for (const scope of [`API_TOKEN=${CANARY}`, SCOPE_CANARY]) { + const canary = scope === SCOPE_CANARY ? SCOPE_CANARY : CANARY; + for (const args of scopeOptionCases({ scope, projectRoot })) { + const result = await runHack({ args }); + expectRefusedWithoutEcho({ + result, + message: INVALID_SCOPE, + canary, + rawArgument: scope, + }); + // Refused before any shell, command, or restart child starts. + expect( + spawnedPrograms(result.spawnLog).filter((name) => name !== "git") + ).toEqual([]); + } + } + expect(await readTree(modernFiles(projectRoot))).toBe(before); + }, + { timeout: 180_000 } +); + +test( + "a well-formed but unknown scope is refused without echoing it", + async () => { + const projectRoot = await createModernProject(); + const before = await readTree(modernFiles(projectRoot)); + // CANARY matches the scope pattern, so it reaches scope resolution. The + // refusal may list configured scopes from project files, never the request. + const cases = scopeOptionCases({ scope: CANARY, projectRoot }).filter( + (args) => args[1] !== "apply" + ); + for (const args of cases) { + const result = await runHack({ args }); + expectRefusedWithoutEcho({ + result, + message: UNKNOWN_SCOPE, + canary: CANARY, + rawArgument: CANARY, + }); + expect(result.stderr).toContain("Configured scopes: global, api."); + expect( + spawnedPrograms(result.spawnLog).filter((name) => name !== "git") + ).toEqual([]); + } + const get = await runHack({ + args: ["env", "get", "GLOBAL_FLAG", "--service", CANARY], + cwd: projectRoot, + }); + expectRefusedWithoutEcho({ + result: get, + message: GET_REFUSAL, + canary: CANARY, + rawArgument: CANARY, + }); + expect(await readTree(modernFiles(projectRoot))).toBe(before); + }, + { timeout: 120_000 } +); + +test("env get argument refusals are value-free before the generic wrapper", () => { + // The CLI replaces every `env get` failure with one generic message, which + // would hide a regression in the handler's own refusals. Assert the parse + // layer the handler uses so a re-echo of the key or scope is still caught. + const cases: readonly { + readonly key: string; + readonly serviceName?: string; + readonly message: string; + readonly canary: string; + }[] = [ + { key: CANARY, message: ENV_KEY_INVALID_MESSAGE, canary: CANARY }, + { + key: `API_TOKEN=${CANARY}`, + message: ENV_KEY_ASSIGNMENT_MESSAGE, + canary: CANARY, + }, + { + key: "API_TOKEN", + serviceName: SCOPE_CANARY, + message: ENV_SCOPE_INVALID_MESSAGE, + canary: SCOPE_CANARY, + }, + { + key: "API_TOKEN", + serviceName: `api=${CANARY}`, + message: ENV_SCOPE_INVALID_MESSAGE, + canary: CANARY, + }, + ]; + for (const entry of cases) { + let thrown: unknown = null; + try { + parseEnvGetArguments({ + key: entry.key, + serviceName: entry.serviceName, + }); + } catch (error: unknown) { + thrown = error; + } + expect(thrown).toBeInstanceOf(CliUsageError); + const message = (thrown as Error).message; + expect(message).toBe(entry.message); + expect(message).not.toContain(entry.canary); + } + expect( + parseEnvGetArguments({ key: "API_TOKEN", serviceName: " api " }) + ).toEqual({ key: "API_TOKEN", scope: "api" }); + expect( + parseEnvGetArguments({ key: "API_TOKEN", serviceName: undefined }) + ).toEqual({ key: "API_TOKEN", scope: "global" }); +}); + +test("execution scope parsing refuses conflicts and malformed scopes value-free", () => { + expect( + parseExecutionScopeOptions({ scopeName: undefined, serviceName: " " }) + ).toBeNull(); + expect( + parseExecutionScopeOptions({ scopeName: "api", serviceName: undefined }) + ).toBe("api"); + for (const input of [ + { scopeName: `API_TOKEN=${CANARY}`, serviceName: undefined }, + { scopeName: undefined, serviceName: `API_TOKEN=${CANARY}` }, + { scopeName: "api", serviceName: CANARY }, + ]) { + let message = ""; + try { + parseExecutionScopeOptions(input); + } catch (error: unknown) { + message = error instanceof Error ? error.message : String(error); + } + expect(message.length).toBeGreaterThan(0); + expect(message).not.toContain(CANARY); + } +}); + +/** Every file or directory name under `dir`, recursively. */ +async function listNames(dir: string): Promise { + const entries = await readdir(dir, { recursive: true }); + return entries.map((entry) => String(entry)); +} + +/** No file anywhere under the temp root may carry the canary in its name. */ +async function expectNoCanaryFileNames(canary: string): Promise { + const names = await listNames(requireRoot()); + expect( + names.filter((name) => name.toLowerCase().includes(canary.toLowerCase())) + ).toEqual([]); +} + +function expectNoNonGitChild(result: RunResult): void { + expect( + spawnedPrograms(result.spawnLog).filter((name) => name !== "git") + ).toEqual([]); +} + +/** Command paths (e.g. ["env", "get"]) whose spec includes `option`. */ +function commandPathsWithOption(option: unknown): readonly string[][] { + const out: string[][] = []; + const walk = ( + commands: readonly AnyCommandSpec[], + prefix: readonly string[] + ): void => { + for (const command of commands) { + const path = [...prefix, command.name]; + if (command.options.some((entry) => entry === option)) { + out.push(path); + } + walk(command.subcommands, path); + } + }; + walk(CLI_SPEC.commands, []); + return out; +} + +/** Arguments that let each env/host command reach its handler. */ +function handlerArguments(path: readonly string[]): readonly string[] { + switch (path.join(" ")) { + case "env get": + case "env explain": + case "env unset": + return ["API_TOKEN"]; + case "env add": + case "env set": + return ["API_TOKEN", "value"]; + case "env apply": + return ["--service", "api"]; + case "env exec": + case "host exec": + return ["--", "true"]; + default: + return []; + } +} + +/** `env get` replaces every failure with one fixed message. */ +function expectedRefusal(path: readonly string[], message: string): string { + return path.join(" ") === "env get" ? GET_REFUSAL : message; +} + +/** Value-bearing --env spellings: assignments and non-canonical names. */ +const ENV_CANARY_VALUES: readonly { + readonly value: string; + readonly canary: string; +}[] = [ + // Previously slugged to "xecho-canary-...", printed, and used as a file name. + { value: `X=${CANARY}`, canary: CANARY }, + // Previously case-folded to "echo-scope-canary-...". + { value: SCOPE_CANARY, canary: SCOPE_CANARY }, + { value: `${CANARY}.yaml`, canary: CANARY }, +]; + +test( + "every command accepting --env refuses a value-bearing --env before any project work", + async () => { + const paths = commandPathsWithOption(optEnv); + // Guard against the enumeration silently finding nothing. + expect(paths.map((path) => path.join(" "))).toEqual( + expect.arrayContaining([ + "up", + "down", + "restart", + "run", + "exec", + "env add", + "env apply", + "env get", + "host exec", + "config plan", + "session start", + ]) + ); + // No .hack/ here: any refusal other than the --env one would mean the + // command discovered (or failed to discover) a project before --env. + const empty = join(requireRoot(), "empty"); + await mkdir(empty, { recursive: true }); + for (const path of paths) { + for (const entry of ENV_CANARY_VALUES) { + const result = await runHack({ + args: [...path, "--env", entry.value], + cwd: empty, + }); + expectRefusedWithoutEcho({ + result, + message: expectedRefusal(path, INVALID_ENV_SELECTION_MESSAGE), + canary: entry.canary, + rawArgument: entry.value, + }); + expectNoNonGitChild(result); + } + } + await expectNoCanaryFileNames(CANARY); + await expectNoCanaryFileNames(SCOPE_CANARY); + }, + { timeout: 300_000 } +); + +test( + "env and host commands refuse a value-bearing --env in a real project without creating overlay files", + async () => { + const projectRoot = await createModernProject(); + const before = await readTree(modernFiles(projectRoot)); + const beforeNames = await listNames(join(projectRoot, ".hack")); + const paths = commandPathsWithOption(optEnv).filter( + (path) => path[0] === "env" || path[0] === "host" + ); + for (const path of paths) { + for (const entry of ENV_CANARY_VALUES) { + const result = await runHack({ + args: [ + ...path, + "--env", + entry.value, + "--path", + projectRoot, + ...handlerArguments(path), + ], + }); + expectRefusedWithoutEcho({ + result, + message: expectedRefusal(path, INVALID_ENV_SELECTION_MESSAGE), + canary: entry.canary, + rawArgument: entry.value, + }); + expectNoNonGitChild(result); + } + } + // `env add --value-stdin` refuses before reading stdin: an empty stdin + // would otherwise be refused as an empty value. + const stdinResult = await runHack({ + args: [ + "env", + "add", + "API_TOKEN", + "--secret", + "--value-stdin", + "--env", + `X=${CANARY}`, + ], + cwd: projectRoot, + stdin: "", + }); + expectRefusedWithoutEcho({ + result: stdinResult, + message: INVALID_ENV_SELECTION_MESSAGE, + canary: CANARY, + rawArgument: `X=${CANARY}`, + }); + expect(stdinResult.stderr).not.toContain("empty value"); + expect(await readTree(modernFiles(projectRoot))).toBe(before); + expect(await listNames(join(projectRoot, ".hack"))).toEqual(beforeNames); + await expectNoCanaryFileNames(CANARY); + await expectNoCanaryFileNames(SCOPE_CANARY); + + // The documented base aliases keep working. + for (const alias of ["base", "DEFAULT", "none", ""]) { + const result = await runHack({ + args: ["env", "list", "--env", alias, "--json", "--path", projectRoot], + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain("GLOBAL_FLAG"); + } + }, + { timeout: 300_000 } +); + +test( + "env and host commands refuse a value-bearing --project without echoing it", + async () => { + const paths = commandPathsWithOption(optProject).filter( + // `host ps` only filters its listing by name and never echoes it. + (path) => + (path[0] === "env" || path[0] === "host") && + path.join(" ") !== "host ps" + ); + expect(paths.length).toBeGreaterThanOrEqual(12); + const cases: readonly { + readonly value: string; + readonly canary: string; + readonly message: string; + }[] = [ + // Previously printed as Unknown project "xecho-canary-...". + { + value: `x=${CANARY}`, + canary: CANARY, + message: INVALID_PROJECT_OPTION_MESSAGE, + }, + { + value: `${CANARY}.x`, + canary: CANARY, + message: INVALID_PROJECT_OPTION_MESSAGE, + }, + // A legacy alias spelling still selects by its slug, but an unknown + // non-canonical selector is not echoed back in slugged form. + { + value: SCOPE_CANARY, + canary: SCOPE_CANARY, + message: "Unknown project. Run 'hack init'", + }, + ]; + const cwd = join(requireRoot(), "empty"); + await mkdir(cwd, { recursive: true }); + for (const path of paths) { + for (const entry of cases) { + const result = await runHack({ + args: [...path, "--project", entry.value, ...handlerArguments(path)], + cwd, + }); + expectRefusedWithoutEcho({ + result, + message: expectedRefusal(path, entry.message), + canary: entry.canary, + rawArgument: entry.value, + }); + expectNoNonGitChild(result); + } + } + await expectNoCanaryFileNames(CANARY); + await expectNoCanaryFileNames(SCOPE_CANARY); + }, + { timeout: 300_000 } +); + +test( + "env and host commands refuse a value-bearing --target without echoing it", + async () => { + const projectRoot = await createModernProject(); + const before = await readTree(modernFiles(projectRoot)); + const paths = [ + ["env", "explain"], + ["env", "exec"], + ["env", "shell"], + ["host", "exec"], + ["host", "shell"], + ]; + for (const path of paths) { + for (const entry of [ + { value: `x=${CANARY}`, canary: CANARY }, + { value: SCOPE_CANARY, canary: SCOPE_CANARY }, + ]) { + const result = await runHack({ + args: [ + ...path, + "--target", + entry.value, + "--path", + projectRoot, + ...handlerArguments(path), + ], + }); + expectRefusedWithoutEcho({ + result, + message: "Invalid --target value. Expected one of: host, compose.", + canary: entry.canary, + rawArgument: entry.value, + }); + expectNoNonGitChild(result); + } + } + expect(await readTree(modernFiles(projectRoot))).toBe(before); + await expectNoCanaryFileNames(CANARY); + await expectNoCanaryFileNames(SCOPE_CANARY); + }, + { timeout: 120_000 } +); + +test("overlay and project selectors are never slugged from user input", () => { + // Shared by the CLI and the daemon env routes (which map undefined to a + // fixed `invalid_env` error). + expect(parseEnvConfigSelection(undefined)).toBeUndefined(); + expect(parseEnvConfigSelection(" ")).toBeNull(); + expect(parseEnvConfigSelection("NONE")).toBeNull(); + expect(parseEnvConfigSelection(" qa ")).toBe("qa"); + expect(parseEnvConfigSelection("qa-2")).toBe("qa-2"); + for (const value of [ + `X=${CANARY}`, + SCOPE_CANARY, + "QA", + "q_a", + "q a", + "q--a", + "-qa", + `${CANARY}.yaml`, + "../qa", + ]) { + expect(parseEnvConfigSelection(value)).toBeUndefined(); + } + + expect(parseProjectSelectorOption({ projectOpt: " my-app " })).toEqual({ + name: "my-app", + canonical: true, + }); + // Legacy alias spellings still select their canonical slug. + expect(parseProjectSelectorOption({ projectOpt: "My_App" })).toEqual({ + name: "my-app", + canonical: false, + }); + for (const value of [`x=${CANARY}`, `${CANARY}.x`, "a:b", "", "---"]) { + let message = ""; + try { + parseProjectSelectorOption({ projectOpt: value }); + } catch (error: unknown) { + expect(error).toBeInstanceOf(CliUsageError); + message = error instanceof Error ? error.message : String(error); + } + expect(message).toBe(INVALID_PROJECT_OPTION_MESSAGE); + } +}); diff --git a/tests/env-selection-parity.test.ts b/tests/env-selection-parity.test.ts index 52f174ba4..eeb6a7c7a 100644 --- a/tests/env-selection-parity.test.ts +++ b/tests/env-selection-parity.test.ts @@ -10,7 +10,10 @@ import { import { tmpdir } from "node:os"; import { join } from "node:path"; import { CliUsageError } from "../src/cli/command.ts"; -import { resolveRequestedEnvName } from "../src/cli/env-selection.ts"; +import { + INVALID_ENV_SELECTION_MESSAGE, + resolveRequestedEnvName, +} from "../src/cli/env-selection.ts"; import { optEnv } from "../src/cli/options.ts"; import { runCli } from "../src/cli/run.ts"; import { __testOnlySessionCommand } from "../src/commands/session.ts"; @@ -216,7 +219,7 @@ test("an unusable --env name is refused by every surface without reading values" for (const result of [plan, list]) { expect(JSON.parse(result.stdout)).toEqual({ ok: false, - error: { code: "E_USAGE", message: "Invalid --env value." }, + error: { code: "E_USAGE", message: INVALID_ENV_SELECTION_MESSAGE }, }); } expect(get.stdout).toBe(""); @@ -262,16 +265,21 @@ test("authored native dispatch refuses an unusable --env before selecting a proj overlay: INVALID_NAME, }).catch((value: unknown) => value); expect(error).toBeInstanceOf(CliUsageError); - expect(error).toHaveProperty("message", "Invalid --env value."); + expect(error).toHaveProperty("message", INVALID_ENV_SELECTION_MESSAGE); }); test("the shared resolver maps base spellings to null and keeps workspace names env-base", () => { const { buildScopedWorkspaceBaseName } = __testOnlySessionCommand; expect(resolveRequestedEnvName({ envOption: undefined })).toBeUndefined(); - expect(resolveRequestedEnvName({ envOption: " QA " })).toBe("qa"); - expect(() => resolveRequestedEnvName({ envOption: INVALID_NAME })).toThrow( - "Invalid --env value." - ); + expect(resolveRequestedEnvName({ envOption: " qa " })).toBe("qa"); + expect(resolveRequestedEnvName({ envOption: "qa-2" })).toBe("qa-2"); + // Names are never slugged: a non-canonical spelling is refused, not + // rewritten into a different overlay name. + for (const spelling of [INVALID_NAME, "QA", "q_a", "q a", "qa.", "X=qa"]) { + expect(() => resolveRequestedEnvName({ envOption: spelling })).toThrow( + INVALID_ENV_SELECTION_MESSAGE + ); + } for (const spelling of BASE_SPELLINGS) { const envName = resolveRequestedEnvName({ envOption: spelling }); expect(envName).toBeNull(); diff --git a/tests/env-value-stdin-cli.test.ts b/tests/env-value-stdin-cli.test.ts new file mode 100644 index 000000000..b84f1e6a8 --- /dev/null +++ b/tests/env-value-stdin-cli.test.ts @@ -0,0 +1,406 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +/** + * End-to-end `--value-stdin` checks in fresh CLI subprocesses. + * + * Isolation: every run gets a temp HOME and global config, a temp project, + * and either the project-key YAML store or an injected `encrypted_file` + * backend with an env-provided key. No keychain, 1Password, Docker, or agent + * client config is reached. A preload records argv/env of every child process + * the CLI spawns so the sentinel can be proven absent from them. + */ + +const REPO_ROOT = resolve(import.meta.dir, ".."); +const ENTRYPOINT = resolve(REPO_ROOT, "index.ts"); +const RECORDER = resolve(import.meta.dir, "fixtures/spawn-argv-recorder.ts"); +const SENTINEL = "stdin-sentinel-c1-5b8e2d"; +const SECRETS_FILE_KEY = "env-value-stdin-test-key"; + +type RunResult = { + readonly exitCode: number; + readonly stdout: string; + readonly stderr: string; + readonly spawnLog: string; +}; + +let root: string | null = null; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "hack-env-value-stdin-")); + await mkdir(join(root, "home"), { recursive: true }); +}); + +afterEach(async () => { + if (root) { + await rm(root, { recursive: true, force: true }); + root = null; + } +}); + +function requireRoot(): string { + if (!root) { + throw new Error("Missing temp root"); + } + return root; +} + +function isolatedEnv(spawnLog: string): Record { + const base = requireRoot(); + return { + PATH: process.env.PATH ?? "/usr/bin:/bin", + HOME: join(base, "home"), + TMPDIR: join(base, "home"), + HACK_GLOBAL_CONFIG_PATH: join(base, "home", "hack-config.json"), + HACK_LOGGER: "console", + HACK_SECRETS_FILE_KEY: SECRETS_FILE_KEY, + HACK_TEST_SPAWN_ARGV_LOG: spawnLog, + }; +} + +async function runHack(input: { + readonly args: readonly string[]; + readonly stdin?: string | Uint8Array | "ignore"; + readonly cwd?: string; +}): Promise { + const base = requireRoot(); + const spawnLog = join(base, `spawn-${crypto.randomUUID()}.jsonl`); + const stdinInput = input.stdin ?? "ignore"; + const proc = Bun.spawn( + [process.execPath, "--preload", RECORDER, ENTRYPOINT, ...input.args], + { + cwd: input.cwd ?? REPO_ROOT, + env: isolatedEnv(spawnLog), + stdin: stdinInput === "ignore" ? "ignore" : "pipe", + stdout: "pipe", + stderr: "pipe", + } + ); + if (stdinInput !== "ignore" && proc.stdin) { + proc.stdin.write(stdinInput); + await proc.stdin.end(); + } + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + const spawnLogText = await readFile(spawnLog, "utf8").catch(() => ""); + return { exitCode, stdout, stderr, spawnLog: spawnLogText }; +} + +function expectNoSentinel(result: RunResult): void { + expect(result.stdout).not.toContain(SENTINEL); + expect(result.stderr).not.toContain(SENTINEL); + expect(result.spawnLog).not.toContain(SENTINEL); +} + +async function createModernProject(): Promise { + const projectRoot = join(requireRoot(), "modern"); + const projectDir = join(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + join(projectDir, "docker-compose.yml"), + "services:\n api:\n image: alpine:3.20\n" + ); + await writeFile( + join(projectDir, "hack.config.json"), + `${JSON.stringify({ name: "value-stdin-modern", dev_host: "value-stdin-modern.hack" }, null, 2)}\n` + ); + await writeFile( + join(projectDir, "hack.env.default.yaml"), + 'version: 1\nenvironment: default\nsecretsprovider: project_key\nvalues:\n global:\n GLOBAL_FLAG: "1"\n' + ); + return projectRoot; +} + +async function readProjectEnvFiles(projectRoot: string): Promise { + const files = ["hack.env.default.yaml", "hack.env.local.yaml", ".env"]; + const contents = await Promise.all( + files.map((file) => + readFile(join(projectRoot, ".hack", file), "utf8").catch(() => "") + ) + ); + return contents.join("\n"); +} + +async function envGet(input: { + readonly projectRoot: string; + readonly key: string; + readonly service?: string; +}): Promise { + return await runHack({ + args: [ + "env", + "get", + input.key, + "--path", + input.projectRoot, + ...(input.service ? ["--service", input.service] : []), + ], + }); +} + +test( + "env add --secret --value-stdin stores the piped value without exposing it", + async () => { + const projectRoot = await createModernProject(); + const result = await runHack({ + args: [ + "env", + "add", + "API_TOKEN", + "--service", + "api", + "--secret", + "--value-stdin", + "--path", + projectRoot, + ], + stdin: `${SENTINEL}\n`, + }); + expect(result.exitCode).toBe(0); + expectNoSentinel(result); + // The recorder is live: the write path's git probe was captured. + expect(result.spawnLog).toContain('"git"'); + + // Stored encrypted, never as plaintext in project files. + expect(await readProjectEnvFiles(projectRoot)).not.toContain(SENTINEL); + + // Positive control for the output scan: an explicit disclosure does + // contain the value, with exactly one trailing newline stripped. + const read = await envGet({ + projectRoot, + key: "API_TOKEN", + service: "api", + }); + expect(read.exitCode).toBe(0); + expect(read.stdout).toBe(SENTINEL); + }, + { timeout: 30_000 } +); + +test( + "env set --value-stdin keeps every newline beyond the single stripped one", + async () => { + const projectRoot = await createModernProject(); + const result = await runHack({ + args: ["env", "set", "MULTILINE", "--value-stdin", "--path", projectRoot], + stdin: `${SENTINEL}\nsecond\n\n`, + }); + expect(result.exitCode).toBe(0); + expectNoSentinel(result); + const read = await envGet({ projectRoot, key: "MULTILINE" }); + expect(read.exitCode).toBe(0); + expect(read.stdout).toBe(`${SENTINEL}\nsecond\n`); + }, + { timeout: 30_000 } +); + +test( + "the value never appears in the CLI process listing while it waits on stdin", + async () => { + const projectRoot = await createModernProject(); + const base = requireRoot(); + const spawnLog = join(base, "spawn-ps.jsonl"); + const proc = Bun.spawn( + [ + process.execPath, + "--preload", + RECORDER, + ENTRYPOINT, + "env", + "add", + "PS_TOKEN", + "--secret", + "--value-stdin", + "--path", + projectRoot, + ], + { + cwd: REPO_ROOT, + env: isolatedEnv(spawnLog), + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + } + ); + const ps = Bun.spawnSync(["ps", "-ww", "-o", "args=", "-p", `${proc.pid}`]); + const listing = ps.stdout.toString(); + proc.stdin.write(`${SENTINEL}\n`); + await proc.stdin.end(); + const [exitCode, stderr] = await Promise.all([ + proc.exited, + new Response(proc.stderr).text(), + ]); + expect(listing).toContain("--value-stdin"); + expect(listing).not.toContain(SENTINEL); + expect(exitCode).toBe(0); + expect(stderr).not.toContain(SENTINEL); + }, + { timeout: 30_000 } +); + +test( + "positional value plus --value-stdin is refused without echoing the value", + async () => { + const projectRoot = await createModernProject(); + const before = await readProjectEnvFiles(projectRoot); + const cases: readonly (readonly string[])[] = [ + ["env", "add", "API_TOKEN", SENTINEL, "--secret", "--value-stdin"], + ["env", "set", "API_TOKEN", SENTINEL, "--value-stdin"], + ["env", "set", `API_TOKEN=${SENTINEL}`, "--value-stdin"], + ["env", "add", `API_TOKEN=${SENTINEL}`, "--value-stdin"], + ]; + for (const args of cases) { + const result = await runHack({ + args: [...args, "--path", projectRoot], + stdin: "piped-should-not-be-read\n", + }); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain( + "--value-stdin cannot be combined with a value argument" + ); + expectNoSentinel(result); + } + expect(await readProjectEnvFiles(projectRoot)).toBe(before); + }, + { timeout: 60_000 } +); + +test( + "oversize and empty stdin are refused and nothing is stored", + async () => { + const projectRoot = await createModernProject(); + const before = await readProjectEnvFiles(projectRoot); + + const oversize = await runHack({ + args: [ + "env", + "add", + "BIG_TOKEN", + "--secret", + "--value-stdin", + "--path", + projectRoot, + ], + stdin: SENTINEL.repeat(Math.ceil((64 * 1024 + 1) / SENTINEL.length)), + }); + expect(oversize.exitCode).toBe(1); + expect(oversize.stderr).toContain("exceeds the 65536-byte limit"); + expectNoSentinel(oversize); + + for (const stdin of ["", "\n"]) { + const empty = await runHack({ + args: [ + "env", + "add", + "EMPTY_TOKEN", + "--value-stdin", + "--path", + projectRoot, + ], + stdin, + }); + expect(empty.exitCode).toBe(1); + expect(empty.stderr).toContain("--value-stdin received an empty value"); + } + + const missingKey = await runHack({ + args: ["env", "add", "--value-stdin", "--path", projectRoot], + stdin: `${SENTINEL}\n`, + }); + expect(missingKey.exitCode).toBe(1); + expect(missingKey.stderr).toContain("--value-stdin requires the key"); + expectNoSentinel(missingKey); + + expect(await readProjectEnvFiles(projectRoot)).toBe(before); + }, + { timeout: 60_000 } +); + +test( + "non-TTY refusal suggests --value-stdin instead of a value argument", + async () => { + const projectRoot = await createModernProject(); + const result = await runHack({ + args: ["env", "add", "API_TOKEN", "--secret", "--path", projectRoot], + }); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("E_INTERACTIVE_REQUIRED"); + expect(result.stderr).toContain("--value-stdin"); + expect(result.stderr).not.toContain("Pass the value as an argument"); + expect(result.stderr).not.toContain("hack env add KEY value --secret"); + }, + { timeout: 30_000 } +); + +test( + "legacy env set --secret --value-stdin writes through an injected encrypted_file store", + async () => { + const base = requireRoot(); + const projectRoot = join(base, "legacy"); + const projectDir = join(projectRoot, ".hack"); + const storePath = join(base, "secrets.enc.json"); + await mkdir(projectDir, { recursive: true }); + await writeFile(join(projectDir, "docker-compose.yml"), "services: {}\n"); + await writeFile( + join(projectDir, "hack.config.json"), + `${JSON.stringify( + { + name: "value-stdin-legacy", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: storePath }, + }, + }, + }, + null, + 2 + )}\n` + ); + await writeFile( + join(projectDir, "hack.env.json"), + `${JSON.stringify( + { + version: 1, + vars: [{ key: "API_KEY", required: false, source: "keychain" }], + }, + null, + 2 + )}\n` + ); + + const result = await runHack({ + args: ["env", "set", "API_KEY", "--secret", "--value-stdin"], + stdin: `${SENTINEL}\r\n`, + cwd: projectRoot, + }); + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain("encrypted_file"); + expectNoSentinel(result); + expect(await readFile(storePath, "utf8")).not.toContain(SENTINEL); + expect( + await readFile(join(projectDir, ".env"), "utf8").catch(() => "") + ).not.toContain(SENTINEL); + + const list = await runHack({ + args: ["env", "list", "--show-secrets", "--json"], + cwd: projectRoot, + }); + expect(list.exitCode).toBe(0); + const payload = JSON.parse(list.stdout) as { + readonly vars: ReadonlyArray<{ + readonly key: string; + readonly value: string | null; + }>; + }; + expect(payload.vars.find((entry) => entry.key === "API_KEY")?.value).toBe( + SENTINEL + ); + }, + { timeout: 60_000 } +); diff --git a/tests/env-value-stdin.test.ts b/tests/env-value-stdin.test.ts new file mode 100644 index 000000000..34a7dfd7e --- /dev/null +++ b/tests/env-value-stdin.test.ts @@ -0,0 +1,261 @@ +import { describe, expect, test } from "bun:test"; + +import { CliUsageError } from "../src/cli/command.ts"; +import { + assertEnvValueStdinUsage, + ENV_VALUE_STDIN_MAX_BYTES, + ENV_VALUE_STDIN_REFUSAL_REASONS, + EnvValueStdinError, + type EnvValueStdinSource, + readEnvValueFromStdin, + stripOneTrailingNewline, +} from "../src/lib/env-value-stdin.ts"; + +const SENTINEL = "stdin-sentinel-7f3a9c"; + +function sourceFrom(input: { + readonly chunks: readonly (string | Uint8Array)[]; + readonly isTTY?: boolean; +}): EnvValueStdinSource & { opened: number } { + const encoder = new TextEncoder(); + const state = { + opened: 0, + isTTY: input.isTTY ?? false, + open: () => { + state.opened += 1; + return new ReadableStream({ + start(controller) { + for (const chunk of input.chunks) { + controller.enqueue( + typeof chunk === "string" ? encoder.encode(chunk) : chunk + ); + } + controller.close(); + }, + }); + }, + }; + return state; +} + +/** Never-ending producer: proves the reader stops at the bound. */ +function endlessSource(): EnvValueStdinSource & { + pulled: number; + canceled: boolean; +} { + const chunk = new TextEncoder().encode(SENTINEL.repeat(64)); + const state = { + pulled: 0, + canceled: false, + isTTY: false, + open: () => + new ReadableStream({ + pull(controller) { + state.pulled += 1; + controller.enqueue(chunk.slice()); + }, + cancel() { + state.canceled = true; + }, + }), + }; + return state; +} + +async function refusal(promise: Promise): Promise { + try { + await promise; + } catch (error: unknown) { + if (error instanceof EnvValueStdinError) { + return error; + } + throw error; + } + throw new Error("expected a --value-stdin refusal"); +} + +/** + * Leak detector shared by every refusal assertion: the caller's value (or any + * fragment of it) must never appear in a diagnostic. + */ +function expectValueFree(error: unknown, value: string): void { + const rendered = + error instanceof Error + ? `${error.name}\n${error.message}\n${error.stack ?? ""}\n${JSON.stringify(error)}` + : String(error); + for (const fragment of [value, value.slice(0, 8), value.slice(-8)]) { + if (rendered.includes(fragment)) { + throw new Error("diagnostic leaked caller input"); + } + } +} + +describe("readEnvValueFromStdin", () => { + test("returns piped input unchanged when it has no trailing newline", async () => { + const value = await readEnvValueFromStdin({ + source: sourceFrom({ chunks: [SENTINEL] }), + }); + expect(value).toBe(SENTINEL); + }); + + test("strips exactly one trailing newline", async () => { + expect( + await readEnvValueFromStdin({ + source: sourceFrom({ chunks: [`${SENTINEL}\n`] }), + }) + ).toBe(SENTINEL); + expect( + await readEnvValueFromStdin({ + source: sourceFrom({ chunks: [`${SENTINEL}\n\n`] }), + }) + ).toBe(`${SENTINEL}\n`); + expect( + await readEnvValueFromStdin({ + source: sourceFrom({ chunks: [`${SENTINEL}\r\n`] }), + }) + ).toBe(SENTINEL); + expect( + await readEnvValueFromStdin({ + source: sourceFrom({ chunks: ["line1\nline2\n"] }), + }) + ).toBe("line1\nline2"); + expect( + await readEnvValueFromStdin({ + source: sourceFrom({ chunks: [` ${SENTINEL} \n`] }), + }) + ).toBe(` ${SENTINEL} `); + }); + + test("reassembles multi-byte characters split across chunks", async () => { + const bytes = new TextEncoder().encode("päss☃\n"); + const value = await readEnvValueFromStdin({ + source: sourceFrom({ chunks: [bytes.slice(0, 2), bytes.slice(2)] }), + }); + expect(value).toBe("päss☃"); + }); + + test("accepts input exactly at the byte bound", async () => { + const value = await readEnvValueFromStdin({ + source: sourceFrom({ chunks: ["a".repeat(ENV_VALUE_STDIN_MAX_BYTES)] }), + }); + expect(value.length).toBe(ENV_VALUE_STDIN_MAX_BYTES); + }); + + test("refuses oversize input with a closed error and stops reading", async () => { + const endless = endlessSource(); + const error = await refusal( + readEnvValueFromStdin({ source: endless, maxBytes: 4096 }) + ); + expect(error.reason).toBe("oversize"); + expect(error).toBeInstanceOf(CliUsageError); + expect(endless.canceled).toBe(true); + expect(endless.pulled).toBeLessThan(16); + expectValueFree(error, SENTINEL); + + const atDefault = await refusal( + readEnvValueFromStdin({ + source: sourceFrom({ + chunks: ["a".repeat(ENV_VALUE_STDIN_MAX_BYTES), "b"], + }), + }) + ); + expect(atDefault.reason).toBe("oversize"); + }); + + test("refuses empty and newline-only input", async () => { + for (const chunks of [[], [""], ["\n"], ["\r\n"]]) { + const error = await refusal( + readEnvValueFromStdin({ source: sourceFrom({ chunks }) }) + ); + expect(error.reason).toBe("empty"); + } + }); + + test("refuses an interactive TTY stdin before reading any byte", async () => { + const source = sourceFrom({ chunks: [SENTINEL], isTTY: true }); + const error = await refusal(readEnvValueFromStdin({ source })); + expect(error.reason).toBe("interactive_tty"); + expect(error.message).toContain("hidden prompt"); + expect(source.opened).toBe(0); + }); + + test("refuses invalid UTF-8 and NUL bytes without echoing input", async () => { + const invalid = new Uint8Array([ + ...new TextEncoder().encode(SENTINEL), + 0xff, + 0xfe, + ]); + const utf8 = await refusal( + readEnvValueFromStdin({ source: sourceFrom({ chunks: [invalid] }) }) + ); + expect(utf8.reason).toBe("invalid_utf8"); + expectValueFree(utf8, SENTINEL); + + const nul = await refusal( + readEnvValueFromStdin({ + source: sourceFrom({ chunks: [`${SENTINEL}\0tail`] }), + }) + ); + expect(nul.reason).toBe("nul_byte"); + expectValueFree(nul, SENTINEL); + }); + + test("maps read failures to a closed error that drops the cause", async () => { + const source: EnvValueStdinSource = { + isTTY: false, + open: () => + new ReadableStream({ + pull(controller) { + controller.error(new Error(`backend said ${SENTINEL}`)); + }, + }), + }; + const error = await refusal(readEnvValueFromStdin({ source })); + expect(error.reason).toBe("read_failed"); + expect(error.cause).toBeUndefined(); + expectValueFree(error, SENTINEL); + }); +}); + +describe("assertEnvValueStdinUsage", () => { + test("refuses a positional value and a missing key", () => { + expect(() => + assertEnvValueStdinUsage({ hasKey: true, hasPositionalValue: true }) + ).toThrow(EnvValueStdinError); + expect(() => + assertEnvValueStdinUsage({ hasKey: false, hasPositionalValue: false }) + ).toThrow(EnvValueStdinError); + expect(() => + assertEnvValueStdinUsage({ hasKey: true, hasPositionalValue: false }) + ).not.toThrow(); + }); +}); + +describe("refusal diagnostics", () => { + test("every refusal message is fixed and value-free", () => { + for (const reason of ENV_VALUE_STDIN_REFUSAL_REASONS) { + const error = new EnvValueStdinError(reason); + expect(error.message).toStartWith("--value-stdin"); + expect(error.message).toContain("Nothing was stored."); + expectValueFree(error, SENTINEL); + } + }); + + test("negative control: the leak detector catches a value in an error message", () => { + const leaky = new CliUsageError(`Invalid value: ${SENTINEL}`); + expect(() => expectValueFree(leaky, SENTINEL)).toThrow( + "diagnostic leaked caller input" + ); + const partial = new Error(`value starts with ${SENTINEL.slice(0, 8)}`); + expect(() => expectValueFree(partial, SENTINEL)).toThrow( + "diagnostic leaked caller input" + ); + }); +}); + +test("stripOneTrailingNewline removes at most one newline", () => { + expect(stripOneTrailingNewline("a\n\n")).toBe("a\n"); + expect(stripOneTrailingNewline("a\r\n")).toBe("a"); + expect(stripOneTrailingNewline("a\r")).toBe("a\r"); + expect(stripOneTrailingNewline("a")).toBe("a"); +}); diff --git a/tests/fixtures/spawn-argv-recorder.ts b/tests/fixtures/spawn-argv-recorder.ts new file mode 100644 index 000000000..8f2c166d3 --- /dev/null +++ b/tests/fixtures/spawn-argv-recorder.ts @@ -0,0 +1,85 @@ +/** + * Test preload: records argv and env of every child process the CLI spawns. + * + * Loaded with `bun --preload` in a fresh subprocess only. Each spawn appends + * one JSON line `{ via, argv, env }` to the file named by + * `HACK_TEST_SPAWN_ARGV_LOG`, then delegates to the original implementation, + * so behavior is unchanged. Tests scan the log for a secret sentinel. + */ +import { appendFileSync } from "node:fs"; +import { createRequire } from "node:module"; + +const logPath = process.env.HACK_TEST_SPAWN_ARGV_LOG; + +function record(input: { + readonly via: string; + readonly argv: unknown; + readonly env: unknown; +}): void { + if (!logPath) { + return; + } + appendFileSync( + logPath, + `${JSON.stringify({ via: input.via, argv: input.argv, env: input.env ?? null })}\n` + ); +} + +function bunSpawnArgv(args: readonly unknown[]): { + readonly argv: unknown; + readonly env: unknown; +} { + const [first, second] = args; + if (Array.isArray(first)) { + const options = (second ?? {}) as { readonly env?: unknown }; + return { argv: first, env: options.env }; + } + const options = (first ?? {}) as { + readonly cmd?: unknown; + readonly env?: unknown; + }; + return { argv: options.cmd, env: options.env }; +} + +const originalSpawn = Bun.spawn; +const originalSpawnSync = Bun.spawnSync; + +Bun.spawn = ((...args: unknown[]) => { + record({ via: "Bun.spawn", ...bunSpawnArgv(args) }); + return Reflect.apply(originalSpawn, Bun, args); +}) as typeof Bun.spawn; + +Bun.spawnSync = ((...args: unknown[]) => { + record({ via: "Bun.spawnSync", ...bunSpawnArgv(args) }); + return Reflect.apply(originalSpawnSync, Bun, args); +}) as typeof Bun.spawnSync; + +const require = createRequire(import.meta.url); +const childProcess = require("node:child_process") as Record; +for (const name of [ + "spawn", + "spawnSync", + "exec", + "execSync", + "execFile", + "execFileSync", + "fork", +]) { + const original = childProcess[name]; + if (typeof original !== "function") { + continue; + } + childProcess[name] = (...args: unknown[]) => { + const [command, maybeArgs, maybeOptions] = args; + const argList = Array.isArray(maybeArgs) ? maybeArgs : []; + const options = (Array.isArray(maybeArgs) ? maybeOptions : maybeArgs) as + | { readonly env?: unknown } + | undefined; + record({ + via: `child_process.${name}`, + argv: [command, ...argList], + env: options?.env, + }); + return Reflect.apply(original, childProcess, args); + }; +} diff --git a/tests/native-authored-command-dispatch.test.ts b/tests/native-authored-command-dispatch.test.ts index cb7806dc8..9637a6bdd 100644 --- a/tests/native-authored-command-dispatch.test.ts +++ b/tests/native-authored-command-dispatch.test.ts @@ -308,7 +308,7 @@ macTest.each( macTest.each([ { option: "base", overlay: "base", metadata: null }, { option: undefined, overlay: "inherit", metadata: "qa" }, - { option: " QA ", overlay: { named: "qa" }, metadata: "qa" }, + { option: " qa ", overlay: { named: "qa" }, metadata: "qa" }, ])( "source CLI env $option binds the documented selection into the native source before values", async ({ option, overlay, metadata }) => { diff --git a/tests/project-env-config.test.ts b/tests/project-env-config.test.ts index b0a555c93..2d4c10fa9 100644 --- a/tests/project-env-config.test.ts +++ b/tests/project-env-config.test.ts @@ -25,6 +25,7 @@ import { upsertDotEnvValue } from "../src/lib/hack-env.ts"; import { readProjectDefaultEnvConfig } from "../src/lib/project.ts"; import { assertValidProjectEnvScopeName, + ENV_SCOPE_UNKNOWN_MESSAGE, ensureHackDirGitignore, ensureProjectEnvSecretKey, inspectLegacyComposeEnvFileReferences, @@ -38,8 +39,10 @@ import { resolveProjectEnvConfig, resolveProjectEnvLocalConfigPath, resolveProjectEnvSharedKeyPath, + selectProjectEnvValues, selectProjectEnvValuesForExecutionTarget, setProjectEnvValue, + unknownProjectEnvScopeMessage, unsetProjectEnvValue, } from "../src/lib/project-env-config.ts"; import { resolveSecretStore } from "../src/lib/secret-store.ts"; @@ -378,15 +381,50 @@ test("materializeProjectEnv rejects unknown service scopes", async () => { secret: false, }); - await expect( - materializeProjectEnv({ - projectRoot: repo.projectRoot, - projectDir: repo.projectDir, - envName: null, - serviceName: "typo_service", - serviceNames: ["api", "web"], - }) - ).rejects.toThrow("Unknown env scope: typo_service"); + const error = await materializeProjectEnv({ + projectRoot: repo.projectRoot, + projectDir: repo.projectDir, + envName: null, + serviceName: "typo_service", + serviceNames: ["api", "web"], + }).then( + () => null, + (caught: unknown) => caught + ); + expect(error).toBeInstanceOf(Error); + const message = (error as Error).message; + // Configured scope names come from compose/env files and may be listed; + // the requested argument never is. + expect(message).toBe( + `${ENV_SCOPE_UNKNOWN_MESSAGE} Configured scopes: global, api, web.` + ); + expect(message).not.toContain("typo_service"); +}); + +test("unknown-scope refusals list configured scopes but never the request", () => { + const canary = "api_token=unknown-scope-canary-7c2e"; + const resolved = { + globalEnv: {}, + serviceEnv: { web: {}, api: {} }, + hostTargetEnv: {}, + } as unknown as Parameters[0]["resolved"]; + for (const scopeName of [canary, "constructor", "__proto__", "toString"]) { + let message = ""; + try { + selectProjectEnvValues({ resolved, scopeName }); + } catch (error: unknown) { + message = error instanceof Error ? error.message : String(error); + } + expect(message).toBe( + `${ENV_SCOPE_UNKNOWN_MESSAGE} Configured scopes: global, api, web.` + ); + expect(message).not.toContain(scopeName); + } + + const many = unknownProjectEnvScopeMessage({ + configuredScopes: Array.from({ length: 30 }, (_, i) => `svc${i + 100}`), + }); + expect(many).toContain("(and 11 more)"); }); test("project env scopes accept compose-compatible underscores and dots", () => { @@ -405,6 +443,51 @@ test("project env scopes accept compose-compatible underscores and dots", () => }); }); +test("env key and scope refusals never echo the rejected argument", async () => { + const canary = "echo-canary-lib-5e1c"; + const refusal = (fn: () => unknown): string => { + try { + fn(); + } catch (error: unknown) { + return error instanceof Error ? error.message : String(error); + } + throw new Error("expected a refusal"); + }; + const messages = [ + refusal(() => parseProjectEnvTarget({ keyOrPath: `API_TOKEN=${canary}` })), + refusal(() => + parseProjectEnvTarget({ keyOrPath: `API_TOKEN=${canary}.tail` }) + ), + refusal(() => parseProjectEnvTarget({ keyOrPath: canary })), + refusal(() => parseProjectEnvTarget({ keyOrPath: `api.${canary}` })), + refusal(() => + parseProjectEnvTarget({ keyOrPath: `Bad${canary}.API_TOKEN` }) + ), + refusal(() => + assertValidProjectEnvScopeName({ scopeName: `Bad${canary}` }) + ), + ]; + for (const message of messages) { + expect(message).toMatch(/^Invalid env (key|scope): /); + expect(message).not.toContain(canary); + expect(message).not.toContain("API_TOKEN="); + } + + const repo = await createRepo(); + const write = setProjectEnvValue({ + projectRoot: repo.projectRoot, + projectDir: repo.projectDir, + envName: null, + scope: "global", + key: `API_TOKEN=${canary}`, + value: canary, + secret: true, + }); + await expect(write).rejects.toThrow("Invalid env key: keys must match"); + const error = await write.catch((caught: unknown) => caught); + expect(error instanceof Error ? error.message : "").not.toContain(canary); +}); + test("resolveProjectEnvConfig falls back to HACK_ENV_SECRET_KEY when the key file is missing", async () => { const repo = await createRepo();