mirror of
https://github.com/wirenboard/agent-vm.git
synced 2026-07-09 16:00:54 +00:00
ce745ece6d
Folds in fourteen findings from the code review of the previous two
commits. Most are 'the commit advertised something it didn't actually
deliver' — straightforward to make honest.
**Observability commit was a no-op — revert it.**
- `main.rs` filter went back to the simple `warn` default. The previous
`microsandbox_network::proxy=info,microsandbox_network::tls::proxy=info`
targeted modules that (a) only emit `tracing::debug!` (so info
filtered them out) and (b) run in the `msb sandbox` subprocess whose
own tracing init reads no env at all. Net behaviour: zero proxy
logs surfaced; the doc-comment claim was wishful thinking.
- `run.rs` post-mortem hint trimmed to one accurate line. Dropped the
pointers to `msb.stderr.log` and `msb-exit.log` (vendor `spawn.rs`
inherits stderr instead of teeing; `handle.rs::wait` writes no exit
log). What remains: a single `(see <runtime.log> for the in-VM tail)`
appended via `with_context`. Avoids the duplicated error output the
old code produced — anyhow's main termination prints the chain once
rather than the previous "eprintln before return then re-print on
exit" combo.
- `runtime_log_path()` replaces `msb_sandbox_log_dir`; comment no
longer claims to "mirror" upstream, and the final fallback matches
`microsandbox_utils::resolve_home()` (`./.microsandbox`) rather than
the previous incorrect `/var/lib/microsandbox`. Hint now also fires
in the non-TTY branch's `with_context` and `anyhow::bail!` paths,
not just TTY — CI users are the ones most likely to need the
pointer.
**Chrome MCP cleanups.**
- `certutil` trust string `-t TC` → `-t C,,`. The previous form set
the SSL column to `TC` (T = trusted issuer of *client* certs, C =
trusted issuer of server certs). We only want server-cert trust;
the T silently broadened scope. `C,,` is the textbook idiom.
- Wrapper's `--preserve-env` allow-list expanded to forward the env
vars chrome-devtools-mcp itself documents
(`CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS`, `CI`, `DEBUG`), agentd-
set proxy config (`HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` plus
lowercase variants), and basic locale (`TZ`, `LANG`, `LC_ALL`,
`TMPDIR`). Without these, user MCP-config `env:` blocks and
documented opt-outs were silently stripped by sudo's `env_reset`.
- Wrapper now `cd /home/chrome` before exec'ing. Inherited cwd was
`/workspace` (the project bind-mount), which the chrome user (UID
9999) cannot write — any MCP tool emitting a relative path
(`./trace.json`, `./screenshot.png`, `--logFile=./debug.log`)
EACCES'd silently. `/home/chrome` is chrome-owned and writable.
- Pre-warm `chrome-devtools-mcp@latest` into chrome's npm cache at
image build time (`/home/chrome/.npm/_npx/...`). Previously every
first MCP call paid a multi-second `npx -y` fetch and broke
entirely when registry.npmjs.org was unreachable. The image gains
~21 MiB; cold launches drop the fetch.
- Bash prelude's certutil block is now gated on `AGENT_VM_NO_CHROME_MCP`
not being set. Previously the env-var opt-out only suppressed the
MCP entry in `claude.json`; the sudo→chrome→certutil round-trip
ran on every launch regardless, exposing the opt-out user to the
exact sudoers/NSS failure modes they were trying to avoid.
**Sticky opt-out fixed.** `write_default_claude_root_state` now
always owns the `chrome-devtools` key: `AGENT_VM_NO_CHROME_MCP=1`
explicitly `mcp.remove("chrome-devtools")`s a previously-written
entry instead of leaving it sticky in the on-disk merged JSON.
**OPENCODE placeholder rename completed.** `OPENCODE_OPENAI_ACCESS_PLACEHOLDER`
JWT signature is now `msb-opencode-placeholder-a-v2` (was
`MSB_OPENCODE_v1`), matching the rename rationale spelled out on
`OPENAI_ID_PLACEHOLDER`: non-token-shaped sentinels so Anthropic
doesn't flag transcripts that contain them.
**Stale literals swept.** `ARCHITECTURE.md` reads the new names at
all five reference sites. Six tests in `intercept_hook.rs` that
hardcoded `MSB_PLACEHOLDER_GH_TOKEN_v1[_xxxxxxxx]` as a standalone
fixture now use `secrets::GH_TOKEN_PLACEHOLDER` directly — so a
future placeholder shape change is exercised by the existing
regression assertions instead of slipping past them.
End-to-end verified inside a fresh sandbox:
- `claude mcp list` → ✓ Connected
- `navigate_page` https://example.com + `take_snapshot` returns the
real "Example Domain" content
- NSS DB lists only the microsandbox CA with trust `C,,` (not `CT,,`)
- `chrome-devtools-mcp` package present in /home/chrome/.npm/_npx/
out of the box (no per-launch fetch)
- `AGENT_VM_NO_CHROME_MCP=1`: claude.json mcpServers is `{}` (entry
removed, not just absent on new files); NSS DB is empty (certutil
prelude skipped)
- Wrapper preserves CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS, CI,
DEBUG, HTTP_PROXY, TZ, NODE_EXTRA_CA_CERTS across the sudo hop
- 73 cargo unit tests pass (incl. the intercept_hook tests now using
the constant)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|---|---|---|
| .. | ||
| build.sh | ||
| Dockerfile | ||