You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A command spawned by computer_run_command was given the computer process's own environment (agent-computer/src/shell.ts:81). Under docker-compose.yml that is one variable, COMPUTER_TOKEN. Under the one-container image it is the container's environment, which docker/s6/s6-rc.d/computer/run:1 hands over deliberately and which docs/deployment.md fills from --env-file .env: the key that decrypts stored credentials, the database URL, the model key. env returned all of them, and the default policy permits the tool.
Now a command gets what a command needs. PATH, the locale and terminal names, and the proxy variables, because an apt-get behind a corporate proxy reaches nothing without them. HOME is the workspace, set last so nothing can move it. COMPUTER_SHELL_ENV names anything else a deployment wants passed through, comma separated and read literally, so naming a secret there is an operator's decision rather than the default it was.
An allow list rather than a deny list, because a deny list is the secrets that existed on the day it was written, and the next variable somebody adds to a deployment is not on it.
This is a floor, not the boundary, and the image already says why: Dockerfile:134-137 notes that a Bot can become root, that per-Bot computers and gVisor are what make that sane, and that "in a container shared between Bots, or one holding a database, a Bot with sudo can reach all of it." Root in a shared container can read another process's environment whatever this function returns. What this removes is the one-word version of it. The larger question, whether a shell belongs in a container that also holds the database and is shared between every Bot, is in the issue rather than in this branch, because it is a product decision and not a bug fix.
Where it runs
New state that outlives a request? None. commandEnvironment is a pure function of the environment it is handed and the workspace path.
What happens on the second replica? The same thing. Every computer process filters its own environment identically; there is nothing shared to disagree about.
Anything serialised? Nothing. No writes.
Anything fanned out to a browser? No.
New listener, port, or schedule? No. One new optional variable, COMPUTER_SHELL_ENV, read at spawn time.
Boundary and audit
Every acting call still goes through the gateway: resolve, decide, audit, then act. Untouched — this changes what the hands are holding, not who decides.
New refusals and new failures each write a row. No new refusal or failure path; a command that relied on an inherited variable now sees it unset rather than being refused.
Nothing new is trusted from the client. COMPUTER_SHELL_ENV is deployment configuration, read from the computer's own environment, never from a request.
Proof
commandEnvironment is exported and pure, taking the environment as an argument the way egressFor does, so the cases can be read without a container. agent-computer/tests/shell.test.ts covers seven: the eight secrets a one-container deployment holds are absent; PATH and the locale survive; HOME is the workspace whatever the deployment's HOME was; a proxied deployment keeps HTTPS_PROXY and no_proxy; COMPUTER_SHELL_ENV passes two named variables with surrounding spaces tolerated and brings nothing else with them; naming a variable that is not set does not invent an empty one, because test -z "$X" and test -v X are different questions; and naming HOME in that list cannot move a command's home out of its workspace.
bun test tests/shell.test.ts from agent-computer reports 7 pass, 0 fail, 18 expect() calls.
The remaining gates I could not run here, and would rather say so than imply otherwise.bunx biome exits with an access violation on this machine and bun run typecheck cannot resolve yaml, zod and others; both reproduce on a clean checkout of main with no changes, so they are this Windows setup rather than this branch, and I could not run the container either. I have kept every line inside 80 columns and followed the formatting of the file beside it, but CI is the real check on that, and I will fix whatever it reports.
Reviewed by reading rather than running: no ENV in the Dockerfile that a command plausibly needs is dropped, there is no DEBIAN_FRONTEND to lose, and sudo resets the environment by default in any case, so sudo apt-get install behaves exactly as before.
Closing this in favour of #68, which got here first and is the better change.
It does three things mine did not. It strips userinfo out of the proxy URLs, so a proxy password does not survive the scrub. It copies every LC_* category rather than the one I happened to name. And it sets DEBIAN_FRONTEND=noninteractive, which I considered and talked myself out of, on the grounds that the image never set it — the right question was not what the image sets, it was what the tool description tells the model to run, which is apt-get install.
It also found the thing I walked straight past: -lc makes it a login shell, so it sources $HOME/.bash_profile, and HOME is the workspace a Bot writes with computer_write_file. A Bot could leave a file there that every later command runs first, and the audit row would still read whatever innocent thing was asked for. That is a trail describing something other than what ran, which is worse than the environment read I filed, and -c is the right answer to it.
Nothing here is worth keeping over that. #66 has the reasoning and #68 has the fix.
One thing to carry forward rather than lose: #66's second half is not addressed by either branch, and should not be closed silently along with the issue. Dockerfile:134-137 sets the precondition for the shell — "in a container shared between Bots, or one holding a database, a Bot with sudo can reach all of it" — and the one-container image both omits the supervisor, so every Bot shares one computer, and offers EMBEDDED_POSTGRES=on. Scrubbing the environment is a floor under that, not an answer to it. Happy to open it as its own issue once #68 lands so it does not disappear.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
A command spawned by
computer_run_commandwas given the computer process's own environment (agent-computer/src/shell.ts:81). Underdocker-compose.ymlthat is one variable,COMPUTER_TOKEN. Under the one-container image it is the container's environment, whichdocker/s6/s6-rc.d/computer/run:1hands over deliberately and whichdocs/deployment.mdfills from--env-file .env: the key that decrypts stored credentials, the database URL, the model key.envreturned all of them, and the default policy permits the tool.Now a command gets what a command needs.
PATH, the locale and terminal names, and the proxy variables, because anapt-getbehind a corporate proxy reaches nothing without them.HOMEis the workspace, set last so nothing can move it.COMPUTER_SHELL_ENVnames anything else a deployment wants passed through, comma separated and read literally, so naming a secret there is an operator's decision rather than the default it was.An allow list rather than a deny list, because a deny list is the secrets that existed on the day it was written, and the next variable somebody adds to a deployment is not on it.
Closes #66.
What this does not do
This is a floor, not the boundary, and the image already says why:
Dockerfile:134-137notes that a Bot can become root, that per-Bot computers and gVisor are what make that sane, and that "in a container shared between Bots, or one holding a database, a Bot with sudo can reach all of it." Root in a shared container can read another process's environment whatever this function returns. What this removes is the one-word version of it. The larger question, whether a shell belongs in a container that also holds the database and is shared between every Bot, is in the issue rather than in this branch, because it is a product decision and not a bug fix.Where it runs
commandEnvironmentis a pure function of the environment it is handed and the workspace path.COMPUTER_SHELL_ENV, read at spawn time.Boundary and audit
COMPUTER_SHELL_ENVis deployment configuration, read from the computer's own environment, never from a request.Proof
commandEnvironmentis exported and pure, taking the environment as an argument the wayegressFordoes, so the cases can be read without a container.agent-computer/tests/shell.test.tscovers seven: the eight secrets a one-container deployment holds are absent;PATHand the locale survive;HOMEis the workspace whatever the deployment'sHOMEwas; a proxied deployment keepsHTTPS_PROXYandno_proxy;COMPUTER_SHELL_ENVpasses two named variables with surrounding spaces tolerated and brings nothing else with them; naming a variable that is not set does not invent an empty one, becausetest -z "$X"andtest -v Xare different questions; and namingHOMEin that list cannot move a command's home out of its workspace.bun test tests/shell.test.tsfromagent-computerreports 7 pass, 0 fail, 18expect()calls.The remaining gates I could not run here, and would rather say so than imply otherwise.
bunx biomeexits with an access violation on this machine andbun run typecheckcannot resolveyaml,zodand others; both reproduce on a clean checkout ofmainwith no changes, so they are this Windows setup rather than this branch, and I could not run the container either. I have kept every line inside 80 columns and followed the formatting of the file beside it, but CI is the real check on that, and I will fix whatever it reports.Reviewed by reading rather than running: no
ENVin theDockerfilethat a command plausibly needs is dropped, there is noDEBIAN_FRONTENDto lose, andsudoresets the environment by default in any case, sosudo apt-get installbehaves exactly as before.