Judge app state by write access, not ownership (#66) - #71
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
Also, from auditing the rest of the ownership surface:
2>/dev/null || truebeside an unguarded chmod, so the failure that actually breaks Wolf Den was the one being hidden. Both now warn and let the deploy continue.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.