From 1520d3ff64a1c1bcaa28878e0e1699ba5b319992 Mon Sep 17 00:00:00 2001 From: Tien Le Date: Tue, 6 Oct 2026 15:17:10 +0000 Subject: [PATCH 1/2] Repair conflicting Claude and Codex managed settings without prompting --- docs/os-managed-settings-design.md | 34 ++--- src/ucode/agents/claude.py | 12 +- src/ucode/agents/codex.py | 12 +- src/ucode/managed_files.py | 37 +++--- tests/README.md | 7 +- tests/test_agent_claude.py | 36 ++++-- tests/test_agent_codex.py | 66 +++++++--- tests/test_managed_files.py | 194 +++++++++++++++++++++++++++-- 8 files changed, 310 insertions(+), 88 deletions(-) diff --git a/docs/os-managed-settings-design.md b/docs/os-managed-settings-design.md index bf190775a..0f69a0fbc 100644 --- a/docs/os-managed-settings-design.md +++ b/docs/os-managed-settings-design.md @@ -13,8 +13,9 @@ The two stacked PRs make precedence handling deterministic: managed-settings path. After both PRs merge, interactive configuration reconciles the agent's OS-managed file by default. -Non-interactive and CI execution never elevates privileges and instead uses local settings when the -managed file is compatible. +Non-interactive execution uses local settings when the managed file is compatible. Both agents repair +conflicting managed settings using `sudo -n`, which requires existing authorization and never prompts +for a password. If that repair is denied, the agent does not launch. ## Configuration Files @@ -28,10 +29,14 @@ configuration, except for Claude subscription relay. ## Interactive Detection -An invocation may modify OS-managed settings only when standard input is a TTY. Standard output does -not affect the decision, so piping logs does not disable an otherwise interactive configuration. -CI, pipes, cron jobs, and headless subprocesses normally have non-TTY standard input and therefore -remain local-only. +Standard input determines whether UG may request administrator permission interactively. Standard +output does not affect the decision. Without a TTY, either agent may repair an existing conflicting +file through one-shot `sudo -n` with stdin disconnected; absent or compatible files remain local-only. +A denied repair blocks launch. + +`managed_writes_allowed` centralizes this policy: its default remains interactive-only, while the +shared reconciliation/writer permits an existing-file repair attempt. The writer derives sudo's +prompt mode and stdin handling itself; the predicate does not probe or grant authorization. This is a new shared ucode distinction. The previous implementation inferred interactivity from command shape in some flows and did not guard managed-file writes consistently. @@ -46,12 +51,13 @@ command shape in some flows and did not guard managed-file writes consistently. | Interactive | Already identical | Continue without a backup, write, or `sudo` invocation. | | Non-interactive | Absent | Use the local ucode file. Do not create the managed file. | | Non-interactive | Ucode-owned values absent or equal | Use the local ucode file. Do not modify the managed file. | -| Non-interactive | Ucode-owned value conflicts | Stop before launching because the higher-precedence value would override ucode. | +| Non-interactive | Ucode-owned value conflicts | Repair through `sudo -n`; stop before launching if authorization or verification fails. | | Any | Invalid, unreadable, or symlinked | Stop without modifying the file because precedence cannot be established safely. | `ucode configure`, first-time `ucode claude` or `ucode codex`, and later launches all use the same agent-specific reconciliation path. A first-time launch from an interactive terminal can therefore -request administrator permission. A first-time non-interactive launch remains local-only. +request administrator permission. A first-time non-interactive launch uses local settings unless +the agent must repair an existing conflicting managed file. For Claude, turning off UG tracing preserves the live OS-managed telemetry values unchanged, including values UG wrote on an earlier run. The same applies when the workspace has no managed @@ -99,8 +105,8 @@ errors: - retries once only when device management restored the exact pre-write contents; - preserves a concurrently changed policy instead of overwriting it. -The privilege boundary is interactive. Non-interactive paths do not invoke either normal `sudo` or -`sudo -n`. +Interactive updates may request administrator permission. Non-interactive conflict repairs use only +`sudo -n` with stdin disconnected. Root access cannot sustainably override an actively enforced policy. If an MDM process immediately restores the original file twice, ucode stops with an error instead of repeatedly fighting the @@ -157,7 +163,7 @@ to unchanged launches. The verification scopes distinguish: -- an interactively reconciled managed file; +- a managed file reconciled interactively or through an authorized non-prompting repair; - a managed file verified as compatible with local settings; - a managed file verified as compatible with Claude relay. @@ -196,9 +202,9 @@ result. An identical file produces no elevation message. Representative blockers are: ```text -Claude Code configuration cannot be applied non-interactively because OS-managed settings at - override ucode values: env.ANTHROPIC_BASE_URL. Run `ucode configure --agent claude` from an -interactive terminal or contact your administrator. +Claude Code cannot start because ucode could not update : sudo: a password is required. +Run the ucode command from an interactive terminal and approve the administrator prompt, or contact +your administrator. ``` ```text diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 68e3f1d00..f803c34cf 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -1694,15 +1694,8 @@ def _reconcile_managed_settings( managed_before = copy.deepcopy(existing) desired_settings = compose(existing) _preserve_permission_denies(managed_before, desired_settings) - if not managed_writes_allowed(): - conflicts = _managed_settings_conflicts(managed_before, desired_settings, owned_paths) - if conflicts: - raise RuntimeError( - "Claude Code configuration cannot be applied non-interactively because " - f"OS-managed settings at {path} override ucode values: {', '.join(conflicts)}. " - "Run `ucode configure --agent claude` from an interactive terminal or contact " - "your administrator." - ) + conflicts = _managed_settings_conflicts(managed_before, desired_settings, owned_paths) + if not managed_writes_allowed() and not conflicts: mark_managed_file_verified(state, "claude", path, scope="local-compatible") return try: @@ -1715,7 +1708,6 @@ def _reconcile_managed_settings( parser=_parse_managed_settings, ) except ManagedFileWriteUnavailable: - conflicts = _managed_settings_conflicts(managed_before, desired_settings, owned_paths) if conflicts: raise print_warning( diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 3c04a93ff..1ff680c14 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -606,15 +606,8 @@ def _reconcile_managed_config(state: dict, compose: Callable[[dict], dict]) -> N ) from exc managed_before = copy.deepcopy(existing) desired_doc = compose(existing) - if not managed_writes_allowed(): - conflicts = managed_file_conflicts(managed_before, desired_doc, MANAGED_KEYS) - if conflicts: - raise RuntimeError( - "Codex configuration cannot be applied non-interactively because OS-managed " - f"settings at {path} override ucode values: {', '.join(conflicts)}. Run `ucode " - "configure --agent codex` from an interactive terminal or contact your " - "administrator." - ) + conflicts = managed_file_conflicts(managed_before, desired_doc, MANAGED_KEYS) + if not managed_writes_allowed() and not conflicts: mark_managed_file_verified(state, "codex", path, scope="local-compatible") return try: @@ -627,7 +620,6 @@ def _reconcile_managed_config(state: dict, compose: Callable[[dict], dict]) -> N parser=_parse_managed_config, ) except ManagedFileWriteUnavailable: - conflicts = managed_file_conflicts(managed_before, desired_doc, MANAGED_KEYS) if conflicts: raise print_warning_err( diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index 7cdcea540..bf9f23829 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -1,8 +1,8 @@ """Safely manage root-owned, highest-precedence agent settings files. Interactive updates preserve unrelated policy, retain a private baseline for ``ucode revert``, and -verify the privileged atomic replacement. Non-interactive runs only check whether existing managed -values are compatible with ucode's local settings. +verify the privileged atomic replacement. Existing-file repairs may run without a terminal; +the shared writer uses sudo's non-prompting mode without consuming the caller's standard input. """ from __future__ import annotations @@ -168,9 +168,9 @@ def managed_file_scope(state: dict, tool: str) -> str: return scope if isinstance(scope, str) else "managed" -def managed_writes_allowed() -> bool: - """Managed writes are interactive setup work; scripts and CI use local settings.""" - return sys.stdin.isatty() +def managed_writes_allowed(*, repair_existing: bool = False) -> bool: + """Allow interactive writes or existing-file repair attempts; sudo authorizes the write.""" + return sys.stdin.isatty() or repair_existing @contextmanager @@ -468,17 +468,17 @@ def reconcile_managed_file( f"{display}: OS-managed settings aren't supported on this platform; skipped {path}." ) return "unsupported" - if not managed_writes_allowed() and not is_dry_run(): - raise RuntimeError( - f"Refusing to update {display} managed settings at {path} non-interactively. " - "Run the command from an interactive terminal." - ) if path.is_symlink(): raise RuntimeError( f"Refusing to update {display} managed settings through symlink {path}. " "Replace it with a regular file or contact your administrator." ) current_text = read_managed_file(path) + if not managed_writes_allowed(repair_existing=current_text is not None) and not is_dry_run(): + raise RuntimeError( + f"Refusing to update {display} managed settings at {path} non-interactively. " + "Run the command from an interactive terminal." + ) if current_text == desired_text: return "unchanged" if current_text is not None: @@ -497,7 +497,8 @@ def reconcile_managed_file( created = current_text is None _ensure_backup(tool, path, current_text) - _print_managed_write_permission(display) + if managed_writes_allowed(): + _print_managed_write_permission(display) if read_managed_file(path) != current_text: raise RuntimeError( f"{display} managed settings changed while ucode was preparing the update. " @@ -1009,16 +1010,17 @@ def _sudo_remove(path: Path) -> None: def _sudo_replace(path: Path, desired_text: str) -> None: """Atomically replace ``path`` while preserving metadata and file flags.""" - if not managed_writes_allowed(): - raise RuntimeError("Refusing to invoke sudo for managed settings non-interactively.") _validate_sudo_replace_target(path) + if not managed_writes_allowed(repair_existing=path.is_file()): + raise RuntimeError("Refusing to create managed settings non-interactively.") + non_interactive = not managed_writes_allowed() with tempfile.NamedTemporaryFile( mode="w", suffix=path.suffix or ".tmp", delete=False, encoding="utf-8" ) as tmp: tmp.write(desired_text) tmp_path = tmp.name try: - if _managed_write_session_depth: + if _managed_write_session_depth and not non_interactive: _session_worker().replace(path, tmp_path) else: subprocess_cross_os.run( @@ -1027,6 +1029,7 @@ def _sudo_replace(path: Path, desired_text: str) -> None: tmp_path, str(path), ), + stdin=subprocess.DEVNULL if non_interactive else None, capture_output=True, text=True, check=True, @@ -1105,7 +1108,5 @@ def _sudo_failure_message(path: Path, display: str, exc: subprocess.CalledProces def _sudo_command(*args: str) -> list[str]: - """Build a sudo command only for an explicitly interactive managed-file operation.""" - if not managed_writes_allowed(): - raise RuntimeError("Refusing to invoke sudo for managed settings non-interactively.") - return [_SUDO, *args] + """Never prompt for privileged commands when no terminal is available.""" + return [_SUDO, *([] if managed_writes_allowed() else ["-n"]), *args] diff --git a/tests/README.md b/tests/README.md index 66d24fa4f..8aa92efa9 100644 --- a/tests/README.md +++ b/tests/README.md @@ -82,8 +82,11 @@ catalogs. It also checks explicit search-model precedence and preservation of an existing Isaac server when no GPT model is available. These component tests mock external discovery and the Claude CLI; they do not establish live search coverage. -`test_agent_claude.py` covers OS-managed telemetry ownership and headless configuration. These are -unit/component regressions, not automated Isaac or live telemetry-export coverage. +`test_agent_claude.py`, `test_agent_codex.py`, and `test_managed_files.py` cover headless repair of +conflicting managed settings, compatible/absent no-ops, policy and backup preservation, denied +authorization, and one-shot `sudo -n` with disconnected stdin. These are component regressions with +privileged writes replaced by temporary-file operations, not automated Isaac, real sudo-policy, or +live telemetry-export coverage. Managed smart defaults are covered by `test_managed_config.py`, `test_cli.py`, `test_managed_setup.py`, `test_databricks.py`, and `test_managed_budget.py`: parsing the diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index e9bb3436a..2274508fb 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -1264,7 +1264,7 @@ def test_managed_file_wholesale_overwrite_survives_real_reconcile_round_trip( ) monkeypatch.setattr(claude, "_managed_settings_path", lambda: managed_path) monkeypatch.setattr(claude, "managed_writes_allowed", lambda: True) - monkeypatch.setattr(managed_files, "managed_writes_allowed", lambda: True) + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: True) monkeypatch.setattr(claude, "backup_existing_file", lambda *a, **kw: True) monkeypatch.setattr(claude, "save_state", lambda state: None) monkeypatch.setattr(claude, "ug_version", lambda: "1.0") @@ -1326,7 +1326,7 @@ def _write(target, text): monkeypatch.setattr(managed_files, "_sudo_remove", lambda *a: sudo_writes.append("remove")) monkeypatch.setattr(claude, "_managed_settings_path", lambda: managed_path) monkeypatch.setattr(claude, "managed_writes_allowed", lambda: True) - monkeypatch.setattr(managed_files, "managed_writes_allowed", lambda: True) + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: True) monkeypatch.setattr(claude, "backup_existing_file", lambda *a, **kw: True) monkeypatch.setattr(claude, "save_state", lambda state: None) monkeypatch.setattr(claude, "ug_version", lambda: "1.0") @@ -1968,20 +1968,29 @@ def test_noninteractive_uses_local_settings_when_managed_file_is_compatible(self assert managed_writes == [] - def test_noninteractive_fails_when_managed_file_conflicts(self, monkeypatch): + def test_noninteractive_repairs_conflicting_managed_settings_without_prompting( + self, monkeypatch + ): private_writes: list = [] managed_writes: list = [] existing = { - str(FAKE_MANAGED_PATH): {"env": {"ANTHROPIC_BASE_URL": "https://other.example.com"}} + str(FAKE_MANAGED_PATH): { + "apiKeyHelper": "isaac auth token", + "env": {"ANTHROPIC_BASE_URL": "https://other.example.com", "ISAAC_ONLY": "keep"}, + "permissions": {"deny": ["Read(secret.txt)"]}, + } } self._patch(monkeypatch, private_writes, managed_writes, existing) monkeypatch.setattr(claude, "managed_writes_allowed", lambda: False) - state = {"workspace": WS, "codex_models": []} - with pytest.raises(RuntimeError, match="cannot be applied non-interactively"): - claude.write_tool_config(state, "databricks-claude-sonnet-4") + claude.write_tool_config({"workspace": WS, "codex_models": []}, None) - assert managed_writes == [] + assert len(managed_writes) == 1 + written = json.loads(managed_writes[0][1]) + assert written["apiKeyHelper"] == private_writes[0][1]["apiKeyHelper"] + assert written["env"]["ANTHROPIC_BASE_URL"] == f"{WS}/ai-gateway/anthropic" + assert written["env"]["ISAAC_ONLY"] == "keep" + assert "Read(secret.txt)" in written["permissions"]["deny"] def test_headless_isaac_headers_and_telemetry_are_compatible(self, monkeypatch): private_writes: list = [] @@ -2065,7 +2074,12 @@ def test_explicit_ug_tracing_still_rejects_conflicting_managed_telemetry(self, m ) monkeypatch.setattr(claude, "managed_writes_allowed", lambda: False) - with pytest.raises(RuntimeError, match="env.OTEL_TRACES_EXPORTER"): + def deny_managed_write(*args, **kwargs): + raise managed_files.ManagedFileWriteUnavailable("sudo denied") + + monkeypatch.setattr(claude, "reconcile_managed_file", deny_managed_write) + + with pytest.raises(managed_files.ManagedFileWriteUnavailable, match="sudo denied"): claude.write_tool_config( {"workspace": WS, "codex_models": [], "claude_otel_tracing": True}, None ) @@ -2103,13 +2117,15 @@ def deny_managed_write(*args, **kwargs): assert "continuing with local settings" in warnings[0] assert verified == [{"scope": "local-compatible"}] - def test_sudo_failure_remains_fatal_when_managed_file_conflicts(self, monkeypatch): + @pytest.mark.parametrize("interactive", [False, True]) + def test_sudo_failure_remains_fatal_when_managed_file_conflicts(self, monkeypatch, interactive): private_writes: list = [] managed_writes: list = [] existing = { str(FAKE_MANAGED_PATH): {"env": {"ANTHROPIC_BASE_URL": "https://other.example.com"}} } self._patch(monkeypatch, private_writes, managed_writes, existing) + monkeypatch.setattr(claude, "managed_writes_allowed", lambda: interactive) def deny_managed_write(*args, **kwargs): raise managed_files.ManagedFileWriteUnavailable("sudo denied") diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index 935e9712e..b29b71f13 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -1674,7 +1674,7 @@ def test_managed_config_preserves_other_keys(self, tmp_path, monkeypatch): assert doc["approval_policy"] == "on-request" assert "model" not in doc - def _sudo_counting_env(self, tmp_path, monkeypatch): + def _sudo_counting_env(self, tmp_path, monkeypatch, *, non_interactive=False): """Real reconcile flow (semantic no-op check included) with sudo writes counted.""" config_path = tmp_path / ".codex" / "ucode.config.toml" managed_path = tmp_path / "etc-codex" / "managed_config.toml" @@ -1683,16 +1683,24 @@ def _sudo_counting_env(self, tmp_path, monkeypatch): monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", tmp_path / "codex-ucode-config.backup.toml") monkeypatch.setattr(codex, "agent_version", lambda binary: "0.134.0") monkeypatch.setattr(codex, "save_state", lambda state: None) - monkeypatch.setattr(codex, "managed_writes_allowed", lambda: True) + monkeypatch.setattr(codex, "managed_writes_allowed", lambda: not non_interactive) monkeypatch.setattr(codex, "codex_managed_config_path", lambda: managed_path) monkeypatch.setattr(managed_files, "managed_files_supported", lambda: True) - monkeypatch.setattr(managed_files, "managed_writes_allowed", lambda: True) + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: not non_interactive) monkeypatch.setattr(managed_files, "MANAGED_BACKUP_DIR", tmp_path / "managed-backups") monkeypatch.setattr( managed_files, "MANAGED_BACKUP_MANIFEST_PATH", tmp_path / "managed-backups" / "m.json" ) + if non_interactive: + monkeypatch.setattr( + managed_files, + "_print_managed_write_permission", + lambda *args: pytest.fail("headless repair must not ask for a password"), + ) + def _write(target, text): + assert target == managed_path sudo_writes.append(text) Path(target).parent.mkdir(parents=True, exist_ok=True) Path(target).write_text(text, encoding="utf-8") @@ -1816,28 +1824,50 @@ def test_noninteractive_uses_local_config_when_managed_config_is_compatible( _, managed_path = self._patch(tmp_path, monkeypatch) monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) state = {"workspace": WS, "codex_models": ["gpt-5"]} + codex.write_tool_config(state) + assert not managed_path.exists() - def test_noninteractive_preserves_unrelated_managed_config(self, tmp_path, monkeypatch): - _, managed_path = self._patch(tmp_path, monkeypatch) - managed_path.parent.mkdir(parents=True, exist_ok=True) + def test_noninteractive_compatible_managed_config_is_read_only(self, tmp_path, monkeypatch): + managed_path, sudo_writes = self._sudo_counting_env( + tmp_path, monkeypatch, non_interactive=True + ) original = 'approval_policy = "on-request"\n' + managed_path.parent.mkdir(parents=True, exist_ok=True) managed_path.write_text(original, encoding="utf-8") - monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) + before = managed_files.managed_file_fingerprint(managed_path) + state = {"workspace": WS, "codex_models": ["gpt-5"]} - codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + codex.write_tool_config(state) - assert managed_path.read_text(encoding="utf-8") == original + assert read_toml_safe(codex.CODEX_CONFIG_PATH)["model_provider"] == "Databricks" + assert managed_files.read_managed_file(managed_path) == original + assert managed_files.managed_file_fingerprint(managed_path) == before + assert sudo_writes == [] + assert not managed_files.MANAGED_BACKUP_DIR.exists() + assert state["managed_file_fingerprints"]["codex"]["scope"] == "local-compatible" + assert codex.managed_config_is_current(state) - def test_noninteractive_fails_when_managed_config_conflicts(self, tmp_path, monkeypatch): - _, managed_path = self._patch(tmp_path, monkeypatch) + def test_noninteractive_repairs_conflicting_managed_config_without_prompting( + self, tmp_path, monkeypatch + ): + managed_path, sudo_writes = self._sudo_counting_env( + tmp_path, monkeypatch, non_interactive=True + ) managed_path.parent.mkdir(parents=True, exist_ok=True) - managed_path.write_text('model_provider = "enterprise"\n', encoding="utf-8") - monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) + original = 'model_provider = "enterprise"\napproval_policy = "on-request"\n' + managed_path.write_text(original, encoding="utf-8") + state = {"workspace": WS, "codex_models": ["gpt-5"]} - with pytest.raises(RuntimeError, match="cannot be applied non-interactively"): - codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + codex.write_tool_config(state) + + written = read_toml_safe(managed_path) + local_provider = read_toml_safe(codex.CODEX_CONFIG_PATH)["model_providers"]["Databricks"] + assert written["model_provider"] == "Databricks" + assert written["approval_policy"] == "on-request" + assert written["model_providers"]["Databricks"] == local_provider + assert len(sudo_writes) == 1 def test_invalid_managed_toml_is_not_modified(self, tmp_path, monkeypatch): _, managed_path = self._patch(tmp_path, monkeypatch) @@ -1877,10 +1907,14 @@ def deny_managed_write(*args, **kwargs): assert "continuing with local settings" in warnings[0] assert verified == [{"scope": "local-compatible"}] - def test_sudo_failure_remains_fatal_when_managed_config_conflicts(self, tmp_path, monkeypatch): + @pytest.mark.parametrize("interactive", [False, True]) + def test_sudo_failure_remains_fatal_when_managed_config_conflicts( + self, tmp_path, monkeypatch, interactive + ): _, managed_path = self._patch(tmp_path, monkeypatch) managed_path.parent.mkdir(parents=True, exist_ok=True) managed_path.write_text('model_provider = "enterprise"\n', encoding="utf-8") + monkeypatch.setattr(codex, "managed_writes_allowed", lambda: interactive) def deny_managed_write(*args, **kwargs): raise managed_files.ManagedFileWriteUnavailable("sudo denied") diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py index 606a7ba8f..0ee459d40 100644 --- a/tests/test_managed_files.py +++ b/tests/test_managed_files.py @@ -441,9 +441,60 @@ def run(command, **kwargs): assert str(path) not in command[3] assert str(parent) not in command[3] assert "$(not-a-command)\n" not in command[3] - assert kwargs == {"capture_output": True, "text": True, "check": True} + assert kwargs == {"capture_output": True, "text": True, "check": True, "stdin": None} assert source_path is not None and not source_path.exists() + @pytest.mark.parametrize("denied", [False, True]) + def test_noninteractive_repair_uses_sudo_n_without_session_or_stdin( + self, tmp_path, monkeypatch, denied + ): + calls: list = [] + source_paths: list[Path] = [] + path = tmp_path / "managed-settings.json" + path.write_text('{"apiKeyHelper": "isaac"}') + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: False) + monkeypatch.setattr(managed_files, "current_os", lambda: managed_files.OS.LINUX) + monkeypatch.setitem( + managed_files._SUDO_REPLACE_TARGETS, managed_files.OS.LINUX, frozenset({path}) + ) + + def run(command, **kwargs): + source = Path(command[-2]) + source_paths.append(source) + assert source.read_text() == '{"apiKeyHelper": "ug auth token"}' + calls.append((command, kwargs)) + if denied: + raise subprocess.CalledProcessError( + 1, command, stderr="sudo: a password is required" + ) + return subprocess.CompletedProcess(command, 0, stdout="", stderr="") + + monkeypatch.setattr(managed_files.subprocess_cross_os, "run", run) + monkeypatch.setattr( + managed_files.subprocess_cross_os, + "popen", + lambda *args, **kwargs: pytest.fail( + "headless repair must not start an interactive worker" + ), + ) + + with managed_files.managed_write_session(): + if denied: + with pytest.raises(subprocess.CalledProcessError): + _REAL_SUDO_REPLACE(path, '{"apiKeyHelper": "ug auth token"}') + else: + _REAL_SUDO_REPLACE(path, '{"apiKeyHelper": "ug auth token"}') + + assert len(calls) == 1 + command, kwargs = calls[0] + assert command[:4] == ["/usr/bin/sudo", "-n", "/bin/sh", "-c"] + assert command[4] == managed_files._SUDO_REPLACE_SCRIPT + assert command[5:8] == ["ucode-managed-replace", "once", "linux"] + assert command[-1] == str(path) + assert kwargs["stdin"] == subprocess.DEVNULL + assert kwargs["check"] is True + assert all(not source.exists() for source in source_paths) + def test_rejects_target_outside_fixed_allowlist_before_sudo(self, tmp_path, monkeypatch): monkeypatch.setattr( managed_files.subprocess, @@ -626,6 +677,127 @@ def test_real_shell_creates_missing_file(self, tmp_path, parent_exists): class TestManagedFileLifecycle: + def test_noninteractive_creation_is_blocked(self, tmp_path, backup_dir, monkeypatch): + path = tmp_path / "managed.json" + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: False) + monkeypatch.setattr( + managed_files, + "_sudo_replace", + lambda *args: pytest.fail("headless creation must not write"), + ) + + with pytest.raises(RuntimeError, match="Refusing to update .* non-interactively"): + managed_files.reconcile_managed_file( + path, + '{"apiKeyHelper": "ug"}', + tool="claude", + display="Claude Code", + owned_paths=[["apiKeyHelper"]], + parser=json.loads, + ) + + assert not path.exists() + assert not backup_dir.exists() + + def test_noninteractive_revert_is_blocked(self, tmp_path, backup_dir, monkeypatch): + path = tmp_path / "managed.json" + path.write_text('{"enterprise": true}\n', encoding="utf-8") + monkeypatch.setattr( + managed_files, + "_sudo_replace", + lambda target, text: target.write_text(text, encoding="utf-8"), + ) + managed_files.reconcile_managed_file( + path, + '{"enterprise": true, "ucode": true}\n', + tool="claude", + display="Claude Code", + owned_paths=[["ucode"]], + parser=json.loads, + ) + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: False) + + with pytest.raises(RuntimeError, match="Cannot restore .* non-interactively"): + managed_files.revert_managed_file( + "claude", display="Claude Code", parser=json.loads, dumper=json.dumps + ) + + assert path.read_text(encoding="utf-8") == '{"enterprise": true, "ucode": true}\n' + + def test_noninteractive_repair_preserves_policy_and_backup( + self, tmp_path, backup_dir, monkeypatch + ): + path = tmp_path / "managed.json" + original = '{"apiKeyHelper": "isaac", "permissions": {"deny": ["Read(secret.txt)"]}}\n' + desired = '{"apiKeyHelper": "ug", "permissions": {"deny": ["Read(secret.txt)"]}}\n' + path.write_text(original) + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: False) + + def replace(target, text): + target.write_text(text) + + monkeypatch.setattr(managed_files, "_sudo_replace", replace) + monkeypatch.setattr( + managed_files, + "_print_managed_write_permission", + lambda *args: pytest.fail("headless repair must not ask for a password"), + ) + + assert ( + managed_files.reconcile_managed_file( + path, + desired, + tool="claude", + display="Claude Code", + owned_paths=[["apiKeyHelper"]], + parser=json.loads, + ) + == "written" + ) + assert path.read_text() == desired + assert (backup_dir / "manifest.json").exists() + snapshots = managed_files.managed_file_snapshots("claude", json.loads) + assert snapshots.original_before_ug == json.loads(original) + assert snapshots.last_applied_by_ug == json.loads(desired) + + def test_noninteractive_permission_failure_preserves_file_and_does_not_retry( + self, tmp_path, backup_dir, monkeypatch + ): + path = tmp_path / "managed.json" + original = '{"apiKeyHelper": "isaac"}\n' + path.write_text(original) + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: False) + calls: list = [] + + def deny_write(target, text): + calls.append(target) + raise subprocess.CalledProcessError( + 1, ["sudo", "-n"], stderr="sudo: a password is required" + ) + + monkeypatch.setattr(managed_files, "_sudo_replace", deny_write) + monkeypatch.setattr( + managed_files, + "_print_managed_write_permission", + lambda *args: pytest.fail("headless repair must not ask for a password"), + ) + + with pytest.raises(managed_files.ManagedFileWriteUnavailable, match="password is required"): + managed_files.reconcile_managed_file( + path, + '{"apiKeyHelper": "ug"}\n', + tool="claude", + display="Claude Code", + owned_paths=[["apiKeyHelper"]], + parser=json.loads, + ) + + assert path.read_text() == original + assert calls == [path] + assert managed_files.managed_file_snapshots( + "claude", json.loads + ).original_before_ug == json.loads(original) + def test_dry_run_does_not_write_or_backup(self, tmp_path, backup_dir, monkeypatch): path = tmp_path / "managed.json" config_io.set_dry_run(True) @@ -1128,14 +1300,20 @@ def no_sudo(*args, **kwargs): assert path.read_text() == admin_text # exact bytes preserved -def test_managed_writes_disabled_without_tty(monkeypatch): - monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: False) +@pytest.mark.parametrize( + ("interactive", "repair_existing", "expected"), + [(False, False, False), (False, True, True), (True, False, True)], +) +def test_managed_writes_policy(monkeypatch, interactive, repair_existing, expected): + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: interactive) - assert managed_files.managed_writes_allowed() is False + assert managed_files.managed_writes_allowed() is interactive + assert managed_files.managed_writes_allowed(repair_existing=repair_existing) is expected -def test_sudo_command_refuses_noninteractive_execution(monkeypatch): - monkeypatch.setattr(managed_files, "managed_writes_allowed", lambda: False) +@pytest.mark.parametrize("interactive", [False, True]) +def test_sudo_command_derives_prompt_policy(monkeypatch, interactive): + monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: interactive) - with pytest.raises(RuntimeError, match="Refusing to invoke sudo"): - managed_files._sudo_command("cp", "a", "b") + expected = ["/usr/bin/sudo", *([] if interactive else ["-n"]), "cp", "a", "b"] + assert managed_files._sudo_command("cp", "a", "b") == expected From eef3ebb74b2d509b2595f80d267a39da8ebccf52 Mon Sep 17 00:00:00 2001 From: Tien Le Date: Wed, 7 Oct 2026 15:36:58 +0000 Subject: [PATCH 2/2] Name the sudo prompt-mode check separately from the write gate `managed_writes_allowed()` was doing double duty: with `repair_existing` it is the write-authorization gate, but its no-arg form was borrowed as an "is a terminal attached?" check at the sudo-mode sites, so `non_interactive = not managed_writes_allowed()` read backwards. Extract `_sudo_may_prompt()` for the TTY/prompt-mode check and build `managed_writes_allowed()` on top of it; the three prompt-mode sites now call `_sudo_may_prompt()` while the authorization-gate sites keep `managed_writes_allowed`. Behavior is unchanged (`_sudo_may_prompt()` equals the former no-arg form). Co-authored-by: Isaac --- src/ucode/managed_files.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index bf9f23829..eecc07d0e 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -168,9 +168,14 @@ def managed_file_scope(state: dict, tool: str) -> str: return scope if isinstance(scope, str) else "managed" +def _sudo_may_prompt() -> bool: + """A terminal is attached, so sudo can prompt and the persistent worker is usable.""" + return sys.stdin.isatty() + + def managed_writes_allowed(*, repair_existing: bool = False) -> bool: - """Allow interactive writes or existing-file repair attempts; sudo authorizes the write.""" - return sys.stdin.isatty() or repair_existing + """Writes need either an interactive terminal or an existing file to repair in place.""" + return _sudo_may_prompt() or repair_existing @contextmanager @@ -497,7 +502,7 @@ def reconcile_managed_file( created = current_text is None _ensure_backup(tool, path, current_text) - if managed_writes_allowed(): + if _sudo_may_prompt(): _print_managed_write_permission(display) if read_managed_file(path) != current_text: raise RuntimeError( @@ -1013,7 +1018,7 @@ def _sudo_replace(path: Path, desired_text: str) -> None: _validate_sudo_replace_target(path) if not managed_writes_allowed(repair_existing=path.is_file()): raise RuntimeError("Refusing to create managed settings non-interactively.") - non_interactive = not managed_writes_allowed() + non_interactive = not _sudo_may_prompt() with tempfile.NamedTemporaryFile( mode="w", suffix=path.suffix or ".tmp", delete=False, encoding="utf-8" ) as tmp: @@ -1109,4 +1114,4 @@ def _sudo_failure_message(path: Path, display: str, exc: subprocess.CalledProces def _sudo_command(*args: str) -> list[str]: """Never prompt for privileged commands when no terminal is available.""" - return [_SUDO, *([] if managed_writes_allowed() else ["-n"]), *args] + return [_SUDO, *([] if _sudo_may_prompt() else ["-n"]), *args]