Skip to content

fix(cli): a project .env cannot set the feedback email in another letter case - #5018

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/cli-dotenv-feedback-email-case
Oct 4, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/cli-dotenv-feedback-email-case

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

What

A project .env can no longer set the feedback email by spelling its name in another letter case. hyperframes_feedback_email=… (or any mixed case) in a project .env is now skipped, the same as HYPERFRAMES_FEEDBACK_EMAIL=… already was. hyperframes capture, which loads the .env above its output folder, now applies the same rules instead of copying every key, the feedback email included.

Why

Only the app that launches the CLI may attach an email to feedback; an agent can write a project .env, so applyDotEnv skips that key. The skip compared the name exactly. On Windows, environment variable names are case-blind, so process.env.hyperframes_feedback_email = x sets the same variable feedbackEmail() reads, and feedback that should be anonymous would carry an email from the project file.

Capture had its own .env parser with no skip at all. Feedback is sent only by the feedback command, in its own process, so that copy could not leak the email today, but it was a second set of rules for the same file.

How

  • applyDotEnv (packages/cli/src/utils/dotEnv.ts) compares the key upper-cased against FEEDBACK_EMAIL_ENV. feedbackEmail() is unchanged.
  • loadEnvFile (packages/cli/src/capture/scaffolding.ts) keeps its walk up to five parent folders and hands the first .env it finds to applyDotEnv, which is now the one owner of how a project .env is applied.
  • Side effects for capture, matching the CLI's own .env handling: export KEY=… lines and inline # comments are parsed, a quoted value ends at its closing quote (so a double-quoted JSON value with unescaped inner quotes is cut short, as it already was for the CLI's own .env), and a variable already set to an empty string is no longer overwritten by the file.

Test plan

  • dotEnv.test.ts: "never takes the feedback email under another letter case" (lower and mixed case) fails on main (expected { …(3) } to deeply equal { OTHER: '1' }).
  • scaffolding.test.ts: "loadEnvFile never takes the feedback email from a project file, in any letter case" writes a .env one folder above the start folder; it fails on main (expected [ 'a@example.com', …(2) ] to deeply equal [ undefined, undefined, 'kept' ]) and checks a quoted ordinary key is still loaded.
  • dotEnv, feedbackSource, scaffolding and captureAttempt tests: 21 pass, 3 runs in a row (Linux). Pre-commit typecheck, oxlint, oxfmt and the comment ratchet pass.
  • Not run on a native Windows machine: the tests check the rule directly, and the Windows behaviour it guards is Node's documented case-blind process.env.

Size

Under the 100-line floor on purpose: a standalone privacy fix with nothing open to carry it.

No visible change

CLI only; nothing under Studio or the player.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 4, 2026 14:55
@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 1556 (base branch 1556), smooth 1364 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 62e0c17b. A project .env can no longer set the feedback email under any spelling of the name, and hyperframes capture's .env loader now goes through the same applyDotEnv. I found nothing that blocks; the capture-side behaviour changes below are ones the PR body already lists.

It touches 4 files (dotEnv.ts, scaffolding.ts and their tests). Not draft, not stacked: 2 commits on main, same 4 files in main...62e0c17b. main is 4 commits ahead but none touch these files.

What I verified (head tarball, deps built, NODE_ENV=test)

  • dotEnv, feedbackSource, scaffolding, captureAttempt tests: 21/21, 3 runs, matching the PR body. tsc --noEmit on packages/cli, oxlint and oxfmt --check clean.
  • Both new tests fail on base. Head dotEnv.ts with the base scaffolding.ts fails the capture test; the base dotEnv.ts fails both.
  • 9 mutants: 8 caught (no toUpperCase, wrong-case compare, skip dropped, overwrite of existing keys, capture applying to a scratch env, a one-level walk, export prefix dropped, inline-comment stripping dropped). 1 survives, below.
  • Probe of what applyDotEnv does with a feedback-email line: exact, lower, mixed case, export-prefixed, padded with spaces, after a BOM and with CRLF are all skipped, and OTHER=1 beside them still loads. Only the key spelling is matched, so on Linux and macOS a lower-case hyperframes_feedback_email is now dropped too; that is a distinct variable nothing reads, so it is harmless.
  • Only commands/feedback.ts:281 reads feedbackEmail(), so the PR's statement that capture's old copy could not leak the email today holds.

Non-blocking

  1. Two capture-side changes can switch vision captioning off silently. Both are in the PR body, and both match what cli.ts already does for the cwd .env, but no test pins either. I ran the old inline loader against applyDotEnv:
    • HYPERFRAMES_VERTEX_SERVICE_ACCOUNT="{"type":"service_account",…}" (double-quoted, inner quotes unescaped) used to load whole; it now loads as {, and contentExtractor.ts:378 then reports "not valid JSON; skipped vision captioning". Single-quoted and unquoted JSON load the same as before. dotenv-style parsers read the unescaped form whole, so this is the case most likely to surprise someone.
    • A shell variable already set to an empty string (GEMINI_API_KEY=) is no longer filled from the .env above the output folder. The old loader replaced empty values.
      A line in the changelog or a pinning test for each would make it deliberate.
  2. "One owner" holds for packages/cli. The media-use skill has its own loadEnvFromDir (skills/media-use/audio/scripts/lib/heygen.mjs) that still copies every key, the feedback email in any case included. It runs in the skill scripts' own processes, which never call feedbackEmail(), so it is the same non-leak as capture's old copy. Worth knowing if that script ever shells out to hyperframes feedback.
  3. Survivor: capture's "first .env found wins" break is not pinned (a walk that keeps loading parent .env files passes). That behaviour is unchanged by this PR.
  4. Not exercised: native Windows. The tests check the rule directly, and the Windows case-blind process.env behaviour it guards is Node's documented one.

CI. All checks at this head finished: 76 passed, 5 skipped, 0 failing, 0 pending, including the edit-accuracy gate (1556, same as base). CI is a reference; the verdict rests on the evidence above.

Reviewed on the PR head 62e0c17b; no other review exists on it. This is a review verdict, not authorization to merge or deploy beyond what the gate already does.

— Review by tai (pr-review)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit dcc7210 Oct 4, 2026
152 checks passed
@miguel-heygen
miguel-heygen deleted the fix/cli-dotenv-feedback-email-case branch October 4, 2026 16:06
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.

2 participants