games-on-whales/unraid-plugin

Fork 0

mirror of https://github.com/games-on-whales/unraid-plugin.git synced 2026-10-15 13:02:00 +00:00

[PR #71] [MERGED] Judge app state by write access, not ownership (#66) #77

New issue

Closed

opened 2026-10-15 12:55:25 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:55:25 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/games-on-whales/unraid-plugin/pull/71
Author: @JBailes
Created: 8/16/2026
Status: ✅ Merged
Merged: 8/16/2026
Merged by: @JBailes

Base: main ← Head: fix/issue-66-app-state-access

📝 Commits (1)

b83d8c9 Judge app state by write access, not ownership (#66)

📊 Changes

6 files changed (+361 additions, -27 deletions)

View changed files

➕ .github/workflows/tests.yml (+68 -0)
📝 scripts/app-state.sh (+127 -13)
📝 scripts/deploy.sh (+27 -5)
📝 scripts/diagnose.sh (+51 -7)
📝 scripts/utils.sh (+3 -0)
📝 tests/app-state.sh (+85 -2)

📄 Description

The latest report on #66 has the tell in it: the stamp reads 99:100 — so the migration ran and finished clean — and the tree is still full of root-owned paths. The repair is not failing, it is being undone.

Everything named in that report is created by root, on every app launch:

profile-data//, /udev/*
Wolf, create_directories() in state/sessions.hpp and runners/docker.cpp
$HOME/.steam, $HOME/homebrew/services
the gow steam image's cont-init.d/system-services.sh, which runs as root
before dropping to PUID
$HOME/homebrew/plugins
Decky's PluginLoader, deliberately

So a host-side chown can never hold, and #70 was right to reach for inheritable ACLs. Two gaps remained.

The ACL only reached depth 2. prepare_app_state_roots seeds -maxdepth 2 (state root, profile, app HOME), so directories that predate the seeding — .steam and homebrew, at depth 3 — carry no ACL, and neither do their children. Those are genuinely unwritable by the run uid. That is the live breakage.

And ownership was the wrong signal. first_foreign_path and diagnose.sh compared uid/gid, so root-owned-but-writable paths were reported as breakage forever and re-triggered a full chown -R over a Steam library on every deploy. The doctor told the user to run Install, and Install could not satisfy the check.

run_ids_can_write() asks the kernel via setpriv + access(2), so an inherited ACL counts. It refuses to probe as uid 0, where root bypasses the permission bits and every answer would be "writable".
first_foreign_path -> first_unwritable_path: ownership is now only the cheap pre-filter that enumerates candidates.
repair_app_state_acls() applies the ACL recursively so pre-existing deep trees gain inheritance, gated on the same drift condition as the chown, and running before it — ACLs are the half that survives the next launch.
migrate and diagnose now share APP_STATE_PROBE_DEPTH. They disagreed (3 vs 4), and homebrew/plugins sat in the gap: a path the doctor reported and the repair never looked at.
diagnose reports access rather than ownership, prints mode and the ACL entry for paths that really are stuck, and says so plainly when it had to fall back to comparing uids.

Also, from auditing the rest of the ownership surface:

deploy.sh no longer creates ${APPDATA}/steam. It existed only for a /etc/wolf/steam mount removed with the identity mount, and nothing has read it since. Existing ones are left alone.
covers/ and compatibilitytools.d/ keep 1000:1000: they lost their own mounts but wolf-den still reaches them through ${APPDATA}:/etc/wolf, at the same in-container paths.
that chown was 2>/dev/null || true beside an unguarded chmod, so the failure that actually breaks Wolf Den was the one being hidden. Both now warn and let the deploy continue.
utils.sh gains the shebang shellcheck needs to lint it.

Tests: the suite's load-bearing assertions were skipping on any host without root and setfacl, which is most of them — three fixes shipped for this issue without those assertions ever executing. It now reproduces the reported tree, asserts red before green, and checks that a root-created directory is writable by inheritance alone. CI runs it as root with real ACLs and fails the build if it skips, because a green check that proves nothing is worse than a red one.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/games-on-whales/unraid-plugin/pull/71 **Author:** [@JBailes](https://github.com/JBailes) **Created:** 8/16/2026 **Status:** ✅ Merged **Merged:** 8/16/2026 **Merged by:** [@JBailes](https://github.com/JBailes) **Base:** `main` ← **Head:** `fix/issue-66-app-state-access` --- ### 📝 Commits (1) - [`b83d8c9`](https://github.com/games-on-whales/unraid-plugin/commit/b83d8c93d843d996e3a29a4b5b905c7ea63a83a1) Judge app state by write access, not ownership (#66) ### 📊 Changes **6 files changed** (+361 additions, -27 deletions) <details> <summary>View changed files</summary> ➕ `.github/workflows/tests.yml` (+68 -0) 📝 `scripts/app-state.sh` (+127 -13) 📝 `scripts/deploy.sh` (+27 -5) 📝 `scripts/diagnose.sh` (+51 -7) 📝 `scripts/utils.sh` (+3 -0) 📝 `tests/app-state.sh` (+85 -2) </details> ### 📄 Description The latest report on #66 has the tell in it: the stamp reads 99:100 — so the migration ran and finished clean — and the tree is still full of root-owned paths. The repair is not failing, it is being undone. Everything named in that report is created by root, on every app launch: profile-data/<profile>/<app>, <app>/udev/* Wolf, create_directories() in state/sessions.hpp and runners/docker.cpp $HOME/.steam, $HOME/homebrew/services the gow steam image's cont-init.d/system-services.sh, which runs as root before dropping to PUID $HOME/homebrew/plugins Decky's PluginLoader, deliberately So a host-side chown can never hold, and #70 was right to reach for inheritable ACLs. Two gaps remained. The ACL only reached depth 2. prepare_app_state_roots seeds -maxdepth 2 (state root, profile, app HOME), so directories that predate the seeding — .steam and homebrew, at depth 3 — carry no ACL, and neither do their children. Those are genuinely unwritable by the run uid. That is the live breakage. And ownership was the wrong signal. first_foreign_path and diagnose.sh compared uid/gid, so root-owned-but-writable paths were reported as breakage forever and re-triggered a full chown -R over a Steam library on every deploy. The doctor told the user to run Install, and Install could not satisfy the check. - run_ids_can_write() asks the kernel via setpriv + access(2), so an inherited ACL counts. It refuses to probe as uid 0, where root bypasses the permission bits and every answer would be "writable". - first_foreign_path -> first_unwritable_path: ownership is now only the cheap pre-filter that enumerates candidates. - repair_app_state_acls() applies the ACL recursively so pre-existing deep trees gain inheritance, gated on the same drift condition as the chown, and running before it — ACLs are the half that survives the next launch. - migrate and diagnose now share APP_STATE_PROBE_DEPTH. They disagreed (3 vs 4), and homebrew/plugins sat in the gap: a path the doctor reported and the repair never looked at. - diagnose reports access rather than ownership, prints mode and the ACL entry for paths that really are stuck, and says so plainly when it had to fall back to comparing uids. Also, from auditing the rest of the ownership surface: - deploy.sh no longer creates ${APPDATA}/steam. It existed only for a /etc/wolf/steam mount removed with the identity mount, and nothing has read it since. Existing ones are left alone. - covers/ and compatibilitytools.d/ keep 1000:1000: they lost their own mounts but wolf-den still reaches them through ${APPDATA}:/etc/wolf, at the same in-container paths. - that chown was `2>/dev/null || true` beside an unguarded chmod, so the failure that actually breaks Wolf Den was the one being hidden. Both now warn and let the deploy continue. - utils.sh gains the shebang shellcheck needs to lint it. Tests: the suite's load-bearing assertions were skipping on any host without root and setfacl, which is most of them — three fixes shipped for this issue without those assertions ever executing. It now reproduces the reported tree, asserts red before green, and checks that a root-created directory is writable by inheritance alone. CI runs it as root with real ACLs and fails the build if it skips, because a green check that proves nothing is worse than a red one. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-15 12:55:25 +00:00

closed this issue
added the
pull-request
label

No labels

enhancement

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

games-on-whales/unraid-plugin#77

No description provided.

Rows
Columns