diff --git a/README.md b/README.md index 0677cd6..25926d2 100644 --- a/README.md +++ b/README.md @@ -261,6 +261,15 @@ git: IdentityFile: /Users/didericis/.ssh/id_ed25519_gitea KnownHostKey: ssh-ed25519 AAAA... +# Optional per-bottle git identity. When set, `git config --global +# user.name` / `user.email` are applied inside the bottle at +# provisioning so the agent's commits land with this attribution +# instead of git refusing to commit. Either field can be set +# independently. Issue #86. +git_user: + name: "Eric Bauerfeld" + email: "eric+claude@dideric.is" + # Routes declared here are held by a per-bottle cred-proxy sidecar, # not the agent. Each route names a path the agent dials, the # upstream the proxy forwards to, an auth_scheme, and a token_ref diff --git a/claude_bottle/backend/docker/provision/git.py b/claude_bottle/backend/docker/provision/git.py index 991422b..7cd797a 100644 --- a/claude_bottle/backend/docker/provision/git.py +++ b/claude_bottle/backend/docker/provision/git.py @@ -1,6 +1,6 @@ """Git provisioning inside a running Docker bottle. -Two concerns, both about git in the agent: +Three concerns, all about git in the agent: 1. If --cwd was passed AND the host cwd has a .git, copy that .git into /home/node/workspace/.git so the agent operates on the @@ -11,6 +11,9 @@ Two concerns, both about git in the agent: ls-remote) transparently hits the per-agent git-gate. The gate mirrors the upstream in both directions, so URL rewriting is symmetric. + 3. If the bottle declares `git_user` (issue #86), set + `git config --global user.{name,email}` inside the bottle so + the agent's commits are attributed to that identity. """ from __future__ import annotations @@ -26,10 +29,11 @@ from ..bottle_plan import DockerBottlePlan def provision_git(plan: DockerBottlePlan, target: str) -> None: - """Set up git inside the bottle. Runs both subcases; each no-ops - when its condition isn't met.""" + """Set up git inside the bottle. Runs all three subcases; each + no-ops when its condition isn't met.""" _provision_cwd_git(plan, target) _provision_git_gate_config(plan, target) + _provision_git_user(plan, target) def _provision_cwd_git(plan: DockerBottlePlan, target: str) -> None: @@ -78,3 +82,40 @@ def _provision_git_gate_config(plan: DockerBottlePlan, target: str) -> None: ) docker_mod.docker_exec_root(container, ["chown", "node:node", container_gitconfig]) docker_mod.docker_exec_root(container, ["chmod", "644", container_gitconfig]) + + +def _provision_git_user(plan: DockerBottlePlan, target: str) -> None: + """Apply `git config --global user.{name,email}` inside the + bottle so the agent's commits are attributed to the operator- + chosen identity instead of the agent image's default + (which is no user — git would refuse to commit at all + until the agent ran its own `git config`). + + Runs as the `node` user so `--global` lands in + `/home/node/.gitconfig` (matching the existing + `_provision_git_gate_config` write location). No-op when the + bottle didn't declare `git_user`. + + Each field set independently — name-only or email-only + configs only run the `git config` line for the field + present.""" + bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name) + gu = bottle.git_user + if gu.is_empty(): + return + if gu.name: + info(f"git config --global user.name = {gu.name!r}") + subprocess.run( + ["docker", "exec", "-u", "node", target, + "git", "config", "--global", "user.name", gu.name], + stdout=subprocess.DEVNULL, + check=True, + ) + if gu.email: + info(f"git config --global user.email = {gu.email!r}") + subprocess.run( + ["docker", "exec", "-u", "node", target, + "git", "config", "--global", "user.email", gu.email], + stdout=subprocess.DEVNULL, + check=True, + ) diff --git a/claude_bottle/backend/smolmachines/provision/git.py b/claude_bottle/backend/smolmachines/provision/git.py index dc3fefb..42bdb39 100644 --- a/claude_bottle/backend/smolmachines/provision/git.py +++ b/claude_bottle/backend/smolmachines/provision/git.py @@ -1,7 +1,7 @@ """Git provisioning inside a running smolmachines bottle (PRD 0023 chunk 4d). -Two concerns, both about git in the agent: +Three concerns, all about git in the agent: 1. If --cwd was passed AND the host cwd has a .git, copy that .git into /home/node/workspace/.git so the agent operates on @@ -11,6 +11,9 @@ Two concerns, both about git in the agent: against a declared upstream transparently hits the per-bottle git-gate. The gate mirrors the upstream in both directions, so URL rewriting is symmetric. + 3. If the bottle declares `git_user` (issue #86), set + `git config --global user.{name,email}` inside the guest so + the agent's commits are attributed to that identity. Differs from `backend.docker.provision.git` in one address detail: the TSI-allowlisted guest can only reach the bundle's pinned IP @@ -44,10 +47,11 @@ def _guest_home() -> str: def provision_git(plan: SmolmachinesBottlePlan, target: str) -> None: - """Set up git inside the guest. Runs both subcases; each + """Set up git inside the guest. Runs all three subcases; each no-ops when its condition isn't met.""" _provision_cwd_git(plan, target) _provision_git_gate_config(plan, target) + _provision_git_user(plan, target) def _provision_cwd_git(plan: SmolmachinesBottlePlan, target: str) -> None: @@ -101,3 +105,37 @@ def _provision_git_gate_config(plan: SmolmachinesBottlePlan, target: str) -> Non _smolvm.machine_cp(str(config_file), f"{target}:{guest_gitconfig}") _smolvm.machine_exec(target, ["chown", "node:node", guest_gitconfig]) _smolvm.machine_exec(target, ["chmod", "644", guest_gitconfig]) + + +def _provision_git_user( + plan: SmolmachinesBottlePlan, target: str, +) -> None: + """Apply `git config --global user.{name,email}` inside the + guest as the node user so --global lands in the same + `/home/node/.gitconfig` that `_provision_git_gate_config` + writes to. No-op when the bottle didn't declare `git_user`. + + Runs via `runuser -u node --`; HOME is forced via smolvm's + `-e` flag because runuser (without -l) inherits root's + HOME=/root, which would put --global in the wrong file.""" + bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name) + gu = bottle.git_user + if gu.is_empty(): + return + env = {"HOME": _guest_home(), "USER": "node"} + if gu.name: + info(f"git config --global user.name = {gu.name!r}") + _smolvm.machine_exec( + target, + ["runuser", "-u", "node", "--", + "git", "config", "--global", "user.name", gu.name], + env=env, + ) + if gu.email: + info(f"git config --global user.email = {gu.email!r}") + _smolvm.machine_exec( + target, + ["runuser", "-u", "node", "--", + "git", "config", "--global", "user.email", gu.email], + env=env, + ) diff --git a/claude_bottle/manifest.py b/claude_bottle/manifest.py index 6725fec..97a3a53 100644 --- a/claude_bottle/manifest.py +++ b/claude_bottle/manifest.py @@ -14,7 +14,9 @@ the system prompt, for bottles the body is human documentation Bottle schema (frontmatter): env: { : , ... } git: [ , ... ] + git_user: { name: , email: } # optional egress: { routes: [ , ... ] } + supervise: # optional Agent schema (frontmatter): bottle: # required @@ -157,6 +159,54 @@ EGRESS_SINGLETON_ROLES = frozenset({ }) +@dataclass(frozen=True) +class GitUser: + """Per-bottle `git config --global user.name` / `user.email` + pair (issue #86). The agent's commits inside the bottle are + attributed to this identity rather than the agent image's + image-baked default (no user, or whatever the image dropped + in). Either or both fields can be set independently. + + `from_dict` is forgiving on shape (a single missing field is + fine — we just skip that config line at provisioning) but + strict on types (string-or-die).""" + + name: str = "" + email: str = "" + + @classmethod + def from_dict(cls, bottle_name: str, raw: object) -> "GitUser": + d = _as_json_object(raw, f"bottle '{bottle_name}' git_user") + for k in d.keys(): + if k not in {"name", "email"}: + die( + f"bottle '{bottle_name}' git_user has unknown key {k!r}; " + f"allowed: name, email" + ) + name = d.get("name", "") + email = d.get("email", "") + if not isinstance(name, str): + die( + f"bottle '{bottle_name}' git_user.name must be a string " + f"(was {type(name).__name__})" + ) + if not isinstance(email, str): + die( + f"bottle '{bottle_name}' git_user.email must be a string " + f"(was {type(email).__name__})" + ) + if not name and not email: + die( + f"bottle '{bottle_name}' git_user is set but neither " + f"name nor email is non-empty; remove the block or " + f"fill at least one field." + ) + return cls(name=name, email=email) + + def is_empty(self) -> bool: + return not self.name and not self.email + + @dataclass(frozen=True) class EgressRoute: """One route on the per-bottle egress sidecar (PRD 0017). @@ -344,6 +394,12 @@ class EgressConfig: class Bottle: env: Mapping[str, str] = field(default_factory=_empty_str_dict) git: tuple[GitEntry, ...] = () + # Per-bottle git identity (issue #86). Empty default — bottles + # that don't set `git_user:` in the manifest skip the + # `git config --global` step entirely. Set independently of + # the `git:` upstream list above: a bottle can declare a user + # identity without any git-gate upstreams, and vice versa. + git_user: GitUser = field(default_factory=GitUser) egress: EgressConfig = field(default_factory=EgressConfig) # Opt-in per-bottle stuck-recovery sidecar (PRD 0013). When true, # the launch step brings up a supervise sidecar that exposes three @@ -422,6 +478,12 @@ class Bottle: f"See docs/prds/0017-egress-via-mitmproxy.md." ) + git_user = ( + GitUser.from_dict(name, d["git_user"]) + if "git_user" in d + else GitUser() + ) + egress = ( EgressConfig.from_dict(name, d["egress"]) if "egress" in d @@ -436,7 +498,7 @@ class Bottle: ) return cls( - env=env, git=git, egress=egress, + env=env, git=git, git_user=git_user, egress=egress, supervise=supervise_raw, ) @@ -772,7 +834,7 @@ _FILENAME_RX = re.compile(r"^[a-z][a-z0-9-]*$") # sets dies with a "did you mean" pointer — typos shouldn't silently # ghost into an empty config. _BOTTLE_KEYS = frozenset( - {"env", "git", "egress", "supervise"} + {"env", "git", "git_user", "egress", "supervise"} ) _AGENT_KEYS_REQUIRED = frozenset({"bottle"}) _AGENT_KEYS_OPTIONAL = frozenset({"skills"}) diff --git a/tests/unit/test_docker_provision_git_user.py b/tests/unit/test_docker_provision_git_user.py new file mode 100644 index 0000000..9cd5728 --- /dev/null +++ b/tests/unit/test_docker_provision_git_user.py @@ -0,0 +1,140 @@ +"""Unit: docker backend `_provision_git_user` (issue #86). + +Mocks `subprocess.run` and asserts the `docker exec -u node … +git config --global …` argv shape. The cwd + git-gate passes +are covered indirectly by the existing integration-shaped tests +in test_smolmachines_provision; this file targets just the new +git_user pass.""" + +from __future__ import annotations + +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from claude_bottle.backend import BottleSpec +from claude_bottle.backend.docker.bottle_plan import DockerBottlePlan +from claude_bottle.backend.docker.provision import git as _git +from claude_bottle.egress import EgressPlan +from claude_bottle.git_gate import GitGatePlan +from claude_bottle.manifest import Manifest +from claude_bottle.pipelock import PipelockProxyPlan + + +def _plan(*, git_user: dict | None = None, + stage_dir: Path | None = None) -> DockerBottlePlan: + bottle_json: dict = {} + if git_user is not None: + bottle_json["git_user"] = git_user + manifest = Manifest.from_json_obj({ + "bottles": {"dev": bottle_json}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, + }) + spec = BottleSpec( + manifest=manifest, agent_name="demo", + copy_cwd=False, user_cwd="/tmp/x", + ) + return DockerBottlePlan( + spec=spec, + stage_dir=stage_dir or Path("/tmp/stage"), + slug="demo-abc12", + container_name="claude-bottle-demo-abc12", + container_name_pinned=False, + image="claude-bottle:latest", + derived_image="", + runtime_image="claude-bottle:latest", + dockerfile_path="", + env_file=Path("/tmp/agent.env"), + forwarded_env={}, + prompt_file=Path("/tmp/prompt.txt"), + proxy_plan=PipelockProxyPlan( + yaml_path=Path("/tmp/pipelock.yaml"), slug="demo-abc12", + ), + git_gate_plan=GitGatePlan( + slug="demo-abc12", + entrypoint_script=Path("/tmp/git-gate-entrypoint.sh"), + hook_script=Path("/tmp/git-gate-hook"), + access_hook_script=Path("/tmp/git-gate-access-hook"), + upstreams=(), + ), + egress_plan=EgressPlan( + slug="demo-abc12", + routes_path=Path("/tmp/routes.yaml"), + routes=(), + token_env_map={}, + ), + supervise_plan=None, + use_runsc=False, + ) + + +def _git_config_calls(mock_run) -> list[list[str]]: + """Filter `subprocess.run` calls down to the ones that run + `git config --global` inside the bottle, returning each argv.""" + out: list[list[str]] = [] + for call in mock_run.call_args_list: + argv = call.args[0] + if (len(argv) >= 5 + and argv[0] == "docker" and argv[1] == "exec" + and "git" in argv and "config" in argv): + out.append(list(argv)) + return out + + +class TestProvisionGitUser(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory(prefix="cb-prov-git-user.") + self.stage = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def test_noop_when_no_git_user(self): + with patch.object(_git.subprocess, "run") as run: + _git._provision_git_user( + _plan(stage_dir=self.stage), "claude-bottle-demo-abc12", + ) + self.assertEqual([], _git_config_calls(run)) + + def test_sets_name_and_email(self): + plan = _plan( + git_user={"name": "Eric Bauerfeld", "email": "eric@dideric.is"}, + stage_dir=self.stage, + ) + with patch.object(_git.subprocess, "run") as run: + _git._provision_git_user(plan, "claude-bottle-demo-abc12") + calls = _git_config_calls(run) + self.assertEqual(2, len(calls)) + # All `docker exec` invocations run as `-u node` so the + # --global config lands in /home/node/.gitconfig. + for argv in calls: + self.assertEqual( + ["docker", "exec", "-u", "node", "claude-bottle-demo-abc12", + "git", "config", "--global"], + argv[:8], + ) + self.assertEqual(["user.name", "Eric Bauerfeld"], calls[0][8:]) + self.assertEqual(["user.email", "eric@dideric.is"], calls[1][8:]) + + def test_name_only_sets_only_name(self): + plan = _plan(git_user={"name": "Bot"}, stage_dir=self.stage) + with patch.object(_git.subprocess, "run") as run: + _git._provision_git_user(plan, "claude-bottle-demo-abc12") + calls = _git_config_calls(run) + self.assertEqual(1, len(calls)) + self.assertEqual(["user.name", "Bot"], calls[0][8:]) + + def test_email_only_sets_only_email(self): + plan = _plan( + git_user={"email": "bot@example.com"}, stage_dir=self.stage, + ) + with patch.object(_git.subprocess, "run") as run: + _git._provision_git_user(plan, "claude-bottle-demo-abc12") + calls = _git_config_calls(run) + self.assertEqual(1, len(calls)) + self.assertEqual(["user.email", "bot@example.com"], calls[0][8:]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_manifest_git_user.py b/tests/unit/test_manifest_git_user.py new file mode 100644 index 0000000..bf950d5 --- /dev/null +++ b/tests/unit/test_manifest_git_user.py @@ -0,0 +1,109 @@ +"""Unit: Bottle.git_user manifest parsing + validation (issue #86).""" + +import contextlib +import io +import unittest + +from claude_bottle.log import Die +from claude_bottle.manifest import GitUser, Manifest + + +def _die_message(callable_, *args, **kwargs) -> str: + """Run `callable_` expecting it to die, return the stderr text + so tests can assert specifics. `die()` prints to stderr then + raises Die(1) — the exit code is in the exception, the human + message is in stderr.""" + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + try: + callable_(*args, **kwargs) + except Die: + return buf.getvalue() + raise AssertionError("expected Die was not raised") + + +def _manifest(git_user): + return { + "bottles": {"dev": {"git_user": git_user}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, + } + + +class TestGitUserParsing(unittest.TestCase): + def test_parses_both_fields(self): + m = Manifest.from_json_obj(_manifest({ + "name": "Eric Bauerfeld", + "email": "eric+claude@dideric.is", + })) + u = m.bottles["dev"].git_user + self.assertEqual("Eric Bauerfeld", u.name) + self.assertEqual("eric+claude@dideric.is", u.email) + self.assertFalse(u.is_empty()) + + def test_name_only(self): + m = Manifest.from_json_obj(_manifest({"name": "Bot"})) + u = m.bottles["dev"].git_user + self.assertEqual("Bot", u.name) + self.assertEqual("", u.email) + + def test_email_only(self): + m = Manifest.from_json_obj(_manifest({"email": "bot@example.com"})) + u = m.bottles["dev"].git_user + self.assertEqual("", u.name) + self.assertEqual("bot@example.com", u.email) + + def test_omitted_defaults_to_empty(self): + # No git_user block at all → empty GitUser, is_empty True → + # provisioner skips the `git config` step entirely. + m = Manifest.from_json_obj({ + "bottles": {"dev": {}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, + }) + u = m.bottles["dev"].git_user + self.assertTrue(u.is_empty()) + + def test_both_empty_strings_dies(self): + # An explicit `git_user: {name: "", email: ""}` is a typo + # / half-finished edit; fail loudly rather than silently + # no-op (the operator clearly meant to configure something). + msg = _die_message( + Manifest.from_json_obj, _manifest({"name": "", "email": ""}), + ) + self.assertIn("neither name nor email", msg) + + def test_unknown_key_dies(self): + msg = _die_message( + Manifest.from_json_obj, + _manifest({"name": "Bot", "username": "bot"}), + ) + self.assertIn("unknown key", msg) + self.assertIn("username", msg) + + def test_non_string_name_dies(self): + msg = _die_message( + Manifest.from_json_obj, _manifest({"name": 42}), + ) + self.assertIn("git_user.name must be a string", msg) + + def test_non_string_email_dies(self): + msg = _die_message( + Manifest.from_json_obj, _manifest({"email": ["x@y.z"]}), + ) + self.assertIn("git_user.email must be a string", msg) + + +class TestGitUserDirect(unittest.TestCase): + """Direct GitUser dataclass exercises (no manifest wrapper).""" + + def test_is_empty_default(self): + self.assertTrue(GitUser().is_empty()) + + def test_is_empty_false_when_name_set(self): + self.assertFalse(GitUser(name="x").is_empty()) + + def test_is_empty_false_when_email_set(self): + self.assertFalse(GitUser(email="x@y").is_empty()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_smolmachines_provision.py b/tests/unit/test_smolmachines_provision.py index d738363..9cfa6dc 100644 --- a/tests/unit/test_smolmachines_provision.py +++ b/tests/unit/test_smolmachines_provision.py @@ -36,6 +36,7 @@ def _plan( agent_prompt: str = "", skills: list[str] | None = None, git: list[GitEntry] = (), + git_user: dict | None = None, copy_cwd: bool = False, user_cwd: str = "/tmp/x", stage_dir: Path | None = None, @@ -57,6 +58,8 @@ def _plan( } for g in git ] + if git_user is not None: + bottle_json["git_user"] = git_user if supervise: bottle_json["supervise"] = True manifest = Manifest.from_json_obj({ @@ -467,6 +470,75 @@ class TestProvisionGit(unittest.TestCase): ) +class TestProvisionGitUser(unittest.TestCase): + """`_provision_git_user` runs `git config --global` inside the + guest as the node user with HOME forced via `smolvm -e` + (otherwise --global lands in /root/.gitconfig). No-op when the + bottle didn't declare git_user (issue #86).""" + + def _git_config_calls(self, mock_exec): + """Filter machine_exec calls down to git-config invocations, + return list of (argv, env-dict) tuples.""" + out = [] + for c in mock_exec.call_args_list: + argv = c.args[1] if len(c.args) > 1 else c.kwargs.get("argv", []) + if "git" in argv and "config" in argv: + out.append((argv, c.kwargs.get("env") or {})) + return out + + def test_noop_when_no_git_user(self): + with patch( + "claude_bottle.backend.smolmachines.provision.git._smolvm.machine_exec" + ) as ex: + _git._provision_git_user(_plan(), "claude-bottle-demo-abc12") + self.assertEqual([], self._git_config_calls(ex)) + + def test_sets_name_and_email_as_node(self): + plan = _plan(git_user={ + "name": "Eric Bauerfeld", + "email": "eric@dideric.is", + }) + with patch( + "claude_bottle.backend.smolmachines.provision.git._smolvm.machine_exec" + ) as ex: + _git._provision_git_user(plan, "claude-bottle-demo-abc12") + calls = self._git_config_calls(ex) + self.assertEqual(2, len(calls)) + # Both go through `runuser -u node --` so they run as node; + # HOME is forced via smolvm -e so --global writes to + # /home/node/.gitconfig and not /root/.gitconfig. + for argv, env in calls: + self.assertEqual( + ["runuser", "-u", "node", "--", + "git", "config", "--global"], + argv[:7], + ) + self.assertEqual("/home/node", env.get("HOME")) + self.assertEqual("node", env.get("USER")) + self.assertEqual(["user.name", "Eric Bauerfeld"], calls[0][0][7:]) + self.assertEqual(["user.email", "eric@dideric.is"], calls[1][0][7:]) + + def test_name_only(self): + plan = _plan(git_user={"name": "Bot"}) + with patch( + "claude_bottle.backend.smolmachines.provision.git._smolvm.machine_exec" + ) as ex: + _git._provision_git_user(plan, "claude-bottle-demo-abc12") + calls = self._git_config_calls(ex) + self.assertEqual(1, len(calls)) + self.assertEqual(["user.name", "Bot"], calls[0][0][7:]) + + def test_email_only(self): + plan = _plan(git_user={"email": "bot@example.com"}) + with patch( + "claude_bottle.backend.smolmachines.provision.git._smolvm.machine_exec" + ) as ex: + _git._provision_git_user(plan, "claude-bottle-demo-abc12") + calls = self._git_config_calls(ex) + self.assertEqual(1, len(calls)) + self.assertEqual(["user.email", "bot@example.com"], calls[0][0][7:]) + + class TestProvisionSupervise(unittest.TestCase): def test_noop_when_supervise_not_enabled(self): with patch(