fix(skills): shorten Testbox SSH socket paths

This commit is contained in:
Peter Steinberger
2026-07-15 08:52:05 +01:00
parent 2afa9de438
commit 1fe3a5f557
4 changed files with 341 additions and 741 deletions
+6
View File
@@ -235,6 +235,12 @@ happen before the test shell starts:
OPENCLAW_TESTBOX=1 "$AUTOREVIEW" --parallel-tests "pnpm check:changed"
```
On POSIX, the helper puts this isolated Testbox home under the short, sticky
system `/tmp`; Blacksmith creates an SSH control socket below that home, and a
long macOS `TMPDIR` can exceed the Unix-socket path limit. With an older helper,
prefix the outer autoreview process with `TMPDIR=/tmp`. Setting `TMPDIR` inside
the quoted test command is too late because the isolated home already exists.
This is the narrow trusted-maintainer-code exception: it stages only the Blacksmith
credential file into the temporary home so the command can delegate remotely. Never
use this credential-hydrated path for untrusted contributor or fork code. Run other
+27 -1
View File
@@ -968,6 +968,29 @@ def safe_temp_root(repo: Path) -> Path:
return root
def parallel_test_temp_root(repo: Path) -> Path:
root = safe_temp_root(repo)
if os.name == "nt" or not env_truthy("OPENCLAW_TESTBOX"):
return root
# Blacksmith stores its SSH control socket below HOME. macOS temp roots can
# already consume the Unix-socket path budget before the socket name.
try:
short_root = Path("/tmp").resolve(strict=True)
short_root_stat = short_root.stat()
except OSError as exc:
raise SystemExit(f"unable to resolve short Testbox temp root: {exc}") from exc
if not stat.S_ISDIR(short_root_stat.st_mode):
raise SystemExit("short Testbox temp root must be a directory")
if is_within(short_root, repo.resolve()):
raise SystemExit("short Testbox temp root must be outside the reviewed repository")
if short_root_stat.st_mode & stat.S_IWOTH and not (
short_root_stat.st_mode & stat.S_ISVTX
):
raise SystemExit("world-writable Testbox temp root must have the sticky bit set")
return short_root
def safe_proxy_url(value: str) -> bool:
try:
candidate = value if "://" in value else f"http://{value}"
@@ -8971,7 +8994,10 @@ def start_parallel_tests(
) -> tuple[subprocess.Popen, float]:
print(f"tests: {command}")
test_home = Path(
tempfile.mkdtemp(prefix="autoreview-test-home-", dir=safe_temp_root(repo))
tempfile.mkdtemp(
prefix="autoreview-test-home-",
dir=parallel_test_temp_root(repo),
)
)
try:
env = safe_test_env(repo, test_home)
@@ -4149,6 +4149,53 @@ class AutoreviewHardeningTests(unittest.TestCase):
):
self.helper["safe_temp_root"](repo)
@unittest.skipIf(os.name == "nt", "POSIX Testbox temp-root behavior")
def test_testbox_parallel_test_temp_root_stays_within_socket_limit(self) -> None:
with tempfile.TemporaryDirectory() as tempdir:
root = Path(tempdir)
repo = init_repo(root)
long_temp = root / ("macos-temp-root-" + "x" * 96)
long_temp.mkdir()
with mock.patch.object(
tempfile,
"gettempdir",
return_value=str(long_temp),
), mock.patch.dict(
os.environ,
{"OPENCLAW_TESTBOX": "1"},
):
selected = self.helper["parallel_test_temp_root"](repo)
self.assertEqual(selected, Path("/tmp").resolve())
socket_path = (
selected
/ ("autoreview-test-home-" + "x" * 8)
/ ".blacksmith"
/ "c"
/ "6d146d2f25180c1d.sock"
)
self.assertLess(len(os.fsencode(socket_path)), 104)
def test_parallel_test_temp_root_keeps_configured_root_without_testbox(self) -> None:
with tempfile.TemporaryDirectory() as tempdir:
root = Path(tempdir)
repo = init_repo(root)
configured_temp = root / "configured-temp"
configured_temp.mkdir()
with mock.patch.object(
tempfile,
"gettempdir",
return_value=str(configured_temp),
), mock.patch.dict(
os.environ,
{"OPENCLAW_TESTBOX": "0"},
):
selected = self.helper["parallel_test_temp_root"](repo)
self.assertEqual(selected, configured_temp.resolve())
def test_claude_fable_alias_requires_fable_safe_mode_version(self) -> None:
args = argparse.Namespace(
claude_bin="claude",
File diff suppressed because it is too large Load Diff