mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-26 06:02:22 +00:00
* fix(eval): bind the resolved node to a fresh sandbox path, not one under /usr #2607 bound the resolved `node` to /usr/local/bin/node, but that path lives inside the /usr tree that _runtime_mount_args already read-only-binds wholesale. The second real workflow_dispatch run on the self-hosted runner (https://github.com/abhigyanpatwari/GitNexus/actions/runs/29840270554) failed immediately in the bubblewrap preflight: "bwrap: Can't create file at /usr/local/bin/node: Read-only file system" -- bwrap can't create a new mount-point file inside a tree it already bound read-only when the real path doesn't already exist there on the host, which is exactly the self-hosted case this bind exists to fix. Introduces SANDBOX_NODE (/opt/claude/node), a fresh path outside every tree _runtime_mount_args binds, following the same pattern SANDBOX_CLAUDE and SANDBOX_PYTHON3 already use. Updates the two real call sites (sanitized_graph.py, runner_sessions.py) to use the constant instead of the hardcoded literal, so the fix can't drift out of sync with itself again, and re-exports it from runner.py alongside the other SANDBOX_* names for the real-bwrap tests that reference it directly. Adds a real-bwrap test (gated behind GITNEXUS_REQUIRE_BWRAP_CANARY, same as the existing ones) that copies a real node binary to a path outside every bound tree and actually launches bwrap against it -- an argv-construction test alone can't catch a bwrap-level "Read-only file system" error, only a real invocation can, and that's exactly the gap that let #2607's version of this fix through review looking correct. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(eval): don't let the new real-bwrap test's node-mock break bwrap's own resolution CI caught this immediately: the new test_real_bubblewrap_runs_node_from_outside_the_bound_trees monkeypatched shutil.which to return None for anything but "node", but prepare_sandbox's own bwrap/claude resolution (_resolve_executable) goes through shutil.which too -- so the test broke bwrap discovery before the sandbox it's supposed to exercise could even be built ("SandboxError: required executable is unavailable: bwrap"). Delegate to the real shutil.which for every other name instead of blanket-returning None. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
cb5a8e242f
commit
3fc8abe084
6 changed files with 76 additions and 22 deletions
|
|
@ -4,6 +4,7 @@ from __future__ import annotations
|
|||
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import stat
|
||||
import subprocess
|
||||
import sys
|
||||
|
|
@ -19,6 +20,7 @@ from workflow_bench.process_control import ManagedProcessResult, run_managed
|
|||
from workflow_bench.proposer_sandbox import (
|
||||
MAX_BUNDLE_BYTES,
|
||||
MAX_EVIDENCE_FILE_BYTES,
|
||||
SANDBOX_NODE,
|
||||
SANDBOX_PYTHON3,
|
||||
SANDBOX_SHELL_PREFIX,
|
||||
SANDBOX_USER_SKILLS,
|
||||
|
|
@ -192,14 +194,19 @@ def test_sandbox_command_has_minimal_mounts_and_no_host_root_bind(tmp_path: Path
|
|||
assert not private_root.exists()
|
||||
|
||||
|
||||
def test_runtime_mounts_bind_the_resolved_node_wherever_path_puts_it(monkeypatch) -> None:
|
||||
def test_runtime_mounts_bind_the_resolved_node_to_a_fresh_sandbox_path(monkeypatch) -> None:
|
||||
# sanitized_graph.py and runner_sessions.py invoke the sandboxed graph CLI
|
||||
# at the fixed path /usr/local/bin/node. That's only covered by the /usr
|
||||
# bind when node happens to live under /usr/local/bin on the host -- true
|
||||
# on GitHub-hosted runner images, but not on a self-hosted runner where
|
||||
# actions/setup-node installs into its own tool-cache directory instead
|
||||
# (observed empirically: "bwrap: execvp /usr/local/bin/node: No such file
|
||||
# or directory" on a fresh self-hosted runner).
|
||||
# via SANDBOX_NODE. node's real host location varies (GitHub-hosted
|
||||
# runner images happen to have one under /usr/local/bin; a self-hosted
|
||||
# runner's actions/setup-node installs into its own tool-cache directory
|
||||
# instead), so this must bind to a FRESH sandbox path like /opt/claude/...
|
||||
# rather than anywhere under /usr, /bin, /lib, or /lib64: those are
|
||||
# already read-only bound by this same function, and bwrap can't create
|
||||
# a new mount-point file inside an already-read-only tree when the real
|
||||
# path doesn't already exist there on the host (observed empirically:
|
||||
# "bwrap: Can't create file at /usr/local/bin/node: Read-only file
|
||||
# system" when this bind first targeted that path on a self-hosted
|
||||
# runner where node isn't really there).
|
||||
monkeypatch.setattr(
|
||||
"workflow_bench.proposer_sandbox.shutil.which",
|
||||
lambda name: "/opt/hostedtoolcache/node/22.18.0/x64/bin/node" if name == "node" else None,
|
||||
|
|
@ -207,13 +214,14 @@ def test_runtime_mounts_bind_the_resolved_node_wherever_path_puts_it(monkeypatch
|
|||
args = _runtime_mount_args()
|
||||
node_index = args.index("/opt/hostedtoolcache/node/22.18.0/x64/bin/node")
|
||||
assert args[node_index - 1] == "--ro-bind"
|
||||
assert args[node_index + 1] == "/usr/local/bin/node"
|
||||
assert args[node_index + 1] == SANDBOX_NODE
|
||||
assert not any(SANDBOX_NODE.startswith(bound + "/") for bound in ("/usr", "/bin", "/lib", "/lib64"))
|
||||
|
||||
|
||||
def test_runtime_mounts_skip_the_node_bind_when_node_is_unresolvable(monkeypatch) -> None:
|
||||
monkeypatch.setattr("workflow_bench.proposer_sandbox.shutil.which", lambda name: None)
|
||||
args = _runtime_mount_args()
|
||||
assert "/usr/local/bin/node" not in args
|
||||
assert SANDBOX_NODE not in args
|
||||
|
||||
|
||||
def test_stricter_prefix_freezes_evaluated_skills_and_can_unshare_network(tmp_path: Path) -> None:
|
||||
|
|
@ -244,6 +252,43 @@ def test_stricter_prefix_freezes_evaluated_skills_and_can_unshare_network(tmp_pa
|
|||
assert prefix[user_index - 2] == "--ro-bind"
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
os.environ.get("GITNEXUS_REQUIRE_BWRAP_CANARY") != "1",
|
||||
reason="real Bubblewrap canary is mandatory in the named Ubuntu CI job",
|
||||
)
|
||||
def test_real_bubblewrap_runs_node_from_outside_the_bound_trees(tmp_path: Path, monkeypatch) -> None:
|
||||
# Reproduces the self-hosted-runner failure directly: node resolved from
|
||||
# a path outside /usr, /bin, /lib, /lib64 (actions/setup-node's own
|
||||
# tool-cache convention) must still be reachable inside the sandbox at
|
||||
# SANDBOX_NODE. A real node copied to a fresh, non-system location stands
|
||||
# in for the tool-cache install; argv-construction tests alone can't
|
||||
# catch a bwrap-level "Can't create file ...: Read-only file system"
|
||||
# (the actual error this fix resolves), only a real bwrap invocation can.
|
||||
real_node = shutil.which("node")
|
||||
if not real_node:
|
||||
pytest.skip("no node on PATH to relocate for this canary")
|
||||
toolcache = tmp_path / "toolcache"
|
||||
toolcache.mkdir()
|
||||
relocated_node = toolcache / "node"
|
||||
shutil.copy2(real_node, relocated_node)
|
||||
relocated_node.chmod(0o755)
|
||||
# Only fake "node"'s resolution -- prepare_sandbox's own bwrap/claude
|
||||
# lookups (_resolve_executable) also go through shutil.which, and must
|
||||
# keep resolving for real or preflight fails before the sandbox is even
|
||||
# built.
|
||||
real_which = shutil.which
|
||||
monkeypatch.setattr(
|
||||
"workflow_bench.proposer_sandbox.shutil.which",
|
||||
lambda name: str(relocated_node) if name == "node" else real_which(name),
|
||||
)
|
||||
|
||||
clone = tmp_path / "clone"
|
||||
clone.mkdir()
|
||||
with prepare_sandbox(clone=clone, claude_bin=Path(sys.executable), preflight=True) as sandbox:
|
||||
result = sandbox.run([SANDBOX_NODE, "--version"], timeout=10)
|
||||
assert result.ok, result.stderr_tail
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
os.environ.get("GITNEXUS_REQUIRE_BWRAP_CANARY") != "1",
|
||||
reason="real Bubblewrap canary is mandatory in the named Ubuntu CI job",
|
||||
|
|
|
|||
|
|
@ -438,11 +438,11 @@ def test_real_bubblewrap_runtime_mount_imports_cli_without_exposing_checkout(tmp
|
|||
preflight=True,
|
||||
) as sandbox:
|
||||
visibility = sandbox.run(
|
||||
["/usr/local/bin/node", "-e", visibility_script],
|
||||
[runner.SANDBOX_NODE, "-e", visibility_script],
|
||||
timeout=10,
|
||||
)
|
||||
imported = sandbox.run(
|
||||
["/usr/local/bin/node", runner.SANDBOX_GITNEXUS_ENTRYPOINT, "--version"],
|
||||
[runner.SANDBOX_NODE, runner.SANDBOX_GITNEXUS_ENTRYPOINT, "--version"],
|
||||
timeout=10,
|
||||
)
|
||||
# --version never reaches the `analyze` command, which is loaded via a
|
||||
|
|
@ -451,7 +451,7 @@ def test_real_bubblewrap_runtime_mount_imports_cli_without_exposing_checkout(tmp
|
|||
# resolve-analyze-cmd.cjs. Require the compiled analyze module
|
||||
# directly so this canary actually exercises that chain.
|
||||
analyze_imported = sandbox.run(
|
||||
["/usr/local/bin/node", "-e", f"require('{runner.SANDBOX_GITNEXUS}/dist/cli/analyze.js')"],
|
||||
[runner.SANDBOX_NODE, "-e", f"require('{runner.SANDBOX_GITNEXUS}/dist/cli/analyze.js')"],
|
||||
timeout=10,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ SANDBOX_TMP = "/tmp"
|
|||
SANDBOX_CLAUDE = "/opt/claude/claude"
|
||||
SANDBOX_SHELL_PREFIX = "/opt/claude/shell-prefix"
|
||||
SANDBOX_PYTHON3 = "/opt/claude/python3"
|
||||
SANDBOX_NODE = "/opt/claude/node"
|
||||
SANDBOX_PATH = "/opt/claude:/usr/local/bin:/usr/bin:/bin"
|
||||
SANDBOX_GITNEXUS = "/opt/gitnexus"
|
||||
SANDBOX_GITNEXUS_SHARED = "/opt/gitnexus-shared"
|
||||
|
|
@ -355,16 +356,19 @@ def _runtime_mount_args() -> list[str]:
|
|||
if path.exists():
|
||||
args += ["--ro-bind", raw, raw]
|
||||
# sanitized_graph.py and runner_sessions.py invoke the sandboxed graph
|
||||
# CLI at the fixed path /usr/local/bin/node. That's only covered by the
|
||||
# /usr bind above when node happens to live under /usr/local/bin on the
|
||||
# host -- true on GitHub-hosted runner images, but not on a self-hosted
|
||||
# runner where actions/setup-node installs into its own tool-cache
|
||||
# directory instead. Bind whatever `node` actually resolves to on PATH
|
||||
# to that same fixed sandbox path so both call sites keep working
|
||||
# regardless of where the host actually put it.
|
||||
# CLI via SANDBOX_NODE. Bind whatever `node` actually resolves to on PATH
|
||||
# there -- true node location varies by host (GitHub-hosted runner images
|
||||
# happen to have one under /usr/local/bin; a self-hosted runner's
|
||||
# actions/setup-node installs into its own tool-cache directory instead).
|
||||
# Target must be a fresh path like /opt/claude/... rather than anywhere
|
||||
# under /usr, /bin, /lib, or /lib64: those are already read-only bound
|
||||
# above, and bwrap can't create a new mount-point file inside an
|
||||
# already-read-only tree when the real path doesn't already exist there
|
||||
# (the exact case a self-hosted runner hits, and the reason this bind
|
||||
# exists at all).
|
||||
node_bin = shutil.which("node")
|
||||
if node_bin:
|
||||
args += ["--ro-bind", node_bin, "/usr/local/bin/node"]
|
||||
args += ["--ro-bind", node_bin, SANDBOX_NODE]
|
||||
for raw in (
|
||||
"/etc/ssl",
|
||||
"/etc/hosts",
|
||||
|
|
|
|||
|
|
@ -78,6 +78,7 @@ from .proposer_sandbox import (
|
|||
SANDBOX_GITNEXUS as SANDBOX_GITNEXUS,
|
||||
SANDBOX_GITNEXUS_REGISTRY,
|
||||
SANDBOX_GITNEXUS_SHARED as SANDBOX_GITNEXUS_SHARED,
|
||||
SANDBOX_NODE as SANDBOX_NODE,
|
||||
SANDBOX_WORKSPACE,
|
||||
ReadOnlyMount,
|
||||
SandboxError,
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ from .proposer_sandbox import (
|
|||
SANDBOX_GITNEXUS,
|
||||
SANDBOX_GITNEXUS_REGISTRY,
|
||||
SANDBOX_HOME,
|
||||
SANDBOX_NODE,
|
||||
SANDBOX_TMP,
|
||||
SANDBOX_WORKSPACE,
|
||||
SandboxError,
|
||||
|
|
@ -50,6 +51,8 @@ def measured_cost(raw: Any) -> float | None:
|
|||
if not math.isfinite(raw) or raw < 0:
|
||||
return None
|
||||
return float(raw)
|
||||
|
||||
|
||||
SANDBOX_GITNEXUS_ENTRYPOINT = f"{SANDBOX_GITNEXUS}/dist/cli/index.js"
|
||||
SENSITIVE_EVENT_KEYS = frozenset(
|
||||
{
|
||||
|
|
@ -113,7 +116,7 @@ def sandbox_mcp_config() -> str:
|
|||
"PATH=/usr/local/bin:/usr/bin:/bin",
|
||||
"LANG=C.UTF-8",
|
||||
"GIT_TERMINAL_PROMPT=0",
|
||||
"/usr/local/bin/node",
|
||||
SANDBOX_NODE,
|
||||
SANDBOX_GITNEXUS_ENTRYPOINT,
|
||||
"mcp",
|
||||
],
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ from .process_control import ManagedProcessError, run_managed
|
|||
from .proposer_sandbox import (
|
||||
SANDBOX_GITNEXUS,
|
||||
SANDBOX_HOME,
|
||||
SANDBOX_NODE,
|
||||
SANDBOX_WORKSPACE,
|
||||
ReadOnlyMount,
|
||||
SandboxError,
|
||||
|
|
@ -248,7 +249,7 @@ def _run_graph_cli(
|
|||
) -> bytes | None:
|
||||
command = [
|
||||
*prefix,
|
||||
"/usr/local/bin/node",
|
||||
SANDBOX_NODE,
|
||||
SANDBOX_GITNEXUS_ENTRYPOINT,
|
||||
*arguments,
|
||||
]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue