security: added slot_uid-aware git env and workspace ownership handling
- Added optional slot_uid handling to push requests and git operations, validating IDs 1..65535. - Passed slot-specific subprocess kwargs into git helpers, including safe.directory, HOME, user/group and umask. - Injected slot-safe repo env helpers to scrub secrets, force git identity, and disable terminal prompts. - Adjusted workspace and cache permissioning so slot users own workspaces with targeted chmod/chown behavior. - Added tests for slot UID propagation, safe-directory env, and cross-slot push/retry behavior in unit and e2e suites.
This commit is contained in:
+5
-4
@@ -50,13 +50,14 @@ ENV PYTHONDONTWRITEBYTECODE=1 \
|
||||
PIP_DISABLE_PIP_VERSION_CHECK=1 \
|
||||
BUN_INSTALL=/opt/bun \
|
||||
PI_ROOT=/work/pi \
|
||||
# Persistent build caches under the /data volume so cargo target,
|
||||
# rustup toolchains, and bun's global package cache are shared across
|
||||
# every per-issue worktree AND survive container restarts.
|
||||
# Persistent build caches under the /data volume so cargo target and
|
||||
# rustup toolchains are shared across every per-issue worktree and
|
||||
# survive container restarts. Bun's install cache is deliberately
|
||||
# workspace-private at runtime; bun chmod/chown behavior makes a shared
|
||||
# cross-slot cache unreliable.
|
||||
CARGO_HOME=/data/cache/cargo \
|
||||
CARGO_TARGET_DIR=/data/cache/cargo-target \
|
||||
RUSTUP_HOME=/data/cache/rustup \
|
||||
BUN_INSTALL_CACHE_DIR=/data/cache/bun-cache \
|
||||
PATH=/opt/bun/bin:/usr/local/cargo/bin:/usr/local/bin:/usr/bin:/bin
|
||||
|
||||
RUN apt-get update \
|
||||
|
||||
+18
-15
@@ -23,6 +23,15 @@ elif [[ "${1:-}" == *"robomp.proxy"* ]]; then
|
||||
fi
|
||||
|
||||
/usr/sbin/groupadd -f -g 2000 omp
|
||||
max_slots="${ROBOMP_MAX_CONCURRENCY:-8}"
|
||||
for i in $(seq 1 "$max_slots"); do
|
||||
user="omp-$i"
|
||||
slot_group="omp-$i"
|
||||
slot_id=$((2000 + i))
|
||||
/usr/sbin/groupadd -f -g "$slot_id" "$slot_group"
|
||||
id -u "$user" >/dev/null 2>&1 || /usr/sbin/useradd -u "$slot_id" -g "$slot_group" -G omp -M -N -s /usr/sbin/nologin "$user"
|
||||
/usr/sbin/usermod -g "$slot_group" -a -G omp "$user"
|
||||
done
|
||||
|
||||
if [ "$is_proxy_role" -eq 1 ]; then
|
||||
exec "$@"
|
||||
@@ -34,23 +43,17 @@ if [ ! -d "$PI_ROOT/packages/coding-agent" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
max_slots="${ROBOMP_MAX_CONCURRENCY:-8}"
|
||||
for i in $(seq 1 "$max_slots"); do
|
||||
user="omp-$i"
|
||||
slot_group="omp-$i"
|
||||
slot_id=$((2000 + i))
|
||||
/usr/sbin/groupadd -f -g "$slot_id" "$slot_group"
|
||||
id -u "$user" >/dev/null 2>&1 || /usr/sbin/useradd -u "$slot_id" -g "$slot_group" -G omp -M -N -s /usr/sbin/nologin "$user"
|
||||
/usr/sbin/usermod -g "$slot_group" -a -G omp "$user"
|
||||
done
|
||||
|
||||
mkdir -p /data/workspaces /data/workspaces/_pool /data/logs
|
||||
# Persistent build caches under the /data volume. CARGO_HOME, CARGO_TARGET_DIR,
|
||||
# RUSTUP_HOME, and BUN_INSTALL_CACHE_DIR are pinned to these paths in the image
|
||||
# ENV so every per-issue worktree shares one cargo target and one bun cache.
|
||||
mkdir -p /data/cache/cargo /data/cache/cargo-target /data/cache/rustup /data/cache/bun-cache
|
||||
# Persistent build caches under the /data volume. CARGO_HOME,
|
||||
# CARGO_TARGET_DIR, and RUSTUP_HOME are pinned to these paths in the image ENV
|
||||
# so every per-issue worktree shares one cargo target/toolchain. Bun install
|
||||
# cache is workspace-private; a shared cache is unsafe across slot users
|
||||
# because bun may chmod/chown its cache root to the first writer.
|
||||
mkdir -p /data/cache/cargo /data/cache/cargo-target /data/cache/rustup
|
||||
chown -R root:omp /data/cache /data/workspaces/_pool
|
||||
chmod -R u=rwX,g=rwsX,o= /data/cache /data/workspaces/_pool
|
||||
find /data/cache /data/workspaces/_pool -type d -exec chmod 2770 {} +
|
||||
find /data/cache /data/workspaces/_pool -type f -perm /111 -exec chmod 0770 {} +
|
||||
find /data/cache /data/workspaces/_pool -type f ! -perm /111 -exec chmod 0660 {} +
|
||||
chmod 0700 /data/logs
|
||||
|
||||
|
||||
|
||||
+111
-20
@@ -16,11 +16,14 @@ from __future__ import annotations
|
||||
import base64
|
||||
import logging
|
||||
import os
|
||||
import platform
|
||||
import re
|
||||
import subprocess
|
||||
from collections.abc import Mapping
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from urllib.parse import urlparse
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -34,6 +37,43 @@ _BAD_OBJECT_REF_RE = re.compile(
|
||||
)
|
||||
_FETCH_PRUNE_REPAIR_ATTEMPTS = 8
|
||||
|
||||
_SHARED_OMP_GID = 2000
|
||||
_AGENT_HOME = Path("/srv/agent-home")
|
||||
|
||||
|
||||
def _slot_permissions_active(slot_uid: int | None) -> bool:
|
||||
return slot_uid is not None and platform.system() == "Linux" and os.geteuid() == 0
|
||||
|
||||
|
||||
def _slot_subprocess_kwargs(slot_uid: int | None) -> dict[str, Any]:
|
||||
if not _slot_permissions_active(slot_uid):
|
||||
return {}
|
||||
assert slot_uid is not None
|
||||
return {"user": slot_uid, "group": slot_uid, "extra_groups": [_SHARED_OMP_GID], "umask": 0o002}
|
||||
|
||||
|
||||
def _append_safe_directory(env: dict[str, str], repo_dir: Path) -> None:
|
||||
count = int(env.get("GIT_CONFIG_COUNT", "0"))
|
||||
env[f"GIT_CONFIG_KEY_{count}"] = "safe.directory"
|
||||
env[f"GIT_CONFIG_VALUE_{count}"] = str(repo_dir)
|
||||
env["GIT_CONFIG_COUNT"] = str(count + 1)
|
||||
|
||||
|
||||
def _local_remote_safe_directory(remote_url: str, *, cwd: Path) -> Path | None:
|
||||
"""Return a local filesystem remote path that git may need whitelisted."""
|
||||
raw = remote_url.strip()
|
||||
if not raw:
|
||||
return None
|
||||
if raw.startswith("file://"):
|
||||
parsed = urlparse(raw)
|
||||
if parsed.netloc not in ("", "localhost"):
|
||||
return None
|
||||
return Path(parsed.path)
|
||||
if "://" in raw or re.match(r"^[^/\\s]+:", raw):
|
||||
return None
|
||||
path = Path(raw)
|
||||
return path if path.is_absolute() else (cwd / path).resolve()
|
||||
|
||||
|
||||
def redact_credentials(text: str | None) -> str:
|
||||
"""Strip `user:password@` from any embedded URL in `text`."""
|
||||
@@ -86,6 +126,11 @@ def _run_git(
|
||||
cwd: Path | None,
|
||||
token: str | None,
|
||||
extra_env: Mapping[str, str] | None = None,
|
||||
safe_directory: Path | None = None,
|
||||
user: int | None = None,
|
||||
group: int | None = None,
|
||||
extra_groups: list[int] | tuple[int, ...] | None = None,
|
||||
umask: int | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
"""Run `git <args>` with optional PAT injection via `--config-env`.
|
||||
@@ -101,8 +146,13 @@ def _run_git(
|
||||
`_DEFAULT_GIT_TIMEOUT_SECONDS`.
|
||||
"""
|
||||
env: dict[str, str] = {**os.environ, "GIT_TERMINAL_PROMPT": "0"}
|
||||
if user is not None and _AGENT_HOME.is_dir():
|
||||
env["HOME"] = str(_AGENT_HOME)
|
||||
if extra_env:
|
||||
env.update(extra_env)
|
||||
if safe_directory is not None:
|
||||
_append_safe_directory(env, safe_directory)
|
||||
|
||||
cmd: list[str] = ["git"]
|
||||
if token:
|
||||
env[AUTH_ENV_VAR] = _basic_auth_header(token)
|
||||
@@ -110,6 +160,15 @@ def _run_git(
|
||||
cmd.extend(args)
|
||||
log.debug("git", extra={"cmd": _redacted_cmd(cmd), "cwd": str(cwd) if cwd else None})
|
||||
effective_timeout = _DEFAULT_GIT_TIMEOUT_SECONDS if timeout is None else timeout
|
||||
subprocess_kwargs: dict[str, Any] = {}
|
||||
if user is not None:
|
||||
subprocess_kwargs["user"] = user
|
||||
if group is not None:
|
||||
subprocess_kwargs["group"] = group
|
||||
if extra_groups is not None:
|
||||
subprocess_kwargs["extra_groups"] = extra_groups
|
||||
if umask is not None:
|
||||
subprocess_kwargs["umask"] = umask
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
cmd,
|
||||
@@ -119,6 +178,7 @@ def _run_git(
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=effective_timeout,
|
||||
**subprocess_kwargs,
|
||||
)
|
||||
except subprocess.TimeoutExpired as exc:
|
||||
# `subprocess.run` already kills the direct child when the timeout
|
||||
@@ -327,6 +387,7 @@ def clone(
|
||||
clone_url: str,
|
||||
default_branch: str,
|
||||
token: str | None,
|
||||
safe_directory: Path | None = None,
|
||||
) -> None:
|
||||
"""Fresh `git clone --filter=blob:none` into `target`."""
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
@@ -339,10 +400,10 @@ def clone(
|
||||
clone_url,
|
||||
str(target),
|
||||
]
|
||||
_check(_run_git(args, cwd=None, token=token), ["git", *args])
|
||||
_check(_run_git(args, cwd=None, token=token, safe_directory=safe_directory), ["git", *args])
|
||||
|
||||
|
||||
def fetch_prune(repo_dir: Path, *, token: str | None) -> None:
|
||||
def fetch_prune(repo_dir: Path, *, token: str | None, safe_directory: Path | None = None) -> None:
|
||||
"""`git fetch --prune origin` on the shared pool clone.
|
||||
|
||||
Pool clones are long-lived. If a transient git object alternate leaks into
|
||||
@@ -356,7 +417,7 @@ def fetch_prune(repo_dir: Path, *, token: str | None) -> None:
|
||||
_prune_missing_alternates(repo_dir)
|
||||
last_proc: subprocess.CompletedProcess[str] | None = None
|
||||
for _ in range(_FETCH_PRUNE_REPAIR_ATTEMPTS):
|
||||
proc = _run_git(args, cwd=repo_dir, token=token)
|
||||
proc = _run_git(args, cwd=repo_dir, token=token, safe_directory=safe_directory)
|
||||
if proc.returncode == 0:
|
||||
return
|
||||
last_proc = proc
|
||||
@@ -367,10 +428,10 @@ def fetch_prune(repo_dir: Path, *, token: str | None) -> None:
|
||||
_check(last_proc, ["git", *args])
|
||||
|
||||
|
||||
def fetch_ref(repo_dir: Path, ref: str, *, token: str | None) -> None:
|
||||
def fetch_ref(repo_dir: Path, ref: str, *, token: str | None, safe_directory: Path | None = None) -> None:
|
||||
"""`git fetch origin <ref>` (best-effort: caller decides to swallow)."""
|
||||
args = ["fetch", "origin", ref]
|
||||
proc = _run_git(args, cwd=repo_dir, token=token)
|
||||
proc = _run_git(args, cwd=repo_dir, token=token, safe_directory=safe_directory)
|
||||
if proc.returncode != 0:
|
||||
log.debug(
|
||||
"fetch_ref non-fatal failure",
|
||||
@@ -392,22 +453,29 @@ class HeadDriftError(GitCommandError):
|
||||
"""
|
||||
|
||||
|
||||
def rev_parse_head(repo_dir: Path) -> str:
|
||||
def rev_parse_head(
|
||||
repo_dir: Path,
|
||||
*,
|
||||
safe_directory: Path | None = None,
|
||||
user: int | None = None,
|
||||
group: int | None = None,
|
||||
extra_groups: list[int] | tuple[int, ...] | None = None,
|
||||
umask: int | None = None,
|
||||
) -> str:
|
||||
"""Return the SHA of HEAD or raise GitCommandError."""
|
||||
proc = subprocess.run(
|
||||
["git", "rev-parse", "HEAD"],
|
||||
cwd=str(repo_dir),
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
args = ["rev-parse", "HEAD"]
|
||||
proc = _run_git(
|
||||
args,
|
||||
cwd=repo_dir,
|
||||
token=None,
|
||||
safe_directory=safe_directory,
|
||||
user=user,
|
||||
group=group,
|
||||
extra_groups=extra_groups,
|
||||
umask=umask,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
raise GitCommandError(
|
||||
["git", "rev-parse", "HEAD"],
|
||||
proc.returncode,
|
||||
proc.stdout,
|
||||
proc.stderr,
|
||||
)
|
||||
raise GitCommandError(["git", *args], proc.returncode, proc.stdout, proc.stderr)
|
||||
return proc.stdout.strip()
|
||||
|
||||
|
||||
@@ -417,6 +485,8 @@ def push(
|
||||
branch: str,
|
||||
expected_head: str | None,
|
||||
token: str | None,
|
||||
slot_uid: int | None = None,
|
||||
safe_directory: Path | None = None,
|
||||
) -> PushResult:
|
||||
"""`git push --force-with-lease=<ref>:<sha> --set-upstream origin <branch>` from `repo_dir`.
|
||||
|
||||
@@ -440,7 +510,12 @@ def push(
|
||||
the push is aborted with `HeadDriftError`. This is a separate concern from
|
||||
`--force-with-lease`, which compares against the remote ref.
|
||||
"""
|
||||
head = rev_parse_head(repo_dir)
|
||||
slot_kwargs = _slot_subprocess_kwargs(slot_uid)
|
||||
git_safe_directory = safe_directory
|
||||
if git_safe_directory is None and slot_kwargs:
|
||||
git_safe_directory = repo_dir
|
||||
|
||||
head = rev_parse_head(repo_dir, safe_directory=git_safe_directory, **slot_kwargs)
|
||||
if expected_head and head != expected_head:
|
||||
raise HeadDriftError(
|
||||
["git", "push"],
|
||||
@@ -455,11 +530,27 @@ def push(
|
||||
["rev-parse", "--verify", "--quiet", f"refs/remotes/origin/{branch}"],
|
||||
cwd=repo_dir,
|
||||
token=None,
|
||||
safe_directory=git_safe_directory,
|
||||
**slot_kwargs,
|
||||
)
|
||||
expected_remote = probe.stdout.strip() if probe.returncode == 0 else ""
|
||||
push_extra_env: dict[str, str] | None = None
|
||||
origin = _run_git(
|
||||
["remote", "get-url", "origin"], cwd=repo_dir, token=None, safe_directory=git_safe_directory, **slot_kwargs
|
||||
)
|
||||
if origin.returncode == 0:
|
||||
local_remote = _local_remote_safe_directory(origin.stdout, cwd=repo_dir)
|
||||
if local_remote is not None:
|
||||
push_extra_env = {}
|
||||
_append_safe_directory(push_extra_env, local_remote)
|
||||
lease = f"--force-with-lease=refs/heads/{branch}:{expected_remote}"
|
||||
args = ["push", lease, "--set-upstream", "origin", branch]
|
||||
_check(_run_git(args, cwd=repo_dir, token=token), ["git", *args])
|
||||
_check(
|
||||
_run_git(
|
||||
args, cwd=repo_dir, token=token, extra_env=push_extra_env, safe_directory=git_safe_directory, **slot_kwargs
|
||||
),
|
||||
["git", *args],
|
||||
)
|
||||
return PushResult(head=head, branch=branch)
|
||||
|
||||
|
||||
|
||||
+94
-76
@@ -9,6 +9,7 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import subprocess
|
||||
import time
|
||||
from collections.abc import Callable, Mapping
|
||||
@@ -22,14 +23,32 @@ from omp_rpc import HostTool, HostToolContext, RpcCommandError, host_tool
|
||||
from robomp import persona
|
||||
from robomp.config import Settings
|
||||
from robomp.db import Database, issue_key
|
||||
from robomp.git_ops import GitCommandError, HeadDriftError, rev_parse_head
|
||||
from robomp.git_ops import GitCommandError, HeadDriftError
|
||||
from robomp.github_backend import GitHubBackend
|
||||
from robomp.github_client import GitHubError, IssueInfo, RepoInfo
|
||||
from robomp.sandbox import GitTransport, Workspace, rename_workspace_branch, validate_branch_slug, workspace_key
|
||||
from robomp.sandbox import (
|
||||
GitTransport,
|
||||
Workspace,
|
||||
_prepare_slot_runtime_env,
|
||||
_safe_directory_env,
|
||||
_share_git_metadata_with_slots,
|
||||
_slot_permissions_active,
|
||||
_slot_subprocess_kwargs,
|
||||
rename_workspace_branch,
|
||||
validate_branch_slug,
|
||||
workspace_key,
|
||||
)
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
_PRE_PR_FIX_COMMAND = ("bun", "run", "fix")
|
||||
_PRE_PR_CHECK_COMMAND = ("bun", "check")
|
||||
_REPO_COMMAND_SCRUBBED_ENV_KEYS: tuple[str, ...] = (
|
||||
"GITHUB_TOKEN",
|
||||
"GITHUB_WEBHOOK_SECRET",
|
||||
"ROBOMP_REPLAY_TOKEN",
|
||||
"ROBOMP_GH_PROXY_HMAC_KEY",
|
||||
)
|
||||
_AGENT_HOME = Path("/srv/agent-home")
|
||||
_PRE_PR_FIX_TIMEOUT_SECONDS = 600.0
|
||||
_PRE_PR_CHECK_TIMEOUT_SECONDS = 600.0
|
||||
_PRE_PR_CHECK_MAX_OUTPUT = 12_000
|
||||
@@ -127,6 +146,55 @@ def _raise_command(message: str) -> NoReturn:
|
||||
raise RpcCommandError(message, error={"message": message})
|
||||
|
||||
|
||||
def _git_identity_env(author_name: str, author_email: str) -> dict[str, str]:
|
||||
"""Environment forcing agent git commits to use the configured bot identity."""
|
||||
return {
|
||||
"GIT_AUTHOR_NAME": author_name,
|
||||
"GIT_AUTHOR_EMAIL": author_email,
|
||||
"GIT_COMMITTER_NAME": author_name,
|
||||
"GIT_COMMITTER_EMAIL": author_email,
|
||||
}
|
||||
|
||||
|
||||
def _repo_command_env(bindings: ToolBindings) -> dict[str, str]:
|
||||
"""Environment for repo-owned commands (`bun`, formatter, local git).
|
||||
|
||||
These commands execute code from the checked-out repository, so they must
|
||||
not inherit GitHub credentials from the orchestrator. They also need the
|
||||
exact same HOME/XDG/TMP/Bun cache paths as the agent process; otherwise
|
||||
host-side pre-publish gates validate a different machine than the agent saw.
|
||||
"""
|
||||
env = os.environ.copy()
|
||||
for key in _REPO_COMMAND_SCRUBBED_ENV_KEYS:
|
||||
env[key] = ""
|
||||
if _AGENT_HOME.is_dir():
|
||||
env["HOME"] = str(_AGENT_HOME)
|
||||
env.update(_prepare_slot_runtime_env(bindings.workspace, bindings.slot_uid))
|
||||
env.update(_safe_directory_env(bindings.workspace.repo_dir))
|
||||
env.update(_git_identity_env(bindings.author_name, bindings.author_email))
|
||||
env["GIT_TERMINAL_PROMPT"] = "0"
|
||||
return env
|
||||
|
||||
|
||||
def _run_repo_command(
|
||||
bindings: ToolBindings,
|
||||
cmd: list[str] | tuple[str, ...],
|
||||
*,
|
||||
timeout: float | None = None,
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
"""Run a repo-local command with agent-equivalent permissions and env."""
|
||||
return subprocess.run(
|
||||
list(cmd),
|
||||
cwd=str(bindings.workspace.repo_dir),
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=timeout,
|
||||
env=_repo_command_env(bindings),
|
||||
**_slot_subprocess_kwargs(bindings.slot_uid),
|
||||
)
|
||||
|
||||
|
||||
def _has_bun_script(repo_dir: Path, name: str) -> bool:
|
||||
"""Return True iff `package.json` defines a `scripts.<name>` entry.
|
||||
|
||||
@@ -198,19 +266,12 @@ def _run_pre_publish_bun_fix(
|
||||
"""
|
||||
if not _has_bun_script(bindings.workspace.repo_dir, "fix"):
|
||||
return
|
||||
repo_dir = str(bindings.workspace.repo_dir)
|
||||
# Dirty-tree gate BEFORE the formatter so any pre-existing uncommitted
|
||||
# edit isn't silently swept into the `style: bun run fix` commit by the
|
||||
# `git add -A` below. The agent owns the worktree end-to-end; any diff
|
||||
# not already in a commit is a workflow bug it must resolve before we
|
||||
# mutate the tree further.
|
||||
pre_status = subprocess.run(
|
||||
["git", "status", "--porcelain", "--untracked-files=normal"],
|
||||
cwd=repo_dir,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
pre_status = _run_repo_command(bindings, ["git", "status", "--porcelain", "--untracked-files=normal"])
|
||||
if pre_status.stdout.strip():
|
||||
dirty = "\n ".join(pre_status.stdout.strip().splitlines())
|
||||
msg = (
|
||||
@@ -231,14 +292,7 @@ def _run_pre_publish_bun_fix(
|
||||
)
|
||||
return
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
_PRE_PR_FIX_COMMAND,
|
||||
cwd=repo_dir,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=_PRE_PR_FIX_TIMEOUT_SECONDS,
|
||||
)
|
||||
proc = _run_repo_command(bindings, _PRE_PR_FIX_COMMAND, timeout=_PRE_PR_FIX_TIMEOUT_SECONDS)
|
||||
except FileNotFoundError:
|
||||
msg = f"refusing to {stage}: `bun run fix` is required before {stage}, but `bun` is not on PATH."
|
||||
_audit(bindings, tool_name, args, error=msg)
|
||||
@@ -264,29 +318,18 @@ def _run_pre_publish_bun_fix(
|
||||
_audit(bindings, tool_name, args, error=msg)
|
||||
_raise_command(msg)
|
||||
|
||||
status = subprocess.run(
|
||||
["git", "status", "--porcelain", "--untracked-files=normal"],
|
||||
cwd=repo_dir,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
status = _run_repo_command(bindings, ["git", "status", "--porcelain", "--untracked-files=normal"])
|
||||
if not status.stdout.strip():
|
||||
return
|
||||
|
||||
add = subprocess.run(
|
||||
["git", "add", "-A"],
|
||||
cwd=repo_dir,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
add = _run_repo_command(bindings, ["git", "add", "-A"])
|
||||
if add.returncode != 0:
|
||||
err = (add.stderr or add.stdout).strip()
|
||||
msg = f"refusing to {stage}: `git add -A` failed after `bun run fix`: {err}"
|
||||
_audit(bindings, tool_name, args, error=msg)
|
||||
_raise_command(msg)
|
||||
commit = subprocess.run(
|
||||
commit = _run_repo_command(
|
||||
bindings,
|
||||
[
|
||||
"git",
|
||||
"-c",
|
||||
@@ -297,10 +340,6 @@ def _run_pre_publish_bun_fix(
|
||||
"-m",
|
||||
_PRE_PR_FIX_COMMIT_SUBJECT,
|
||||
],
|
||||
cwd=repo_dir,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if commit.returncode != 0:
|
||||
err = (commit.stderr or commit.stdout).strip()
|
||||
@@ -332,14 +371,7 @@ def _run_pre_publish_bun_check(
|
||||
if not _has_bun_script(bindings.workspace.repo_dir, "check"):
|
||||
return
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
_PRE_PR_CHECK_COMMAND,
|
||||
cwd=str(bindings.workspace.repo_dir),
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=_PRE_PR_CHECK_TIMEOUT_SECONDS,
|
||||
)
|
||||
proc = _run_repo_command(bindings, _PRE_PR_CHECK_COMMAND, timeout=_PRE_PR_CHECK_TIMEOUT_SECONDS)
|
||||
except FileNotFoundError:
|
||||
msg = f"refusing to {stage}: `bun check` is required before {stage}, but `bun` is not on PATH."
|
||||
_audit(bindings, tool_name, args, error=msg)
|
||||
@@ -486,40 +518,24 @@ def _guarded_push_branch(bindings: ToolBindings, args: Mapping[str, Any], tool_n
|
||||
_raise_command(
|
||||
f"refusing to push: branch={branch!r} does not match workspace branch {bindings.workspace.branch!r}."
|
||||
)
|
||||
repo_dir = str(bindings.workspace.repo_dir)
|
||||
# Re-pin the configured identity right before push (cheap; idempotent).
|
||||
subprocess.run(
|
||||
["git", "config", "user.email", bindings.author_email],
|
||||
cwd=repo_dir,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "config", "user.name", bindings.author_name],
|
||||
cwd=repo_dir,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
_run_repo_command(bindings, ["git", "config", "user.email", bindings.author_email])
|
||||
_run_repo_command(bindings, ["git", "config", "user.name", bindings.author_name])
|
||||
repo_dir_path = bindings.workspace.repo_dir
|
||||
try:
|
||||
head_sha = rev_parse_head(repo_dir_path)
|
||||
except GitCommandError as exc:
|
||||
err = (exc.stderr or exc.stdout).strip() or f"exit {exc.returncode}"
|
||||
head_proc = _run_repo_command(bindings, ["git", "rev-parse", "HEAD"])
|
||||
if head_proc.returncode != 0:
|
||||
err = (head_proc.stderr or head_proc.stdout).strip() or f"exit {head_proc.returncode}"
|
||||
_audit(bindings, tool_name, args, error=err)
|
||||
_raise_command(f"git rev-parse failed: {err}")
|
||||
head_sha = head_proc.stdout.strip()
|
||||
|
||||
# Identity gate: every commit between the base branch and HEAD must
|
||||
# carry the configured author. Refuse to push otherwise so the agent
|
||||
# fixes it (`git commit --amend --reset-author --no-edit`).
|
||||
base = bindings.repo.default_branch
|
||||
identities = subprocess.run(
|
||||
identities = _run_repo_command(
|
||||
bindings,
|
||||
["git", "log", "--format=%H%x09%ae%x09%an", f"origin/{base}..HEAD"],
|
||||
cwd=repo_dir,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if identities.returncode != 0:
|
||||
err = (identities.stderr or identities.stdout).strip()
|
||||
@@ -551,13 +567,7 @@ def _guarded_push_branch(bindings: ToolBindings, args: Mapping[str, Any], tool_n
|
||||
# forgot to `git add && git commit`, files dropped by package managers, etc.)
|
||||
# would silently land in the PR review delta but not in the commit history.
|
||||
# Reject so the agent either commits or stashes them.
|
||||
status = subprocess.run(
|
||||
["git", "status", "--porcelain", "--untracked-files=normal"],
|
||||
cwd=repo_dir,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
status = _run_repo_command(bindings, ["git", "status", "--porcelain", "--untracked-files=normal"])
|
||||
if status.stdout.strip():
|
||||
dirty = "\n ".join(status.stdout.strip().splitlines())
|
||||
msg = (
|
||||
@@ -576,6 +586,7 @@ def _guarded_push_branch(bindings: ToolBindings, args: Mapping[str, Any], tool_n
|
||||
repo_dir=repo_dir_path,
|
||||
branch=branch,
|
||||
expected_head=head_sha,
|
||||
slot_uid=bindings.slot_uid,
|
||||
)
|
||||
except HeadDriftError:
|
||||
msg = (
|
||||
@@ -592,6 +603,7 @@ def _guarded_push_branch(bindings: ToolBindings, args: Mapping[str, Any], tool_n
|
||||
msg = f"gh-proxy rejected push: {exc.status} {exc.message}"
|
||||
_audit(bindings, tool_name, args, error=msg)
|
||||
_raise_command(msg)
|
||||
_share_git_metadata_with_slots(repo_dir_path, bindings.slot_uid)
|
||||
_audit(bindings, tool_name, args, result={"head": result.head, "branch": result.branch})
|
||||
return result.head
|
||||
|
||||
@@ -803,6 +815,12 @@ def _build_repro_record(bindings: ToolBindings) -> HostTool[Any, Any]:
|
||||
f"## Output\n\n```\n{output}\n```\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
# Single-ownership invariant: workspace files belong to the active
|
||||
# slot. The orchestrator (root) wrote this file directly, so hand it
|
||||
# over before the audit row lands so the agent can edit/delete it.
|
||||
if _slot_permissions_active(bindings.slot_uid):
|
||||
assert bindings.slot_uid is not None
|
||||
os.chown(target, bindings.slot_uid, bindings.slot_uid)
|
||||
_audit(bindings, "repro_record", args, result={"path": str(target.relative_to(bindings.workspace.root))})
|
||||
return "recorded"
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import os
|
||||
import subprocess
|
||||
from collections.abc import AsyncIterator
|
||||
from contextlib import asynccontextmanager
|
||||
@@ -43,6 +44,7 @@ from robomp.git_ops import (
|
||||
)
|
||||
from robomp.github_client import GitHubClient, GitHubError
|
||||
from robomp.proxy_hmac import HEADER_SIGNATURE, HEADER_TIMESTAMP, verify
|
||||
from robomp.sandbox import _safe_directory_env, _slot_subprocess_kwargs
|
||||
from robomp.sandbox import workspace_key as compute_workspace_key
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -102,6 +104,14 @@ def _require_int(value: Any, field: str) -> int:
|
||||
return value
|
||||
|
||||
|
||||
def _optional_slot_uid(value: Any) -> int | None:
|
||||
if value is None:
|
||||
return None
|
||||
if not isinstance(value, int) or isinstance(value, bool) or not (0 < value < 65536):
|
||||
raise HTTPException(400, "missing/invalid 'slot_uid'")
|
||||
return value
|
||||
|
||||
|
||||
def _optional_str_list(value: Any, field: str) -> list[str] | None:
|
||||
if value is None:
|
||||
return None
|
||||
@@ -140,8 +150,10 @@ def _resolve_hmac_key(cfg: Settings) -> bytes:
|
||||
_ORIGIN_READ_TIMEOUT_SECONDS = 5.0
|
||||
|
||||
|
||||
def _read_origin_url(repo_dir: Path) -> str:
|
||||
def _read_origin_url(repo_dir: Path, slot_uid: int | None = None) -> str:
|
||||
"""Return the worktree's `origin` remote URL, or raise HTTPException."""
|
||||
env = {**os.environ, "GIT_TERMINAL_PROMPT": "0"}
|
||||
env.update(_safe_directory_env(repo_dir))
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
["git", "-C", str(repo_dir), "remote", "get-url", "origin"],
|
||||
@@ -149,6 +161,8 @@ def _read_origin_url(repo_dir: Path) -> str:
|
||||
text=True,
|
||||
check=False,
|
||||
timeout=_ORIGIN_READ_TIMEOUT_SECONDS,
|
||||
env=env,
|
||||
**_slot_subprocess_kwargs(slot_uid),
|
||||
)
|
||||
except subprocess.TimeoutExpired as exc:
|
||||
raise HTTPException(504, "timeout reading origin url") from exc
|
||||
@@ -161,7 +175,7 @@ def _read_origin_url(repo_dir: Path) -> str:
|
||||
return proc.stdout.strip()
|
||||
|
||||
|
||||
def _assert_origin_safe_for_repo(repo_dir: Path, expected_repo: str) -> None:
|
||||
def _assert_origin_safe_for_repo(repo_dir: Path, expected_repo: str, slot_uid: int | None = None) -> None:
|
||||
"""Refuse the push if the worktree's `origin` would leak the PAT.
|
||||
|
||||
The PAT is injected via `--config-env http.extraHeader=…` (see
|
||||
@@ -178,7 +192,7 @@ def _assert_origin_safe_for_repo(repo_dir: Path, expected_repo: str) -> None:
|
||||
`git remote set-url origin https://evil.example/x.git` and the proxy
|
||||
would happily push (with the PAT) to that remote.
|
||||
"""
|
||||
url = _read_origin_url(repo_dir)
|
||||
url = _read_origin_url(repo_dir, slot_uid=slot_uid)
|
||||
parsed = urlparse(url)
|
||||
scheme = (parsed.scheme or "").lower()
|
||||
if scheme not in ("http", "https"):
|
||||
@@ -561,6 +575,7 @@ def create_proxy_app(settings: Settings) -> FastAPI:
|
||||
workspace_key = _require_str(data.get("workspace_key"), "workspace_key")
|
||||
branch = _require_str(data.get("branch"), "branch")
|
||||
expected_head = _require_str(data.get("expected_head"), "expected_head")
|
||||
slot_uid = _optional_slot_uid(data.get("slot_uid"))
|
||||
# Sanity-check workspace_key matches the repo claim.
|
||||
expected_prefix = repo.replace("/", "__") + "__"
|
||||
if not workspace_key.startswith(expected_prefix):
|
||||
@@ -570,7 +585,7 @@ def create_proxy_app(settings: Settings) -> FastAPI:
|
||||
raise HTTPException(404, f"workspace not found: {workspace_key}")
|
||||
# Block attacker-controlled `origin` from being a PAT exfil channel.
|
||||
# MUST run BEFORE any subprocess that would inject the token header.
|
||||
await asyncio.to_thread(_assert_origin_safe_for_repo, repo_dir, repo)
|
||||
await asyncio.to_thread(_assert_origin_safe_for_repo, repo_dir, repo, slot_uid)
|
||||
try:
|
||||
result = await _run_git_op(
|
||||
git_push,
|
||||
@@ -578,6 +593,7 @@ def create_proxy_app(settings: Settings) -> FastAPI:
|
||||
branch=branch,
|
||||
expected_head=expected_head,
|
||||
token=_resolve_token(settings),
|
||||
slot_uid=slot_uid,
|
||||
)
|
||||
except HeadDriftError as exc:
|
||||
return _git_error_response(exc, head_drift=True)
|
||||
|
||||
@@ -368,17 +368,18 @@ class ProxyGitTransport:
|
||||
repo_dir: Path,
|
||||
branch: str,
|
||||
expected_head: str,
|
||||
slot_uid: int | None = None,
|
||||
) -> PushResult:
|
||||
del repo_dir
|
||||
data = self._post(
|
||||
"/gh/v1/git/push",
|
||||
{
|
||||
"repo": repo,
|
||||
"workspace_key": workspace_key,
|
||||
"branch": branch,
|
||||
"expected_head": expected_head,
|
||||
},
|
||||
)
|
||||
body: dict[str, Any] = {
|
||||
"repo": repo,
|
||||
"workspace_key": workspace_key,
|
||||
"branch": branch,
|
||||
"expected_head": expected_head,
|
||||
}
|
||||
if slot_uid is not None:
|
||||
body["slot_uid"] = slot_uid
|
||||
data = self._post("/gh/v1/git/push", body)
|
||||
return PushResult(head=str(data.get("head") or expected_head), branch=str(data.get("branch") or branch))
|
||||
|
||||
|
||||
|
||||
+192
-20
@@ -8,6 +8,32 @@ in `robomp.proxy_client` forwards the same set of operations over HMAC RPC.
|
||||
|
||||
Per-issue worktree add/remove stays local — those operations only touch the
|
||||
shared on-disk pool clone, no remote authentication required.
|
||||
|
||||
Permission model
|
||||
----------------
|
||||
There are four ownership zones on disk; do not let them blur:
|
||||
|
||||
1. **Workspace tree** (`/data/workspaces/<key>/`, including `repo/`,
|
||||
`.omp-session/`, `context/`, `artifacts/`, `.omp-tmp/`, `.omp-xdg/`):
|
||||
single-owner. Owned by the active slot UID/GID (`omp-N`) with mode
|
||||
`u=rwX,g=rwX,o=` (effectively `0770` dirs / `0660` files; the group is
|
||||
the slot's own private gid so group bits are functionally identical to
|
||||
owner-only). The orchestrator (root) reads/writes via uid-0 bypass when
|
||||
it must, and drops to the slot for any subprocess that touches paths the
|
||||
agent will revisit. `ensure_workspace` + `_chown_workspace` are the
|
||||
single point of truth for this zone — no other helper sets ownership
|
||||
inside `ws_root`.
|
||||
2. **Clone pool** (`/data/workspaces/_pool/<owner>__<repo>/`): genuinely
|
||||
multi-slot. Owned by `root:omp` (gid 2000) with setgid `02770`; cross-slot
|
||||
writes are bridged by `_share_git_metadata_with_slots`.
|
||||
3. **Language tool caches** (`/data/cache/{cargo,cargo-target,rustup,bun-cache}`):
|
||||
multi-slot. Owned by `root:omp` with setgid `02770`; provisioned by
|
||||
`entrypoint.sh`.
|
||||
4. **Agent HOME template** (`/srv/agent-home`): read-only, `root:root`
|
||||
`0755/0644`.
|
||||
|
||||
Bun's install cache stays workspace-private (zone 1) on purpose — bun
|
||||
chmod/utimes its own cache root, which breaks any shared-cache scheme.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -24,7 +50,7 @@ import stat
|
||||
import subprocess
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Protocol
|
||||
from typing import Any, Protocol
|
||||
|
||||
from robomp.git_ops import (
|
||||
GitCommandError,
|
||||
@@ -86,6 +112,22 @@ def workspace_key(repo: str, number: int) -> str:
|
||||
return f"{repo.replace('/', '__')}__{number}"
|
||||
|
||||
|
||||
def _safe_directory_env(repo_dir: Path) -> dict[str, str]:
|
||||
"""Return a Git config env overlay whitelisting ``repo_dir`` as safe."""
|
||||
return {
|
||||
"GIT_CONFIG_COUNT": "1",
|
||||
"GIT_CONFIG_KEY_0": "safe.directory",
|
||||
"GIT_CONFIG_VALUE_0": str(repo_dir),
|
||||
}
|
||||
|
||||
|
||||
def _git_env_for_repo(repo_dir: Path) -> dict[str, str]:
|
||||
env = os.environ.copy()
|
||||
env.update(_safe_directory_env(repo_dir))
|
||||
env["GIT_TERMINAL_PROMPT"] = "0"
|
||||
return env
|
||||
|
||||
|
||||
def make_branch(*, issue_number: int, title: str, seed: str | None = None) -> str:
|
||||
return f"farm/{_short_hex(seed or f'{issue_number}-{title}')}/{_slug(title or f'issue-{issue_number}')}"
|
||||
|
||||
@@ -150,6 +192,7 @@ def rename_workspace_branch(
|
||||
proc = _safe_run(
|
||||
["git", "branch", "-m", workspace.branch, new_branch],
|
||||
cwd=workspace.repo_dir,
|
||||
**_slot_subprocess_kwargs(slot_uid),
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
raise GitCommandError(
|
||||
@@ -194,6 +237,7 @@ class GitTransport(Protocol):
|
||||
repo_dir: Path,
|
||||
branch: str,
|
||||
expected_head: str,
|
||||
slot_uid: int | None = None,
|
||||
) -> PushResult:
|
||||
"""Push `branch` to origin. MUST refuse if HEAD has drifted from `expected_head`."""
|
||||
...
|
||||
@@ -232,15 +276,16 @@ class LocalGitTransport:
|
||||
repo_dir: Path,
|
||||
branch: str,
|
||||
expected_head: str,
|
||||
slot_uid: int | None = None,
|
||||
) -> PushResult:
|
||||
del repo, workspace_key
|
||||
return git_push(repo_dir, branch=branch, expected_head=expected_head, token=self._token)
|
||||
return git_push(repo_dir, branch=branch, expected_head=expected_head, token=self._token, slot_uid=slot_uid)
|
||||
|
||||
|
||||
# ---------- low-level helpers retained for callers expecting old shape ----------
|
||||
|
||||
|
||||
def _safe_run(cmd: list[str], *, cwd: Path | None = None) -> subprocess.CompletedProcess[str]:
|
||||
def _safe_run(cmd: list[str], *, cwd: Path | None = None, **kwargs: Any) -> subprocess.CompletedProcess[str]:
|
||||
"""Run without raising; caller decides on returncode. Credentials are redacted from any captured output."""
|
||||
proc = subprocess.run(
|
||||
cmd,
|
||||
@@ -248,6 +293,7 @@ def _safe_run(cmd: list[str], *, cwd: Path | None = None) -> subprocess.Complete
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
**kwargs,
|
||||
)
|
||||
if proc.stdout:
|
||||
proc.stdout = redact_credentials(proc.stdout)
|
||||
@@ -338,7 +384,18 @@ def _reap_slot(slot_uid: int | None) -> None:
|
||||
|
||||
|
||||
def _prepare_slot_tmpdir(workspace: Workspace, slot_uid: int | None) -> Path:
|
||||
"""Create the per-workspace temp directory used by the agent subprocess."""
|
||||
"""Return the per-workspace tmpdir path, idempotently provisioning it.
|
||||
|
||||
Ownership/mode is set by ``_chown_workspace`` as part of the workspace's
|
||||
single-ownership invariant; this helper only:
|
||||
|
||||
- replaces any non-directory at ``.omp-tmp`` (symlink-protection: a user
|
||||
who plants a symlink there could redirect later writes outside the
|
||||
workspace regardless of who owns the destination), and
|
||||
- ``mkdir(mode=0o700, exist_ok=True)`` as a safety net for callers that
|
||||
run before ``ensure_workspace`` (e.g. unit tests with ``slot_uid=None``).
|
||||
"""
|
||||
del slot_uid # ownership is _chown_workspace's job; kept for call-site parity
|
||||
tmpdir = workspace.root / ".omp-tmp"
|
||||
try:
|
||||
st = tmpdir.lstat()
|
||||
@@ -348,13 +405,89 @@ def _prepare_slot_tmpdir(workspace: Workspace, slot_uid: int | None) -> Path:
|
||||
if not stat.S_ISDIR(st.st_mode):
|
||||
tmpdir.unlink()
|
||||
tmpdir.mkdir(mode=0o700, parents=True, exist_ok=True)
|
||||
if _slot_permissions_active(slot_uid):
|
||||
assert slot_uid is not None
|
||||
os.chown(tmpdir, slot_uid, slot_uid)
|
||||
tmpdir.chmod(0o700)
|
||||
return tmpdir
|
||||
|
||||
|
||||
def _slot_subprocess_kwargs(slot_uid: int | None) -> dict[str, Any]:
|
||||
"""Return subprocess identity kwargs for commands that should run as a slot.
|
||||
|
||||
`preexec_fn` is intentionally avoided: the worker runs tasks in threads,
|
||||
and `subprocess` warns that `preexec_fn` is unsafe in multithreaded
|
||||
parents. Python's native `user` / `group` / `extra_groups` parameters do
|
||||
the setuid/setgid work in the child safely.
|
||||
"""
|
||||
if not _slot_permissions_active(slot_uid):
|
||||
return {}
|
||||
assert slot_uid is not None
|
||||
return {"user": slot_uid, "group": slot_uid, "extra_groups": [_SHARED_OMP_GID], "umask": 0o002}
|
||||
|
||||
|
||||
def _prepare_slot_runtime_env(workspace: Workspace, slot_uid: int | None) -> dict[str, str]:
|
||||
"""Compute the env overlay (TMPDIR + XDG_*) for slot-side subprocesses.
|
||||
|
||||
Pure env helper: ownership of the workspace tree (including these XDG
|
||||
paths and the bun install cache) is the single responsibility of
|
||||
``ensure_workspace``/``_chown_workspace``. The mkdir calls here exist
|
||||
only as a safety net for callers that bypass ``ensure_workspace`` (unit
|
||||
tests) or for the case where a runtime dir was deleted mid-process.
|
||||
|
||||
Cargo/rustup/target caches live under ``/data/cache/*`` (container ENV)
|
||||
and are group-shared via ``omp``. Bun's install cache is explicitly
|
||||
workspace-private because bun chmod/chowns its cache root, which makes a
|
||||
cross-slot shared cache a permanent source of permission failures.
|
||||
"""
|
||||
tmpdir = _prepare_slot_tmpdir(workspace, slot_uid)
|
||||
xdg_root = workspace.root / ".omp-xdg"
|
||||
xdg_data = xdg_root / "data"
|
||||
xdg_state = xdg_root / "state"
|
||||
xdg_cache = xdg_root / "cache"
|
||||
bun_cache = xdg_cache / "bun-install"
|
||||
|
||||
for base in (xdg_data, xdg_state, xdg_cache):
|
||||
base.mkdir(parents=True, exist_ok=True)
|
||||
(base / "omp").mkdir(parents=True, exist_ok=True)
|
||||
bun_cache.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
return {
|
||||
"TMPDIR": str(tmpdir),
|
||||
"TMP": str(tmpdir),
|
||||
"TEMP": str(tmpdir),
|
||||
"XDG_DATA_HOME": str(xdg_data),
|
||||
"XDG_STATE_HOME": str(xdg_state),
|
||||
"XDG_CACHE_HOME": str(xdg_cache),
|
||||
"BUN_INSTALL_CACHE_DIR": str(bun_cache),
|
||||
}
|
||||
|
||||
|
||||
def _provision_runtime_dirs(ws_root: Path) -> None:
|
||||
"""Create the runtime dirs that ``_chown_workspace`` will hand to the slot.
|
||||
|
||||
Runs immediately before ``_chown_workspace`` so the recursive chown sweep
|
||||
picks up ``.omp-tmp`` and the per-workspace XDG tree. Without this,
|
||||
``_prepare_slot_runtime_env`` would create them later from the orchestrator
|
||||
process — leaving root-owned cache roots that bun/biome/cargo cannot
|
||||
chmod/utime, the original source of the recurring permission failures.
|
||||
|
||||
Symlink-safe on ``.omp-tmp`` (replaces a planted non-directory in place).
|
||||
"""
|
||||
tmpdir = ws_root / ".omp-tmp"
|
||||
try:
|
||||
st = tmpdir.lstat()
|
||||
except FileNotFoundError:
|
||||
pass
|
||||
else:
|
||||
if not stat.S_ISDIR(st.st_mode):
|
||||
tmpdir.unlink()
|
||||
tmpdir.mkdir(mode=0o700, parents=True, exist_ok=True)
|
||||
|
||||
xdg_root = ws_root / ".omp-xdg"
|
||||
for sub in ("data", "state", "cache"):
|
||||
base = xdg_root / sub
|
||||
base.mkdir(parents=True, exist_ok=True)
|
||||
(base / "omp").mkdir(parents=True, exist_ok=True)
|
||||
(xdg_root / "cache" / "bun-install").mkdir(parents=True, exist_ok=True)
|
||||
|
||||
|
||||
def _grant_group_bits(path: Path, *, gid: int, bits: int) -> None:
|
||||
try:
|
||||
st = path.lstat()
|
||||
@@ -436,19 +569,31 @@ def _share_git_metadata_with_slots(repo_dir: Path, slot_uid: int | None) -> None
|
||||
_grant_tree(common_dir / rel, gid=gid, files_group_writable=True)
|
||||
|
||||
|
||||
# slot_uid is also the slot-private GID created by entrypoint.sh. Do not use
|
||||
# the shared omp group for the workspace tree; that would let every slot read
|
||||
# every other slot's checkout, artifacts, context, and .omp-session. A retry may
|
||||
# acquire a different slot, so we recursively hand the private workspace tree to
|
||||
# the current slot before launching `omp --continue`.
|
||||
def _chown_workspace(ws_root: Path, slot_uid: int | None) -> None:
|
||||
"""Hand the entire workspace tree to the active slot UID/GID.
|
||||
|
||||
Single-ownership invariant: every file under ``ws_root`` ends up owned by
|
||||
``slot_uid:slot_uid`` with mode ``u=rwX,g=rwX,o=`` (``0770`` dirs / ``0660``
|
||||
files). The slot's GID is its own private gid (created by entrypoint.sh),
|
||||
so the group bits are functionally identical to owner-only — they exist
|
||||
for parity with the existing pattern and to make accidental future
|
||||
``setgid`` use safe.
|
||||
|
||||
The orchestrator (root) keeps read/write access via uid-0 bypass; any
|
||||
subprocess that touches paths the agent will revisit MUST drop to the slot
|
||||
via ``_slot_subprocess_kwargs`` so tools like bun/biome/cargo (which
|
||||
chmod/utime their own cache state) never encounter a non-owner file.
|
||||
|
||||
Self-healing on re-entry: an existing workspace left over from the old
|
||||
``root:slot`` model gets re-chown'd on the next ``ensure_workspace`` call.
|
||||
"""
|
||||
if slot_uid is None:
|
||||
return
|
||||
if platform.system() != "Linux":
|
||||
return
|
||||
if os.geteuid() != 0:
|
||||
return
|
||||
subprocess.run(["chown", "-R", f"0:{slot_uid}", str(ws_root)], check=True)
|
||||
subprocess.run(["chown", "-R", f"{slot_uid}:{slot_uid}", str(ws_root)], check=True)
|
||||
subprocess.run(["chmod", "-R", "u=rwX,g=rwX,o=", str(ws_root)], check=True)
|
||||
|
||||
|
||||
@@ -544,7 +689,20 @@ class SandboxManager:
|
||||
seed=f"{repo}#{number}",
|
||||
)
|
||||
|
||||
if not (repo_dir / ".git").exists():
|
||||
repo_exists = (repo_dir / ".git").exists()
|
||||
workspace_prepared = False
|
||||
slot_git_kwargs = _slot_subprocess_kwargs(slot_uid)
|
||||
slot_git_env: dict[str, str] | None = None
|
||||
if repo_exists:
|
||||
# Existing workspaces are already slot-owned from the previous run.
|
||||
# Refresh pool-side group bits, then hand the tree to the current
|
||||
# slot before running any git command inside the worktree; root's
|
||||
# uid-0 bypass does not bypass git's safe.directory ownership check.
|
||||
_share_git_metadata_with_slots(repo_dir, slot_uid)
|
||||
_provision_runtime_dirs(ws_root)
|
||||
_chown_workspace(ws_root, slot_uid)
|
||||
workspace_prepared = True
|
||||
if not repo_exists:
|
||||
# Make sure the requested start point exists locally (best-effort).
|
||||
# For follow-ups on an existing PR, `existing_branch` is the remote
|
||||
# head branch we need to amend; starting from default would silently
|
||||
@@ -575,7 +733,13 @@ class SandboxManager:
|
||||
cwd=pool,
|
||||
)
|
||||
else:
|
||||
current = _safe_run(["git", "symbolic-ref", "--quiet", "--short", "HEAD"], cwd=repo_dir)
|
||||
slot_git_env = _git_env_for_repo(repo_dir)
|
||||
current = _safe_run(
|
||||
["git", "symbolic-ref", "--quiet", "--short", "HEAD"],
|
||||
cwd=repo_dir,
|
||||
env=slot_git_env,
|
||||
**slot_git_kwargs,
|
||||
)
|
||||
if current.returncode == 0 and current.stdout.strip():
|
||||
branch = current.stdout.strip()
|
||||
if existing_branch is not None and existing_branch != branch:
|
||||
@@ -584,11 +748,19 @@ class SandboxManager:
|
||||
existing_branch,
|
||||
branch,
|
||||
)
|
||||
# Identity is set on the worktree's shared config; idempotent.
|
||||
_safe_run(["git", "config", "user.email", author_email], cwd=repo_dir)
|
||||
_safe_run(["git", "config", "user.name", author_name], cwd=repo_dir)
|
||||
if not workspace_prepared:
|
||||
_share_git_metadata_with_slots(repo_dir, slot_uid)
|
||||
_provision_runtime_dirs(ws_root)
|
||||
_chown_workspace(ws_root, slot_uid)
|
||||
if slot_git_env is None:
|
||||
slot_git_env = _git_env_for_repo(repo_dir)
|
||||
# Identity is set on the worktree's shared config; idempotent. Run as
|
||||
# the slot after the chown so git never trips over safe.directory.
|
||||
for command in (["git", "config", "user.email", author_email], ["git", "config", "user.name", author_name]):
|
||||
proc = _safe_run(command, cwd=repo_dir, env=slot_git_env, **slot_git_kwargs)
|
||||
if proc.returncode != 0:
|
||||
raise GitCommandError(command, proc.returncode, proc.stdout, proc.stderr)
|
||||
_share_git_metadata_with_slots(repo_dir, slot_uid)
|
||||
_chown_workspace(ws_root, slot_uid)
|
||||
return Workspace(
|
||||
root=ws_root,
|
||||
repo_dir=repo_dir,
|
||||
|
||||
+5
-48
@@ -16,7 +16,6 @@ import asyncio
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import threading
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
@@ -36,8 +35,8 @@ from robomp.config import Settings
|
||||
from robomp.db import Database, issue_key
|
||||
from robomp.github_backend import GitHubBackend
|
||||
from robomp.github_client import CommentInfo, IssueInfo, RepoInfo
|
||||
from robomp.host_tools import AbortController, ToolBindings
|
||||
from robomp.sandbox import GitTransport, Workspace, _prepare_slot_tmpdir
|
||||
from robomp.host_tools import AbortController, ToolBindings, _git_identity_env
|
||||
from robomp.sandbox import GitTransport, Workspace, _prepare_slot_runtime_env, _safe_directory_env
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -186,48 +185,6 @@ def _build_extra_env(settings: Settings) -> dict[str, str]:
|
||||
return env
|
||||
|
||||
|
||||
def _prepare_xdg_dirs(workspace: Workspace, slot_uid: int | None) -> dict[str, str]:
|
||||
"""Prepare per-workspace XDG homes for mutable omp state."""
|
||||
xdg_root = workspace.root / ".omp-xdg"
|
||||
homes = {
|
||||
"XDG_DATA_HOME": xdg_root / "data",
|
||||
"XDG_STATE_HOME": xdg_root / "state",
|
||||
"XDG_CACHE_HOME": xdg_root / "cache",
|
||||
}
|
||||
should_chown = slot_uid is not None and os.geteuid() == 0
|
||||
for base in homes.values():
|
||||
omp_dir = base / "omp"
|
||||
base.mkdir(parents=True, exist_ok=True)
|
||||
omp_dir.mkdir(parents=True, exist_ok=True)
|
||||
if not should_chown:
|
||||
continue
|
||||
assert slot_uid is not None
|
||||
for path in (base, omp_dir):
|
||||
try:
|
||||
os.chown(path, 0, slot_uid)
|
||||
path.chmod(0o770)
|
||||
except OSError as exc:
|
||||
log.warning("Failed to make XDG directory accessible to slot user %s: %s", path, exc)
|
||||
if should_chown:
|
||||
assert slot_uid is not None
|
||||
# `sandbox._chown_workspace` runs `chown -R 0:slot` on the entire
|
||||
# workspace tree, which flips slot-created cache files under
|
||||
# `.omp-xdg/` (e.g. bun's `.pile` install cache) from `slot:slot`
|
||||
# to `root:slot`. The next bun install hits `PermissionDenied`
|
||||
# because bun chmod/utime's its own cache files and needs owner.
|
||||
# Restore slot ownership recursively so bun (and any other tool
|
||||
# using XDG paths) can touch its own cache.
|
||||
try:
|
||||
subprocess.run(
|
||||
["chown", "-R", f"{slot_uid}:{slot_uid}", str(xdg_root)],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
except (OSError, subprocess.CalledProcessError) as exc:
|
||||
log.warning("Failed to recursively chown XDG root to slot %s: %s", slot_uid, exc)
|
||||
return {key: str(path) for key, path in homes.items()}
|
||||
|
||||
|
||||
_TERMINAL_TRIAGE_TOOLS: frozenset[str] = frozenset({"gh_open_pr", "mark_unable_to_reproduce", "abort_task"})
|
||||
_PR_REQUIRING_CLASSIFICATIONS: frozenset[str] = frozenset({"bug", "documentation"})
|
||||
|
||||
@@ -451,9 +408,9 @@ def _run_rpc_blocking(
|
||||
log.debug("delta", extra={"issue": bindings.issue_key, "delta": str(ev.get("delta", ""))[:200]})
|
||||
|
||||
rpc_env = _build_extra_env(settings)
|
||||
slot_tmpdir = str(_prepare_slot_tmpdir(inputs.workspace, inputs.slot_uid))
|
||||
rpc_env.update({"TMPDIR": slot_tmpdir, "TMP": slot_tmpdir, "TEMP": slot_tmpdir})
|
||||
rpc_env.update(_prepare_xdg_dirs(inputs.workspace, inputs.slot_uid))
|
||||
rpc_env.update(_prepare_slot_runtime_env(inputs.workspace, inputs.slot_uid))
|
||||
rpc_env.update(_safe_directory_env(bindings.workspace.repo_dir))
|
||||
rpc_env.update(_git_identity_env(inputs.settings.resolved_author_name, inputs.settings.git_author_email))
|
||||
resuming = _has_prior_session(bindings.workspace.session_dir)
|
||||
extra_args: tuple[str, ...] = ("--continue",) if resuming else ()
|
||||
log.info(
|
||||
|
||||
+159
-1
@@ -12,6 +12,7 @@ import httpx
|
||||
import pytest
|
||||
from omp_rpc import HostToolContext, RpcCommandError
|
||||
|
||||
from robomp import host_tools
|
||||
from robomp.db import Database
|
||||
from robomp.github_client import GitHubClient, IssueInfo, RepoInfo
|
||||
from robomp.host_tools import AbortController, ToolBindings, build
|
||||
@@ -74,7 +75,7 @@ def _stop_loop(loop: asyncio.AbstractEventLoop, t: threading.Thread) -> None:
|
||||
|
||||
|
||||
def _bindings(
|
||||
db: Database, tmp_path: Path, transport: httpx.MockTransport
|
||||
db: Database, tmp_path: Path, transport: httpx.MockTransport, *, slot_uid: int | None = None
|
||||
) -> tuple[ToolBindings, asyncio.AbstractEventLoop, threading.Thread]:
|
||||
github = GitHubClient("token", transport=transport)
|
||||
loop, thread = _make_loop_in_background()
|
||||
@@ -88,6 +89,7 @@ def _bindings(
|
||||
loop=loop,
|
||||
author_name="robomp-bot",
|
||||
author_email="robomp-bot@example.invalid",
|
||||
slot_uid=slot_uid,
|
||||
)
|
||||
db.upsert_issue(
|
||||
key=bindings.issue_key,
|
||||
@@ -104,6 +106,137 @@ def _ctx() -> HostToolContext[Any]:
|
||||
return HostToolContext(tool_call_id="tc-1", _cancel_event=threading.Event(), _send_update=lambda _payload: None)
|
||||
|
||||
|
||||
def test_repo_command_env_scrubs_secrets_and_uses_workspace_cache(
|
||||
db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.setenv("GITHUB_TOKEN", "secret-token")
|
||||
monkeypatch.setenv("GITHUB_WEBHOOK_SECRET", "secret-webhook")
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_HMAC_KEY", "secret-proxy")
|
||||
monkeypatch.setenv("BUN_INSTALL_CACHE_DIR", "/data/cache/bun-cache")
|
||||
|
||||
bindings, loop, thread = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)), slot_uid=2001)
|
||||
try:
|
||||
env = host_tools._repo_command_env(bindings)
|
||||
finally:
|
||||
_stop_loop(loop, thread)
|
||||
|
||||
assert env["GITHUB_TOKEN"] == ""
|
||||
assert env["GITHUB_WEBHOOK_SECRET"] == ""
|
||||
assert env["ROBOMP_GH_PROXY_HMAC_KEY"] == ""
|
||||
assert env["BUN_INSTALL_CACHE_DIR"] == str(bindings.workspace.root / ".omp-xdg" / "cache" / "bun-install")
|
||||
assert env["XDG_CACHE_HOME"] == str(bindings.workspace.root / ".omp-xdg" / "cache")
|
||||
assert env["TMPDIR"] == str(bindings.workspace.root / ".omp-tmp")
|
||||
assert env["GIT_CONFIG_COUNT"] == "1"
|
||||
assert env["GIT_CONFIG_KEY_0"] == "safe.directory"
|
||||
assert env["GIT_CONFIG_VALUE_0"] == str(bindings.workspace.repo_dir)
|
||||
assert env["GIT_AUTHOR_NAME"] == bindings.author_name
|
||||
assert env["GIT_AUTHOR_EMAIL"] == bindings.author_email
|
||||
assert env["GIT_COMMITTER_NAME"] == bindings.author_name
|
||||
assert env["GIT_COMMITTER_EMAIL"] == bindings.author_email
|
||||
assert (bindings.workspace.root / ".omp-tmp").is_dir()
|
||||
|
||||
|
||||
def test_run_repo_command_uses_slot_identity_kwargs(
|
||||
db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
import subprocess
|
||||
|
||||
bindings, loop, thread = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)), slot_uid=2001)
|
||||
captured: dict[str, Any] = {}
|
||||
|
||||
monkeypatch.setattr(
|
||||
host_tools,
|
||||
"_slot_subprocess_kwargs",
|
||||
lambda uid: {"user": uid, "group": uid, "extra_groups": [2000], "umask": 0o002},
|
||||
)
|
||||
|
||||
def fake_run(cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]:
|
||||
captured["cmd"] = cmd
|
||||
captured["kwargs"] = kwargs
|
||||
return subprocess.CompletedProcess(cmd, 0, "ok", "")
|
||||
|
||||
monkeypatch.setattr(host_tools.subprocess, "run", fake_run) # type: ignore[attr-defined]
|
||||
try:
|
||||
proc = host_tools._run_repo_command(bindings, ["git", "status"])
|
||||
finally:
|
||||
_stop_loop(loop, thread)
|
||||
|
||||
assert proc.stdout == "ok"
|
||||
assert captured["cmd"] == ["git", "status"]
|
||||
kwargs = captured["kwargs"]
|
||||
assert kwargs["cwd"] == str(bindings.workspace.repo_dir)
|
||||
assert kwargs["user"] == 2001
|
||||
assert kwargs["group"] == 2001
|
||||
assert kwargs["extra_groups"] == [2000]
|
||||
assert kwargs["umask"] == 0o002
|
||||
assert kwargs["env"]["BUN_INSTALL_CACHE_DIR"].endswith("/.omp-xdg/cache/bun-install")
|
||||
|
||||
|
||||
def test_guarded_push_branch_rev_parse_runs_via_repo_command_and_passes_slot_uid(
|
||||
db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
import subprocess
|
||||
from dataclasses import replace
|
||||
|
||||
from robomp.git_ops import PushResult
|
||||
|
||||
class RecordingTransport:
|
||||
def __init__(self) -> None:
|
||||
self.calls: list[dict[str, Any]] = []
|
||||
|
||||
def push_branch(self, **kwargs: Any) -> PushResult:
|
||||
self.calls.append(kwargs)
|
||||
return PushResult(head=str(kwargs["expected_head"]), branch=str(kwargs["branch"]))
|
||||
|
||||
transport = RecordingTransport()
|
||||
bindings, loop, thread = _bindings(
|
||||
db,
|
||||
tmp_path,
|
||||
httpx.MockTransport(lambda _r: httpx.Response(500)),
|
||||
slot_uid=2001,
|
||||
)
|
||||
bindings = replace(bindings, git_transport=transport)
|
||||
commands: list[list[str]] = []
|
||||
|
||||
def fake_run_repo_command(
|
||||
command_bindings: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
del timeout
|
||||
assert command_bindings.slot_uid == 2001
|
||||
command = list(cmd)
|
||||
commands.append(command)
|
||||
if command == ["git", "rev-parse", "HEAD"]:
|
||||
return subprocess.CompletedProcess(command, 0, "abc123\n", "")
|
||||
if command[:3] == ["git", "log", "--format=%H%x09%ae%x09%an"]:
|
||||
return subprocess.CompletedProcess(
|
||||
command,
|
||||
0,
|
||||
"abc123\trobomp-bot@example.invalid\trobomp-bot\n",
|
||||
"",
|
||||
)
|
||||
return subprocess.CompletedProcess(command, 0, "", "")
|
||||
|
||||
monkeypatch.setattr(host_tools, "_run_repo_command", fake_run_repo_command)
|
||||
monkeypatch.setattr(host_tools, "_share_git_metadata_with_slots", lambda _repo_dir, _slot_uid: None)
|
||||
try:
|
||||
head = host_tools._guarded_push_branch(bindings, {}, "gh_push_branch", bindings.workspace.branch)
|
||||
finally:
|
||||
_stop_loop(loop, thread)
|
||||
|
||||
assert head == "abc123"
|
||||
assert ["git", "rev-parse", "HEAD"] in commands
|
||||
assert transport.calls == [
|
||||
{
|
||||
"repo": "octo/widget",
|
||||
"workspace_key": "octo__widget__42",
|
||||
"repo_dir": bindings.workspace.repo_dir,
|
||||
"branch": bindings.workspace.branch,
|
||||
"expected_head": "abc123",
|
||||
"slot_uid": 2001,
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def test_gh_post_comment_happy_path(db: Database, tmp_path: Path) -> None:
|
||||
captured: dict[str, Any] = {}
|
||||
|
||||
@@ -248,6 +381,31 @@ def test_repro_record_writes_transcript(db: Database, tmp_path: Path) -> None:
|
||||
_stop_loop(loop, t)
|
||||
|
||||
|
||||
def test_repro_record_chowns_to_slot_when_root(db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
chowns: list[tuple[Path, int, int]] = []
|
||||
monkeypatch.setattr(host_tools, "_slot_permissions_active", lambda slot_uid: slot_uid is not None)
|
||||
monkeypatch.setattr("robomp.host_tools.os.chown", lambda path, uid, gid: chowns.append((Path(path), uid, gid)))
|
||||
|
||||
bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda r: httpx.Response(500)), slot_uid=2001)
|
||||
try:
|
||||
tool = next(x for x in build(bindings) if x.name == "repro_record")
|
||||
result = tool.execute(
|
||||
{
|
||||
"title": "panic on empty input",
|
||||
"command": "bun test foo.test.ts",
|
||||
"output": "Error: boom",
|
||||
"exit_code": 1,
|
||||
},
|
||||
_ctx(),
|
||||
)
|
||||
assert result == "recorded"
|
||||
files = list(bindings.workspace.repro_dir.iterdir())
|
||||
assert len(files) == 1
|
||||
assert chowns == [(files[0], 2001, 2001)]
|
||||
finally:
|
||||
_stop_loop(loop, t)
|
||||
|
||||
|
||||
def test_repro_record_rejects_bad_args(db: Database, tmp_path: Path) -> None:
|
||||
bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda r: httpx.Response(500)))
|
||||
try:
|
||||
|
||||
@@ -0,0 +1,335 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
import platform
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
from collections.abc import Iterator
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp import host_tools
|
||||
from robomp.db import Database
|
||||
from robomp.github_backend import GitHubBackend
|
||||
from robomp.github_client import IssueInfo, RepoInfo
|
||||
from robomp.sandbox import LocalGitTransport, SandboxManager, Workspace
|
||||
|
||||
pytestmark = pytest.mark.skipif(
|
||||
os.environ.get("ROBOMP_PERMISSION_E2E") != "1",
|
||||
reason="set ROBOMP_PERMISSION_E2E=1 to run slot-permission e2e tests",
|
||||
)
|
||||
|
||||
_SLOT_ONE = 2001
|
||||
_SLOT_TWO = 2002
|
||||
_SHARED_OMP_GID = 2000
|
||||
_AUTHOR_NAME = "robomp-bot"
|
||||
_AUTHOR_EMAIL = "robomp-bot@example.invalid"
|
||||
_REPO = "octo/permission-e2e"
|
||||
|
||||
|
||||
def _require_linux_root_toolchain() -> None:
|
||||
if platform.system() != "Linux" or os.geteuid() != 0:
|
||||
pytest.skip("slot permission e2e tests require Linux root so subprocesses can drop to omp-N UIDs")
|
||||
missing = [cmd for cmd in ("git", "bun", "cargo", "python3") if shutil.which(cmd) is None]
|
||||
if missing:
|
||||
pytest.skip(f"slot permission e2e tests require tools on PATH: {', '.join(missing)}")
|
||||
|
||||
|
||||
def _git(args: list[str], cwd: Path, *, env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(
|
||||
["git", *args],
|
||||
cwd=str(cwd),
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
)
|
||||
|
||||
|
||||
def _write_seed_repo(seed: Path) -> None:
|
||||
(seed / "src").mkdir(parents=True)
|
||||
(seed / "crates" / "core" / "src").mkdir(parents=True)
|
||||
(seed / "package.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"name": "permission-e2e",
|
||||
"private": True,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"check": "bun run check:ts && cargo check --workspace",
|
||||
"check:ts": "biome check src/index.ts",
|
||||
"fix": "biome check --write --unsafe src/index.ts",
|
||||
},
|
||||
"devDependencies": {"@biomejs/biome": "^2.4.14"},
|
||||
},
|
||||
indent=2,
|
||||
)
|
||||
+ "\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / ".gitignore").write_text("node_modules/\n", encoding="utf-8")
|
||||
(seed / "src" / "index.ts").write_text("export const answer = 42;\n", encoding="utf-8")
|
||||
(seed / "Cargo.toml").write_text(
|
||||
'[workspace]\nmembers = ["crates/core"]\nresolver = "2"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / "rust-toolchain.toml").write_text(
|
||||
'[toolchain]\nchannel = "stable"\nprofile = "minimal"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / "crates" / "core" / "Cargo.toml").write_text(
|
||||
'[package]\nname = "permission-e2e-core"\nversion = "0.1.0"\nedition = "2021"\n\n[lib]\npath = "src/lib.rs"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / "crates" / "core" / "src" / "lib.rs").write_text(
|
||||
"pub fn answer() -> u32 {\n 42\n}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def slot_tmp_path() -> Iterator[Path]:
|
||||
root = Path(tempfile.mkdtemp(prefix="robomp-permission-e2e-", dir="/tmp"))
|
||||
root.chmod(0o755)
|
||||
try:
|
||||
yield root
|
||||
finally:
|
||||
shutil.rmtree(root, ignore_errors=True)
|
||||
|
||||
|
||||
def _share_tree_with_slots(path: Path) -> None:
|
||||
for root, dirs, files in os.walk(path):
|
||||
root_path = Path(root)
|
||||
os.chown(root_path, 0, _SHARED_OMP_GID)
|
||||
root_path.chmod(0o2770)
|
||||
for dirname in dirs:
|
||||
child = root_path / dirname
|
||||
os.chown(child, 0, _SHARED_OMP_GID)
|
||||
child.chmod(0o2770)
|
||||
for filename in files:
|
||||
child = root_path / filename
|
||||
executable = child.stat().st_mode & 0o111
|
||||
os.chown(child, 0, _SHARED_OMP_GID)
|
||||
child.chmod(0o770 if executable else 0o660)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def upstream_repo(slot_tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
||||
upstream = slot_tmp_path / "upstream.git"
|
||||
seed = slot_tmp_path / "seed"
|
||||
seed.mkdir()
|
||||
_write_seed_repo(seed)
|
||||
|
||||
_git(["init", "--initial-branch=main", "--bare", str(upstream)], cwd=slot_tmp_path)
|
||||
_git(["init", "--initial-branch=main", str(seed)], cwd=slot_tmp_path)
|
||||
_git(["-C", str(seed), "add", "."], cwd=slot_tmp_path)
|
||||
commit_env = os.environ | {
|
||||
"GIT_AUTHOR_NAME": "seed",
|
||||
"GIT_AUTHOR_EMAIL": "seed@example.invalid",
|
||||
"GIT_COMMITTER_NAME": "seed",
|
||||
"GIT_COMMITTER_EMAIL": "seed@example.invalid",
|
||||
}
|
||||
_git(["-C", str(seed), "commit", "-m", "seed"], cwd=slot_tmp_path, env=commit_env)
|
||||
_git(["-C", str(seed), "remote", "add", "origin", str(upstream)], cwd=slot_tmp_path)
|
||||
_git(["-C", str(seed), "push", "origin", "main"], cwd=slot_tmp_path)
|
||||
_share_tree_with_slots(upstream)
|
||||
git_system_config = slot_tmp_path / "git-system.conf"
|
||||
_git(["config", "--file", str(git_system_config), "--add", "safe.directory", str(upstream)], cwd=slot_tmp_path)
|
||||
git_system_config.chmod(0o644)
|
||||
monkeypatch.setenv("GIT_CONFIG_SYSTEM", str(git_system_config))
|
||||
return upstream
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def tool_loop() -> Iterator[asyncio.AbstractEventLoop]:
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
yield loop
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
|
||||
def _ensure_workspace(
|
||||
root: Path, upstream: Path, *, number: int, slot_uid: int, existing_branch: str | None = None
|
||||
) -> Workspace:
|
||||
manager = SandboxManager(root, transport=LocalGitTransport(token=None))
|
||||
return manager.ensure_workspace(
|
||||
repo=_REPO,
|
||||
number=number,
|
||||
title="permission e2e",
|
||||
clone_url=str(upstream),
|
||||
default_branch="main",
|
||||
existing_branch=existing_branch,
|
||||
author_name=_AUTHOR_NAME,
|
||||
author_email=_AUTHOR_EMAIL,
|
||||
slot_uid=slot_uid,
|
||||
)
|
||||
|
||||
|
||||
def _bindings(
|
||||
*,
|
||||
db: Database,
|
||||
tool_loop: asyncio.AbstractEventLoop,
|
||||
workspace: Workspace,
|
||||
upstream: Path,
|
||||
slot_uid: int,
|
||||
) -> host_tools.ToolBindings:
|
||||
repo = RepoInfo(full_name=_REPO, default_branch="main", clone_url=str(upstream), private=False)
|
||||
issue = IssueInfo(
|
||||
repo=_REPO,
|
||||
number=workspace.issue_number,
|
||||
title="permission e2e",
|
||||
body="",
|
||||
state="open",
|
||||
author="human",
|
||||
labels=(),
|
||||
is_pull_request=False,
|
||||
)
|
||||
return host_tools.ToolBindings(
|
||||
db=db,
|
||||
github=cast(GitHubBackend, object()), # not used by these local-only host-tool paths
|
||||
git_transport=LocalGitTransport(token=None),
|
||||
repo=repo,
|
||||
issue=issue,
|
||||
workspace=workspace,
|
||||
loop=tool_loop,
|
||||
author_name=_AUTHOR_NAME,
|
||||
author_email=_AUTHOR_EMAIL,
|
||||
slot_uid=slot_uid,
|
||||
)
|
||||
|
||||
|
||||
def _run_ok(
|
||||
bindings: host_tools.ToolBindings,
|
||||
cmd: list[str] | tuple[str, ...],
|
||||
*,
|
||||
timeout: float = 180.0,
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
proc = host_tools._run_repo_command(bindings, cmd, timeout=timeout)
|
||||
assert proc.returncode == 0, (
|
||||
f"command failed as slot {bindings.slot_uid}: {' '.join(cmd)}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"
|
||||
)
|
||||
return proc
|
||||
|
||||
|
||||
def _write_as_slot(bindings: host_tools.ToolBindings, relative_path: str, content: str) -> None:
|
||||
_run_ok(
|
||||
bindings,
|
||||
[
|
||||
"python3",
|
||||
"-c",
|
||||
(
|
||||
"from pathlib import Path; "
|
||||
"Path(__import__('sys').argv[1]).parent.mkdir(parents=True, exist_ok=True); "
|
||||
"Path(__import__('sys').argv[1]).write_text(__import__('sys').argv[2], encoding='utf-8')"
|
||||
),
|
||||
relative_path,
|
||||
content,
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
def _prepare_shared_cargo_cache(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
||||
cargo_home = tmp_path / "shared-cache" / "cargo"
|
||||
cargo_target = tmp_path / "shared-cache" / "cargo-target"
|
||||
for path in (cargo_home, cargo_target):
|
||||
path.mkdir(parents=True)
|
||||
os.chown(path, 0, _SHARED_OMP_GID)
|
||||
path.chmod(0o2770)
|
||||
monkeypatch.setenv("CARGO_HOME", str(cargo_home))
|
||||
monkeypatch.setenv("CARGO_TARGET_DIR", str(cargo_target))
|
||||
return cargo_target
|
||||
|
||||
|
||||
def test_slot_workspace_runs_bun_biome_cargo_and_git_after_root_reentry(
|
||||
slot_tmp_path: Path,
|
||||
upstream_repo: Path,
|
||||
db: Database,
|
||||
tool_loop: asyncio.AbstractEventLoop,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
_require_linux_root_toolchain()
|
||||
cargo_target = _prepare_shared_cargo_cache(slot_tmp_path, monkeypatch)
|
||||
workspaces = slot_tmp_path / "workspaces"
|
||||
|
||||
first = _ensure_workspace(workspaces, upstream_repo, number=101, slot_uid=_SLOT_ONE)
|
||||
stale_bun_cache = first.root / ".omp-xdg" / "cache" / "bun-install" / "root-owned-stale"
|
||||
stale_bun_cache.mkdir(parents=True, exist_ok=True)
|
||||
stale_marker = stale_bun_cache / "marker.txt"
|
||||
stale_marker.write_text("root-owned\n", encoding="utf-8")
|
||||
stale_bun_cache.chmod(0o700)
|
||||
stale_marker.chmod(0o600)
|
||||
|
||||
workspace = _ensure_workspace(
|
||||
workspaces,
|
||||
upstream_repo,
|
||||
number=101,
|
||||
slot_uid=_SLOT_ONE,
|
||||
existing_branch=first.branch,
|
||||
)
|
||||
bindings = _bindings(db=db, tool_loop=tool_loop, workspace=workspace, upstream=upstream_repo, slot_uid=_SLOT_ONE)
|
||||
|
||||
_run_ok(bindings, ["bun", "install", "--no-progress"], timeout=300.0)
|
||||
_run_ok(bindings, ["bun", "run", "check:ts"], timeout=180.0)
|
||||
_run_ok(bindings, ["cargo", "check", "--workspace"], timeout=600.0)
|
||||
host_tools._run_pre_publish_bun_check(bindings, {}, tool_name="gh_push_branch", stage="push")
|
||||
|
||||
runtime_env = host_tools._repo_command_env(bindings)
|
||||
bun_cache = Path(runtime_env["BUN_INSTALL_CACHE_DIR"])
|
||||
assert bun_cache.is_dir()
|
||||
assert bun_cache.stat().st_uid == _SLOT_ONE
|
||||
assert stale_marker.stat().st_uid == _SLOT_ONE
|
||||
assert (cargo_target / "debug").is_dir()
|
||||
assert (cargo_target / "debug").stat().st_gid == _SHARED_OMP_GID
|
||||
|
||||
_write_as_slot(bindings, "src/slot-generated.ts", "export const generatedBySlot = true;\n")
|
||||
_run_ok(bindings, ["git", "add", "src/slot-generated.ts", "Cargo.lock", "bun.lock"])
|
||||
_run_ok(bindings, ["git", "commit", "-m", "slot generated file"])
|
||||
status = _run_ok(bindings, ["git", "status", "--porcelain", "--untracked-files=normal"])
|
||||
assert status.stdout.strip() == ""
|
||||
|
||||
|
||||
def test_git_pool_metadata_survives_root_push_and_retry_slot(
|
||||
slot_tmp_path: Path,
|
||||
upstream_repo: Path,
|
||||
db: Database,
|
||||
tool_loop: asyncio.AbstractEventLoop,
|
||||
) -> None:
|
||||
_require_linux_root_toolchain()
|
||||
workspaces = slot_tmp_path / "workspaces"
|
||||
|
||||
first = _ensure_workspace(workspaces, upstream_repo, number=102, slot_uid=_SLOT_ONE)
|
||||
first_bindings = _bindings(db=db, tool_loop=tool_loop, workspace=first, upstream=upstream_repo, slot_uid=_SLOT_ONE)
|
||||
_write_as_slot(first_bindings, "src/first-slot.ts", "export const firstSlot = 1;\n")
|
||||
_run_ok(first_bindings, ["git", "add", "src/first-slot.ts"])
|
||||
_run_ok(first_bindings, ["git", "commit", "-m", "first slot commit"])
|
||||
|
||||
first_head = host_tools._guarded_push_branch(first_bindings, {}, "gh_push_branch", first.branch)
|
||||
remote_head = _git(["--git-dir", str(upstream_repo), "rev-parse", first.branch], cwd=slot_tmp_path).stdout.strip()
|
||||
assert remote_head == first_head
|
||||
|
||||
retry = _ensure_workspace(
|
||||
workspaces,
|
||||
upstream_repo,
|
||||
number=102,
|
||||
slot_uid=_SLOT_TWO,
|
||||
existing_branch=first.branch,
|
||||
)
|
||||
retry_bindings = _bindings(db=db, tool_loop=tool_loop, workspace=retry, upstream=upstream_repo, slot_uid=_SLOT_TWO)
|
||||
|
||||
_run_ok(retry_bindings, ["git", "fsck", "--no-progress"], timeout=180.0)
|
||||
_write_as_slot(retry_bindings, "src/retry-slot.ts", "export const retrySlot = 2;\n")
|
||||
_run_ok(retry_bindings, ["git", "add", "src/retry-slot.ts"])
|
||||
_run_ok(retry_bindings, ["git", "commit", "-m", "retry slot commit"])
|
||||
|
||||
retry_head = host_tools._guarded_push_branch(retry_bindings, {}, "gh_push_branch", retry.branch)
|
||||
remote_retry_head = _git(
|
||||
["--git-dir", str(upstream_repo), "rev-parse", retry.branch], cwd=slot_tmp_path
|
||||
).stdout.strip()
|
||||
assert remote_retry_head == retry_head
|
||||
assert retry_head != first_head
|
||||
@@ -498,6 +498,38 @@ def test_proxy_git_transport_push_head_drift(proxy_settings: Settings, upstream_
|
||||
assert not _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
def test_proxy_git_transport_push_slot_uid_body() -> None:
|
||||
captured: list[dict[str, object]] = []
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
captured.append(json.loads(request.content))
|
||||
return httpx.Response(200, json={"head": "abc123", "branch": "farm/abc/feat"})
|
||||
|
||||
transport = ProxyGitTransport(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.MockTransport(handler),
|
||||
)
|
||||
transport.push_branch(
|
||||
repo="octo/widget",
|
||||
workspace_key="octo__widget__1",
|
||||
repo_dir=Path("/unused"),
|
||||
branch="farm/abc/feat",
|
||||
expected_head="abc123",
|
||||
slot_uid=2001,
|
||||
)
|
||||
transport.push_branch(
|
||||
repo="octo/widget",
|
||||
workspace_key="octo__widget__1",
|
||||
repo_dir=Path("/unused"),
|
||||
branch="farm/abc/feat",
|
||||
expected_head="abc123",
|
||||
)
|
||||
|
||||
assert captured[0]["slot_uid"] == 2001
|
||||
assert "slot_uid" not in captured[1]
|
||||
|
||||
|
||||
# Sanity: signed POST headers from ProxyGitTransport._post verify cleanly.
|
||||
def test_proxy_git_transport_post_headers_verify() -> None:
|
||||
captured: list[httpx.Request] = []
|
||||
|
||||
@@ -156,6 +156,36 @@ async def _async_client(app) -> httpx.AsyncClient:
|
||||
)
|
||||
|
||||
|
||||
def test_read_origin_url_uses_safe_directory_and_slot_identity(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
from robomp.proxy import server as proxy_server
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
repo_dir = tmp_path / "repo"
|
||||
|
||||
def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
captured["cmd"] = cmd
|
||||
captured.update(kwargs)
|
||||
return subprocess.CompletedProcess(cmd, 0, "https://github.com/octo/widget.git\n", "")
|
||||
|
||||
monkeypatch.setattr("robomp.proxy.server.subprocess.run", fake_run)
|
||||
monkeypatch.setattr(
|
||||
"robomp.proxy.server._slot_subprocess_kwargs",
|
||||
lambda uid: {"user": uid, "group": uid, "extra_groups": [2000], "umask": 0o002},
|
||||
)
|
||||
|
||||
assert proxy_server._read_origin_url(repo_dir, slot_uid=2001) == "https://github.com/octo/widget.git"
|
||||
|
||||
env = captured["env"]
|
||||
assert isinstance(env, dict)
|
||||
assert env["GIT_CONFIG_COUNT"] == "1"
|
||||
assert env["GIT_CONFIG_KEY_0"] == "safe.directory"
|
||||
assert env["GIT_CONFIG_VALUE_0"] == str(repo_dir)
|
||||
assert captured["user"] == 2001
|
||||
assert captured["group"] == 2001
|
||||
assert captured["extra_groups"] == [2000]
|
||||
assert captured["umask"] == 0o002
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# HMAC behavior
|
||||
# ============================================================================
|
||||
@@ -725,6 +755,62 @@ async def test_git_push_happy_path(proxy_settings: Settings, upstream_repo: Path
|
||||
assert _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
async def test_git_push_passes_slot_uid_to_git_push(
|
||||
proxy_settings: Settings, upstream_repo: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
from robomp.git_ops import PushResult
|
||||
|
||||
branch = "farm/abc/slot"
|
||||
repo_dir, head = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_git_push(path: Path, **kwargs: object) -> PushResult:
|
||||
captured["path"] = path
|
||||
captured.update(kwargs)
|
||||
return PushResult(head=head, branch=branch)
|
||||
|
||||
monkeypatch.setattr("robomp.proxy.server.git_push", fake_git_push)
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"'
|
||||
+ branch.encode()
|
||||
+ b'","expected_head":"'
|
||||
+ head.encode()
|
||||
+ b'","slot_uid":2001}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert captured["path"] == repo_dir
|
||||
assert captured["slot_uid"] == 2001
|
||||
|
||||
|
||||
@pytest.mark.parametrize("slot_uid", [0, -1, 65536])
|
||||
async def test_git_push_rejects_invalid_slot_uid(proxy_settings: Settings, slot_uid: int) -> None:
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"x","expected_head":"'
|
||||
+ (b"0" * 40)
|
||||
+ b'","slot_uid":'
|
||||
+ str(slot_uid).encode()
|
||||
+ b"}"
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "slot_uid" in resp.text
|
||||
|
||||
|
||||
async def test_git_push_head_drift(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
branch = "farm/abc/drift"
|
||||
_, _ = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
|
||||
+320
-4
@@ -13,10 +13,14 @@ from robomp.sandbox import (
|
||||
SandboxManager,
|
||||
Workspace,
|
||||
_chown_workspace,
|
||||
_prepare_slot_runtime_env,
|
||||
_prepare_slot_tmpdir,
|
||||
_provision_runtime_dirs,
|
||||
_reap_slot,
|
||||
_safe_directory_env,
|
||||
_share_git_metadata_with_slots,
|
||||
_slot_pids,
|
||||
_slot_subprocess_kwargs,
|
||||
make_branch,
|
||||
rename_workspace_branch,
|
||||
workspace_key,
|
||||
@@ -155,6 +159,47 @@ def test_rename_workspace_branch_refreshes_shared_metadata(tmp_path: Path, monke
|
||||
assert calls == [(repo_dir, 2004)]
|
||||
|
||||
|
||||
def test_rename_workspace_branch_runs_git_as_slot_when_permissions_active(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
root = tmp_path / "ws"
|
||||
repo_dir = root / "repo"
|
||||
repo_dir.mkdir(parents=True)
|
||||
initial = "farm/abc12345/some-issue"
|
||||
ws = Workspace(
|
||||
root=root,
|
||||
repo_dir=repo_dir,
|
||||
session_dir=root / ".omp-session",
|
||||
context_dir=root / "context",
|
||||
artifacts_dir=root / "artifacts",
|
||||
branch=initial,
|
||||
repo_full_name="octo/widget",
|
||||
issue_number=1,
|
||||
)
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
captured["cmd"] = cmd
|
||||
captured["kwargs"] = kwargs
|
||||
return subprocess.CompletedProcess(cmd, 0, "", "")
|
||||
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
monkeypatch.setattr("robomp.sandbox.os.geteuid", lambda: 0)
|
||||
monkeypatch.setattr("robomp.sandbox.subprocess.run", fake_run)
|
||||
monkeypatch.setattr("robomp.sandbox._share_git_metadata_with_slots", lambda _repo_dir, _slot_uid: None)
|
||||
|
||||
new_branch = rename_workspace_branch(ws, "fix-json-bom", slot_uid=2004)
|
||||
|
||||
assert new_branch == "farm/abc12345/fix-json-bom"
|
||||
assert captured["cmd"] == ["git", "branch", "-m", initial, "farm/abc12345/fix-json-bom"]
|
||||
kwargs = captured["kwargs"]
|
||||
assert isinstance(kwargs, dict)
|
||||
assert kwargs["cwd"] == str(repo_dir)
|
||||
assert kwargs["user"] == 2004
|
||||
assert kwargs["group"] == 2004
|
||||
assert kwargs["extra_groups"] == [2000]
|
||||
|
||||
|
||||
def test_rename_workspace_branch_is_idempotent_when_slug_unchanged(tmp_path: Path) -> None:
|
||||
root = tmp_path / "ws"
|
||||
repo_dir = root / "repo"
|
||||
@@ -384,11 +429,63 @@ def test_chown_workspace_runs_chown_and_chmod_as_root_on_linux(tmp_path: Path, m
|
||||
|
||||
# 2001 is the slot-private GID matching the slot UID, not the shared omp group.
|
||||
assert calls == [
|
||||
(["chown", "-R", "0:2001", str(tmp_path)], True),
|
||||
(["chown", "-R", "2001:2001", str(tmp_path)], True),
|
||||
(["chmod", "-R", "u=rwX,g=rwX,o=", str(tmp_path)], True),
|
||||
]
|
||||
|
||||
|
||||
def test_chown_workspace_makes_workspace_slot_owned(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
subdir = tmp_path / "subdir"
|
||||
subdir.mkdir()
|
||||
file_path = subdir / "file.txt"
|
||||
file_path.write_text("data\n", encoding="utf-8")
|
||||
tmp_path.chmod(0o777)
|
||||
subdir.chmod(0o777)
|
||||
file_path.chmod(0o777)
|
||||
owned: dict[Path, tuple[int, int]] = {}
|
||||
|
||||
def fake_run(cmd: list[str], *, check: bool) -> None:
|
||||
assert check
|
||||
if cmd[:2] == ["chown", "-R"]:
|
||||
uid_text, gid_text = cmd[2].split(":", 1)
|
||||
root = Path(cmd[3])
|
||||
uid = int(uid_text)
|
||||
gid = int(gid_text)
|
||||
owned[root] = (uid, gid)
|
||||
for current_root, dirs, files in os.walk(root):
|
||||
current = Path(current_root)
|
||||
owned[current] = (uid, gid)
|
||||
for dirname in dirs:
|
||||
owned[current / dirname] = (uid, gid)
|
||||
for filename in files:
|
||||
owned[current / filename] = (uid, gid)
|
||||
elif cmd[:3] == ["chmod", "-R", "u=rwX,g=rwX,o="]:
|
||||
root = Path(cmd[3])
|
||||
root.chmod(0o770)
|
||||
for current_root, dirs, files in os.walk(root):
|
||||
current = Path(current_root)
|
||||
current.chmod(0o770)
|
||||
for dirname in dirs:
|
||||
(current / dirname).chmod(0o770)
|
||||
for filename in files:
|
||||
(current / filename).chmod(0o660)
|
||||
else:
|
||||
raise AssertionError(f"unexpected command: {cmd!r}")
|
||||
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
monkeypatch.setattr("robomp.sandbox.os.geteuid", lambda: 0)
|
||||
monkeypatch.setattr("robomp.sandbox.subprocess.run", fake_run)
|
||||
|
||||
_chown_workspace(tmp_path, 2001)
|
||||
|
||||
assert owned[tmp_path] == (2001, 2001)
|
||||
assert owned[subdir] == (2001, 2001)
|
||||
assert owned[file_path] == (2001, 2001)
|
||||
assert stat.S_IMODE(tmp_path.stat().st_mode) == 0o770
|
||||
assert stat.S_IMODE(subdir.stat().st_mode) == 0o770
|
||||
assert stat.S_IMODE(file_path.stat().st_mode) == 0o660
|
||||
|
||||
|
||||
def test_slot_pids_reads_proc_status_and_skips_zombies(tmp_path: Path) -> None:
|
||||
nonnumeric = tmp_path / "self"
|
||||
nonnumeric.mkdir()
|
||||
@@ -442,7 +539,7 @@ def test_reap_slot_kills_slot_uid_on_linux_root(monkeypatch: pytest.MonkeyPatch)
|
||||
assert calls == [(111, signal.SIGKILL), (222, signal.SIGKILL)]
|
||||
|
||||
|
||||
def test_prepare_slot_tmpdir_chowns_slot_and_locks_down(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_prepare_slot_tmpdir_mkdirs_without_chown(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
chowns: list[tuple[Path, int, int]] = []
|
||||
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
@@ -454,7 +551,7 @@ def test_prepare_slot_tmpdir_chowns_slot_and_locks_down(tmp_path: Path, monkeypa
|
||||
assert tmpdir == tmp_path / ".omp-tmp"
|
||||
assert tmpdir.is_dir()
|
||||
assert stat.S_IMODE(tmpdir.stat().st_mode) == 0o700
|
||||
assert chowns == [(tmpdir, 2001, 2001)]
|
||||
assert chowns == []
|
||||
|
||||
|
||||
def test_prepare_slot_tmpdir_replaces_symlink_without_touching_target(tmp_path: Path) -> None:
|
||||
@@ -471,6 +568,73 @@ def test_prepare_slot_tmpdir_replaces_symlink_without_touching_target(tmp_path:
|
||||
assert target.is_dir()
|
||||
|
||||
|
||||
def test_provision_runtime_dirs_replaces_tmpdir_symlink_and_creates_xdg_tree(tmp_path: Path) -> None:
|
||||
target = tmp_path / "target"
|
||||
target.mkdir()
|
||||
tmpdir = tmp_path / ".omp-tmp"
|
||||
tmpdir.symlink_to(target, target_is_directory=True)
|
||||
|
||||
_provision_runtime_dirs(tmp_path)
|
||||
|
||||
assert tmpdir.is_dir()
|
||||
assert not tmpdir.is_symlink()
|
||||
assert target.is_dir()
|
||||
assert stat.S_IMODE(tmpdir.stat().st_mode) == 0o700
|
||||
for base in (tmp_path / ".omp-xdg" / "data", tmp_path / ".omp-xdg" / "state", tmp_path / ".omp-xdg" / "cache"):
|
||||
assert base.is_dir()
|
||||
assert (base / "omp").is_dir()
|
||||
assert (tmp_path / ".omp-xdg" / "cache" / "bun-install").is_dir()
|
||||
|
||||
|
||||
def test_safe_directory_env_scopes_single_repo_path(tmp_path: Path) -> None:
|
||||
repo_dir = tmp_path / "repo"
|
||||
|
||||
assert _safe_directory_env(repo_dir) == {
|
||||
"GIT_CONFIG_COUNT": "1",
|
||||
"GIT_CONFIG_KEY_0": "safe.directory",
|
||||
"GIT_CONFIG_VALUE_0": str(repo_dir),
|
||||
}
|
||||
|
||||
|
||||
def test_slot_subprocess_kwargs_run_as_slot_on_linux_root(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
monkeypatch.setattr("robomp.sandbox.os.geteuid", lambda: 0)
|
||||
|
||||
assert _slot_subprocess_kwargs(2001) == {
|
||||
"user": 2001,
|
||||
"group": 2001,
|
||||
"extra_groups": [2000],
|
||||
"umask": 0o002,
|
||||
}
|
||||
|
||||
|
||||
def test_prepare_slot_runtime_env_returns_workspace_private_paths_without_chown(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
chowns: list[tuple[Path, int, int]] = []
|
||||
calls: list[list[str]] = []
|
||||
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
monkeypatch.setattr("robomp.sandbox.os.geteuid", lambda: 0)
|
||||
monkeypatch.setattr("robomp.sandbox.os.chown", lambda path, uid, gid: chowns.append((Path(path), uid, gid)))
|
||||
monkeypatch.setattr("robomp.sandbox.subprocess.run", lambda cmd, **_kwargs: calls.append(cmd))
|
||||
|
||||
ws = _workspace(tmp_path)
|
||||
bun_cache = ws.root / ".omp-xdg" / "cache" / "bun-install"
|
||||
|
||||
env = _prepare_slot_runtime_env(ws, 2001)
|
||||
|
||||
assert env["TMPDIR"] == str(ws.root / ".omp-tmp")
|
||||
assert env["XDG_CACHE_HOME"] == str(ws.root / ".omp-xdg" / "cache")
|
||||
assert env["BUN_INSTALL_CACHE_DIR"] == str(bun_cache)
|
||||
for base in (ws.root / ".omp-xdg" / "data", ws.root / ".omp-xdg" / "state", ws.root / ".omp-xdg" / "cache"):
|
||||
assert base.is_dir()
|
||||
assert (base / "omp").is_dir()
|
||||
assert bun_cache.is_dir()
|
||||
assert chowns == []
|
||||
assert calls == []
|
||||
|
||||
|
||||
def test_share_git_metadata_keeps_pool_writable_for_retry_slot(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
repo_dir = tmp_path / "workspaces" / "octo__widget__43" / "repo"
|
||||
repo_dir.mkdir(parents=True)
|
||||
@@ -560,7 +724,12 @@ def test_ensure_workspace_refreshes_permissions_for_retry_slot_and_session(
|
||||
assert ws2.session_dir == ws1.session_dir
|
||||
assert transcript.is_file()
|
||||
assert ws2.branch == ws1.branch
|
||||
assert shared == [(ws1.repo_dir, 2001), (ws1.repo_dir, 2002)]
|
||||
assert shared == [
|
||||
(ws1.repo_dir, 2001),
|
||||
(ws1.repo_dir, 2001),
|
||||
(ws1.repo_dir, 2002),
|
||||
(ws1.repo_dir, 2002),
|
||||
]
|
||||
assert chowns == [(ws1.root, 2001), (ws1.root, 2002)]
|
||||
|
||||
|
||||
@@ -593,6 +762,66 @@ def test_ensure_workspace_preserves_checked_out_branch_on_replay(tmp_path: Path,
|
||||
assert ws2.branch == renamed
|
||||
|
||||
|
||||
def test_ensure_workspace_runs_existing_worktree_git_as_slot_after_chown(
|
||||
tmp_path: Path, upstream_repo: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
mgr = SandboxManager(tmp_path / "workspaces")
|
||||
ws1 = mgr.ensure_workspace(
|
||||
repo="octo/widget",
|
||||
number=47,
|
||||
title="retry me",
|
||||
clone_url=str(upstream_repo),
|
||||
default_branch="main",
|
||||
slot_uid=None,
|
||||
author_name="robomp-bot",
|
||||
author_email="robomp-bot@example.invalid",
|
||||
)
|
||||
events: list[tuple[str, int | None]] = []
|
||||
git_calls: list[tuple[list[str], dict[str, object]]] = []
|
||||
|
||||
def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
git_calls.append((cmd, kwargs))
|
||||
if cmd[:3] == ["git", "remote", "get-url"]:
|
||||
return subprocess.CompletedProcess(cmd, 0, f"{upstream_repo}\n", "")
|
||||
if cmd[:4] == ["git", "symbolic-ref", "--quiet", "--short"]:
|
||||
user = kwargs.get("user")
|
||||
events.append(("symbolic-ref", user if isinstance(user, int) else None))
|
||||
return subprocess.CompletedProcess(cmd, 0, f"{ws1.branch}\n", "")
|
||||
if cmd[:2] == ["git", "config"]:
|
||||
user = kwargs.get("user")
|
||||
events.append(("config", user if isinstance(user, int) else None))
|
||||
return subprocess.CompletedProcess(cmd, 0, "", "")
|
||||
|
||||
def record_chown(_ws_root: Path, slot_uid: int | None) -> None:
|
||||
events.append(("chown", slot_uid))
|
||||
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
monkeypatch.setattr("robomp.sandbox.os.geteuid", lambda: 0)
|
||||
monkeypatch.setattr("robomp.sandbox.subprocess.run", fake_run)
|
||||
monkeypatch.setattr("robomp.sandbox._chown_workspace", record_chown)
|
||||
monkeypatch.setattr("robomp.sandbox._share_git_metadata_with_slots", lambda _repo_dir, _slot_uid: None)
|
||||
|
||||
ws2 = mgr.ensure_workspace(
|
||||
repo="octo/widget",
|
||||
number=47,
|
||||
title="retry me",
|
||||
clone_url=str(upstream_repo),
|
||||
default_branch="main",
|
||||
slot_uid=2002,
|
||||
author_name="robomp-bot",
|
||||
author_email="robomp-bot@example.invalid",
|
||||
)
|
||||
|
||||
assert ws2.branch == ws1.branch
|
||||
assert events[0] == ("chown", 2002)
|
||||
assert ("symbolic-ref", 2002) in events
|
||||
assert events.index(("chown", 2002)) < events.index(("symbolic-ref", 2002))
|
||||
assert events.count(("config", 2002)) == 2
|
||||
worktree_git = [kwargs for cmd, kwargs in git_calls if cmd[:2] in (["git", "symbolic-ref"], ["git", "config"])]
|
||||
assert worktree_git
|
||||
assert all(kwargs["user"] == 2002 and kwargs["group"] == 2002 for kwargs in worktree_git)
|
||||
|
||||
|
||||
def test_ensure_workspace_invokes_slot_chown(
|
||||
tmp_path: Path, upstream_repo: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
@@ -618,6 +847,57 @@ def test_ensure_workspace_invokes_slot_chown(
|
||||
assert calls == [(ws.root, 2001)]
|
||||
|
||||
|
||||
def test_ensure_workspace_provisions_and_slot_owns_runtime_dirs(
|
||||
tmp_path: Path, upstream_repo: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
owned: dict[Path, tuple[int, int]] = {}
|
||||
runtime_paths: list[Path] = []
|
||||
|
||||
def record_chown(ws_root: Path, slot_uid: int | None) -> None:
|
||||
assert slot_uid is not None
|
||||
paths = [
|
||||
ws_root / ".omp-tmp",
|
||||
ws_root / ".omp-xdg" / "data",
|
||||
ws_root / ".omp-xdg" / "data" / "omp",
|
||||
ws_root / ".omp-xdg" / "state",
|
||||
ws_root / ".omp-xdg" / "state" / "omp",
|
||||
ws_root / ".omp-xdg" / "cache",
|
||||
ws_root / ".omp-xdg" / "cache" / "omp",
|
||||
ws_root / ".omp-xdg" / "cache" / "bun-install",
|
||||
]
|
||||
runtime_paths.extend(paths)
|
||||
for path in paths:
|
||||
assert path.is_dir()
|
||||
owned[path] = (slot_uid, slot_uid)
|
||||
|
||||
monkeypatch.setattr("robomp.sandbox._chown_workspace", record_chown)
|
||||
mgr = SandboxManager(tmp_path / "workspaces")
|
||||
|
||||
ws = mgr.ensure_workspace(
|
||||
repo="octo/widget",
|
||||
number=46,
|
||||
title="runtime perms",
|
||||
clone_url=str(upstream_repo),
|
||||
default_branch="main",
|
||||
slot_uid=2001,
|
||||
author_name="robomp-bot",
|
||||
author_email="robomp-bot@example.invalid",
|
||||
)
|
||||
|
||||
assert runtime_paths
|
||||
assert set(runtime_paths) == {
|
||||
ws.root / ".omp-tmp",
|
||||
ws.root / ".omp-xdg" / "data",
|
||||
ws.root / ".omp-xdg" / "data" / "omp",
|
||||
ws.root / ".omp-xdg" / "state",
|
||||
ws.root / ".omp-xdg" / "state" / "omp",
|
||||
ws.root / ".omp-xdg" / "cache",
|
||||
ws.root / ".omp-xdg" / "cache" / "omp",
|
||||
ws.root / ".omp-xdg" / "cache" / "bun-install",
|
||||
}
|
||||
assert set(owned.values()) == {(2001, 2001)}
|
||||
|
||||
|
||||
def test_ensure_workspace_is_idempotent(tmp_path: Path, upstream_repo: Path) -> None:
|
||||
mgr = SandboxManager(tmp_path / "workspaces")
|
||||
ws1 = mgr.ensure_workspace(
|
||||
@@ -856,6 +1136,42 @@ def test_push_force_with_lease_refuses_when_origin_moved(tmp_path: Path, upstrea
|
||||
)
|
||||
|
||||
|
||||
def test_run_git_injects_safe_directory_and_subprocess_identity(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
from robomp.git_ops import _run_git
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
captured["cmd"] = cmd
|
||||
captured.update(kwargs)
|
||||
return subprocess.CompletedProcess(cmd, 0, "", "")
|
||||
|
||||
monkeypatch.setattr("robomp.git_ops.subprocess.run", fake_run)
|
||||
|
||||
_run_git(
|
||||
["status"],
|
||||
cwd=tmp_path,
|
||||
token=None,
|
||||
safe_directory=Path("/x"),
|
||||
user=2001,
|
||||
group=2001,
|
||||
extra_groups=[2000],
|
||||
umask=0o002,
|
||||
)
|
||||
|
||||
env = captured["env"]
|
||||
assert isinstance(env, dict)
|
||||
assert env["GIT_CONFIG_COUNT"] == "1"
|
||||
assert env["GIT_CONFIG_KEY_0"] == "safe.directory"
|
||||
assert env["GIT_CONFIG_VALUE_0"] == "/x"
|
||||
assert captured["user"] == 2001
|
||||
assert captured["group"] == 2001
|
||||
assert captured["extra_groups"] == [2000]
|
||||
assert captured["umask"] == 0o002
|
||||
|
||||
|
||||
def test_run_git_kills_hung_child(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""A `git` invocation that hangs past the timeout must be killed and
|
||||
raised as `GitCommandError(124)` rather than pinning the calling
|
||||
|
||||
+15
-6
@@ -285,17 +285,25 @@ async def test_run_rpc_uses_workspace_xdg_dirs_without_slot(tmp_path: Path, sett
|
||||
assert env["TMPDIR"] == str(tmpdir)
|
||||
assert env["TMP"] == str(tmpdir)
|
||||
assert env["TEMP"] == str(tmpdir)
|
||||
assert env["GIT_CONFIG_COUNT"] == "1"
|
||||
assert env["GIT_CONFIG_KEY_0"] == "safe.directory"
|
||||
assert env["GIT_CONFIG_VALUE_0"] == str(inputs.workspace.repo_dir)
|
||||
assert env["GIT_AUTHOR_NAME"] == settings.resolved_author_name
|
||||
assert env["GIT_AUTHOR_EMAIL"] == settings.git_author_email
|
||||
assert env["GIT_COMMITTER_NAME"] == settings.resolved_author_name
|
||||
assert env["GIT_COMMITTER_EMAIL"] == settings.git_author_email
|
||||
assert tmpdir.is_dir()
|
||||
assert stat.S_IMODE(tmpdir.stat().st_mode) == 0o700
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_chowns_workspace_xdg_dirs_for_slot(
|
||||
async def test_run_rpc_uses_workspace_xdg_dirs_for_slot_without_chown(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
chown_calls: list[tuple[Path, int, int]] = []
|
||||
monkeypatch.setattr("robomp.worker.os.geteuid", lambda: 0)
|
||||
monkeypatch.setattr("robomp.worker.os.chown", lambda path, uid, gid: chown_calls.append((Path(path), uid, gid)))
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
monkeypatch.setattr("robomp.sandbox.os.geteuid", lambda: 0)
|
||||
monkeypatch.setattr("robomp.sandbox.os.chown", lambda path, uid, gid: chown_calls.append((Path(path), uid, gid)))
|
||||
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False, slot_uid=2001)
|
||||
loop = asyncio.new_event_loop()
|
||||
@@ -311,11 +319,12 @@ async def test_run_rpc_chowns_workspace_xdg_dirs_for_slot(
|
||||
loop.close()
|
||||
|
||||
env = _FakeRpcClient.instances[0].kwargs["env"]
|
||||
expected_dirs = set()
|
||||
for key in ("XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_CACHE_HOME"):
|
||||
base = Path(env[key])
|
||||
expected_dirs.update({base, base / "omp"})
|
||||
assert set(chown_calls) == {(path, 0, 2001) for path in expected_dirs}
|
||||
assert base.is_dir()
|
||||
assert (base / "omp").is_dir()
|
||||
assert Path(env["BUN_INSTALL_CACHE_DIR"]).is_dir()
|
||||
assert chown_calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user