feat: owned mode defaults to a non-root service user - #333
Merged
Conversation
#327's claim is that every install runs an unrestricted root shell, including the managed ones, and that the edit denies from #314/#318/#322 do not change that. They do not: `permission.edit` gates the runtime's edit tool, `bash` is a separate key that is unset and therefore allowed, and a root service reaches every denied path through `bash -c`, `wp eval`, or a PHP one-liner. The service user is the only lever in that list the kernel enforces — a non-root agent cannot apt install, cannot rewrite a unit, cannot read another site's credentials, and cannot write wp-admin whichever tool it reaches for. Scoped to owned mode deliberately. That is the shape aimed at owners who cannot evaluate the risk; a site whose operator never opens a terminal should not carry an agent with a root shell. A workspace install belongs to a developer who chose it, so its default is untouched — changing it here would be an unrelated behaviour change smuggled in under a security fix — and it gets the migration path instead. Existing installs are never flipped implicitly, in either mode. That is the #204 rule, and here it would additionally strand the agent's state in /root, which is the #93 footgun arrived at from the other direction. An owned install still on root now gets told to run --migrate-non-root and chooses its own moment to take the service down. The mechanical part is that setup.sh resolves the source mode AFTER detection — the mode is read from the site, and detection is what finds the site — so the identity has to be re-derived once the mode is known. Flipping RUN_AS_ROOT alone would leave SERVICE_USER, the service home, and the Kimaki data dir all still pointing at root: the half-applied identity behind both #204 and #93. detect_service_identity is split out of detect_environment for that, and lives in lib/detect.sh rather than inline in setup.sh so the decision can be tested directly. Re-derivation exposed one trap. The data dir was `KIMAKI_DATA_DIR="${KIMAKI_DATA_DIR:-<default>}"`, which cannot be recomputed: the second call reads back what the first one wrote and keeps the old identity's home. It now keys off KIMAKI_DATA_DIR_EXPLICIT, which both real entry points set before detecting, and falls back to the old behaviour when the flag machinery has not run at all so a caller that only exported the variable is not silently relocated.
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
1 participant
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.
Addresses #327 (item 1). Builds on #330's migration and #331's rename.
Why the edit rules were never the boundary
#327's claim, verified: the denies shipped in #314/#318/#322 are a guardrail, not containment.
permission.editgates the runtime's edit tool.bashis a separate permission key, unset and therefore allowed. A service running as root reaches every denied path viabash -c 'cat > wp-includes/x.php',wp eval, or a PHP one-liner — regardless of what the edit rules say.The service user is the only lever in that list the kernel enforces. A non-root agent cannot
apt install, cannot rewrite a systemd unit, cannot read another site's credentials, and cannot writewp-admin/whichever tool it reaches for.Scope
ownedopencode) — newworkspaceOwned is the shape aimed at owners who cannot evaluate the risk; a site whose operator never opens a terminal should not carry an agent with a root shell. A workspace install belongs to a developer who chose it — changing that default here would be an unrelated behaviour change smuggled in under a security fix. It gets the migration path instead.
Existing installs are never flipped implicitly, in either mode. That's the #204 rule, and here it would additionally strand the agent's state in
/root— the #93 footgun from the other direction. An owned install still on root is now told to run--migrate-non-rootand picks its own moment to take the service down.The mechanical part
setup.shresolves the source mode after detection — the mode is read from the site, and detection is what finds the site. So the identity has to be re-derived once the mode is known.Flipping
RUN_AS_ROOTalone would leaveSERVICE_USER, the service home, and the Kimaki data dir all still pointing at root: the half-applied identity behind both #204 and #93.detect_service_identityis split out ofdetect_environmentfor that, and the decision lives inlib/detect.shrather than inline insetup.shso it can be tested directly.One trap the re-derivation exposed
The data dir was
KIMAKI_DATA_DIR="${KIMAKI_DATA_DIR:-<default>}"— which cannot be recomputed. The second call reads back what the first one wrote and keeps the old identity's home.It now keys off
KIMAKI_DATA_DIR_EXPLICIT, which both real entry points set before detecting, and falls back to the old behaviour when the flag machinery hasn't run at all, so a caller that only exported the variable isn't silently relocated. Both cases are pinned by tests.Coverage
New
tests/service-identity-defaults.sh(CI job added): fresh owned install lands fully on the opencode identity;--root/--non-rootare not second-guessed; workspace mode untouched in both directions; local mode unaffected; re-derivation idempotent; explicit and bare-exported data dirs both preserved; and static checks thatsetup.shapplies the default in the right window and thatupgrade.shdoes not flip an existing install.Full suite green apart from the 3 pre-existing environmental failures that also fail on
origin/main.Still open on #327
Item 3,
permission.bashshaping per mode. The issue sequences it after this one and is explicit that it is intent-shaping rather than containment — worth having, but it is not the boundary and shouldn't be described as one.