Hand the venv and checkout to the ops group

README.md

@@ -104,6 +104,13 @@ hands over:
 exec "$VENV_DIR/bin/<name>" install "$@"
 ```
 
+It also hands the virtualenv and the checkout to the daemon account with the
+ops group writable, so an operator can pull, hotfix and import a driver to
+test it without sudo. That grants nothing new: they already decide what the
+daemons run, through the configs they edit and the restarts they trigger. git
+is told separately, since it refuses a repository whose owner is not the
+caller no matter the permissions.
+
 Everything else needs no root. `doctor` compares the installed unit and polkit
 rule against what `install` would write now, so a host whose unit predates a
 template change is reported rather than silently stale.

src/instrumentctl/installer.py

@@ -16,6 +16,7 @@
 import os
 import pwd
 import shutil
+import stat
 import subprocess
 import tempfile
 from importlib import resources
@@ -181,6 +182,44 @@ def _sudoers(product: Product, paths: Paths) -> None:
     shutil.chown(product.sudoers, "root", "root")
 
 
+def _hand_to_operators(tree: Path, product: Product) -> None:
+    """Give a tree to the daemon account, writable by the ops group.
+
+    Operators already decide what the daemons run, through the configs they
+    edit and the restarts they trigger, so letting them pull and hotfix the
+    checkout grants nothing new. It spares a sudo for every edit and lets them
+    import a driver to test it.
+    """
+    if not tree.is_dir():
+        return
+    for target in (tree, *tree.rglob("*")):
+        if target.is_symlink():
+            continue
+        shutil.chown(target, product.user, product.ops_group)
+        mode = stat.S_IMODE(target.stat().st_mode)
+        # Setgid on directories so anything created later keeps the group,
+        # rather than the creating operator's own
+        target.chmod(mode | (0o2770 if target.is_dir() else 0o660))
+
+
+def _git_safe_directory(repo: Path) -> None:
+    """Let operators run git here despite the tree belonging to the daemon.
+
+    Since 2.35.2 git refuses a repository whose owner is not the caller, which
+    group permissions do not satisfy.
+    """
+    if not (repo / ".git").exists():
+        return
+    listed = subprocess.run(["git", "config", "--system", "--get-all",
+                             "safe.directory"], check=False, capture_output=True,
+                            text=True)
+    if str(repo) in listed.stdout.split():
+        return
+    subprocess.run(["git", "config", "--system", "--add", "safe.directory",
+                    str(repo)], check=False)
+    _say("git trusts", str(repo))
+
+
 def install(paths: Paths, args: argparse.Namespace) -> int:
     """Converge this host onto what the instrument needs. Root only."""
     product = paths.product
@@ -198,10 +237,10 @@ def install(paths: Paths, args: argparse.Namespace) -> int:
     _directories(product, paths)
     _secrets_and_role(product, paths, args.host_role)
     _artefacts(product, paths)
-    shutil.chown(paths.venv, product.user, product.user)
-    for child in paths.venv.rglob("*"):
-        shutil.chown(child, product.user, product.user)
-    _say("owned", f"{paths.venv} by {product.user}")
+    for tree in (paths.venv, paths.repo):
+        _hand_to_operators(tree, product)
+        _say("owned", f"{tree} by {product.user}:{product.ops_group}")
+    _git_safe_directory(paths.repo)
     subprocess.run(["systemctl", "daemon-reload"], check=False)
 
     print(f"\nHost setup complete. Operators in {product.ops_group} can now "

tests/test_ownership.py

@@ -0,0 +1,71 @@
+"""Handing the venv and checkout to operators."""
+from __future__ import annotations
+
+import os
+import pathlib
+import stat
+from unittest import mock
+
+import pytest
+
+from instrumentctl import installer
+from instrumentctl.product import Product
+
+
+@pytest.fixture(name="tree")
+def tree_fixture(tmp_path: pathlib.Path) -> pathlib.Path:
+    """A venv-shaped tree: an executable, a plain file, nested directories."""
+    (tmp_path / "bin").mkdir()
+    (tmp_path / "lib" / "pkg").mkdir(parents=True)
+    (tmp_path / "bin" / "python3").touch(mode=0o755)
+    (tmp_path / "lib" / "pkg" / "mod.py").touch(mode=0o644)
+    return tmp_path
+
+
+def hand_over(tree: pathlib.Path, product: Product) -> None:
+    """Run the real mode changes; chown needs accounts a test does not have."""
+    with mock.patch.object(installer.shutil, "chown", lambda *a, **k: None):
+        installer._hand_to_operators(tree, product)
+
+
+def mode(path: pathlib.Path) -> int:
+    return stat.S_IMODE(path.stat().st_mode)
+
+
+def test_operators_can_write_every_file(tree: pathlib.Path, product: Product) -> None:
+    hand_over(tree, product)
+    for path in (tree / "bin" / "python3", tree / "lib" / "pkg" / "mod.py"):
+        assert mode(path) & stat.S_IWGRP, path
+
+
+def test_operators_can_write_every_directory(tree: pathlib.Path,
+                                             product: Product) -> None:
+    hand_over(tree, product)
+    for path in (tree, tree / "bin", tree / "lib" / "pkg"):
+        assert mode(path) & stat.S_IWGRP, path
+
+
+def test_executables_stay_executable(tree: pathlib.Path, product: Product) -> None:
+    """Group-write must not cost the venv its interpreter."""
+    hand_over(tree, product)
+    assert mode(tree / "bin" / "python3") & stat.S_IXUSR
+
+
+def test_plain_files_do_not_become_executable(tree: pathlib.Path,
+                                              product: Product) -> None:
+    hand_over(tree, product)
+    assert not mode(tree / "lib" / "pkg" / "mod.py") & stat.S_IXUSR
+
+
+@pytest.mark.skipif(os.geteuid() != 0 or os.uname().sysname == "Darwin",
+                    reason="macOS strips setgid for a non-root caller")
+def test_directories_are_setgid(tree: pathlib.Path, product: Product) -> None:
+    """So a file an operator adds keeps the group the daemon reads as."""
+    hand_over(tree, product)
+    assert mode(tree / "bin") & stat.S_ISGID
+
+
+def test_a_missing_tree_is_not_an_error(tmp_path: pathlib.Path,
+                                        product: Product) -> None:
+    """install runs before the checkout exists on a fresh host."""
+    hand_over(tmp_path / "absent", product)