Skip to content

fix(cli): give a guard refusal its own code instead of LAUNCH_FAILED - #24

Merged
yashau merged 1 commit into
mainfrom
claude/nervous-kare-43c74e
Aug 19, 2026
Merged

fix(cli): give a guard refusal its own code instead of LAUNCH_FAILED#24
yashau merged 1 commit into
mainfrom
claude/nervous-kare-43c74e

Conversation

@yashau

@yashau yashau commented Aug 19, 2026

Copy link
Copy Markdown
Owner

What and why

Found in a live smoke test with prk 2026.819.1.

docs/reference/cli/errors.md documents the guard refusal as UNSAFE_ENVIRONMENT at exit 11. What prk run actually emitted when an environment holds a secret named NODE_OPTIONS:

$ prk run --json -- node -e 'console.log("x")'
{"error":{"code":"LAUNCH_FAILED","hint":"Rename the secret, or pass --allow-unsafe-env ...","message":"refusing to set `NODE_OPTIONS` in the child environment: ..."}}
exit: 1

A three-way mismatch: the code is wrong, and exit 1 matches neither the UNSAFE_ENVIRONMENT row nor the LAUNCH_FAILED row (126, 127).

This matters because the guard is a security control and the docs tell scripts to branch on its code. A pipeline doing case $? in 11) never matched, and exit 1 is the generic bucket, so a refused unsafe environment was indistinguishable from any other unspecified failure. Under --json, a consumer keying on UNSAFE_ENVIRONMENT never saw it.

Root cause

The guard is raised in LaunchSpec::with_secrets, so it arrives as prick_exec::LaunchError::Guard, and thiserror's derived From filed every variant of that type under CliError::Launch. CliError::Guard mapped to UNSAFE_ENVIRONMENT and exit 11 exactly as documented — nothing on a real path ever constructed it. The hint text is identical in both variants, which is what made this easy to miss by eye: only the code and the exit were wrong.

The fix

Hand-write From<LaunchError> for CliError so Guard comes back out, and drop #[from] from the Launch variant so the derive cannot reinstate the bug.

The binary is the right place for this. prick-exec documents its codes as the shell's 126 and 127 rather than the API taxonomy, and this crate's module doc already states that the binary is where library errors are collected and given an exit code, a machine code and a hint.

prick_exec::LaunchError::Guard keeps its own exit code of 1. The CLI no longer reads it — CliError::Launch cannot hold a Guard now — and giving a taxonomy value to a type that explicitly disclaims the taxonomy would be the same confusion pointing the other way. Happy to change that instead if you read the comment at crates/prick-exec/src/error.rs:94-96 as intending 11.

Docs

The LAUNCH_FAILED row was wrong beyond the guard case: InvalidKey, CommandLine and Io all exit 1, so 126, 127 was never the whole set. It now reads 1, 126, 127 with the breakdown spelled out. run.md and using-secrets/index.md already documented 11 / UNSAFE_ENVIRONMENT and are correct as of this change.

How to verify

No Cloudflare account needed.

cargo test -p prk --lib

The regression test builds the refusal the way run does — a real LaunchSpec, the strict guard, and the same ? conversion — rather than by naming CliError::Guard directly. Naming the variant is precisely what hid this: the pre-existing a_guard_refusal_points_at_the_override asserted the mapping in isolation and passed throughout.

To confirm the test earns its place, delete the Guard arm from the new From impl in crates/prk/src/error.rs and re-run:

left: "LAUNCH_FAILED"  right: "UNSAFE_ENVIRONMENT"

That is the reported symptom, reproduced. a_guard_refusal_points_at_the_override still passes with the bug present — the blind spot, confirmed.

Checklist

  • mise run ci passes locally. Full run, exit 0 — including e2e (101 passed), test (623 Rust / 1152 JS), miri, deny, audit, openapi:check and version:check.
  • Tests cover the change. A bug fix has a test that fails without the fix.
  • No secret value can reach stdout, stderr, a log line, an error message, a server-rendered page payload or an audit detail field as a result of this change.
  • Documentation is updated if behaviour, flags or output changed.

GuardError::LoaderControlled carries only the variable name, never its value, and this change moves that error between variants without touching its Display. Variable names are already treated as safe to show — see the LaunchSpec::env_names doc comment.

If this touches encryption or the database

Not applicable.

If this touches authorization

Not applicable. The env guard is a local policy on what may be injected into a child process, not a grant check.

🤖 Generated with Claude Code

`prk run` refusing a loader-controlled secret reported
`{"code":"LAUNCH_FAILED"}` and exit 1, where the docs promise
`UNSAFE_ENVIRONMENT` and exit 11. A pipeline doing `case $? in 11)` never
matched, and exit 1 is the generic bucket, so a refused unsafe environment
was indistinguishable from any other unspecified failure -- for a security
control whose whole point is that a caller can see it fire.

The guard is raised in `with_secrets`, so it arrives as
`prick_exec::LaunchError::Guard`, and thiserror's derived `From` filed every
variant of that type under `CliError::Launch`. `CliError::Guard` mapped to
`UNSAFE_ENVIRONMENT` and 11 exactly as documented; nothing on a real path
ever constructed it.

Hand-write the conversion so `Guard` comes back out. The binary is where
this belongs: `prick-exec` documents its codes as the shell's 126 and 127
rather than the API taxonomy, and this crate's module doc already says the
binary is where library errors are collected and classified.

`prick_exec::LaunchError::Guard` keeps its own exit code of 1. The CLI no
longer reads it -- `CliError::Launch` cannot hold a `Guard` now -- and giving
a taxonomy value to a type that disclaims the taxonomy would be the same
confusion in the other direction.

Also fix the `LAUNCH_FAILED` row, which was wrong beyond the guard:
`InvalidKey`, `CommandLine` and `Io` all exit 1, so `126, 127` was never the
whole set.

The regression test builds the refusal the way `run` does -- real
`LaunchSpec`, strict guard, same `?` -- rather than naming `CliError::Guard`.
Naming the variant is what hid this: the existing test asserted the mapping
in isolation and passed throughout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@yashau
yashau merged commit 2ed6b02 into main Aug 19, 2026
19 checks passed
@yashau
yashau deleted the claude/nervous-kare-43c74e branch August 19, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant