The default Vault impl can leak secrets into child processes via process.env
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 45/100
- Tipo de issue
- Error
- Claridad
- Bastante claro
- Estado de actividad
- Activo
- Stack tecnológico
- node.js, typescript
Línea de trabajo
Comienza con apps/cli/src/config.ts y apps/cli/src/index.ts, y después inspecciona los puntos de entrada que generan procesos hijos en plugins/bash/src/index.ts, plugins/background/src/index.ts, plugins/mcp/src/client.ts y docker-bash. Mapea los flujos de Environment y .env antes de elegir las correcciones acotadas, teniendo en cuenta la dependencia de #62 para el gating de vault.get. Se considera terminado cuando los escenarios propuestos de filtración de Environment están cubiertos por pruebas, se conserva la precedencia y se documentan las migraciones o los cambios de comportamiento.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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:
loadDotEnvpromotes.envintoprocess.env.apps/cli/src/config.ts:70-71writes every parsed key intoprocess.envunless already set. It does this becauseEnvFileVaultreads from an env snapshot, not from the file — both construction sites passprocess.env(index.ts:695,index.ts:846) — so promotion is currently load-bearing, not incidental.bashcopies all ofprocess.envinto every child shell.plugins/bash/src/index.ts:251merges the whole environment beforespawn, with the caller'senvlayered on top. So{"script": "env"}returns every API key in the vault.backgroundandmcpdo the same.background:238spreadsprocess.envinto detached children;mcp/client.ts:40spreads it into every stdio MCP server — so a third-party MCP server is handed the full set on launch..envsits in the project directory, whichdocker-bashmounts.docker-bashgets this right at the process level — it passesenv: {}(docker-bash:495) and inherits nothing. But the project root is mounted read-only at/app, and.envis in it (config.ts:64), socat /app/.envrecovers 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
:romount or acatto 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 frombash/background/mcpis worth doing on its own terms. - Fix 3 becomes more valuable, not less. Once the ambient paths are closed,
vault.getis 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.
- Lenguaje dominante
- TypeScript
- Estrellas
- 5
- Forks
- 2
- Merge medio
- 1 d 4 h
- PR fusionados (30 d)
- 11
Preparar el entorno
Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de MatAtBread/matbot
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 74/100
MatAtBread/matbot#99 ·
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
MatAtBread/matbot#95 ·
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
MatAtBread/matbot#94 ·
Los mantenedores suelen responder en 1 día
-
http: ToolContracts arm duplicates HttpInput inline — named references now expand on the wireAbierto
Dificultad 3/5 Medio día Aptitud para principiantes 66/100
MatAtBread/matbot#100 · 1 comentario ·
Los mantenedores suelen responder en 1 día
-
plugin add/remove: nothing records what an install created, so the undo and the uninstall both guessPosiblemente ocupada @MatAtBread la tomó hoy. Abiertobug
Dificultad 5/5 Más de una semana Aptitud para principiantes 12/100
MatAtBread/matbot#98 · 1 comentario ·
Los mantenedores suelen responder en 1 día
Todos los issues de MatAtBread/matbot
Issues similares
-
afk-ok area:data-quality importer size:S
Dificultad 2/5 1-3 horas Aptitud para principiantes 74/100
enorm-labs/event-junkie#3301 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 62/100
one-zero-eight/apply-bot#39 ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
bigbluebutton/bbb-ui-components-react#108 ·
Los mantenedores suelen responder en 2 días
-
area: app type: bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
ExpidetGames/code-reviewer#308 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
Los mantenedores suelen responder en 1 día