Hacktoberfest 2026:维护者为十月标记出来的 issue,仍然开放、适合新手。 浏览 Hacktoberfest issue

The default Vault impl can leak secrets into child processes via process.env

未关闭
#65 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
4/5
预计耗时
3-5 天
新手友好度
45/100
Issue 类型
缺陷
描述清晰度
基本清楚
活跃度
活跃
领域
backend, cli, security

调研方向

从 apps/cli/src/config.ts 和 apps/cli/src/index.ts 开始,然后检查 plugins/bash/src/index.ts、plugins/background/src/index.ts、plugins/mcp/src/client.ts 和 docker-bash 中创建子进程的入口点。在选择范围明确的修复方案之前,先梳理 Environment 和 .env 的流转,并注意 vault.get gating 对 #62 的依赖。完成的标准是:所提出的 Environment 泄漏场景都有测试覆盖,优先级顺序得到保留,并且迁移或行为变更都有文档记录。

由索引模型根据 Issue 内容生成。

描述

Problem

Every secret the Vault holds is readable by any tool that can spawn a process or read a file. A turn only has to run env.

The mechanism is four steps, none of which is individually unreasonable:

  1. loadDotEnv promotes .env into process.env. apps/cli/src/config.ts:70-71 writes every parsed key into process.env unless already set. It does this because EnvFileVault reads from an env snapshot, not from the file — both construction sites pass process.env (index.ts:695, index.ts:846) — so promotion is currently load-bearing, not incidental.
  2. bash copies all of process.env into every child shell. plugins/bash/src/index.ts:251 merges the whole environment before spawn, with the caller's env layered on top. So {"script": "env"} returns every API key in the vault.
  3. background and mcp do the same. background:238 spreads process.env into detached children; mcp/client.ts:40 spreads it into every stdio MCP server — so a third-party MCP server is handed the full set on launch.
  4. .env sits in the project directory, which docker-bash mounts. docker-bash gets this right at the process level — it passes env: {} (docker-bash:495) and inherits nothing. But the project root is mounted read-only at /app, and .env is in it (config.ts:64), so cat /app/.env recovers what the env cleanliness denied.

This runs against the existing doctrine that secrets reach tools only through the Vault and only by substitution — the reason http is deliberately kept away from the Vault entirely. The ${NAME} indirection is doing its job at the API surface while the values sit in ambient state underneath it.

This is a property of the default Vault implementation (a .env file), not of the Vault API

Worth being precise about what is broken here: the Vault interface is fine, and the ${NAME} indirection is doing what it was designed to do. The leak is a property of the medium the default node backend chose — a .env file, read through an environment snapshot. Everything above follows from that one decision: the file is in the project directory (so it is mountable and readable), and its values are in process.env (so they are inheritable).

EnvFileVault already acknowledges that its medium constrains it — it overrides unstorableKey to narrow the base vault's name policy to what a .env file can round-trip. The confidentiality constraint is the same kind of thing, just unstated.

A backend built on OS credential storage — macOS Keychain, the Windows Credential Manager, libsecret, or a broker like gpg-agent/ssh-agent/Vault/a cloud secret manager — does not have this shape at all:

  • There is no file in the project directory, so there is nothing for a :ro mount or a cat to recover (fix 4 becomes moot).
  • Nothing is promoted into process.env, so there is nothing for a child process to inherit (fix 1 becomes moot, and fix 2 stops mattering for vault secrets).
  • Several of these media have their own per-process consent prompt, which is the same shape as fix 3 and a useful precedent for how it should feel.

The swap point already exists and is already documented as a swap point: register('Vault', impl) behind the capture-safe forwarding proxy, so a reference held across the swap keeps resolving to the live backend. A more secure vault is a plugin, not a port.

Two things that do not follow from this, and are the reason the fixes below still stand:

  • Fix 2 is independent. An operator who exports secrets in the real environment (CI, docker run -e, a systemd unit) is leaking them to every child regardless of which vault is registered. Dropping blanket inheritance from bash/background/mcp is worth doing on its own terms.
  • Fix 3 becomes more valuable, not less. Once the ambient paths are closed, vault.get is the remaining path, and it is the one the design intends. A gate there is what makes "which tool asked for which secret" an answerable question.

So the honest framing is: fixes 1 and 4 are mitigations for a medium that is the default because it is zero-configuration, and the durable answer for an installation that cares is a different registered backend. That backend does not exist yet, and is probably its own issue.

Not the fix: trapping process.env

The tempting answer is to interpose on process in the tool execution environment and prompt on access. It doesn't work here, for two independent reasons.

It misses the mechanism. Steps 2–4 are child_process and fs, not property access on process.env. A proxy over the accessor stops none of them.

It isn't enforceable in-process. Plugins are loaded with a plain dynamic import() and get the real globals; there is no module isolation and the design does not claim any. A shadowed process is bypassed by globalThis.process, await import('node:process'), createRequire('process'), /proc/self/environ, or reading .env directly. Against a hostile plugin it is theatre; against a careless one it has the value of a lint rule, and should be priced as one.

The general point: once a plugin is loaded it has full Node capability, so an in-process capability trap is not a boundary. The gate belongs at the secret, not at the accessor.

Proposed fixes

In value order. The first three are small and independent.

1. Stop promoting .env into process.env

loadDotEnv returns the parsed map instead of mutating; the vault is constructed from { ...dotEnv, ...definedProcessEnv }, preserving today's precedence (a real environment variable wins, which is what the if (!(key in process.env)) guard means today). The wizard's carry-across at index.ts:701 becomes an in-memory merge rather than an assignment to process.env.

Cheap: the return value of loadDotEnv is discarded at both call sites (index.ts:777, index.ts:787), so changing its shape breaks nothing. This alone de-fangs steps 2 and 3 for every secret that lives in .env rather than the real environment.

2. Stop inheriting the environment in child-spawning tools

docker-bash already demonstrates the target behaviour (env: {}). Make bash match: no inherited environment by default, with inheritance available explicitly. background and mcp likewise — an MCP server should receive the variables its config names, not everything.

This is a behaviour change and needs a migration note: a script relying on an inherited PATH, HOME or TERM will break. Options are an allowlist of non-secret variables, or full inheritance behind an explicit opt-in. The allowlist is probably right for bash, since the common need is shell hygiene rather than secrets.

3. Gate vault.get per tool

The intended path deserves a gate even after the leaks are closed. ctx.vault is already on ToolContext, so this is one wrapper at the same seam as the other gates — gate id vault.read:<NAME>, decided by the PermissionGate from #62. Absent a gate, it asks; an install that doesn't want to be asked registers a gate plugin.

4. Move .env out of the project directory

Closes step 4, which the first three don't touch: docker-bash keeps its clean environment and can no longer read the file either. Needs a decision about where (alongside .data? outside it?) and a migration for existing installs, so it is the most disruptive of the four and the easiest to defer.

Blast radius

Change Files Size
1. loadDotEnv returns a map; vault built from the merge apps/cli/src/config.ts, apps/cli/src/index.ts (3 sites) ~15 lines
2. bash env allowlist / opt-in plugins/bash/src/index.ts ~15 lines + contract doc
2. background, mcp plugins/background/src/index.ts, plugins/mcp/src/client.ts ~10 lines
3. vault.get gate core executor wrap (shared with #62) ~15 lines
4. .env relocation apps/cli/src/config.ts, env-vault.ts, docs, migration larger — defer
Tests a turn that runs env recovers nothing; precedence preserved; MCP server receives only its configured vars new

Nothing in plugin-api changes, and no frontend changes.

Relation to #62

Independent of #62 for fixes 1, 2 and 4 — those are leaks and should be fixed regardless of whether a permission gate exists. Fix 3 consumes #62's PermissionGate and should land after it.

Worth stating the resulting honest boundary in both places: a loaded plugin has full Node capability, so this issue is not about containing a hostile plugin. It is about not leaving every secret in ambient process state where an ordinary tool call — or a prompt-injected LLM composing a function_tool lambda — recovers the lot by accident.

主要语言
TypeScript
星标
5
派生
2
平均合并
1 天 2 小时
30 天内合并 PR
12

环境准备

这个项目没有提供开发容器、Dockerfile 或贡献指南,环境需要你自己搭建:先看它的 README,通用步骤见我们的新手贡献指南。

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

MatAtBread/matbot 的其他 Issue

查看 MatAtBread/matbot 的全部 Issue

相似的 Issue

更多 TypeScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。