fix: the unit's environment must follow the service identity - #343
Merged
Conversation
Both found by migrating h44lacrosse.com and checking the rendered unit before starting anything. THE ENVIRONMENT DID NOT FOLLOW THE IDENTITY The migration produced User=opencode alongside Environment=HOME=/root and KIMAKI_DATA_DIR=/root/.kimaki. Starting that unit runs the agent as a user that cannot read either path — /root is 0700 — so it comes up with no session database and no runtime state. _merge_systemd_env_lines keeps the INSTALLED unit's value for any key the template also sets. That is deliberate and right: operator edits should survive an upgrade. But identity-derived values are not operator edits. They describe an identity that no longer exists. Same defect #204 fixed from the other direction — there the User flipped silently while the state stayed put; here the User moved and the environment stayed put. The filter matches on VALUE, not on a list of key names, and that is what makes it hold. PATH on a long-lived install carries /root/.kimaki/bin, /root/.cargo/bin and /root/.bun/bin; BUN_INSTALL points at /root/.bun. Enumerating keys would have fixed HOME and KIMAKI_DATA_DIR and left the agent with a PATH full of directories it can no longer read. Prefer the property over a list of names (#318). Operator additions that say nothing about the old home still survive. A GUARD AROUND A REQUIRED FUNCTION IS SILENT BREAKAGE The site reclaim chmods the whole tree g+w and then hardens wp-config back to 640. That harden lived in lib/infrastructure.sh, which setup.sh sources and upgrade.sh does not, and I called it behind `declare -F`. On an upgrade the function did not exist, the guard silently skipped it, and h44lacrosse.com came out of the migration with its database credentials group-writable. I wrote that guard to be defensive. It converted a missing dependency into a silent, security-relevant regression, which is strictly worse than the crash it was avoiding. The function now lives in lib/wordpress.sh, which both entry points source, and the call is unconditional. Tests assert it is defined in a lib upgrade.sh actually sources and that it runs after the blanket g+w.
chubes4
added a commit
that referenced
this pull request
Aug 7, 2026
The effective-prompt harness imports dm-context-filter.ts directly. node cannot load TypeScript — it exits ERR_UNKNOWN_FILE_EXTENSION before rendering a single prompt — and the runner reported that crash as "dm-context-filter may be leaking banned phrases". So h44lacrosse.com warned about a possible prompt leak on every single upgrade, for months, while the filter was clean. Verified under bun on that host: OK — 2 scenarios passed, 0 leaks. An alarm that is always wrong is worse than silence, because it teaches everyone to scroll past the one time it is right. I scrolled past it a dozen times today before reading it. Same shape as the declare -F guard fixed in #343: a failure being reported as something it is not. There the error was silent, here it is loud and inaccurate. Both come from a caller assuming it knows why the thing below it failed. The runner now resolves a runtime that can actually import TypeScript — bun on PATH, then the service home, then root's, because a service-identity migration leaves the toolchain behind while the upgrade still runs as root, which is precisely h44's shape. When none exists it says the filter is UNVERIFIED and explicitly disclaims alleging a leak, rather than guessing. A real failure with a working runtime still reports as a possible leak, which is the case that message was written for.
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.
Both bugs found by migrating h44lacrosse.com to non-root and inspecting the rendered unit before starting anything.
1. The environment did not follow the identity
The migration produced:
Starting that runs the agent as a user that cannot read either path —
/rootis0700. It comes up with no session database and no runtime state._merge_systemd_env_lineskeeps the installed unit's value for any key the template also sets. That's deliberate and correct — operator edits should survive an upgrade. But identity-derived values aren't operator edits; they describe an identity that no longer exists.Same defect as #204 from the other direction: there the
User=flipped silently while state stayed put; here theUser=moved and the environment stayed put.The filter matches on value, not on a list of key names, and that's what makes it hold:
Enumerating keys would have fixed
HOMEandKIMAKI_DATA_DIRand left the agent with aPATHfull of directories it can no longer read. Prefer the property over a list of names (#318).Operator additions that say nothing about the old home still survive — asserted both ways.
2. A guard around a required function is silent breakage
The site reclaim chmods the tree
g+w, then hardenswp-config.phpback to640. That harden lived inlib/infrastructure.sh— whichsetup.shsources andupgrade.shdoes not — and I called it behinddeclare -F.So on an upgrade the function didn't exist, the guard silently skipped it, and h44lacrosse.com came out of the migration with its database credentials group-writable (
660, not640).I wrote that guard to be defensive. It converted a missing dependency into a silent, security-relevant regression — strictly worse than the crash it avoided.
Now defined in
lib/wordpress.sh, which both entry points source, and called unconditionally. Tests assert it's defined in a libupgrade.shactually sources, and that it runs after the blanketg+w.The existing
wp-config-permissionstest caught the move — it extracted the function frominfrastructure.shby path. Worth noting that test only began running in CI today, via #335.h44 status
Already corrected by hand and healthy: kimaki active as
opencode, 0 restarts, site 200, WooCommerce/admin/REST/cron all verified. This makes sure no other install hits either bug.