fix(env): hold env keys to the identifier rule env.sh already assumes

[ydflow]

Summary

Fixes #738.

generateEnvFile interpolated the key raw into export <key>=<quoted value>. The value went through shellQuoteValue; the key went through nothing, so a key from the team repo's env/env.yaml could produce either a line that is not valid shell, or one that executes code — in every member's shell, since env is in pushableTypes and any member who can push can put such a key in front of everyone else.

// before
const lines = variables.map(v => `export ${v.key}=${shellQuoteValue(v.value)}`);

parseEnvFile already refused to read back any key outside [A-Za-z_][A-Za-z0-9_]*, which is what made this worse than a one-off: the variable was written into env.sh and then invisible to the CLI, so nothing reported it as undelivered. That regex is now a module-level ENV_KEY_RE shared by both sides, so write and read agree by construction instead of by two copies that can drift.

Change

• generateEnvFile drops a key that is not an identifier rather than emitting a broken or dangerous line. It drops rather than fails on purpose: one member's bad key must not take env.sh down for everyone, and the remaining variables still ship.
• env add rejects such a key up front and names it, because the local command is where the mistake is still visible. Accepting it there would have reported success for a variable that never reaches anyone's shell.

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Test Plan

• npx tsc --noEmit — no errors
• npx vitest run src/__tests__/env-commands.test.ts src/__tests__/env-handler.test.ts — 68 passed, 1 skipped (pre-existing skip)
• npm run build — success, dist/index.js 1.76 MB

RED → GREEN

Two new tests were written before the fix. Both fail on the base commit:

• generateEnvFile › should drop keys that are not valid shell identifiers — the base emits export bad key='oops' and export FOO;touch /tmp/pwned='y'.
• envAdd › refuses a key that would not survive the round trip into env.sh — the base writes it into env.yaml and logs success.

A companion test (should keep keys that are valid shell identifiers, covering _PRIVATE and A1_b2) passes both before and after, so the guard does not narrow what a legitimate team repo can express.

Real-CLI end-to-end verification

Built with npm run build, then drove the produced dist/index.js:

$ node dist/index.js env add "GOOD_KEY" "ok"
(proceeds past validation — reaches the not-initialized check, i.e. accepted)

$ node dist/index.js env add "bad key" "oops"
✖ Invalid env variable name "bad key": use letters, digits and underscores, starting with a letter or underscore.

$ node dist/index.js env add 'FOO;touch /tmp/pwned-e2e' "y"
✖ Invalid env variable name "FOO;touch /tmp/pwned-e2e": use letters, digits and underscores, starting with a letter or underscore.

/tmp/pwned-e2e was checked after the run and does not exist — the injected command never executed.

Generator side, driven directly:

generateEnvFile([{GOOD_KEY, ok}, {bad key, oops}, {'FOO;touch /tmp/pwned', y}])
  => "export GOOD_KEY='ok'\n"        # both bad keys dropped, no payload in output
parseEnvFile(same) => [['GOOD_KEY','ok']]   # write and read agree

Not verified

• src/__tests__/doctor-env-delivery.test.ts has 5 failures on this branch, but they are pre-existing and environmental: they fail identically on an untouched checkout of upstream/main (cd3e0e6) on this Windows machine, and this PR touches neither the doctor path nor anything it calls. I confirmed this by running the same file in a worktree whose only change is an unrelated workflow edit. I have not attempted to fix them.
• The full vitest run suite was not driven to completion (283 files; it exceeds the local time budget), so I scoped verification to the env-related files plus tsc and build.
• Branch is based on cd3e0e6, not the current main tip (8d74aa4); git fetch was failing on this machine. I checked the three intervening commits and none touch src/resources/env.ts, src/env-commands.ts, or the env tests. Happy to rebase if you would rather have it on the tip.

[github-actions]

Findings

• [P1 blocking] src/resources/env.ts:459 filters invalid keys only while generating env.sh. The same unfiltered variables are still written to the line-based backup at src/resources/env.ts:295, counted as successfully synced, and checked by doctor. A key containing \nVALID_KEY can forge a backup entry consumed by MCP interpolation, while ordinary invalid keys produce a false “Synced” result followed by a permanent missing-variable diagnostic. Validate/filter once before all delivery consumers and explicitly warn about rejected keys.
• [P1 blocking] src/env-commands.ts:67 logs the validation failure and returns without setting a non-zero exit status or throwing. Consequently, teamai env add "bad key" value exits successfully despite rejecting the operation, so scripts cannot detect the failure.
• [P1 blocking] The PR changes the accepted syntax for teamai env add and manually authored env/env.yaml, but does not document the identifier requirement in docs/usage-guide.md and docs/usage-guide.zh-CN.md, violating the repository’s synchronized-documentation rule.
• [P1 blocking] The PR description lacks sufficient end-to-end verification of the primary fix. Its real-CLI commands only exercise the early env add validation—one valid case merely reaches the not-initialized error—and generateEnvFile is invoked directly. No initialized real-CLI pull verifies that a malicious repository env.yaml safely updates env.sh, the backup file, reporting, and subsequent doctor behavior.