[billiam-wang]

🥞 Stacked PR

Use this link to review incremental changes.

• mdm-policy-write [Files changed] ← this PR
  • mdm-policy-read [Files changed]
    • mdm-policy-windows [Files changed]

---

Worktree: unity-gateway | Branch: mdm-policy-write

What did you change, and why?

First of two stacked PRs that replace #1228, porting #1105's machine workspace policy. This one only writes it: on Linux and macOS, ug configure for Claude or Codex installs the workspace's managed config as root-owned /etc/unitygateway/managed-settings.json ({workspace, config}, 0644 in a root 0755 folder) using sudo. It rewrites the file only on change and removes it when the workspace publishes none. If sudo is denied, configure fails with steps to fix it.

Launches don't change: they still read the per-user cache. Windows, UG_SKIP_OS_SETTINGS and --user write nothing. #1260 makes launches read the file.

How do you know it works?

New unit tests cover the writer and the install, replace and remove paths. One integration test checks that configure installs a root-owned file. Lint and ty are clean.

This pull request and its description were written by Isaac.

[billiam-wang]

/ug-review

[github-actions]

is this file read anywhere? nothing in src/ reads /etc/unitygateway/managed-settings.json, so configure now asks for sudo (and can hard fail) for a file nothing uses. can we hold this back until a reader lands, or put it behind a gate?

[billiam-wang]

This is intentional. The followup consumes the new file: #1260

[github-actions]

what happens on a shared machine? if one user configures workspace B with no policy, workspace A's machine-wide policy gets deleted for everyone. a non-empty config overwrites it the same way. should we only remove a policy whose workspace matches ours?

[billiam-wang]

We assume single user for ug today, so shared machine issues are not a concern. Since we are explicitly modifying the global files, it is expected that all users use the same workspace.

[github-actions]

will this break current ug users? on linux/macos every ug configure with a published config now needs sudo, and none of the callers (cli.py:830, 1049) catch this. CI, sudo -n runs, and users without sudo now abort halfway through configure, after the per-user cache is already saved and before any agent setup runs. can we downgrade this to print_warning and keep going?

[billiam-wang]

Published config already needed sudo since it writes to the global file. This is not a new requirement.

[github-actions]

ug only installs this policy on Linux/macOS, so on a Windows integration run policy.stat() will raise. can we skip it on nt, or add it to WINDOWS_UNSUPPORTED_NODES in run_integration.py?

[billiam-wang]

Windows is planned as a follow up to prevent scope creep.

[billiam-wang]

See followup PR: #1262

[billiam-wang]

/ug-review

[github-actions]

Polly AI Review

Blocking issues

• [P1] CLI cannot start — src/ucode/managed_files.py:1259. can we remove the text-mode with? the binary-mode with has identical indentation, leaving an empty suite and causing IndentationError on import.
• [P2] Test teardown deletes existing policy — tests/integration/conftest.py:170. can cleanup preserve files present before the test? --installation-only skips preflight, so help/version tests delete existing policy with passwordless sudo or fail teardown otherwise.

Security vulnerabilities

No newly introduced security vulnerability was confirmed. The documented world-readable policy format is intentional; administrators must not place credentials in it.

Non-blocking notes

• tests/integration/test_ug_configure_managed.py:122: can we add ownership/mode checks to test_ug_configure_managed_is_idempotent, including actual workspace/config values, instead of another configure run?
• tests/test_managed_config.py::TestMachinePolicyInstall.test_launch_within_ttl_reads_the_per_user_cache: can we remove this duplicate of TestRefreshTTL.test_fresh_published_cache_short_circuits?
• tests/test_managed_config.py::TestMachinePolicyInstall.test_skip_os_settings_never_writes_the_machine_policy: can we exercise real UG_SKIP_OS_SETTINGS and --user inputs rather than only mock the gate?
• tests/test_managed_config.py::TestMachinePolicyInstall.test_launch_never_writes_the_machine_policy: can we cover expired-cache no-config/disabled responses with an installed file and assert it survives?

Approach

The existing managed writer and shell worker are appropriate boundaries. update_workspace_policy could share more sudo invocation and error handling with _sudo_replace, while retaining its distinct removal protocol.

Summary

Diff size (computed): 19 files; +935 / -36 lines (971 changed text lines); 0 binary files.

Category	Files	Added	Deleted	Share of changed text lines
Tests and test support	13	640	28	68.8%
Documentation	1	52	0	5.4%
Dependencies and lockfiles	0	0	0	0.0%
Implementation / other	5	243	8	25.8%

Shares use additions + deletions, including generated text and lockfiles. Binary files have no line count. Tests include fixtures/helpers under test paths and colocated test/spec files; categories are path-based, not coverage measurements.

Changes

The PR installs a machine-wide workspace-policy envelope during configure, removes it when no policy is published, and adds revert handling. Launches continue using the per-user cache. Supporting changes cover privileged writes, platform and opt-out gates, test isolation, cleanup, and documentation. No dependency or generated-file changes contribute to the diff.

Most of the breadth follows from introducing a privileged filesystem boundary. A concrete implementation reduction is sharing the temporary-source, sudo invocation, and error-normalization machinery between update_workspace_policy and _sudo_replace. The import failure and unsafe test cleanup require correction before approval.

Tests

Test-by-test assessment

The frozen diff’s checksum and both trusted review policies were verified. Static parsing confirmed the import failure; the reconstructed file also matches the diff’s post-image blob hash. No tests, lint, or type checks were executed, so the description’s green-gate claims are not independently confirmed.

Existing TTL, config-scope, and shell replacement/rollback tests provide relevant coverage. No existing test function was directly removed or weakened; fixture changes alter their execution. The supplied checkout has no tests/_helpers/README.md.

Test / case	Behavior protected	Layer	Needed?	Action / rationale
tests/README.md — coverage inventory and deferred-scope edits	Describes coverage and its limits	Documentation	keep	Distinguishes simulated installation, shell behavior, and privileged integration evidence.
tests/conftest.py::_isolate_ucode_state, nested install_test_policy	Prevents ordinary tests writing real machine policy	Fixture/helper	keep	Temporary paths and a guarded fake installer are necessary isolation, not proof of sudo or ownership behavior.
tests/e2e_cuj/conftest.py::cuj	Refuses a pre-existing policy before destructive cleanup	CUJ fixture	keep	Adds the required safety precondition for this entry point.
tests/e2e_cuj/helpers/constants.py::WORKSPACE_POLICY_PATH	Identifies the control-plane policy separately from native agent settings	Constant	keep	Supports precise cleanup and leak checks without excluding native agent files.
tests/e2e_cuj/helpers/session.py::UserSession.revert_machine_wide	Removes test-created policy after failed or skipped public revert	CUJ cleanup helper	keep	A teardown backstop is appropriate because the CUJ entry points first require an absent policy.
tests/e2e_cuj/test_cuj7_model_discovery.py::TestUnmanagedModelDiscovery.live_session	Protects existing machine policy in the separate unmanaged journey	CUJ fixture	keep	This entry point bypasses cuj, so it needs its own precondition. No test body changes here.
tests/integration/README.md — fixture and execution-count edits	Documents integration coverage	Documentation	keep	Update the inventory consistently with the final scenario set; totals were not independently collected.
tests/integration/conftest.py::session	Cleans machine policy between subprocess scenarios	Integration fixture	keep	Necessary isolation, but preserve pre-existing files as requested in the blocking finding.
tests/integration/utils/managed.py::is_managed_config_control_plane_cache	Separates UG policy from native agent-managed configuration	Assertion helper	keep	The exact-path exclusion reflects the new file’s role without weakening native-agent leak assertions.
tests/integration/test_ug_configure_managed.py::test_ug_configure_installs_workspace_policy_machine_wide	Verifies installed CLI, sudo boundary, owner, modes, and envelope	Integration/process + OS	consolidate	Put these assertions into existing test_ug_configure_managed_is_idempotent; also check workspace/config values. This preserves real privileged evidence without another complete configure scenario.
tests/test_integration_runner.py::test_posix_policy_paths_include_the_installed_workspace_policy — Linux/macOS cases	Includes UG policy in clean-runner preflight	Unit	keep	Pure path selection belongs here; platform-specific native paths remain distinguishable.
tests/test_managed_config.py::OTHER_WORKSPACE	Supplies a distinct prior workspace	Test data	keep	Makes cross-workspace transitions explicit without another fixture.
tests/test_managed_config.py::TestMachinePolicyInstall._stub_token	Avoids live authentication	Fixture	keep	A focused external-dependency stub is sufficient for orchestration tests.
tests/test_managed_config.py::TestMachinePolicyInstall._recording_installer, nested record	Records writes while preserving temporary-file effects	Helper	keep	Supports meaningful write-count and resulting-content assertions.
tests/test_managed_config.py::TestMachinePolicyInstall._installed	Reads installed policy for assertions	Helper	keep	Small assertion helper with no hidden scenario setup.
tests/test_managed_config.py::TestMachinePolicyInstall.test_configure_installs_workspace_and_config	Installs the exact envelope while retaining user-cache behavior	Component	keep	Protects both new output and unchanged cache behavior.
tests/test_managed_config.py::TestMachinePolicyInstall.test_unchanged_policy_writes_nothing	Ignores retrieval-time-only changes	Component	keep	Installer call count detects unnecessary privileged rewrites.
tests/test_managed_config.py::TestMachinePolicyInstall.test_changed_policy_is_reinstalled	Replaces materially changed configuration	Component	keep	Checks both replacement and resulting content.
tests/test_managed_config.py::TestMachinePolicyInstall.test_configure_without_policy_removes_the_same_workspaces_policy	Removes a withdrawn policy	Component	keep	Covers authoritative no-config removal for the current workspace.
tests/test_managed_config.py::TestMachinePolicyInstall.test_configure_without_policy_removes_another_workspaces_policy	Removes stale policy after switching workspaces	Component	keep	Distinct state transition from same-workspace withdrawal.
tests/test_managed_config.py::TestMachinePolicyInstall.test_feature_disabled_configure_removes_installed_policy	Removes policy on the feature-disabled response	Component	keep	Exercises a separate response branch and return contract.
tests/test_managed_config.py::TestMachinePolicyInstall.test_launch_never_writes_the_machine_policy	Keeps launches from invoking the installer	Component	keep	Extend the expired-cache removal-triggering branches as noted above; a fresh-cache second launch does not establish them.
tests/test_managed_config.py::TestMachinePolicyInstall.test_launch_within_ttl_reads_the_per_user_cache	Avoids fetching while cached configuration is fresh	Component	remove	Existing TestRefreshTTL.test_fresh_published_cache_short_circuits already protects this behavior; this adds no distinct machine-policy assertion.
tests/test_managed_config.py::TestMachinePolicyInstall.test_denied_install_raises_actionable_error	Turns installer refusal into actionable configure failure	Component	keep	Checks the error, underlying reason, sudo requirement, and recovery guidance.
tests/test_managed_config.py::TestMachinePolicyInstall.test_denied_removal_raises	Fails configure when required removal is denied	Component	keep	Removal follows a distinct orchestration branch from installation.
tests/test_managed_config.py::TestMachinePolicyInstall.test_denied_launch_refresh_does_not_raise	Keeps installer refusal from affecting launch refresh	Component	keep	Uses a refusing installer to detect accidental privileged launch work.
tests/test_managed_config.py::TestMachinePolicyInstall.test_platforms_without_machine_policy_never_write — OS.WINDOWS, OS.OTHER	Skips unsupported machine-policy writes without disabling cache refresh	Component	keep	Both the no-write boundary and retained user-cache behavior matter.
tests/test_managed_config.py::TestMachinePolicyInstall.test_skip_os_settings_never_writes_the_machine_policy	Honors the machine-settings opt-out	Component	keep	Add real environment/user-scope inputs as noted above; existing shared-gate tests do not establish this caller’s wiring.
tests/test_managed_config.py::TestRevertMachinePolicy.test_revert_removes_the_installed_policy	Removes policy and reports removal	Component	keep	Protects the new revert helper’s observable result.
tests/test_managed_config.py::TestRevertMachinePolicy.test_revert_without_a_policy_writes_nothing	Avoids privileged work for an absent file	Component	keep	A fail-if-called installer makes the no-op assertion meaningful.
tests/test_managed_config.py::TestRevertMachinePolicy.test_revert_leaves_the_policy_where_ug_does_not_manage_it — skip/Windows cases	Preserves policy outside the supported management scope	Component	keep	Checks preservation of an existing file, not merely lack of creation.
tests/test_managed_config.py::TestRevertMachinePolicy.test_revert_without_sudo_warns_and_keeps_the_policy	Makes revert failure non-destructive and visible	Component	keep	Protects revert’s softer failure contract, unlike configure’s exception.
tests/test_managed_files.py::TestWorkspacePolicyWriter.policy_path	Redirects the fixed target and models an unprivileged caller	Fixture	keep	Focused setup isolates privileged operations without a live host.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_identical_policy_does_not_invoke_sudo	Avoids sudo when bytes already match	Component	keep	Protects the writer’s own early-return ordering, separate from semantic orchestration equality.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_nonroot_launch_never_probes_or_invokes_sudo — update/removal × interactive cases	Enforces explicit authorization for privileged work	Component	keep	The small matrix covers both prompting and subprocess paths while preserving the original file.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_nonroot_launch_cannot_install_initial_policy	Prevents unauthorized initial creation	Component	keep	Missing-target behavior differs from preserving an existing target.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_changed_interactive_policy_uses_existing_prompting_writer	Reuses the existing interactive writer	Component	keep	Tests dispatch without pretending mocked writes establish sudo behavior.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_noninteractive_policy_update_uses_sudo_n	Bounds unattended sudo and cleans temporary source files	Component	keep	Covers command construction, timeout, and source lifecycle at the lowest reliable layer.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_failed_privileged_update_leaves_original_file	Preserves existing policy on failed installation	Component	keep	Checks the important failure outcome rather than only subprocess status.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_interactive_policy_removal_may_prompt	Uses the interactive removal command and content snapshot	Component	keep	Protects removal-specific command/source construction.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_policy_removal_waits_for_a_password_prompt — interactive/unattended cases	Allows human input while bounding unattended removal	Component	keep	The timeout distinction is a separate observable contract.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_failed_policy_update_reports_why_sudo_failed	Preserves actionable sudo diagnostics during removal	Component	keep	Checks the failure message on the removal path.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_timed_out_policy_update_decodes_sudo_output	Converts byte-valued timeout output into readable diagnostics	Component	keep	Timeout exceptions have a different data shape from ordinary subprocess failures.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_root_policy_update_does_not_need_sudo	Runs the shell worker directly as root	Component	keep	Establishes the privileged command-prefix boundary.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_policy_dry_run_never_invokes_sudo	Prevents privileged writes during dry run	Component	keep	Protects a distinct no-write branch.
tests/test_managed_files.py::TestWorkspacePolicyWriter.test_policy_symlink_is_rejected_before_sudo	Rejects symlink targets without altering referenced data	Component	keep	The new wrapper has its own validation path; the preservation assertion matters.
tests/test_managed_files.py::TestWorkspacePolicyAsRoot.test_direct_root_run_points_sudo_uid_at_root	Corrects inherited invoking-user identity during direct root execution	Component	keep	Protects environment handling not established by the command-prefix assertion.
tests/test_managed_files.py::TestManagedWorkerShell.test_real_shell_enforces_workspace_policy_permissions — new/existing target cases	Enforces content, directory/file modes, and ownership operations	Shell component	keep	Exercises the real shell logic, including existing-file permission repair; privileged integration supplies actual root-ownership evidence.
tests/test_managed_files.py::TestManagedWorkerShell.test_real_shell_removes_only_matching_workspace_policy — unchanged/changed content cases	Removes the expected snapshot and preserves concurrently changed content	Shell component	keep	The new removal protocol needs real-shell coverage of both outcomes, not another live-workspace matrix.

After fixing the import failure, run:

uv run pytest tests/test_managed_config.py tests/test_managed_files.py tests/test_integration_runner.py
just lint

On a disposable POSIX host, verify configure creates the documented owner/modes, repeated configure leaves unchanged policy untouched, and denied sudo produces recovery guidance. Also verify installation-only checks preserve a policy that existed before the test run.

Scope

No clearly unrelated changes were found. The implementation, revert behavior, documentation, preflight checks, and cleanup all support the workspace-policy lifecycle. The installation-only teardown defect is an unsafe implementation of that supporting cleanup, not a separate requested outcome.

---

_{Automated review by Polly · workflow run}