From a5720b00bd9b812994d33de6aedd20f83e9a3973 Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 19:06:46 -0400 Subject: [PATCH 1/6] Skip sudo wrapper when os_user matches bot process user When claude_user equals the current process user, self-sudo fails because sudoers disallows it. Detect this at spawn time using pwd.getpwuid(os.getuid()).pw_name and skip the sudo wrapper, logging a warning about the misconfiguration. The local effective_claude_user variable avoids mutating the instance attribute, which may be read elsewhere. Fixes #191 --- src/kai/claude.py | 23 ++++++++++++++++----- tests/test_claude.py | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/kai/claude.py b/src/kai/claude.py index 9709b4a..b2875e1 100644 --- a/src/kai/claude.py +++ b/src/kai/claude.py @@ -29,6 +29,7 @@ import json import logging import os +import pwd import signal import time from collections.abc import AsyncIterator @@ -210,15 +211,27 @@ async def _ensure_started(self) -> None: # When running as a different user, spawn via sudo -u. # The subprocess runs with the target user's UID, home directory, # and environment - completely isolated from the bot user. - if self.claude_user: - cmd = ["sudo", "-u", self.claude_user, "--"] + claude_cmd + # Skip sudo when the target user is the same as the bot user - + # self-sudo fails (sudoers disallows it) and serves no purpose. + effective_claude_user = self.claude_user + if effective_claude_user: + current_user = pwd.getpwuid(os.getuid()).pw_name + if effective_claude_user == current_user: + log.warning( + "os_user %r matches the bot process user; skipping sudo (no isolation)", + effective_claude_user, + ) + effective_claude_user = None + + if effective_claude_user: + cmd = ["sudo", "-u", effective_claude_user, "--"] + claude_cmd else: cmd = claude_cmd log.info( "Starting persistent Claude process (model=%s, user=%s)", self.model, - self.claude_user or "(same as bot)", + effective_claude_user or "(same as bot)", ) # Build the subprocess environment. Merge order: @@ -247,7 +260,7 @@ async def _ensure_started(self) -> None: # When spawned via sudo, start in a new process group so we can # kill the entire tree (sudo + claude) via os.killpg(). Without # this, killing sudo may orphan the claude process. - start_new_session=bool(self.claude_user), + start_new_session=bool(effective_claude_user), ) self._session_id = None self._fresh_session = True @@ -258,7 +271,7 @@ async def _ensure_started(self) -> None: # with PGID == PID (session leader). Save it now because os.getpgid() # fails after the process exits, but os.killpg() works as long as any # group member is still alive (i.e., the actual claude process). - if self.claude_user: + if effective_claude_user: self._pgid = self._proc.pid # PGID == PID for session leaders else: self._pgid = None diff --git a/tests/test_claude.py b/tests/test_claude.py index 60d8b9f..2e26f3c 100644 --- a/tests/test_claude.py +++ b/tests/test_claude.py @@ -14,6 +14,7 @@ import asyncio import json +import os import signal import time from pathlib import Path @@ -171,6 +172,53 @@ async def test_start_new_session_true_with_claude_user(self): args = mock_exec.call_args assert args[1].get("start_new_session") is True + @pytest.mark.asyncio + async def test_self_sudo_skipped(self, caplog): + """When claude_user matches the current process user, sudo is skipped.""" + import pwd + + current_user = pwd.getpwuid(os.getuid()).pw_name + claude = _make_claude(claude_user=current_user) + + with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec: + mock_proc = MagicMock() + mock_proc.returncode = None + mock_proc.stderr = AsyncMock() + mock_exec.return_value = mock_proc + + with caplog.at_level("WARNING", logger="kai.claude"): + await claude._ensure_started() + + args = mock_exec.call_args + cmd = args[0] + # Command should NOT start with sudo + assert cmd[0] == "claude" + assert "sudo" not in cmd + # No process group isolation + assert args[1].get("start_new_session") is False + assert claude._pgid is None + # Warning was logged + assert "skipping sudo" in caplog.text + + @pytest.mark.asyncio + async def test_different_user_still_uses_sudo(self): + """When claude_user is a different user, sudo is still used.""" + claude = _make_claude(claude_user="some_other_user") + + with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec: + mock_proc = MagicMock() + mock_proc.returncode = None + mock_proc.stderr = AsyncMock() + mock_exec.return_value = mock_proc + + await claude._ensure_started() + + args = mock_exec.call_args + cmd = args[0] + assert cmd[0] == "sudo" + assert cmd[2] == "some_other_user" + assert args[1].get("start_new_session") is True + # ── Process signal handling ────────────────────────────────────────── From c55c30d5dd29c4f8976c70cccdeeb031ab4cd175 Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 19:15:35 -0400 Subject: [PATCH 2/6] Address PR review: handle KeyError in pwd.getpwuid, improve test hermeticity - Wrap pwd.getpwuid() in try/except KeyError for container environments where the UID has no /etc/passwd entry (falls through to sudo path) - Move import pwd to module level in test_claude.py - Patch pwd.getpwuid in test_different_user_still_uses_sudo for hermeticity - Add test_pwd_keyerror_falls_through_to_sudo for the new fallback --- src/kai/claude.py | 8 +++++++- tests/test_claude.py | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/kai/claude.py b/src/kai/claude.py index b2875e1..96a5a1c 100644 --- a/src/kai/claude.py +++ b/src/kai/claude.py @@ -215,7 +215,13 @@ async def _ensure_started(self) -> None: # self-sudo fails (sudoers disallows it) and serves no purpose. effective_claude_user = self.claude_user if effective_claude_user: - current_user = pwd.getpwuid(os.getuid()).pw_name + # getpwuid raises KeyError when the UID has no passwd entry + # (e.g., containers with --user ). Treat that as "unknown + # user" and fall through to the sudo path. + try: + current_user = pwd.getpwuid(os.getuid()).pw_name + except KeyError: + current_user = None if effective_claude_user == current_user: log.warning( "os_user %r matches the bot process user; skipping sudo (no isolation)", diff --git a/tests/test_claude.py b/tests/test_claude.py index 2e26f3c..cb047ab 100644 --- a/tests/test_claude.py +++ b/tests/test_claude.py @@ -15,6 +15,7 @@ import asyncio import json import os +import pwd import signal import time from pathlib import Path @@ -175,8 +176,6 @@ async def test_start_new_session_true_with_claude_user(self): @pytest.mark.asyncio async def test_self_sudo_skipped(self, caplog): """When claude_user matches the current process user, sudo is skipped.""" - import pwd - current_user = pwd.getpwuid(os.getuid()).pw_name claude = _make_claude(claude_user=current_user) @@ -205,7 +204,13 @@ async def test_different_user_still_uses_sudo(self): """When claude_user is a different user, sudo is still used.""" claude = _make_claude(claude_user="some_other_user") - with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec: + # Patch pwd.getpwuid to return a fixed value so the test doesn't + # depend on the real system user being different from "some_other_user". + mock_pw = MagicMock(pw_name="kai") + with ( + patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec, + patch("kai.claude.pwd.getpwuid", return_value=mock_pw), + ): mock_proc = MagicMock() mock_proc.returncode = None mock_proc.stderr = AsyncMock() @@ -219,6 +224,30 @@ async def test_different_user_still_uses_sudo(self): assert cmd[2] == "some_other_user" assert args[1].get("start_new_session") is True + @pytest.mark.asyncio + async def test_pwd_keyerror_falls_through_to_sudo(self): + """When pwd.getpwuid raises KeyError (unmapped UID), sudo is used.""" + claude = _make_claude(claude_user="container_user") + + # Simulate a container environment where the UID has no passwd entry. + with ( + patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec, + patch("kai.claude.pwd.getpwuid", side_effect=KeyError("uid not found")), + ): + mock_proc = MagicMock() + mock_proc.returncode = None + mock_proc.stderr = AsyncMock() + mock_exec.return_value = mock_proc + + await claude._ensure_started() + + args = mock_exec.call_args + cmd = args[0] + # Should still wrap with sudo since we can't determine the user. + assert cmd[0] == "sudo" + assert cmd[2] == "container_user" + assert args[1].get("start_new_session") is True + # ── Process signal handling ────────────────────────────────────────── From 2790ee1e178ae067712b876d4aef9a35506fd8b2 Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 19:17:03 -0400 Subject: [PATCH 3/6] Skip self-sudo test when UID has no passwd entry --- tests/test_claude.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_claude.py b/tests/test_claude.py index cb047ab..f76e743 100644 --- a/tests/test_claude.py +++ b/tests/test_claude.py @@ -176,7 +176,10 @@ async def test_start_new_session_true_with_claude_user(self): @pytest.mark.asyncio async def test_self_sudo_skipped(self, caplog): """When claude_user matches the current process user, sudo is skipped.""" - current_user = pwd.getpwuid(os.getuid()).pw_name + try: + current_user = pwd.getpwuid(os.getuid()).pw_name + except KeyError: + pytest.skip("UID has no passwd entry; self-sudo-skip path not reachable") claude = _make_claude(claude_user=current_user) with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_exec: From 500492509b34a18e1109cbc228da7d7a904152e1 Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 19:26:07 -0400 Subject: [PATCH 4/6] re-review From 60455a92275a444ba279a610b0cb2aed40cfc33c Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 19:27:43 -0400 Subject: [PATCH 5/6] Use claude_user in warning log (covers both os_user and CLAUDE_USER) --- src/kai/claude.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kai/claude.py b/src/kai/claude.py index 96a5a1c..458d692 100644 --- a/src/kai/claude.py +++ b/src/kai/claude.py @@ -224,7 +224,8 @@ async def _ensure_started(self) -> None: current_user = None if effective_claude_user == current_user: log.warning( - "os_user %r matches the bot process user; skipping sudo (no isolation)", + "claude_user %r matches the bot process user; " + "skipping sudo (no isolation)", effective_claude_user, ) effective_claude_user = None From 43b3357297748ca32675e1f37969635dcf06969d Mon Sep 17 00:00:00 2001 From: Daniel Ellison Date: Thu, 26 Mar 2026 19:30:44 -0400 Subject: [PATCH 6/6] Fix ruff formatting on warning log line --- src/kai/claude.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/kai/claude.py b/src/kai/claude.py index 458d692..aed47d3 100644 --- a/src/kai/claude.py +++ b/src/kai/claude.py @@ -224,8 +224,7 @@ async def _ensure_started(self) -> None: current_user = None if effective_claude_user == current_user: log.warning( - "claude_user %r matches the bot process user; " - "skipping sudo (no isolation)", + "claude_user %r matches the bot process user; skipping sudo (no isolation)", effective_claude_user, ) effective_claude_user = None