fix(env): strip teamai env.sh exports in env exec when the project config can't be read (#879)

With an unreadable project config, env exec passed the inherited
environment unchanged, so a value another scope's env.sh exported (a legacy
token, a member override) reached the command while the warning said no team
values were applied. It now removes what the member-environment rule
discounts (env.sh file, record or marker, the env.sh beside the unreadable
config included), keeps the member's own exports, and names the removed keys,
never values, as the failed-declaration path does. With no config at all the
inherited environment is still passed as is.

docs/designs/team-secrets.md

@@ -187,7 +187,7 @@ teamai env exec -- glab mr list     GITLAB_HOST from env.yaml and GITLAB_TOKEN f
 - **Scope.** The directory's scope: the project teamai is set up for there, found through git, so every worktree of a project resolves to that project, else the user scope.
 - **Environment.** The command inherits teamai's environment, overlaid with the scope's variables in the [variable order](#variables) (a scope variable wins over an inherited one), then with its secrets in the [resolution order](#resolution). A key declared as a secret that has no value for this scope is removed from the command's environment, so the command never sees a value `teamai env list` doesn't show for this scope: another team's export, or the member's own export when this team's value names another variable with `--from-env`.
 - **Missing secret.** The [line](#a-missing-secret-tells-the-member-what-to-run) goes to stderr, and the command runs anyway: `gh` and `glab` can still use their own login.
-- **Failures.** When the declarations fail, no variable and no secret is applied, and the command runs with the inherited environment: any `env.yaml` key may be a secret the file declares, so its repo value is not passed on, and every inherited value that is not the member's own (one a teamai `env.sh` exports or exported, by its file, its record or its marker) is removed, named on stderr by key only; when `env.yaml` fails, the secrets are applied and no variable is; when the value file can't be read, every declared key is removed and no variable is applied. Each says so on stderr. A project config that exists but can't be read is named on stderr, and the command runs with the inherited environment: it is not taken for "no scope", nor for the user scope.
+- **Failures.** When the declarations fail, no variable and no secret is applied, and the command runs with the inherited environment: any `env.yaml` key may be a secret the file declares, so its repo value is not passed on, and every inherited value that is not the member's own (one a teamai `env.sh` exports or exported, by its file, its record or its marker) is removed, named on stderr by key only; when `env.yaml` fails, the secrets are applied and no variable is; when the value file can't be read, every declared key is removed and no variable is applied. Each says so on stderr. A project config that exists but can't be read is named on stderr, and the command runs with the inherited environment, without every value that is not the member's own, as when the declarations fail (the `env.sh` beside that config included), named by key only: it is not taken for "no scope", nor for the user scope.
 - **No scope.** With no project or user config, the command runs with the inherited environment and a notice on stderr. Machine values are not applied there, since no team declares which keys the command needs. An HTTP team repo delivers no env here either.
 - **Output.** Everything teamai prints goes to stderr, so the command's stdout can be piped. The exit code is the command's; a command ended by a signal ends teamai with the same signal, or exits 128 + its number for one Node doesn't end on (SIGPIPE, SIGUSR1). A SIGTERM or SIGHUP sent to teamai is passed on. `Ctrl-C` and `Ctrl-\` are not: the terminal already sent them to the command, and a second SIGINT makes tools such as terraform force-quit, so while teamai runs in its terminal's foreground process group it ignores SIGINT and SIGQUIT and waits for the command. It checks once, before starting the command (`ps -o pgid=,tpgid=`). Anywhere else (a background job, a process without a terminal) a SIGINT or SIGQUIT (`kill -INT <pid>`) was sent to teamai alone and is passed on, as it is when `ps` can't say. On Windows the console sends `Ctrl-C` to every process attached to it, so teamai ignores it. Node doesn't say who sent a signal, so while teamai runs in its terminal's foreground a SIGINT or SIGQUIT sent to teamai's PID alone (`kill -INT <pid>` from another shell) is not passed on either and the command keeps running: send teamai SIGTERM, which is passed on, or signal the command's PID. A command that can't be started exits 127.
 - **Nothing written.** No value is written to disk or to `debug.log`. Finding the scope does what every command that finds one does: it may adopt a project partition, save the user scope's role migration, or set up a freshly cloned single-repo project; none of these writes a value.

docs/designs/team-secrets.zh-CN.md

@@ -187,7 +187,7 @@ teamai env exec -- glab mr list     GITLAB_HOST 来自 env.yaml，GITLAB_TOKEN
 - **Scope。** 当前目录的 scope：teamai 在此处配置的项目（通过 git 查找，因此项目的每个 worktree 都解析到该项目），否则是用户 scope。
 - **环境。** 命令继承 teamai 的环境，先按[变量顺序](#变量)叠加该 scope 的变量（scope 变量覆盖继承的同名变量），再按[解析顺序](#解析顺序)叠加它的密钥。声明为密钥、但在该 scope 下没有值的 key 会从命令的环境中移除，因此命令永远拿不到 `teamai env list` 不会显示为该 scope 的值：另一个团队导出的值，或者该团队的值用 `--from-env` 指向另一个变量时成员自己导出的值。
 - **缺少密钥。** 那一[行提示](#缺少密钥时告诉成员该运行什么)输出到 stderr，命令照常运行：`gh` 和 `glab` 仍可以使用它们自己的登录。
-- **失败。** 声明失败时，不应用任何变量和密钥，命令以继承的环境运行：`env.yaml` 中的任何 key 都可能是该文件声明的密钥，因此不传递它在仓库中的值，并移除继承环境中每个不属于成员自己的值（teamai `env.sh` 导出或曾经导出的值，按文件、记录或标记判断），在 stderr 上只列出 key 名；`env.yaml` 失败时，只应用密钥，不应用任何变量；值文件无法读取时，移除所有已声明的 key，也不应用任何变量。每种情况都会在 stderr 上说明。项目配置存在但无法读取时，会在 stderr 上指出该文件，并以继承的环境运行命令：既不当作"没有 scope"，也不回退到用户 scope。
+- **失败。** 声明失败时，不应用任何变量和密钥，命令以继承的环境运行：`env.yaml` 中的任何 key 都可能是该文件声明的密钥，因此不传递它在仓库中的值，并移除继承环境中每个不属于成员自己的值（teamai `env.sh` 导出或曾经导出的值，按文件、记录或标记判断），在 stderr 上只列出 key 名；`env.yaml` 失败时，只应用密钥，不应用任何变量；值文件无法读取时，移除所有已声明的 key，也不应用任何变量。每种情况都会在 stderr 上说明。项目配置存在但无法读取时，会在 stderr 上指出该文件，并以继承的环境运行命令，与声明失败时一样移除每个不属于成员自己的值（包括该配置旁 `env.sh` 导出的值），只列出 key 名：既不当作"没有 scope"，也不回退到用户 scope。
 - **没有 scope。** 既没有项目配置也没有用户配置时，命令以继承的环境运行，并在 stderr 上给出提示。这里不应用本机值，因为没有团队声明命令需要哪些 key。HTTP 团队仓库在这里同样不提供 env。
 - **输出。** teamai 打印的所有内容都输出到 stderr，因此命令的 stdout 可以直接接管道。退出码就是命令的退出码；命令被信号终止时，teamai 以同一信号结束；对于 Node 不会因之退出的信号（SIGPIPE、SIGUSR1），则以 128 + 信号编号退出。发给 teamai 的 SIGTERM 或 SIGHUP 会转发给命令。`Ctrl-C` 和 `Ctrl-\` 不转发：终端已经把它们发给了命令，第二个 SIGINT 会让 terraform 等工具强制退出，因此当 teamai 运行在其终端的前台进程组中时，它忽略 SIGINT 和 SIGQUIT 并等待命令结束。它在启动命令之前检查一次（`ps -o pgid=,tpgid=`）。在其他情况下（后台作业、没有终端的进程），SIGINT 或 SIGQUIT（`kill -INT <pid>`）是只发给 teamai 的，会转发给命令；`ps` 无法判断时也一样转发。在 Windows 上，控制台会把 `Ctrl-C` 发给所有附着在它上面的进程，因此 teamai 忽略它。Node 无法得知信号的发送者，因此当 teamai 运行在其终端的前台时，只发给 teamai PID 的 SIGINT 或 SIGQUIT（例如从另一个 shell 执行 `kill -INT <pid>`）同样不会转发，命令会继续运行：请向 teamai 发送 SIGTERM（会转发），或直接向命令的 PID 发信号。无法启动的命令以 127 退出。
 - **不写入值。** 不会把任何值写入磁盘或 `debug.log`。查找 scope 的行为与其他查找 scope 的命令相同：可能接管项目分区、保存用户 scope 的角色迁移，或为刚克隆的单仓项目完成配置；这些写入都不包含值。

src/__tests__/env-exec.test.ts

@@ -315,6 +315,33 @@ describe('teamai env exec', () => {
     expect(text(stderr)).not.toContain('No teamai config');
   });
 
+  it("removes what another scope's env.sh exported when a project config cannot be read, keeps the member's own exports, and names the keys", async () => {
+    const personal = await team('personal', { 'env/secrets.yaml': GITHUB_SECRET });
+    await userScope(personal.repoPath);
+    const work = await team('work', {});
+    const { root, partition } = await project(work.repoPath);
+    await fse.outputFile(path.join(partition, 'config.yaml'), 'repo: [not a config\n');
+    await fse.outputFile(path.join(home, '.teamai', 'env.sh'), "export GITHUB_TOKEN='fixture-user-scope'\n");
+    await fse.outputFile(path.join(partition, 'env.sh'), "export API_URL='fixture-work-scope'\n");
+    const [marker, digests] = envShMarker(path.join(tmpDir, 'unscanned', '.teamai'), [['GITLAB_TOKEN', 'fixture-marked']]) ?? [];
+    if (!marker || !digests) throw new Error('no marker for the fixture export');
+    vi.stubEnv(marker, digests);
+    vi.stubEnv('GITHUB_TOKEN', 'fixture-user-scope');
+    vi.stubEnv('API_URL', 'fixture-work-scope');
+    vi.stubEnv('GITLAB_TOKEN', 'fixture-marked');
+    vi.stubEnv('SENTRY_TOKEN', 'fixture-hand-export');
+
+    const env = await childEnv(root);
+
+    expect(env.GITHUB_TOKEN).toBeUndefined();
+    expect(env.API_URL).toBeUndefined();
+    expect(env.GITLAB_TOKEN).toBeUndefined();
+    expect(env.SENTRY_TOKEN).toBe('fixture-hand-export');
+    expect(text(stderr)).toContain(path.join(partition, 'config.yaml'));
+    expect(text(stderr)).toMatch(/without (?=.*GITHUB_TOKEN)(?=.*API_URL)(?=.*GITLAB_TOKEN)[A-Z_, ]+, whose values a teamai env\.sh exported/);
+    expect(text(stderr)).not.toMatch(/fixture-(user|work|marked|hand)/);
+  });
+
   it('with no config at all, runs with the inherited environment, applies no machine value, and says so', async () => {
     await writeSecretStore(getMachineSecretsPath(), { GITHUB_TOKEN: { value: 'fixture-machine' } });
     vi.stubEnv('UNRELATED', 'kept');

src/env-exec.ts

@@ -12,11 +12,13 @@
  */
 import { execFile } from 'node:child_process';
 import os from 'node:os';
+import path from 'node:path';
 import { promisify } from 'node:util';
 import crossSpawn from 'cross-spawn';
 import { resolveConfigForDir } from './config.js';
 import { reportMissingSecrets } from './env-advisories.js';
 import { resolveTeamEnv } from './env-resolution.js';
+import { memberEnvironmentWithoutScope, type MemberEnvironment } from './member-env.js';
 import { describeEntryFailure } from './namespaced-entries.js';
 import { envTable } from './resources/env-key.js';
 import { declaredSecretKeys } from './resources/secrets.js';
@@ -120,16 +122,16 @@ export function exitLike(outcome: ExecOutcome): void {
 
 /** The environment the command runs with, reporting on stderr whatever it leaves out. */
 async function commandEnvironment(cwd: string, dryRun: boolean | undefined): Promise<NodeJS.ProcessEnv> {
-  const unreadable: string[] = [];
-  const localConfig = await resolveConfigForDir(
-    cwd,
-    (configPath, error) => { unreadable.push(`${configPath} could not be read: ${error}.`); },
-    { dryRun },
-  );
+  const unreadable: { configPath: string; error: string }[] = [];
+  const localConfig = await resolveConfigForDir(cwd, (configPath, error) => { unreadable.push({ configPath, error }); }, { dryRun });
   if (unreadable.length > 0) {
-    log.warn(`${unreadable.join(' ')} No team env variables or secrets were applied; the command runs with the inherited `
-      + 'environment. Fix the file, or run `teamai init` again in this project.');
-    return inheritedEnvironment();
+    // The project's config is unknown, so as while its declarations fail, a value a teamai env.sh exported is removed.
+    const env = inheritedEnvironment();
+    const removed = withoutTeamExports(env, await memberEnvironmentWithoutScope(unreadable.map(({ configPath }) => path.dirname(configPath))));
+    log.warn(`${unreadable.map(({ configPath, error }) => `${configPath} could not be read: ${error}.`).join(' ')} No team env `
+      + `variables or secrets were applied; the command runs with the inherited environment${exportedClause(removed)}. Fix the `
+      + 'file, or run `teamai init` again in this project.');
+    return env;
   }
   if (!localConfig) {
     log.warn('No teamai config applies to this directory, so the command runs with the inherited environment and no team '
@@ -167,9 +169,7 @@ async function overlayTeamEnv(localConfig: LocalConfig): Promise<NodeJS.ProcessE
     // Any env.yaml key may be a secret the file declares, so no team value is applied (#879 Conflict 14),
     // and one a teamai env.sh exported is a team value, not the member's: it is removed.
     const failures = [variables, declarations].flatMap((entries) => entries.kind === 'failed' ? [describeEntryFailure(entries.failure)] : []);
-    const removed = Object.keys(env).filter((key) => env[key] !== '' && teamEnv.member(key) === undefined);
-    for (const key of removed) delete env[key];
-    const without = removed.length > 0 ? `, and without ${removed.join(', ')}, whose values a teamai env.sh exported` : '';
+    const without = exportedClause(withoutTeamExports(env, teamEnv.member));
     log.warn(`${failures.join(' ')} The command runs with the inherited environment, without team env variables or secrets${without}.`);
     return env;
   }
@@ -196,6 +196,18 @@ async function overlayTeamEnv(localConfig: LocalConfig): Promise<NodeJS.ProcessE
   return env;
 }
 
+/** Remove from `env` every value that is not the member's own (member-env.ts), and return the keys removed. */
+function withoutTeamExports(env: NodeJS.ProcessEnv, member: MemberEnvironment): string[] {
+  const removed = Object.keys(env).filter((key) => env[key] !== '' && member(key) === undefined);
+  for (const key of removed) delete env[key];
+  return removed;
+}
+
+/** The warning's clause naming the keys `withoutTeamExports` removed, never their values. */
+function exportedClause(removed: readonly string[]): string {
+  return removed.length > 0 ? `, and without ${removed.join(', ')}, whose values a teamai env.sh exported` : '';
+}
+
 /**
  * Run the command with the terminal's stdio. A signal sent to teamai alone is
  * passed on, one from the terminal is not, and either way teamai waits for

src/member-env.ts

@@ -27,8 +27,8 @@ import { readFileSafe } from './utils/fs.js';
 /** A key's value in the member's own environment, or undefined. */
 export type MemberEnvironment = (key: string) => string | undefined;
 
-/** Every teamai env.sh on this machine that a shell may have loaded. */
-async function teamaiEnvShPaths(localConfig: LocalConfig): Promise<string[]> {
+/** Every teamai env.sh on this machine that a shell may have loaded, with the one in each of `dataHomes`. */
+async function teamaiEnvShPaths(dataHomes: readonly string[]): Promise<string[]> {
   const home = getTeamaiHomeDir();
   const projects = path.join(home, 'projects');
   let partitions: string[] = [];
@@ -37,7 +37,7 @@ async function teamaiEnvShPaths(localConfig: LocalConfig): Promise<string[]> {
   } catch {
     // No project partitions on this machine.
   }
-  return [...new Set([path.join(home, 'env.sh'), path.join(getDataHome(localConfig), 'env.sh'), ...partitions])];
+  return [...new Set([path.join(home, 'env.sh'), ...dataHomes.map((dataHome) => path.join(dataHome, 'env.sh')), ...partitions])];
 }
 
 /**
@@ -51,10 +51,28 @@ export async function memberEnvironment(
   localConfig: LocalConfig,
   scope: { secretKeys: ReadonlySet<string>; envYaml: ReadonlyMap<string, string> },
   env: NodeJS.ProcessEnv = process.env,
+): Promise<MemberEnvironment> {
+  return memberEnvironmentAt([getDataHome(localConfig)], scope, env);
+}
+
+/**
+ * The member's own environment where the config that governs the directory
+ * cannot be read: no scope declares anything, and `dataHomes` are the
+ * directories of the configs that could not be read, whose env.sh a shell may
+ * have loaded.
+ */
+export function memberEnvironmentWithoutScope(dataHomes: readonly string[], env: NodeJS.ProcessEnv = process.env): Promise<MemberEnvironment> {
+  return memberEnvironmentAt(dataHomes, { secretKeys: new Set(), envYaml: new Map() }, env);
+}
+
+async function memberEnvironmentAt(
+  dataHomes: readonly string[],
+  scope: { secretKeys: ReadonlySet<string>; envYaml: ReadonlyMap<string, string> },
+  env: NodeJS.ProcessEnv,
 ): Promise<MemberEnvironment> {
   const exported: ReadonlyMap<string, string>[] = [];
   const recorded: EnvShExports[] = [];
-  for (const envSh of await teamaiEnvShPaths(localConfig)) {
+  for (const envSh of await teamaiEnvShPaths(dataHomes)) {
     const content = await readFileSafe(envSh);
     if (content !== null) exported.push(parseEnvFile(content));
     recorded.push(await readEnvShExports(envSh));