fix(daemon): orphan-reaper only reaps containarium-managed accounts#920
Merged
Conversation
RunOrphanReaper (#835) swept every /home/<name> directory with an authorized_keys file and no matching <name>-container, with no way to tell a genuine leftover tenant apart from an unrelated admin account sharing the same directory shape. Gate reaping on the account's login shell actually being the containarium-managed shell wrapper instead. Real incident: upgrading a lab host's daemon (enabling this reaper for the first time there) userdel'd an operator's own admin account mid-session, because it happened to have authorized_keys under /home and no matching container. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
userShell() passed directory names straight from os.ReadDir into exec.Command without validation, unlike the sibling getent-shadow call in jump_server.go which relies on callers pre-validating via isValidUsername. Gate reapOnce on isValidUsername before any directory name reaches the shell lookup, making the #nosec justification true rather than just copied from the sibling call. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 task
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
3 participants
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.
Summary
RunOrphanReaper(Orphaned host users + sshpiper user dirs accumulate without bound (683 homes / 1296 sshpiper entries) → keysync bloat #835) scans/home/<name>for any directory with anauthorized_keysfile and deletes the account if no<name>-containerexists. This heuristic can't distinguish a genuine leftover Containarium tenant from an unrelated admin account that happens to share the same directory shape.userdel -r'd an operator's own admin login mid-session — it hadauthorized_keysunder/homeand no matching container, so it looked identical to a genuine orphan.containerShellPath). Real tenant accounts always run this shell; a manually-provisioned admin account (bash/zsh/etc.) never does, regardless of directory shape.Test plan
go build ./...orphan_reaper_test.go: only containarium-shell + no-container accounts get reaped; a bash-shell account is never reaped even with no matching container; unknown/lookup-failure accounts are never reaped (fail-safe); directories with noauthorized_keysnever reach the shell lookup at all.go vet ./...