fix: CAST_AGE_KEY_FILE_<ENV> maps to a name a shell can set #105

Merged
dan-claude-bot merged 1 commit from fix/age-key-env-name into main 2026-07-19 13:13:09 +00:00
dan-claude-bot commented 2026-07-19 12:17:38 +00:00 (Migrated from github.com)

Fixes #102.

What

keyFileFor derived the injected-key variable as CAST_AGE_KEY_FILE_${envName.toUpperCase()} (src/secrets.ts:82). For env drill-b that advertises CAST_AGE_KEY_FILE_DRILL-B — a name no POSIX shell can export — so the per-invocation channel, including the <(pm read …) process-substitution pattern the README teaches, was a wall for every hyphenated environment name. Found live in the 2026-07-19 release drill (two real Coolify instances; env drill-b had to fall back to a standing key on disk).

How

New ageKeyVarFor(envName): uppercase, then map every character outside [A-Z0-9] to _ — env drill-b reads CAST_AGE_KEY_FILE_DRILL_B. The refusal message advertises the mapped name. The standing-key path (~/.config/cast/age-<env>.key) keeps the exact environment name, so two env names that collide on the variable (drill-b/drill.b) still resolve their own keys on disk — the collision costs nothing.

README documents the mapping; CHANGELOG carries the story.

Tests

  • hyphenated env resolves through the mapped var
  • the refusal names the mapped var and the exact-name standing path (with $HOME isolated so a dev machine's real standing key can't fake a pass — that bit me on the first run)

npm test: 598 passed (33 files) · npm run check clean.

🤖 Generated with Claude Code

Fixes #102. ## What `keyFileFor` derived the injected-key variable as `CAST_AGE_KEY_FILE_${envName.toUpperCase()}` (`src/secrets.ts:82`). For env `drill-b` that advertises `CAST_AGE_KEY_FILE_DRILL-B` — a name no POSIX shell can export — so the per-invocation channel, including the `<(pm read …)` process-substitution pattern the README teaches, was a wall for every hyphenated environment name. Found live in the 2026-07-19 release drill (two real Coolify instances; env `drill-b` had to fall back to a standing key on disk). ## How New `ageKeyVarFor(envName)`: uppercase, then map every character outside `[A-Z0-9]` to `_` — env `drill-b` reads `CAST_AGE_KEY_FILE_DRILL_B`. The refusal message advertises the mapped name. The standing-key path (`~/.config/cast/age-<env>.key`) keeps the exact environment name, so two env names that collide on the variable (`drill-b`/`drill.b`) still resolve their own keys on disk — the collision costs nothing. README documents the mapping; CHANGELOG carries the story. ## Tests - hyphenated env resolves through the mapped var - the refusal names the mapped var and the exact-name standing path (with `$HOME` isolated so a dev machine's real standing key can't fake a pass — that bit me on the first run) `npm test`: 598 passed (33 files) · `npm run check` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
danmt (Migrated from github.com) reviewed 2026-07-19 12:17:38 +00:00
claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-19 12:39:04 +00:00
claude-bot-andresmgsl (Migrated from github.com) left a comment

Reviewed — I agree with all of this, no concerns.

ageKeyVarFor is the right factoring — one function owns the var name, so keyFileFor and the refusal message can't drift, and .replace(/[^A-Z0-9]/g, "_") after uppercasing is the correct POSIX-safe mapping. Deliberately keeping the exact env name on the standing-key path (so drill-b/drill.b collide on the var but still resolve their own keys on disk) is the right call, and it's documented. Tests cover both the hit through the mapped var and the refusal advertising the mapped name — with $HOME isolated so a dev machine's standing key can't fake a pass. README updated. Clean.

automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Reviewed — I agree with all of this, no concerns.** `ageKeyVarFor` is the right factoring — one function owns the var name, so `keyFileFor` and the refusal message can't drift, and `.replace(/[^A-Z0-9]/g, "_")` after uppercasing is the correct POSIX-safe mapping. Deliberately keeping the exact env name on the standing-key path (so `drill-b`/`drill.b` collide on the var but still resolve their own keys on disk) is the right call, and it's documented. Tests cover both the hit through the mapped var and the refusal advertising the mapped name — with `$HOME` isolated so a dev machine's standing key can't fake a pass. README updated. Clean. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_
claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:43:21 +00:00
claude-bot-andresmgsl (Migrated from github.com) left a comment

Approve — the fix is correct, minimal, centralized in one function, and the edge cases I probed all hold.

Verified beyond the diff:

  • src/secrets.ts:100 — uppercase-then-replace(/[^A-Z0-9]/g, "_") is the right order (the class is uppercase-only, so replacing first would have eaten lowercase letters), and it is the identity mapping for every previously settable name, so nothing existing breaks. Leading digits can't produce an invalid name thanks to the CAST_AGE_KEY_FILE_ prefix.
  • Repo-wide grep confirms the var name is now built in exactly one place (ageKeyVarFor), so the lookup at src/secrets.ts:82 and the refusal at src/secrets.ts:87 can't drift. README (README.md:604-609) documents the mapping right next to the <(pm read …) example it unblocks.
  • The collision (drill-b/drill.b → same var) is real but benign — a mismatched injected key fails age decryption loudly rather than decrypting the wrong env — and it's documented in the code comment and CHANGELOG. Keeping the exact env name on the standing-key path is the right call.
  • test/secrets.test.ts:122-135 — isolating $HOME so a dev machine's real standing key can't turn the refusal test into a false pass is a nice catch.

Ran locally at 9e10a89: npm run build + npm test → 598/598 passed; npm run check clean.

One optional nit: the new tests exercise only the hyphen case; a dot or mixed-case env name rides the same regex so I don't consider it blocking.

automated review by claude-bot-andresmgsl · heavy-duty-review-bot

**Approve — the fix is correct, minimal, centralized in one function, and the edge cases I probed all hold.** Verified beyond the diff: - `src/secrets.ts:100` — uppercase-then-`replace(/[^A-Z0-9]/g, "_")` is the right order (the class is uppercase-only, so replacing first would have eaten lowercase letters), and it is the identity mapping for every previously settable name, so nothing existing breaks. Leading digits can't produce an invalid name thanks to the `CAST_AGE_KEY_FILE_` prefix. - Repo-wide grep confirms the var name is now built in exactly one place (`ageKeyVarFor`), so the lookup at `src/secrets.ts:82` and the refusal at `src/secrets.ts:87` can't drift. README (`README.md:604-609`) documents the mapping right next to the `<(pm read …)` example it unblocks. - The collision (`drill-b`/`drill.b` → same var) is real but benign — a mismatched injected key fails age decryption loudly rather than decrypting the wrong env — and it's documented in the code comment and CHANGELOG. Keeping the exact env name on the standing-key path is the right call. - `test/secrets.test.ts:122-135` — isolating `$HOME` so a dev machine's real standing key can't turn the refusal test into a false pass is a nice catch. Ran locally at 9e10a89: `npm run build` + `npm test` → 598/598 passed; `npm run check` clean. One optional nit: the new tests exercise only the hyphen case; a dot or mixed-case env name rides the same regex so I don't consider it blocking. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_
grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:49:33 +00:00
grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Why

  • ageKeyVarFor is the right single owner of the variable name: keyFileFor and the refusal message cannot drift, which is exactly how the old bug stayed invisible (refusal advertised a name no shell can set).
  • Mapping is correct for the real failure mode: uppercase then collapse [^A-Z0-9]_ turns drill-b into CAST_AGE_KEY_FILE_DRILL_B, which POSIX export can set (including process-substitution injection).
  • Standing-key path keeps the exact env name (age-drill-b.key), so a var-name collision (drill-b / drill.b) does not merge identities on disk — the collision is confined to the injected channel, and the PR states that honestly.
  • Tests pin both the happy path and the refusal body with $HOME isolation, so a developer standing key cannot fake a pass.

Checked

  • CI green (build/install/reconcile/scope).
  • No other call sites still building the old toUpperCase()-only name.
  • Docs/CHANGELOG match the code.

No blockers.

**Verdict: Approve** — I agree with this as-is. ### Why - `ageKeyVarFor` is the right single owner of the variable name: `keyFileFor` and the refusal message cannot drift, which is exactly how the old bug stayed invisible (refusal advertised a name no shell can set). - Mapping is correct for the real failure mode: uppercase then collapse `[^A-Z0-9]` → `_` turns `drill-b` into `CAST_AGE_KEY_FILE_DRILL_B`, which POSIX `export` can set (including process-substitution injection). - Standing-key path keeps the exact env name (`age-drill-b.key`), so a var-name collision (`drill-b` / `drill.b`) does not merge identities on disk — the collision is confined to the injected channel, and the PR states that honestly. - Tests pin both the happy path and the refusal body with `$HOME` isolation, so a developer standing key cannot fake a pass. ### Checked - CI green (build/install/reconcile/scope). - No other call sites still building the old `toUpperCase()`-only name. - Docs/CHANGELOG match the code. No blockers.
codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:49:50 +00:00
codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

The environment-variable mapping is centralized, preserves existing alphanumeric names, produces a shell-settable name for hyphenated environments, keeps exact environment names for standing-key paths, and is covered by lookup and refusal tests. Current checks are green.

Verdict: I agree with everything and have no additional feedback. The environment-variable mapping is centralized, preserves existing alphanumeric names, produces a shell-settable name for hyphenated environments, keeps exact environment names for standing-key paths, and is covered by lookup and refusal tests. Current checks are green.
dan-claude-bot commented 2026-07-19 12:56:10 +00:00 (Migrated from github.com)

@danmt — handing this one to you. Final state:

What it fixes: cast#102 — the injected-key variable was derived as CAST_AGE_KEY_FILE_${envName.toUpperCase()}, so env drill-b advertised CAST_AGE_KEY_FILE_DRILL-B: a name no POSIX shell can export, which walled off the per-invocation key channel (including the <(pm read …) process-substitution pattern the README teaches) for every hyphenated environment name. Found live in the release drill's two-instance run.

The fix: one function (ageKeyVarFor) now owns the mapping — uppercase, then [^A-Z0-9] → _ — so the lookup and the refusal message can't drift; env drill-b reads CAST_AGE_KEY_FILE_DRILL_B. The standing-key path keeps the exact env name, so names that collide on the variable (drill-b/drill.b) still resolve their own keys on disk. README documents the mapping.

Review history: one round — all three bots approved, no changes requested.

Verification: npm test 598/0 · biome clean. New tests cover the mapped-var hit and the refusal advertising the mapped name, with $HOME isolated so a dev machine's real standing key can't fake a pass.

No open questions from the round. Ready for your review.

🤖 Generated with Claude Code

@danmt — handing this one to you. Final state: **What it fixes:** cast#102 — the injected-key variable was derived as `CAST_AGE_KEY_FILE_${envName.toUpperCase()}`, so env `drill-b` advertised `CAST_AGE_KEY_FILE_DRILL-B`: a name no POSIX shell can export, which walled off the per-invocation key channel (including the `<(pm read …)` process-substitution pattern the README teaches) for every hyphenated environment name. Found live in the release drill's two-instance run. **The fix:** one function (`ageKeyVarFor`) now owns the mapping — uppercase, then `[^A-Z0-9] → _` — so the lookup and the refusal message can't drift; env `drill-b` reads `CAST_AGE_KEY_FILE_DRILL_B`. The standing-key path keeps the exact env name, so names that collide on the variable (`drill-b`/`drill.b`) still resolve their own keys on disk. README documents the mapping. **Review history:** one round — all three bots approved, no changes requested. **Verification:** `npm test` 598/0 · biome clean. New tests cover the mapped-var hit and the refusal advertising the mapped name, with `$HOME` isolated so a dev machine's real standing key can't fake a pass. No open questions from the round. Ready for your review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/cast#105
No description provided.