Loom virtual workspace exec and materialized sandbox fallback - #51
Conversation
Snowy7
left a comment
There was a problem hiding this comment.
CHANGES_REQUESTED (could not submit a formal request-changes review because GitHub rejects requesting changes on my own PR under the authenticated account).
Blocking issue found in commit 626c42d984f7993125f1eb6f3360773bd7b31417.
loom workspace materialize-run does not actually isolate the child process. In AgentWorkspaceSession::run_materialized_command, Loom creates a sandbox and then launches the command with only .current_dir(&cwd). The child process is otherwise unrestricted, so it can write out of .loom/workspaces/materialized/... with .. paths or absolute paths. Those writes bypass capture_sandbox_changes() and therefore bypass the workspace write policy, secret/generated filtering, and overlay accounting.
I reproduced this locally with a tracked temp folder and:
loom workspace materialize-run <folder> --session escape-test -- cmd /C "echo OPENAI_API_KEY=sk-...>..\..\..\..\..\leaked.env"
The command exited 0, reported Captured: 0 changed, 1 unchanged, removed the sandbox, left leaked.env in the shared folder, and loom workspace diff still reported no session changes. That violates the documented/materialized-run contract that it runs in an isolated sandbox and captures changed/created files through the existing workspace write policy.
Please make the fallback enforce an actual boundary before claiming policy-safe capture. At minimum, commands must not be able to mutate the shared folder or other sessions/sandboxes undetected; otherwise the docs and command name need to state that this is only a convenience working directory, not an isolated/policy-safe sandbox.
Verification run:
cargo test -p loom-workspace -p loom-clicargo fmt --all -- --check- GitHub
Rustcheck is passing
Snowy7
left a comment
There was a problem hiding this comment.
APPROVED re-review for e493b067bbf54c5f4a60be4e0c581a16361e0f8c.
GitHub would not allow a formal approving review from the authenticated owner account, so recording the approval as a review comment.
I rechecked the previous materialized-run sandbox escape. The new temp-backed materialized root plus pre/post shared-folder fingerprint guard fixes the silent capture/policy bypass for direct shared-folder mutations: an absolute write back into the real shared folder now exits non-zero, reports the mutated path, keeps the sandbox for inspection, and leaves the workspace overlay diff unchanged.
Verification:
cargo test -p loom-workspace -p loom-clicargo fmt --all -- --checkgit diff --check origin/main...HEAD- Manual adversarial CLI check for absolute shared-folder write from
materialize-run - GitHub
Rustcheck is green; merge state is clean
Residual risk: this is still not an OS-level security sandbox, and the docs now say so. Hostile commands/process trees remain out of scope; this approval is for the stated materialized fallback guard and capture-policy behavior.
Summary
Adds the PR 2 workspace execution layer for Loom agent sessions:
loom workspace execfor deterministic virtual commands over a session view:pwd,ls,cat,stat, literalrg, andwriteloom workspace materialize-runfor isolated real-process fallback sandboxes that hydrate the session view, run a command, and capture changed/created files back through the existing overlay write policySafety and limitations
rgcommand is a literal line search, not full ripgrep compatibility.Validation
cargo test -p loom-workspacecargo test -p loom-clicargo test --workspace