From 898757b7b6382c4c5bf2dfb35e4cd427a26e2212 Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:27:32 +0300 Subject: [PATCH 1/9] Stopped Kernel backends runs on main and dev from cancelling each other; only superseded pull request runs are cancelled Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/kernel-backends.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/kernel-backends.yml b/.github/workflows/kernel-backends.yml index 4d3ffd0..f933338 100644 --- a/.github/workflows/kernel-backends.yml +++ b/.github/workflows/kernel-backends.yml @@ -21,9 +21,11 @@ on: permissions: contents: read +# Only a pull request's superseded runs are cancelled; on a branch, cancelling +# would leave an earlier commit with no result. concurrency: group: kernel-backends-${{ github.ref }} - cancel-in-progress: true + cancel-in-progress: ${{ github.event_name == 'pull_request' }} # An explicit bash shell adds pipefail, so `make | tee` fails when make does. defaults: From 9c487ed985aea9dac05c9385ebf4b91fa5bbd959 Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:28:06 +0300 Subject: [PATCH 2/9] Pinned the Python requirements exactly and added Dependabot to keep them and the Actions versions current The docs build dependencies move into requirements-docs.txt so Dependabot tracks them too; MkDocs stays below 2.0, which drops the theme overrides the site uses. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/dependabot.yml | 20 ++++++++++++++++++++ .github/workflows/docs.yml | 4 ++-- .github/workflows/tests.yml | 7 +++---- CONTRIBUTING.md | 8 ++++++++ requirements-dev.txt | 2 +- requirements-docs.txt | 4 ++++ requirements.txt | 2 +- 7 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 requirements-docs.txt diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..a3f8dca --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,20 @@ +# Weekly update PRs for the pinned Python requirements and the GitHub Actions +# versions. They target dev, like every other change, and must pass the +# required checks before reaching main. +version: 2 +updates: + - package-ecosystem: pip + directory: / + schedule: + interval: weekly + target-branch: dev + ignore: + # MkDocs 2.0 drops the plugin and theme-override systems the docs use. + - dependency-name: mkdocs + versions: [">=2"] + + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + target-branch: dev diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2cf54f6..14278a4 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -7,6 +7,7 @@ on: - 'docs/**' - 'overrides/**' - 'mkdocs.yml' + - 'requirements-docs.txt' workflow_dispatch: permissions: @@ -28,8 +29,7 @@ jobs: with: python-version: '3.12' - # MkDocs 2.0 drops the plugin and theme-override systems this site uses. - - run: pip install 'mkdocs>=1.6,<2' 'mkdocs-material>=9.7,<10' + - run: pip install -r requirements-docs.txt - run: mkdocs build --strict diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2f4bf67..e6ddfe3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -5,8 +5,8 @@ on: branches: [main, dev] pull_request: branches: [main, dev] - # Weekly, so unpinned dependencies or a runner image change that breaks the - # suite shows up without a commit to trigger it. + # Weekly, so a runner image change that breaks the suite shows up without a + # commit to trigger it. Dependency updates arrive as Dependabot PRs. schedule: - cron: '0 6 * * 1' workflow_dispatch: @@ -47,7 +47,6 @@ jobs: with: python-version: '3.12' - # MkDocs 2.0 drops the plugin and theme-override systems this site uses. - - run: pip install 'mkdocs>=1.6,<2' 'mkdocs-material>=9.7,<10' + - run: pip install -r requirements-docs.txt - run: mkdocs build --strict diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 08d13dd..e4a4d36 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,14 @@ Thanks for your interest in contributing! Here's how to get started. ```bash pytest ``` +4. If you changed the docs, build the site the way CI does: + ```bash + pip install -r requirements-docs.txt + mkdocs build --strict + ``` + +Dependencies are pinned exactly; Dependabot opens weekly PRs against `dev` to +update them. ## Testing on WSL2 diff --git a/requirements-dev.txt b/requirements-dev.txt index e079f8a..7345f25 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1 +1 @@ -pytest +pytest==9.1.1 diff --git a/requirements-docs.txt b/requirements-docs.txt new file mode 100644 index 0000000..dad9d91 --- /dev/null +++ b/requirements-docs.txt @@ -0,0 +1,4 @@ +# Docs site build (mkdocs.yml). MkDocs 2.0 drops the plugin and theme-override +# systems this site uses, so it stays on 1.x. +mkdocs==1.6.1 +mkdocs-material==9.7.7 diff --git a/requirements.txt b/requirements.txt index 2a98720..fe9dcbc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1 @@ -fusepy +fusepy==3.0.1 From 3b5832b8b891623d8f86b8496473687bda048225 Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:28:52 +0300 Subject: [PATCH 3/9] Documented that mode bits never change FUSE enforcement and how each backend treats containers running as root Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 10 ++++++++++ bpf/README.md | 2 ++ docs/bpf.md | 2 +- docs/fuse.md | 2 ++ fuse/README.md | 6 ++++++ 5 files changed, 21 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 328b172..c27ef65 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,16 @@ docker run --user 9500 \ The container (UID 9500) will be able to write under `/data` only if you granted its W-bit. +> **Containers running as root.** Docker runs a container as root unless you +> pass `--user`, and each backend treats root differently: +> +> - **BPF** exempts root unconditionally, so a root container can write +> anywhere on an enforced drive. Always pass `--user`. +> - **FUSE** remaps root to the user who launched the shim, so a root container +> gets that user's grants. +> - **kmod** exempts root unless the kernel was built with +> `CONFIG_SECURITY_UGOW_ROOT_EXEMPT=n`. + --- ## Architecture diff --git a/bpf/README.md b/bpf/README.md index 22e9abb..78756d5 100644 --- a/bpf/README.md +++ b/bpf/README.md @@ -35,6 +35,8 @@ Key advantage over the FUSE shim: **cannot be bypassed from userspace**. Every syscall path to the filesystem passes through the LSM hooks. **Root (uid 0) is always exempt** from enforcement to prevent system lockout. +That includes containers: Docker runs a container as root unless it is started +with `--user`, and such a container can write anywhere on an enforced drive. --- diff --git a/docs/bpf.md b/docs/bpf.md index 54c04b5..57708b6 100644 --- a/docs/bpf.md +++ b/docs/bpf.md @@ -29,7 +29,7 @@ The BPF programs hook into the kernel's LSM framework and check every write-clas Key advantage over the FUSE shim: **cannot be bypassed from userspace**. Every syscall path to the filesystem passes through the LSM hooks. !!! info "Root exemption" - Root (uid 0) is always exempt from enforcement to prevent system lockout. + Root (uid 0) is always exempt from enforcement to prevent system lockout. That includes containers: Docker runs a container as root unless it is started with `--user`, and such a container can write anywhere on an enforced drive. ## Prerequisites diff --git a/docs/fuse.md b/docs/fuse.md index af69707..40f95d9 100644 --- a/docs/fuse.md +++ b/docs/fuse.md @@ -31,6 +31,8 @@ Write-class VFS operations -- `access` (for `W_OK`), `open`, `create`, `truncate `chmod` and `utimens` are metadata writes and gated like any other write: the shim runs as root, so an ungated `chmod` would let any caller re-mode every file on the drive. `chmod` additionally masks off the `setuid` and `setgid` bits, which buy nothing on a Windows drive and would be a privilege-escalation primitive. `link` requires the W-bit on the source file as well as the destination parent, so a second name can never confer rights the first one lacked. `chown` remains root-only, tested against the real caller UID rather than the root-remapped one. +Mode bits never grant or revoke write access -- the W-bit comes only from `ugow allow` and `ugow deny`. A granted user's `chmod +w` or `chmod -w` changes the mode stored on the drive, but not who can write: the shim reports each caller's W-bit in place of the stored write bits, and the kernel's permission check uses that reported mode. + ## Prerequisites ```bash diff --git a/fuse/README.md b/fuse/README.md index 94096c6..19b8230 100644 --- a/fuse/README.md +++ b/fuse/README.md @@ -44,6 +44,12 @@ requires the W-bit on the source file as well as the destination parent, so a second name never confers rights the first one lacked. `chown` remains root-only, tested against the real caller UID rather than the root-remapped one. +Mode bits never grant or revoke write access -- the W-bit comes only from +`ugow allow` and `ugow deny`. A granted user's `chmod +w` or `chmod -w` changes +the mode stored on the drive, but not who can write: the shim reports each +caller's W-bit in place of the stored write bits, and the kernel's permission +check uses that reported mode. + --- ## Prerequisites From c6c8593f2b24acddf956169f464ce8f465630be8 Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:30:49 +0300 Subject: [PATCH 4/9] Reframed ACL mirroring as a visibility aid, added acl-cleanup --dry-run, and made cleanup report failures WSL reaches NTFS as the launching Windows user whatever the Linux uid, so the mirrored wsl_ ACEs never change what WSL can do; the docs now say so. acl-cleanup printed "complete" even when it could not list the accounts, and removed them with -ErrorAction SilentlyContinue, which exits 0 on failure. It now prints each account it removed (or would, with --dry-run), exits non-zero on any failure, and reports a missing powershell.exe instead of a traceback. The docs also showed `ugow allow --mirror-acl`, which argparse rejects: the flag belongs to ugow itself and goes before the subcommand. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 7 +++-- cli.py | 21 ++++++++++--- docs/architecture.md | 4 ++- docs/cli.md | 15 ++++----- docs/index.md | 2 +- permstore.py | 68 ++++++++++++++++++++++++++++------------- tests/test_cli.py | 54 ++++++++++++++++++++++++++++++++ tests/test_permstore.py | 63 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 196 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index c27ef65..d8615f5 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ **U**nix **G**rant **O**verlay for **W**indows drives in WSL2. -UGOW adds fine-grained, per-user write control to Windows drives mounted in WSL2. Write operations on `/mnt/c`, `/mnt/d`, etc. are gated by a SQLite-backed permission store and optionally mirrored to NTFS ACLs on the Windows host. +UGOW adds fine-grained, per-user write control to Windows drives mounted in WSL2. Write operations on `/mnt/c`, `/mnt/d`, etc. are gated by a SQLite-backed permission store. --- @@ -37,7 +37,7 @@ I couldn't find a clean solution, so I built one. - **Permission inheritance** -- a grant on a directory applies to all descendants. - **Root exemption** (BPF mode) -- root is always allowed through. In FUSE mode, root is remapped to the user who launched the shim, inheriting their W-bit rights. The kmod makes this a build-time choice (`CONFIG_SECURITY_UGOW_ROOT_EXEMPT`, default `y`). - **Audited denials** -- every refused operation is logged with the operation, UID, and mount-visible path (`journalctl -u wsl-fuse-shim@c.service`). -- **ACL mirroring** -- best-effort creation of corresponding Windows local users (`wsl_`) and NTFS ACL grants via PowerShell/`icacls`. +- **ACL mirroring (visibility only)** -- optionally records each grant on the Windows side as an NTFS ACE for a disabled local account `wsl_`, so it shows up in the folder's Security tab. It enforces nothing: WSL reaches NTFS as the Windows user who launched it, whatever the Linux UID, so an ACE for `wsl_` never changes what WSL can do. - **Unified CLI** -- `ugow allow`, `ugow deny`, `ugow check`, `ugow status`, `ugow list`, `ugow sync`, `ugow acl-cleanup` -- works identically across all backends. --- @@ -148,7 +148,8 @@ sudo ugow check --user 9500 /mnt/c/data # can UID 9500 write here? sudo ugow status /mnt/c/docker # who can write here? sudo ugow list # show all grants sudo ugow sync # replay SQLite into kernel backends -sudo ugow acl-cleanup # drop mirrored wsl_* users with no grants +sudo ugow acl-cleanup --dry-run # list mirrored wsl_* users with no grants +sudo ugow acl-cleanup # ...and remove them ``` Users can be specified by name or numeric UID. All commands require root (`sudo`). The `check` command uses `SUDO_UID` to test the *calling* user's permissions, not root's. diff --git a/cli.py b/cli.py index 15c759f..e3965f6 100644 --- a/cli.py +++ b/cli.py @@ -9,7 +9,7 @@ ugow status Show who can write to a path ugow list List all grants ugow sync Sync SQLite grants into kernel backends - ugow acl-cleanup Drop mirrored Windows users with no grants + ugow acl-cleanup [--dry-run] Drop mirrored Windows users with no grants ugow mount Enable UGOW on a Windows drive ugow unmount Disable UGOW on a Windows drive ugow drives List active UGOW-managed drives @@ -451,8 +451,17 @@ def cmd_sync(args): def cmd_acl_cleanup(args): require_root("acl-cleanup") store = PermStore(db_path=args.db, mirror_acl=False) - store.cleanup_acl() - print("ACL cleanup complete.") + removed, failed = store.cleanup_acl(dry_run=args.dry_run) + + verb = "Would remove" if args.dry_run else "Removed" + for name in removed: + print(f" {verb}: {name}") + for name in failed: + print(f" Failed to remove: {name}", file=sys.stderr) + if not removed and not failed: + print("No mirrored wsl_* accounts without grants.") + if failed: + sys.exit(1) def cmd_list(args): @@ -676,8 +685,10 @@ def main(): sub.add_parser("list", help="List all grants") sub.add_parser("sync", help="Replay SQLite grants into kernel backends (kmod/BPF)") - sub.add_parser("acl-cleanup", - help="Remove mirrored Windows wsl_* users with no grants") + p = sub.add_parser("acl-cleanup", + help="Remove mirrored Windows wsl_* users with no grants") + p.add_argument("--dry-run", action="store_true", + help="List the accounts that would be removed, removing nothing") p = sub.add_parser("mount", help="Enable UGOW on a Windows drive") p.add_argument("drive", help="Drive letter (e.g. d, e, f)") diff --git a/docs/architecture.md b/docs/architecture.md index ced8154..35940fa 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -140,6 +140,8 @@ SQLite connections are per-thread (stored in `threading.local()`). WAL mode is e ### ACL mirroring +ACL mirroring is a visibility aid, not an enforcement layer. WSL reaches NTFS as the Windows user who launched it, whatever the Linux UID, so an ACE for `wsl_` never changes what WSL can do. The mirrored ACEs only show the grant in a folder's Windows Security tab. + When enabled (`--mirror-acl`), grant and revoke operations are queued to a background worker thread that: 1. Creates a Windows local user `wsl_` if it doesn't exist, with a random password, and immediately disables it -- the account exists only as an ACL principal, nothing ever authenticates as it, and a disabled account's SID still resolves in ACLs @@ -150,7 +152,7 @@ Grants are applied inheritably (`(OI)(CI)F`) and recursively (`/T`). Revoke uses Requesting `--mirror-acl` without Windows Administrator privileges raises `AclMirrorUnavailable`, which the CLI turns into an error message. The library never calls `sys.exit()` on its caller's behalf -- the FUSE daemon imports the same module and would otherwise die at startup with no explanation. Flushing pending ACL work is bounded (60 s) and returns a success flag rather than blocking forever if the worker thread has died. -ACL mirroring is best-effort -- if it fails, the SQLite grant still succeeds and the W-bit enforcement layer works independently. `ugow acl-cleanup` removes mirrored `wsl_*` accounts whose UID no longer holds any grant. +ACL mirroring is best-effort -- if it fails, the SQLite grant still succeeds and the W-bit enforcement layer works independently. `ugow acl-cleanup` removes mirrored `wsl_*` accounts whose UID no longer holds any grant; `--dry-run` lists them without removing anything. ### Path conversion diff --git a/docs/cli.md b/docs/cli.md index b54fbaf..fe69434 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -29,14 +29,14 @@ The grant is pushed to every active kernel backend *before* it is committed to S In BPF mode, `allow` also widens the path's Unix mode if it would block writes at the DAC layer before the LSM hooks run, recording the original mode so `deny` can put it back. -??? info "ACL mirroring" - Pass `--mirror-acl` to also create a corresponding NTFS ACL grant on the Windows side via PowerShell. Requires an elevated (Administrator) Windows Terminal session. +??? info "ACL mirroring (visibility only)" + Pass `--mirror-acl`, before the subcommand, to also record the grant on the Windows side. Requires an elevated (Administrator) Windows Terminal session. ```bash - sudo ugow allow --mirror-acl ubuntu /mnt/c/docker + sudo ugow --mirror-acl allow ubuntu /mnt/c/docker ``` - This creates a Windows local user `wsl_` and grants it full control on the Windows path via `icacls`. + This creates a disabled Windows local user `wsl_` and grants it full control on the Windows path via `icacls`, so the grant shows up in the folder's Security tab. It does not enforce anything: WSL reaches NTFS as the Windows user who launched it, whatever the Linux UID, so an ACE for `wsl_` never changes what WSL can do. Enforcement comes only from the UGOW backend. --- @@ -132,10 +132,11 @@ If no kernel backend is active the command reports that there is nothing to sync Remove mirrored Windows `wsl_` users that no longer hold any grant. ```bash +sudo ugow acl-cleanup --dry-run # list what would be removed sudo ugow acl-cleanup ``` -ACL mirroring creates a Windows local user per granted UID. Revoking the last grant for a UID drops its NTFS ACEs but leaves the account behind; this command lists the host's `wsl_*` users and deletes the ones whose UID has zero grants in the store. It has no effect if you have never used `--mirror-acl`. +ACL mirroring creates a Windows local user per granted UID. Revoking the last grant for a UID drops its NTFS ACEs but leaves the account behind; this command lists the host's `wsl_*` users and deletes the ones whose UID has zero grants in the store. It prints each account it removed, or would remove with `--dry-run`, and exits non-zero if any removal failed or the accounts could not be listed. Removing accounts needs an elevated Windows session. It has no effect if you have never used `--mirror-acl`. --- @@ -193,9 +194,9 @@ Shows active FUSE units or BPF-registered devices, depending on the installed mo ## Hidden Flags -These flags are available but hidden from `--help`: +These flags are available but hidden from `--help`. They belong to `ugow` itself, so they go before the subcommand: | Flag | Description | |------|-------------| | `--db ` | Override the SQLite database path (default: `/var/lib/ugow/wperm.db`) | -| `--mirror-acl` | Enable NTFS ACL mirroring on `allow`/`deny` operations | +| `--mirror-acl` | Mirror `allow`/`deny` into NTFS ACLs for visibility; enforces nothing. Goes before the subcommand: `ugow --mirror-acl allow ...` | diff --git a/docs/index.md b/docs/index.md index 31a3173..4c89924 100644 --- a/docs/index.md +++ b/docs/index.md @@ -41,7 +41,7 @@ I couldn't find a clean solution, so I built one. - :material-file-lock: **W-bit enforcement** -- gates `access`, `open`, `create`, `truncate`, `mkdir`, `unlink`, `rmdir`, `rename`, `symlink`, `link`, `chmod`, and `utimens`. A hard link needs the W-bit on both ends, and `setuid`/`setgid` bits can never be set through the shim. - :material-family-tree: **Permission inheritance** -- a grant on a directory applies to all descendants automatically. - :material-text-search: **Audited denials** -- every refused operation is logged with the operation, UID, and path. -- :material-microsoft-windows: **ACL mirroring** -- optionally creates matching NTFS ACL grants via PowerShell/`icacls`. +- :material-microsoft-windows: **ACL mirroring (visibility only)** -- optionally records grants as NTFS ACEs so they show in the Windows Security tab. It enforces nothing: WSL reaches NTFS as the Windows user who launched it, whatever the Linux UID, so an ACE for `wsl_` never changes what WSL can do. - :material-console: **Unified CLI** -- `ugow allow`, `deny`, `check`, `status`, `list`, `sync`, `acl-cleanup` -- works the same across all backends. ## Quick Example diff --git a/permstore.py b/permstore.py index 54e894a..53cdbba 100644 --- a/permstore.py +++ b/permstore.py @@ -443,26 +443,37 @@ def flush_acl(self, timeout=_ACL_FLUSH_TIMEOUT): time.sleep(0.05) return True - def cleanup_acl(self): - """Remove Windows wsl_* users whose UID has zero grants in the DB.""" + def cleanup_acl(self, dry_run=False): + """Remove Windows wsl_* users whose UID has zero grants in the DB. + + Returns (removed, failed): the account names deleted -- or, with + *dry_run*, the ones that would be -- and those whose removal failed. + Raises AclMirrorUnavailable if the accounts cannot be listed, so a + caller never reports a cleanup that did not happen. + """ active_uids = { row[0] for row in self._conn().execute("SELECT DISTINCT uid FROM wperms") } - result = subprocess.run( - [ - _find_powershell(), "-Command", - "Get-LocalUser | Where-Object { $_.Name -like 'wsl_*' } " - "| Select-Object -ExpandProperty Name", - ], - capture_output=True, - text=True, - timeout=_ACL_CMD_TIMEOUT, - ) + try: + result = subprocess.run( + [ + _find_powershell(), "-Command", + "Get-LocalUser | Where-Object { $_.Name -like 'wsl_*' } " + "| Select-Object -ExpandProperty Name", + ], + capture_output=True, + text=True, + timeout=_ACL_CMD_TIMEOUT, + ) + except (OSError, subprocess.TimeoutExpired) as e: + raise AclMirrorUnavailable(f"cannot list Windows accounts: {e}") from e if result.returncode != 0: - log.error("Failed to list wsl_* users: %s", result.stderr.strip()) - return + raise AclMirrorUnavailable( + f"cannot list Windows accounts: {result.stderr.strip()}" + ) + stale = [] for line in result.stdout.strip().splitlines(): name = line.strip() if not name.startswith("wsl_"): @@ -472,19 +483,34 @@ def cleanup_acl(self): except ValueError: continue if uid not in active_uids: + stale.append(name) + if dry_run: + return stale, [] + + removed, failed = [], [] + for name in stale: + try: + # -ErrorAction Stop: SilentlyContinue would exit 0 on failure. rm = subprocess.run( [ _find_powershell(), "-Command", f"Remove-LocalUser -Name '{self._ps_escape(name)}' " - f"-ErrorAction SilentlyContinue", + f"-ErrorAction Stop", ], capture_output=True, text=True, timeout=_ACL_CMD_TIMEOUT, ) - if rm.returncode == 0: - log.info("Removed stale Windows user: %s", name) - else: - log.warning( - "Failed to remove %s: %s", name, rm.stderr.strip() - ) + except (OSError, subprocess.TimeoutExpired) as e: + failed.append(name) + log.warning("Failed to remove %s: %s", name, e) + continue + if rm.returncode == 0: + removed.append(name) + log.info("Removed stale Windows user: %s", name) + else: + failed.append(name) + log.warning( + "Failed to remove %s: %s", name, rm.stderr.strip() + ) + return removed, failed diff --git a/tests/test_cli.py b/tests/test_cli.py index 3772cf5..93923c4 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -409,6 +409,60 @@ def boom(*a, **k): assert "bpftool" in err +class TestAclCleanup: + @pytest.fixture() + def args(self, tmp_path): + def _make(dry_run=False): + return types.SimpleNamespace(db=str(tmp_path / "acl.db"), dry_run=dry_run) + return _make + + @pytest.fixture(autouse=True) + def as_root(self, monkeypatch): + monkeypatch.setattr("os.getuid", lambda: 0) + + def test_dry_run_lists_without_removing(self, monkeypatch, args, capsys): + from cli import cmd_acl_cleanup + + seen = {} + + def cleanup(self, dry_run=False): + seen["dry_run"] = dry_run + return ["wsl_2000"], [] + + monkeypatch.setattr(PermStore, "cleanup_acl", cleanup) + cmd_acl_cleanup(args(dry_run=True)) + assert seen["dry_run"] is True + assert "Would remove: wsl_2000" in capsys.readouterr().out + + def test_failed_removal_exits_nonzero(self, monkeypatch, args, capsys): + from cli import cmd_acl_cleanup + + monkeypatch.setattr(PermStore, "cleanup_acl", + lambda self, dry_run=False: (["wsl_1"], ["wsl_2"])) + with pytest.raises(SystemExit) as exc: + cmd_acl_cleanup(args()) + assert exc.value.code == 1 + out = capsys.readouterr() + assert "Removed: wsl_1" in out.out + assert "Failed to remove: wsl_2" in out.err + + def test_nothing_to_do_says_so(self, monkeypatch, args, capsys): + from cli import cmd_acl_cleanup + + monkeypatch.setattr(PermStore, "cleanup_acl", lambda self, dry_run=False: ([], [])) + cmd_acl_cleanup(args()) + assert "No mirrored wsl_* accounts" in capsys.readouterr().out + + def test_dry_run_flag_parses_after_the_subcommand(self, monkeypatch): + from cli import main + + seen = {} + monkeypatch.setattr("cli.cmd_acl_cleanup", lambda a: seen.setdefault("args", a)) + monkeypatch.setattr("sys.argv", ["ugow", "acl-cleanup", "--dry-run"]) + main() + assert seen["args"].dry_run is True + + # --------------------------------------------------------------------------- # main() arg dispatch # --------------------------------------------------------------------------- diff --git a/tests/test_permstore.py b/tests/test_permstore.py index 279b147..fae9a9f 100644 --- a/tests/test_permstore.py +++ b/tests/test_permstore.py @@ -1,4 +1,5 @@ import os +import subprocess import time import threading import pytest @@ -194,3 +195,65 @@ def grant_range(start, count): assert errors == [], f"Errors during concurrent grants: {errors}" grants = store.list_grants() assert len(grants) == 400 + + +class TestCleanupAcl: + """acl-cleanup must report what it did, never a cleanup that did not happen.""" + + @staticmethod + def _fake_powershell(monkeypatch, listing, remove_rc=0, calls=None): + def run(cmd, **kwargs): + script = cmd[-1] + if calls is not None: + calls.append(script) + if script.startswith("Get-LocalUser"): + return subprocess.CompletedProcess(cmd, 0, listing, "") + return subprocess.CompletedProcess(cmd, remove_rc, "", "denied") + monkeypatch.setattr(permstore_mod.subprocess, "run", run) + + def test_removes_only_accounts_without_grants(self, store, monkeypatch): + store.grant("/mnt/c/keep", 1000) + calls = [] + self._fake_powershell( + monkeypatch, "wsl_1000\nwsl_2000\nwsl_bogus\nAdministrator\n", calls=calls + ) + removed, failed = store.cleanup_acl() + assert removed == ["wsl_2000"] + assert failed == [] + assert any("Remove-LocalUser -Name 'wsl_2000'" in c for c in calls) + assert not any("'wsl_1000'" in c for c in calls if c.startswith("Remove")) + + def test_dry_run_removes_nothing(self, store, monkeypatch): + calls = [] + self._fake_powershell(monkeypatch, "wsl_2000\n", calls=calls) + removed, failed = store.cleanup_acl(dry_run=True) + assert removed == ["wsl_2000"] + assert failed == [] + assert not any(c.startswith("Remove-LocalUser") for c in calls) + + def test_failed_removal_is_reported(self, store, monkeypatch): + self._fake_powershell(monkeypatch, "wsl_2000\n", remove_rc=1) + removed, failed = store.cleanup_acl() + assert removed == [] + assert failed == ["wsl_2000"] + + def test_removal_errors_are_not_silenced(self, store, monkeypatch): + calls = [] + self._fake_powershell(monkeypatch, "wsl_2000\n", calls=calls) + store.cleanup_acl() + removal = [c for c in calls if c.startswith("Remove-LocalUser")][0] + assert "-ErrorAction Stop" in removal + + def test_missing_powershell_raises_instead_of_tracebacking(self, store, monkeypatch): + def run(cmd, **kwargs): + raise FileNotFoundError(cmd[0]) + monkeypatch.setattr(permstore_mod.subprocess, "run", run) + with pytest.raises(permstore_mod.AclMirrorUnavailable): + store.cleanup_acl() + + def test_listing_failure_raises(self, store, monkeypatch): + def run(cmd, **kwargs): + return subprocess.CompletedProcess(cmd, 1, "", "access denied") + monkeypatch.setattr(permstore_mod.subprocess, "run", run) + with pytest.raises(permstore_mod.AclMirrorUnavailable, match="access denied"): + store.cleanup_acl() From 3b4fdd99658fbfc1f4f225837238408f2737b69a Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:31:35 +0300 Subject: [PATCH 5/9] Made ugow mount reload BPF grants after registering a drive, since inode-keyed grants can go stale across a remount A failed reload exits non-zero: the drive is enforced by then, and its grants may be refused until ugow sync succeeds. Co-Authored-By: Claude Opus 5.5 (1M context) --- cli.py | 17 +++++++++++--- docs/cli.md | 2 +- tests/test_cli.py | 57 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/cli.py b/cli.py index e3965f6..77067b7 100644 --- a/cli.py +++ b/cli.py @@ -545,12 +545,23 @@ def cmd_mount(args): file=sys.stderr) sys.exit(1) result = _run([UGOW_PYTHON, UGOW_MANAGE, "add-device", mount_path]) - if result.returncode == 0: - print(f"\nDrive {letter.upper()}: is now enforced by UGOW BPF at {mount_path}") - else: + if result.returncode != 0: print(f"Error: {result.stderr.strip()}", file=sys.stderr) sys.exit(result.returncode) + # BPF grants are keyed by inode, and a Windows drive's inode numbers + # can change across a remount, so reload the map from SQLite now that + # the drive is enforced -- stale keys would match nothing and deny. + result = _run([UGOW_PYTHON, UGOW_MANAGE, "--db", args.db, "sync"]) + if result.returncode != 0: + err = (result.stderr or result.stdout or "").strip() + print(f"Error: drive {letter.upper()}: is enforced, but reloading " + f"its grants failed: {err}\n" + f" Grants on it may be refused until 'sudo ugow sync' succeeds.", + file=sys.stderr) + sys.exit(result.returncode) + print(f"\nDrive {letter.upper()}: is now enforced by UGOW BPF at {mount_path}") + def cmd_unmount(args): require_root("unmount") diff --git a/docs/cli.md b/docs/cli.md index fe69434..cc80c70 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -152,7 +152,7 @@ sudo ugow mount |----------|-------------| | `drive` | Drive letter (e.g. `d`, `e`, `f`) | -In FUSE mode, this starts a systemd unit for the drive. In BPF mode, this registers the device in the BPF target map and records the drive letter in `/var/lib/ugow/drives`, so it is re-registered at the next boot -- device numbers are reassigned on every WSL restart, and a drive left out would come back unenforced. +In FUSE mode, this starts a systemd unit for the drive. In BPF mode, this registers the device in the BPF target map and records the drive letter in `/var/lib/ugow/drives`, so it is re-registered at the next boot -- device numbers are reassigned on every WSL restart, and a drive left out would come back unenforced. It then reloads the BPF grant map from SQLite, like `ugow sync`: grants are keyed by inode, which can change when a drive is remounted. If that reload fails, the command exits non-zero -- the drive is enforced, but its grants may be refused until `ugow sync` succeeds. ```bash sudo ugow mount d diff --git a/tests/test_cli.py b/tests/test_cli.py index 93923c4..13c23d1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -409,6 +409,63 @@ def boom(*a, **k): assert "bpftool" in err +class TestBpfMountResync: + """`ugow mount` in BPF mode must reload inode-keyed grants for the drive.""" + + @pytest.fixture() + def calls(self, monkeypatch): + import subprocess as sp + + class Calls(list): + rcs = {} # ugow_manage subcommand -> return code + + record = Calls() + + def run(cmd): + record.append(cmd) + sub = "sync" if cmd[-1] == "sync" else cmd[-2] + rc = record.rcs.get(sub, 0) + return sp.CompletedProcess(cmd, rc, "", "boom" if rc else "") + + monkeypatch.setattr("os.getuid", lambda: 0) + monkeypatch.setattr("cli._detect_mode", lambda: "bpf") + monkeypatch.setattr("cli.os.path.isdir", lambda p: True) + monkeypatch.setattr("cli._run", run) + record.rcs = {} + return record + + @staticmethod + def _args(): + return types.SimpleNamespace(drive="d", db="/tmp/ugow-test.db") + + def test_registers_then_syncs(self, calls, capsys): + from cli import cmd_mount, UGOW_MANAGE + + cmd_mount(self._args()) + assert [c[1:] for c in calls] == [ + [UGOW_MANAGE, "add-device", "/mnt/d"], + [UGOW_MANAGE, "--db", "/tmp/ugow-test.db", "sync"], + ] + assert "now enforced" in capsys.readouterr().out + + def test_sync_failure_exits_nonzero(self, calls, capsys): + from cli import cmd_mount + + calls.rcs["sync"] = 1 + with pytest.raises(SystemExit) as exc: + cmd_mount(self._args()) + assert exc.value.code == 1 + assert "reloading its grants failed" in capsys.readouterr().err + + def test_failed_registration_does_not_sync(self, calls): + from cli import cmd_mount + + calls.rcs["add-device"] = 1 + with pytest.raises(SystemExit): + cmd_mount(self._args()) + assert len(calls) == 1 + + class TestAclCleanup: @pytest.fixture() def args(self, tmp_path): From 42f7e259dc8b1726529a4ab0aede26c92b02c95f Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:33:39 +0300 Subject: [PATCH 6/9] Resolved grant paths with realpath so grants given through a symlink match what the backends enforce The kernel follows a symlink before the FUSE shim or an LSM hook sees the path, so a grant stored under the link itself never matched in FUSE or kmod mode. allow, deny, check and status now resolve the path (non-strictly, so a not-yet-existing leaf is kept) and print a note when it differs from what was typed. deny also removes a grant stored unresolved by an older version, and the kmod key is resolved for grants replayed by sync. Co-Authored-By: Claude Opus 5.5 (1M context) --- cli.py | 35 +++++++++++++++--- docs/cli.md | 4 ++- tests/test_cli.py | 78 +++++++++++++++++++++++++++++++++++++++++ tests/test_permstore.py | 7 ++++ 4 files changed, 119 insertions(+), 5 deletions(-) diff --git a/cli.py b/cli.py index 77067b7..76cb6b1 100644 --- a/cli.py +++ b/cli.py @@ -162,6 +162,26 @@ def _bpf_revoke(uid, path): return None +# --------------------------------------------------------------------------- +# Path resolution +# --------------------------------------------------------------------------- + +def resolve_path(raw): + """Return the canonical path a grant for *raw* is stored under. + + Symlinks are resolved because enforcement never sees them: the kernel + follows a link before the FUSE shim or an LSM hook is called, so a grant + stored under the link's own path would never match. realpath is non-strict, + so missing trailing components are kept as typed -- a grant may name a path + that does not exist yet. + """ + path = os.path.realpath(raw) + typed = os.path.abspath(raw) + if path != typed: + print(f" note: {typed} resolves to {path}; using that") + return path + + # --------------------------------------------------------------------------- # kmod securityfs helpers # --------------------------------------------------------------------------- @@ -186,6 +206,9 @@ def _kmod_key(path): expressed the same way, paired with the device so identical relative paths on different drives stay distinct. """ + # The kmod renders the dentry the kernel resolved, so the path it is sent + # must be resolved too -- including grants stored before paths were. + path = os.path.realpath(path) st = os.stat(path) dev = f"{os.major(st.st_dev)}:{os.minor(st.st_dev)}" mount = _mount_point(path) @@ -276,7 +299,7 @@ def _restore_dac_after_bpf(store, path): def cmd_allow(args): require_root("allow") uid, username = resolve_user(args.user) - path = os.path.abspath(args.path) + path = resolve_path(args.path) store = PermStore(db_path=args.db, mirror_acl=args.mirror_acl) @@ -313,13 +336,17 @@ def cmd_allow(args): def cmd_deny(args): require_root("deny") uid, username = resolve_user(args.user) - path = os.path.abspath(args.path) + path = resolve_path(args.path) store = PermStore(db_path=args.db, mirror_acl=args.mirror_acl) # Revoke is the reverse: drop the authoritative record first so a partial # failure errs toward denying rather than keeping a stale permission. store.revoke(path, uid) + # Grants made before paths were resolved are stored as typed. + typed = os.path.abspath(args.path) + if typed != path: + store.revoke(typed, uid) errors = [] if _bpf_active(): @@ -348,7 +375,7 @@ def cmd_deny(args): def cmd_check(args): require_root("check") - path = os.path.abspath(args.path) + path = resolve_path(args.path) if hasattr(args, "user") and args.user: uid, username = resolve_user(args.user) @@ -368,7 +395,7 @@ def cmd_check(args): def cmd_status(args): require_root("status") - path = os.path.abspath(args.path) + path = resolve_path(args.path) ancestors = set(_path_ancestors(path)) store = PermStore(db_path=args.db, mirror_acl=False) diff --git a/docs/cli.md b/docs/cli.md index cc80c70..9b2a848 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -13,10 +13,12 @@ sudo ugow allow | Argument | Description | |----------|-------------| | `user` | Username or numeric UID | -| `path` | Absolute path to grant write access on | +| `path` | Path to grant write access on | Grants are inherited -- granting a directory covers all files and subdirectories beneath it. +The path is resolved before it is stored: made absolute, `..` and duplicate slashes removed, and symlinks followed. The kernel follows a symlink before any backend sees the path, so a grant stored under the link itself would never match. If the resolved path differs from what you typed, `ugow` prints a note. `deny`, `check` and `status` resolve the same way; `deny` also removes a grant stored under the unresolved path by an older version. + ```bash sudo ugow allow ubuntu /mnt/c/docker sudo ugow allow 9500 /mnt/c/data diff --git a/tests/test_cli.py b/tests/test_cli.py index 13c23d1..a402159 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -361,6 +361,84 @@ def test_already_writable_path_is_not_recorded(self, tmp_path): assert store.forget_dac_mode(str(target)) is None +class TestPathResolution: + """Grants are stored under the resolved path, since enforcement never sees links.""" + + @pytest.fixture() + def args(self, tmp_path): + def _make(**kwargs): + defaults = {"db": str(tmp_path / "resolve.db"), "mirror_acl": False} + defaults.update(kwargs) + return types.SimpleNamespace(**defaults) + return _make + + @pytest.fixture(autouse=True) + def no_kernel_backends(self, monkeypatch): + monkeypatch.setattr("os.getuid", lambda: 0) + monkeypatch.setattr("cli._bpf_active", lambda: False) + monkeypatch.setattr("cli._kmod_active", lambda: False) + + @pytest.fixture() + def tree(self, tmp_path): + real = tmp_path / "real" + real.mkdir() + link = tmp_path / "link" + link.symlink_to(real) + return os.path.realpath(real), str(link) + + def test_allow_through_a_symlink_stores_the_target(self, args, tree, capsys): + from cli import cmd_allow + + real, link = tree + cmd_allow(args(user="1000", path=link)) + assert PermStore(db_path=args().db).list_grants() == [(real, 1000)] + assert f"resolves to {real}" in capsys.readouterr().out + + def test_missing_leaf_under_a_symlink_is_kept(self, args, tree): + from cli import cmd_allow + + real, link = tree + cmd_allow(args(user="1000", path=os.path.join(link, "not-yet"))) + assert PermStore(db_path=args().db).list_grants() == [ + (os.path.join(real, "not-yet"), 1000) + ] + + def test_parent_references_are_stored_normalized(self, args, tree): + from cli import cmd_allow + + real, _ = tree + cmd_allow(args(user="1000", path=os.path.join(real, "a", "..", "b"))) + assert PermStore(db_path=args().db).list_grants() == [ + (os.path.join(real, "b"), 1000) + ] + + def test_deny_through_the_symlink_removes_the_grant(self, args, tree): + from cli import cmd_allow, cmd_deny + + real, link = tree + cmd_allow(args(user="1000", path=real)) + cmd_deny(args(user="1000", path=link)) + assert PermStore(db_path=args().db).list_grants() == [] + + def test_deny_removes_a_grant_stored_before_resolution(self, args, tree): + from cli import cmd_deny + + _, link = tree + store = PermStore(db_path=args().db) + store.grant(link, 1000) # how older versions stored it + cmd_deny(args(user="1000", path=link)) + assert store.list_grants() == [] + + def test_check_follows_the_symlink(self, args, tree): + from cli import cmd_allow, cmd_check + + real, link = tree + cmd_allow(args(user="1000", path=real)) + with pytest.raises(SystemExit) as exc: + cmd_check(args(user="1000", path=os.path.join(link, "file"))) + assert exc.value.code == 0 + + class TestKmodKey: """The kmod compares superblock-relative paths, so the CLI must send those.""" diff --git a/tests/test_permstore.py b/tests/test_permstore.py index fae9a9f..00ec8c5 100644 --- a/tests/test_permstore.py +++ b/tests/test_permstore.py @@ -112,6 +112,13 @@ def test_redundant_separators_normalized(self, store): store.grant("/mnt/c//data/./sub", 1000) assert store.has_wbit("/mnt/c/data/sub", 1000) is True + def test_parent_references_normalized(self, store): + assert normalize_path("/mnt/c/a/b/../c") == "/mnt/c/a/c" + store.grant("/mnt/c/data/tmp/../sub", 1000) + assert store.list_grants() == [("/mnt/c/data/sub", 1000)] + assert store.has_wbit("/mnt/c/data/sub/file", 1000) is True + assert store.has_wbit("/mnt/c/data/tmp", 1000) is False + def test_revoke_matches_unnormalized_form(self, store): store.grant("/mnt/c/data", 1000) store.revoke("/mnt/c/data/", 1000) From 0c0b17a5a5dbc1529b3c234bae5579db6e4539d5 Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:34:27 +0300 Subject: [PATCH 7/9] Gave files, directories and symlinks created through the FUSE shim to their creator instead of root The shim runs as root, so everything it created was root-owned and the creator could not chmod its own files. New entries now take the caller's real uid and gid (not the launcher remap); an existing file opened with O_CREAT keeps its owner, and a failed chown is logged rather than failing an operation whose entry already exists. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/fuse.md | 2 ++ fuse/README.md | 5 ++++ fuse/shim.py | 32 ++++++++++++++++++++++---- tests/test_shim_ops.py | 52 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/docs/fuse.md b/docs/fuse.md index 40f95d9..e2ca45a 100644 --- a/docs/fuse.md +++ b/docs/fuse.md @@ -33,6 +33,8 @@ Write-class VFS operations -- `access` (for `W_OK`), `open`, `create`, `truncate Mode bits never grant or revoke write access -- the W-bit comes only from `ugow allow` and `ugow deny`. A granted user's `chmod +w` or `chmod -w` changes the mode stored on the drive, but not who can write: the shim reports each caller's W-bit in place of the stored write bits, and the kernel's permission check uses that reported mode. +Files, directories and symlinks created through the shim belong to the user who created them -- their real UID and GID, not the root-remapped one -- rather than to root, which the shim runs as. The backing mount keeps ownership only because it is mounted with `metadata`. + ## Prerequisites ```bash diff --git a/fuse/README.md b/fuse/README.md index 19b8230..860b5ef 100644 --- a/fuse/README.md +++ b/fuse/README.md @@ -50,6 +50,11 @@ the mode stored on the drive, but not who can write: the shim reports each caller's W-bit in place of the stored write bits, and the kernel's permission check uses that reported mode. +Files, directories and symlinks created through the shim belong to the user who +created them -- their real UID and GID, not the root-remapped one -- rather than +to root, which the shim runs as. The backing mount keeps ownership only because +it is mounted with `metadata`. + --- ## Prerequisites diff --git a/fuse/shim.py b/fuse/shim.py index b5dbea4..728984f 100644 --- a/fuse/shim.py +++ b/fuse/shim.py @@ -69,6 +69,23 @@ def _require_parent_wbit(self, op, backing_path, uid): if not self.store.has_wbit(self._grant_path(parent), uid): self._deny(op, parent, uid, "no W permission on parent") + def _give_to_caller(self, full): + """Hand a newly created entry to the user who created it. + + The shim runs as root, so everything it creates starts out owned by + root, and the creator could not chmod its own files. This uses the + real caller uid, not the launcher remap: a root caller keeps root + ownership. It is best-effort, since the backing mount keeps ownership + only with `metadata`, and failing an operation whose entry already + exists would be worse than a wrong owner. + """ + uid, gid, _ = fuse_get_context() + try: + os.lchown(full, uid, gid) + except OSError as e: + log.warning("could not give %s to uid=%d: %s", + self._grant_path(full), uid, e) + def _full_path(self, path): full = os.path.abspath(os.path.join(self.root, path.lstrip("/"))) if not (full == self.root or full.startswith(self.root + os.sep)): @@ -152,8 +169,13 @@ def create(self, path, mode, fi=None): full = self._full_path(path) self._require_parent_wbit("create", full, self._effective_uid()) # fusepy's create() carries no flags, so O_EXCL cannot be honoured here; - # the kernel's lookup-then-create path decides existence instead. - return os.open(full, os.O_WRONLY | os.O_CREAT, _safe_mode(mode)) + # the kernel's lookup-then-create path decides existence instead. Only + # an entry this call created changes owner. + existed = os.path.lexists(full) + fd = os.open(full, os.O_WRONLY | os.O_CREAT, _safe_mode(mode)) + if not existed: + self._give_to_caller(full) + return fd def read(self, path, size, offset, fh): # Positioned I/O: fusepy dispatches multithreaded, so a shared @@ -193,7 +215,8 @@ def fsync(self, path, fdatasync, fh): def mkdir(self, path, mode): full = self._full_path(path) self._require_parent_wbit("mkdir", full, self._effective_uid()) - return os.mkdir(full, _safe_mode(mode)) + os.mkdir(full, _safe_mode(mode)) + self._give_to_caller(full) def rmdir(self, path): full = self._full_path(path) @@ -218,7 +241,8 @@ def symlink(self, target, source): """Create a symlink at *target* pointing to *source*.""" new_link = self._full_path(target) self._require_parent_wbit("symlink", new_link, self._effective_uid()) - return os.symlink(source, new_link) + os.symlink(source, new_link) + self._give_to_caller(new_link) def link(self, target, source): """Create a hard link at *target* referencing *source*.""" diff --git a/tests/test_shim_ops.py b/tests/test_shim_ops.py index 503ba91..498b254 100644 --- a/tests/test_shim_ops.py +++ b/tests/test_shim_ops.py @@ -360,3 +360,55 @@ def test_flush_succeeds(self, shim, store, backing_root, mock_fuse_ctx): fh = shim.open("/fl.txt", os.O_RDONLY) shim.flush("/fl.txt", fh) shim.release("/fl.txt", fh) + + +class TestOwnership: + """The shim runs as root; what a caller creates must belong to the caller.""" + + @pytest.fixture() + def chowns(self, monkeypatch): + calls = [] + monkeypatch.setattr("shim.os.lchown", lambda p, u, g: calls.append((p, u, g))) + return calls + + def test_created_file_goes_to_caller(self, shim, store, backing_root, mock_fuse_ctx, chowns): + mock_fuse_ctx(UID, gid=2000) + store.grant(backing_root, UID) + fh = shim.create("/new.txt", 0o644) + os.close(fh) + assert chowns == [(os.path.join(backing_root, "new.txt"), UID, 2000)] + + def test_existing_file_keeps_its_owner(self, shim, store, backing_root, mock_fuse_ctx, chowns): + mock_fuse_ctx(UID) + store.grant(backing_root, UID) + with open(os.path.join(backing_root, "old.txt"), "w"): + pass + os.close(shim.create("/old.txt", 0o644)) + assert chowns == [] + + def test_mkdir_and_symlink_go_to_caller(self, shim, store, backing_root, mock_fuse_ctx, chowns): + mock_fuse_ctx(UID, gid=2000) + store.grant(backing_root, UID) + shim.mkdir("/d", 0o755) + shim.symlink("/l", "d") + assert chowns == [ + (os.path.join(backing_root, "d"), UID, 2000), + (os.path.join(backing_root, "l"), UID, 2000), + ] + + def test_root_caller_is_not_remapped(self, shim, store, backing_root, mock_fuse_ctx, chowns, monkeypatch): + """Ownership follows the real caller; only W-bit checks use the remap.""" + monkeypatch.setattr("shim.LAUNCHER_UID", UID) + mock_fuse_ctx(0, gid=0) + store.grant(backing_root, UID) + shim.mkdir("/rootdir", 0o755) + assert chowns == [(os.path.join(backing_root, "rootdir"), 0, 0)] + + def test_chown_failure_does_not_fail_the_operation(self, shim, store, backing_root, mock_fuse_ctx, monkeypatch): + def refuse(*a): + raise PermissionError("no metadata") + monkeypatch.setattr("shim.os.lchown", refuse) + mock_fuse_ctx(UID) + store.grant(backing_root, UID) + shim.mkdir("/d", 0o755) + assert os.path.isdir(os.path.join(backing_root, "d")) From 270575f3f5431c962d9ff794269cfe48d5f0496c Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:36:14 +0300 Subject: [PATCH 8/9] Added end-to-end tests that mount the real FUSE shim as root and act on it as an ordinary user The existing shim tests call its methods with a mocked FUSE context, so the kernel's path walk, default_permissions and allow_other never took part. These mount it the way the systemd unit does and check reads, writes, creates, chmod, hard links, renames, ownership of created entries and the denial log through the kernel. They skip without Linux, /dev/fuse or passwordless sudo; CI sets UGOW_FUSE_TESTS=require so a missing prerequisite fails there instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/tests.yml | 6 +- CONTRIBUTING.md | 4 + tests/test_fuse_mount.py | 176 ++++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 tests/test_fuse_mount.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e6ddfe3..1c19b1b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -34,7 +34,11 @@ jobs: pip install -r requirements-dev.txt - name: Run tests - run: pytest tests/ -v + # The real-mount tests skip when FUSE or passwordless sudo is missing; + # here that must fail instead, or the suite could go quietly empty. + env: + UGOW_FUSE_TESTS: require + run: pytest tests/ -v -rs docs: # docs.yml only builds the site on main, as part of deploying it, so a diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e4a4d36..26382e5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,10 @@ Thanks for your interest in contributing! Here's how to get started. ```bash pytest ``` + `tests/test_fuse_mount.py` mounts the real shim as root, so it runs only on + Linux with `/dev/fuse` and passwordless `sudo`, and is skipped otherwise + (`pytest -rs` shows why). CI sets `UGOW_FUSE_TESTS=require`, which turns + that skip into a failure. 4. If you changed the docs, build the site the way CI does: ```bash pip install -r requirements-docs.txt diff --git a/tests/test_fuse_mount.py b/tests/test_fuse_mount.py new file mode 100644 index 0000000..b6537bc --- /dev/null +++ b/tests/test_fuse_mount.py @@ -0,0 +1,176 @@ +"""End-to-end tests against a real FUSE mount of the shim. + +The other shim tests call UGOWShim's methods directly with a mocked FUSE +context. These mount it for real, as root -- the way wsl-fuse-shim@.service +runs it -- and act on the mount as an ordinary user, so the kernel's own path +walk, `default_permissions` and `allow_other` take part in every decision, as +they do in production. + +They need Linux, /dev/fuse and passwordless sudo, and are skipped without +them -- unless UGOW_FUSE_TESTS=require (set in CI), where a missing +prerequisite fails instead of quietly skipping the whole module. +""" + +import errno +import os +import shutil +import subprocess +import sys +import time + +import pytest + +from permstore import PermStore + +REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +SHIM = os.path.join(REPO, "fuse", "shim.py") +REQUIRED = os.environ.get("UGOW_FUSE_TESTS") == "require" + + +def _missing_prerequisites(): + if not sys.platform.startswith("linux"): + return ["Linux"] + missing = [] + if not os.path.exists("/dev/fuse"): + missing.append("/dev/fuse") + if shutil.which("sudo") is None or subprocess.run( + ["sudo", "-n", "true"], capture_output=True + ).returncode != 0: + missing.append("passwordless sudo") + return missing + + +MISSING = _missing_prerequisites() +pytestmark = pytest.mark.skipif( + bool(MISSING) and not REQUIRED, + reason=f"needs {', '.join(MISSING)}", +) + + +@pytest.fixture(scope="module") +def mount(tmp_path_factory): + """Mount the shim as root over a backing tree; yield (mnt, backing).""" + if MISSING: + pytest.fail(f"UGOW_FUSE_TESTS=require, but missing: {', '.join(MISSING)}") + + base = tmp_path_factory.mktemp("fuse") + backing, mnt, db = base / "backing", base / "mnt", str(base / "wperm.db") + for d in ("granted/sub", "locked"): + (backing / d).mkdir(parents=True) + for f in ("granted/f", "locked/f", "locked/chmod-me", "locked/victim", "locked/move-me"): + (backing / f).write_text("x") + (backing / f).chmod(0o666) + mnt.mkdir() + + uid = os.getuid() + # Grants are written before the mount: once the root-run shim opens the + # database, its WAL files belong to root and this process can't write. + store = PermStore(db_path=db, mirror_acl=False) + store.grant(f"{mnt}/granted", uid) + store.close() + + log_path = base / "shim.log" + with open(log_path, "w") as log: + proc = subprocess.Popen( + ["sudo", "-n", "env", f"PYTHONPATH={REPO}", sys.executable, SHIM, + "--db", db, "--launcher-uid", "4242", str(backing), str(mnt)], + stdout=log, stderr=subprocess.STDOUT, + ) + try: + deadline = time.monotonic() + 15 + while not os.path.ismount(mnt): + if proc.poll() is not None or time.monotonic() > deadline: + pytest.fail(f"shim did not mount:\n{log_path.read_text()}") + time.sleep(0.1) + yield mnt, backing, log_path + finally: + subprocess.run(["sudo", "-n", "umount", str(mnt)], capture_output=True) + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + subprocess.run(["sudo", "-n", "kill", str(proc.pid)], capture_output=True) + # Hand root-owned leftovers (the WAL files) back so pytest can clean up. + subprocess.run(["sudo", "-n", "chown", "-R", f"{uid}:{os.getgid()}", str(base)], + capture_output=True) + + +def _errno_of(fn, *args): + try: + fn(*args) + except OSError as e: + return e.errno + return 0 + + +def _open_for_append(path): + with open(path, "a"): + pass + + +class TestRealMount: + def test_unprivileged_user_reaches_a_root_mount(self, mount): + """allow_other: without it, only root could use the mount at all.""" + mnt, _, _ = mount + assert os.path.ismount(mnt) + assert sorted(os.listdir(mnt)) == ["granted", "locked"] + + def test_reads_are_never_gated(self, mount): + mnt, _, _ = mount + with open(mnt / "locked" / "f") as f: + assert f.read() == "x" + + def test_write_needs_a_grant(self, mount): + mnt, _, _ = mount + assert _errno_of(_open_for_append, mnt / "granted" / "f") == 0 + assert _errno_of(_open_for_append, mnt / "locked" / "f") == errno.EACCES + + def test_grant_covers_descendants(self, mount): + mnt, _, _ = mount + assert _errno_of(_open_for_append, mnt / "granted" / "sub" / "new") == 0 + + def test_create_needs_a_grant_on_the_parent(self, mount): + mnt, _, _ = mount + assert _errno_of(_open_for_append, mnt / "locked" / "new") == errno.EACCES + assert not (mnt / "locked" / "new").exists() + + def test_stat_and_access_report_the_callers_wbit(self, mount): + mnt, _, _ = mount + assert os.stat(mnt / "granted" / "f").st_mode & 0o222 == 0o222 + assert os.stat(mnt / "locked" / "f").st_mode & 0o222 == 0 + assert os.access(mnt / "granted" / "f", os.W_OK) + assert not os.access(mnt / "locked" / "f", os.W_OK) + + def test_chmod_by_the_owner_still_needs_a_grant(self, mount): + """The caller owns the file, so only the shim's gate can refuse this.""" + mnt, backing, _ = mount + assert os.stat(mnt / "locked" / "chmod-me").st_uid == os.getuid() + assert _errno_of(os.chmod, mnt / "locked" / "chmod-me", 0o600) == errno.EACCES + assert (backing / "locked" / "chmod-me").stat().st_mode & 0o777 == 0o666 + + def test_hard_link_cannot_move_a_file_into_a_grant(self, mount): + mnt, _, _ = mount + assert _errno_of( + os.link, mnt / "locked" / "victim", mnt / "granted" / "stolen" + ) == errno.EACCES + + def test_rename_out_of_an_ungranted_directory_is_denied(self, mount): + mnt, _, _ = mount + assert _errno_of( + os.rename, mnt / "locked" / "move-me", mnt / "granted" / "moved" + ) == errno.EACCES + + def test_created_entries_belong_to_the_caller(self, mount): + mnt, backing, _ = mount + (mnt / "granted" / "mine").write_text("y") + (mnt / "granted" / "mydir").mkdir() + for name in ("mine", "mydir"): + st = os.lstat(backing / "granted" / name) + assert (st.st_uid, st.st_gid) == (os.getuid(), os.getgid()) + + def test_denials_are_logged(self, mount): + # A chmod by the owner passes the kernel's own check and reaches the + # shim's gate, which logs it; an append-open on a file shown as + # read-only is refused by the kernel before the shim is asked. + mnt, _, log_path = mount + _errno_of(os.chmod, mnt / "locked" / "chmod-me", 0o600) + assert f"deny chmod uid={os.getuid()}" in log_path.read_text() From 23f758613a8b0426f587adba3ba0816b7b6bb6fc Mon Sep 17 00:00:00 2001 From: Adam Streambling Date: Tue, 29 Sep 2026 22:42:00 +0300 Subject: [PATCH 9/9] Made setup.sh install fusepy from requirements.txt, so installs get the version CI tests Co-Authored-By: Claude Opus 5.5 (1M context) --- setup.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.sh b/setup.sh index decc15f..686f6d4 100755 --- a/setup.sh +++ b/setup.sh @@ -319,8 +319,8 @@ if [[ "$MODE" == "fuse" ]]; then fi fi - # Install fusepy into the venv - sudo "$VE/venv/bin/pip" install --upgrade --quiet fusepy + # Install fusepy into the venv, at the version CI tests against + sudo "$VE/venv/bin/pip" install --quiet -r "$SCRIPT_DIR/requirements.txt" # Install shim + permstore + helper scripts for the FUSE daemon if [ -e "$SHIM_BIN" ] && ! grep -q '# Shim: UGOW' "$SHIM_BIN"; then