diff --git a/apps/agor-cli/src/commands/admin/ensure-user.ts b/apps/agor-cli/src/commands/admin/ensure-user.ts index e3914bdf8..0559f94e3 100644 --- a/apps/agor-cli/src/commands/admin/ensure-user.ts +++ b/apps/agor-cli/src/commands/admin/ensure-user.ts @@ -86,7 +86,12 @@ export default class EnsureUser extends Command { // Create the user try { this.log(`Creating Unix user: ${username}`); - await executor.exec(UnixUserCommands.createUser(username, '/bin/bash', homeBase)); + // Only pass --home when the user explicitly overrides the default + // home base; otherwise let useradd use the system default. The + // wrapper validates the supplied path (see docker/sudoers/agor-user-admin). + const homeDir = + homeBase && homeBase !== AGOR_HOME_BASE ? `${homeBase}/${username}` : undefined; + await executor.exec(UnixUserCommands.createUser(username, homeDir)); this.log(`✅ Created Unix user: ${username}`); // Setup ~/agor/worktrees directory diff --git a/apps/agor-docs/pages/guide/multiplayer-unix-isolation.mdx b/apps/agor-docs/pages/guide/multiplayer-unix-isolation.mdx index e86ec4626..8644802ab 100755 --- a/apps/agor-docs/pages/guide/multiplayer-unix-isolation.mdx +++ b/apps/agor-docs/pages/guide/multiplayer-unix-isolation.mdx @@ -113,12 +113,24 @@ storage: ### 3. Sudoers Configuration -The daemon requires sudo access for Unix user/group management. Agor provides a comprehensive, production-ready sudoers file with extensive documentation and security scoping. +The daemon requires sudo access for Unix user/group management. Instead of granting wildcard sudo rules for `useradd`, `usermod`, `chpasswd`, `groupadd`, `gpasswd`, and `find` — each a well-known root-shell path — Agor funnels all privileged operations through a single validated wrapper script: **`agor-user-admin`**. + +The wrapper: + +- Accepts a fixed set of verbs (`add-user`, `delete-user`, `add-group`, `set-password`, `setgid-tree`, `list-symlinks`, …) with strict argument validation +- Re-validates usernames, groupnames, and filesystem paths before calling any tool (regex check, system-account deny-list, `readlink -f` + allowlist) +- Canonicalizes paths so symlinks cannot escape `/home/*/agor/*`, `/home/*/.agor/*`, or `/var/agor/*` +- Rejects chpasswd stdin containing NUL/CR/LF/`:` or non-printable bytes +- Audits every invocation to syslog (`logger -t agor-user-admin`) **Installation:** ```bash -# Download the sudoers file +# 1) Install the wrapper (owned by root, mode 0755) +curl -O https://raw.githubusercontent.com/preset-io/agor/main/docker/sudoers/agor-user-admin +sudo install -o root -g root -m 0755 ./agor-user-admin /usr/local/sbin/agor-user-admin + +# 2) Install the sudoers rule (single NOPASSWD line, no wildcards) curl -O https://raw.githubusercontent.com/preset-io/agor/main/docker/sudoers/agor-daemon.sudoers # CRITICAL: Always validate syntax before installing @@ -127,11 +139,12 @@ sudo visudo -c -f ./agor-daemon.sudoers # Install with correct permissions sudo install -m 0440 ./agor-daemon.sudoers /etc/sudoers.d/agor -# Verify configuration +# 3) Verify sudo -l -U agor +sudo -n /usr/local/sbin/agor-user-admin # should exit 64 "usage: …" ``` -**View the complete file:** [`docker/sudoers/agor-daemon.sudoers`](https://github.com/preset-io/agor/blob/main/docker/sudoers/agor-daemon.sudoers) +**View the files:** [`docker/sudoers/agor-user-admin`](https://github.com/preset-io/agor/blob/main/docker/sudoers/agor-user-admin) · [`docker/sudoers/agor-daemon.sudoers`](https://github.com/preset-io/agor/blob/main/docker/sudoers/agor-daemon.sudoers)
**What the sudoers file enables** @@ -144,23 +157,15 @@ sudo -l -U agor agor ALL=(%agor_users) NOPASSWD: ALL ``` -**User Management:** - -```bash -# Create/manage Unix users for Agor users -agor ALL=(ALL) NOPASSWD: /usr/sbin/useradd * -agor ALL=(ALL) NOPASSWD: /usr/sbin/userdel * -agor ALL=(ALL) NOPASSWD: /usr/sbin/usermod * -agor ALL=(ALL) NOPASSWD: /usr/sbin/chpasswd -``` - -**Group Management (Worktree Isolation):** +**User / group management and password sync — routed through the wrapper:** ```bash -# Create Unix groups per worktree (agor_wt_) -agor ALL=(ALL) NOPASSWD: /usr/sbin/groupadd * -agor ALL=(ALL) NOPASSWD: /usr/sbin/groupdel * -agor ALL=(ALL) NOPASSWD: /usr/sbin/gpasswd * +# ONE NOPASSWD line covers add/delete/lock/unlock users, add/delete groups, +# add/remove user-from-group, set-password, setgid-tree, and symlink prune. +# The wrapper re-validates every argument and only shells out to the +# matching real tool (useradd, usermod, chpasswd, groupadd, groupdel, +# gpasswd, find) with pinned, safe argv. +agor ALL=(root) NOPASSWD: /usr/local/sbin/agor-user-admin ``` **Filesystem Operations (SCOPED TO AGOR PATHS ONLY):** diff --git a/docker/Dockerfile b/docker/Dockerfile index 969ac775f..fa24ce3fd 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -88,6 +88,15 @@ RUN useradd -m -s /bin/bash agor_executor && \ mkdir -p /home/agor_executor/.agor && \ chown -R agor_executor:agor_executor /home/agor_executor +# Install the agor-user-admin wrapper. This is the single sudoers entry point +# for user/group/password/find operations on installations that opt into the +# hardened sudoers file (docker/sudoers/agor-daemon.sudoers). It is installed +# in every image so the restrictive sudoers config can be enabled at any time +# without rebuilding. +COPY docker/sudoers/agor-user-admin /usr/local/sbin/agor-user-admin +RUN chown root:root /usr/local/sbin/agor-user-admin && \ + chmod 0755 /usr/local/sbin/agor-user-admin + # Copy Zellij config for both users COPY docker/zellij-config.kdl /tmp/zellij-config.kdl RUN mkdir -p /home/agor/.config/zellij && \ diff --git a/docker/docker-entrypoint-postgres.sh b/docker/docker-entrypoint-postgres.sh index 11c5f4acc..5c4f6ab18 100755 --- a/docker/docker-entrypoint-postgres.sh +++ b/docker/docker-entrypoint-postgres.sh @@ -26,11 +26,13 @@ create_unix_user() { echo "👤 Creating Unix user: $username" - # Create user with home directory - sudo -n useradd -m -s /bin/bash "$username" + # Create user with home directory. Routed through agor-user-admin so the + # entrypoint exercises the same path as production hardened installs — + # flag-smuggling regressions are caught in dev, not in prod. + sudo -n /usr/local/sbin/agor-user-admin add-user "$username" - # Set password to 'admin' - echo "$username:admin" | sudo -n chpasswd + # Set password to 'admin' (stdin to wrapper; wrapper runs chpasswd internally) + printf 'admin' | sudo -n /usr/local/sbin/agor-user-admin set-password "$username" # Create .agor directory sudo -n mkdir -p "/home/$username/.agor" diff --git a/docker/sudoers/agor-daemon.sudoers b/docker/sudoers/agor-daemon.sudoers index 87d22fa48..4e8ffdcb8 100644 --- a/docker/sudoers/agor-daemon.sudoers +++ b/docker/sudoers/agor-daemon.sudoers @@ -49,26 +49,33 @@ Defaults:agor !requiretty agor ALL=(%agor_users) NOPASSWD: ALL # ============================================================================ -# USER MANAGEMENT - Create/Delete Agor Users +# USER + GROUP MANAGEMENT - via agor-user-admin wrapper # ============================================================================ -# These commands are used when users sign up or are onboarded. -# The daemon creates a corresponding Unix user for each Agor user. - -agor ALL=(ALL) NOPASSWD: /usr/sbin/useradd * -agor ALL=(ALL) NOPASSWD: /usr/sbin/userdel * -agor ALL=(ALL) NOPASSWD: /usr/sbin/usermod * -agor ALL=(ALL) NOPASSWD: /usr/sbin/chpasswd - -# ============================================================================ -# GROUP MANAGEMENT - Worktree Isolation -# ============================================================================ -# Each worktree gets a Unix group (agor_wt_). -# Users with 'all' permission are added to the group. -# Filesystem permissions on the worktree directory enforce access. +# The daemon's user/group/password operations all route through a single +# root-owned wrapper script that validates verbs and arguments internally. +# +# This replaces five previous wildcard NOPASSWD rules that were each a +# root-shell path under sudo (any of them would have let a compromised +# daemon smuggle flags into useradd/usermod/gpasswd/chpasswd/find): +# +# # Superseded by /usr/local/sbin/agor-user-admin — see +# # docker/sudoers/agor-user-admin for the validated verb interface. +# agor ALL=(ALL) NOPASSWD: /usr/sbin/useradd * +# agor ALL=(ALL) NOPASSWD: /usr/sbin/userdel * +# agor ALL=(ALL) NOPASSWD: /usr/sbin/usermod * +# agor ALL=(ALL) NOPASSWD: /usr/sbin/chpasswd +# agor ALL=(ALL) NOPASSWD: /usr/sbin/groupadd * +# agor ALL=(ALL) NOPASSWD: /usr/sbin/groupdel * +# agor ALL=(ALL) NOPASSWD: /usr/sbin/gpasswd * +# agor ALL=(ALL) NOPASSWD: /usr/bin/find * (see below — also collapsed) +# +# Verbs supported by the wrapper (see script for validators + audit log): +# add-user / delete-user [--remove-home] / lock-user / unlock-user +# add-group / delete-group / add-to-group / remove-from-group +# set-password (password on stdin) +# setgid-tree / list-symlinks / prune-all-symlinks / prune-broken-symlinks -agor ALL=(ALL) NOPASSWD: /usr/sbin/groupadd * -agor ALL=(ALL) NOPASSWD: /usr/sbin/groupdel * -agor ALL=(ALL) NOPASSWD: /usr/sbin/gpasswd * +agor ALL=(root) NOPASSWD: /usr/local/sbin/agor-user-admin # ============================================================================ # FILESYSTEM OPERATIONS - Worktree Permissions (SCOPED TO AGOR PATHS) @@ -120,14 +127,13 @@ agor ALL=(ALL) NOPASSWD: /usr/bin/tee /var/agor/* # QUERY AND TRAVERSAL COMMANDS # ============================================================================ # id, getent, readlink, test: read-only state checks, no security risk. -# find: used with -exec chmod g+s for setgid propagation. Note that -# find with -exec can run arbitrary commands as root — command construction -# is controlled in our codebase (group-manager.ts), not from user input. +# `find` is NOT granted here — all legitimate find uses (setgid propagation +# and symlink cleanup) now route through agor-user-admin's setgid-tree / +# list-symlinks / prune-all-symlinks / prune-broken-symlinks verbs. agor ALL=(ALL) NOPASSWD: /usr/bin/id * agor ALL=(ALL) NOPASSWD: /usr/bin/getent * agor ALL=(ALL) NOPASSWD: /usr/bin/readlink * -agor ALL=(ALL) NOPASSWD: /usr/bin/find * agor ALL=(ALL) NOPASSWD: /usr/bin/test * diff --git a/docker/sudoers/agor-user-admin b/docker/sudoers/agor-user-admin new file mode 100644 index 000000000..663360906 --- /dev/null +++ b/docker/sudoers/agor-user-admin @@ -0,0 +1,374 @@ +#!/bin/bash +# agor-user-admin — privileged-operations wrapper for the Agor daemon +# =================================================================== +# +# This single entry point replaces the wildcard sudoers rules that previously +# exposed /usr/sbin/useradd, /usr/sbin/userdel, /usr/sbin/usermod, +# /usr/sbin/gpasswd, /usr/sbin/groupadd, /usr/sbin/groupdel, /usr/sbin/chpasswd, +# and /usr/bin/find directly to the daemon. Each of those binaries is a +# well-known root-shell path under sudo wildcards; consolidating behind a +# validated verb interface closes the flag-smuggling and path-escape avenues +# while keeping the legitimate functionality the daemon needs. +# +# INVOCATION +# /usr/local/sbin/agor-user-admin [args...] # (via sudo) +# +# SUDOERS +# agor ALL=(root) NOPASSWD: /usr/local/sbin/agor-user-admin +# +# INSTALL +# install -o root -g root -m 0755 agor-user-admin /usr/local/sbin/ +# +# EXIT CODES +# 0 success +# 1 underlying tool failure (useradd/chpasswd/... returned non-zero) +# 64 unknown or invalid verb +# 65 invalid argument (bad username, bad path, etc.) +# 66 stdin rejected (password contains NUL/CR/LF/":" or is empty/too long) +# +# AUDIT +# Every invocation is logged to syslog via `logger` with tag +# "agor-user-admin" including the verb and sanitized args (no passwords). +# Operators can grep /var/log/auth.log or journalctl for this tag. +# +# NOTE +# The Node-side validators in packages/core/src/unix/user-manager.ts +# (isValidUnixUsername, assertChpasswdInputSafe) remain as defense-in-depth. +# This wrapper treats its caller as untrusted and re-validates everything. + +set -euo pipefail +umask 0022 + +# Pin a minimal PATH so we are not vulnerable to $PATH lookups. +# All invocations of external tools below use absolute paths regardless. +export PATH=/usr/sbin:/usr/bin:/sbin:/bin +unset IFS +IFS=$' \t\n' + +# Absolute tool paths — we refuse to run if any is missing. +readonly USERADD=/usr/sbin/useradd +readonly USERDEL=/usr/sbin/userdel +readonly USERMOD=/usr/sbin/usermod +readonly GPASSWD=/usr/sbin/gpasswd +readonly GROUPADD=/usr/sbin/groupadd +readonly GROUPDEL=/usr/sbin/groupdel +readonly CHPASSWD=/usr/sbin/chpasswd +readonly FIND=/usr/bin/find +readonly LOGGER=/usr/bin/logger +readonly READLINK=/usr/bin/readlink + +# System accounts the wrapper refuses to touch. Overlaps with canonical +# Debian base users/groups plus the Agor daemon identity itself. +readonly SYSTEM_USERS_DENY=" root daemon bin sys sync games man lp mail news uucp proxy www-data backup list irc gnats nobody _apt systemd-network systemd-resolve systemd-timesync messagebus sshd postgres agor agor_executor " +readonly SYSTEM_GROUPS_DENY=" root daemon bin sys adm tty disk lp mail news uucp man proxy kmem dialout fax voice cdrom floppy tape sudo audio dip www-data backup operator list irc src gnats shadow utmp video sasl plugdev staff crontab ssh wheel agor " + +# Path allowlist for filesystem verbs (setgid-tree, list/prune-symlinks). +# These mirror the existing scoped-chown rules in agor-daemon.sudoers so the +# wrapper does not widen the filesystem surface the daemon can reach. +path_allowed() { + local canon=$1 + case "$canon" in + /home/*/agor|/home/*/agor/*) return 0 ;; + /home/*/.agor|/home/*/.agor/*) return 0 ;; + /var/agor|/var/agor/*) return 0 ;; + *) return 1 ;; + esac +} + +# ---- audit ------------------------------------------------------------------ + +audit() { + # Never log passwords. Callers must sanitize stdin themselves. + local verb=$1; shift + # Arguments are logged as-is; usernames/groups/paths have already been + # validated to a printable character class at this point. + if [[ -x $LOGGER ]]; then + "$LOGGER" -t agor-user-admin -p auth.info -- "verb=$verb args=$*" || true + fi +} + +die() { + local code=$1; shift + printf 'agor-user-admin: %s\n' "$*" >&2 + exit "$code" +} + +# ---- validators ------------------------------------------------------------- + +# Unix username: same rule as isValidUnixUsername() in core/unix/user-manager.ts. +# Start with [a-z_], then [a-z0-9_-]{0,31}. This also rejects "--foo" which +# would otherwise be interpretable as a flag if `--` end-of-options is missed. +validate_username() { + local user=$1 + [[ -n $user ]] || die 65 "empty username" + [[ ${#user} -le 32 ]] || die 65 "username too long (>32): ${user}" + if ! [[ $user =~ ^[a-z_][a-z0-9_-]{0,31}$ ]]; then + die 65 "invalid username shape: ${user}" + fi + case "$SYSTEM_USERS_DENY" in + *" $user "*) die 65 "refusing to touch system user: ${user}" ;; + esac +} + +# Unix groupname: same shape as username. We intentionally share the regex so +# that any future tightening of one also tightens the other. +validate_groupname() { + local group=$1 + [[ -n $group ]] || die 65 "empty groupname" + [[ ${#group} -le 32 ]] || die 65 "groupname too long (>32): ${group}" + if ! [[ $group =~ ^[a-z_][a-z0-9_-]{0,31}$ ]]; then + die 65 "invalid groupname shape: ${group}" + fi + case "$SYSTEM_GROUPS_DENY" in + *" $group "*) die 65 "refusing to touch system group: ${group}" ;; + esac +} + +# Canonicalize + allowlist. Symlinks are resolved by readlink -f, so a symlink +# pointing out of the allowed tree canonicalizes to the *real* path and fails +# the prefix check. +validate_path() { + local raw=$1 + [[ -n $raw ]] || die 65 "empty path" + [[ $raw == /* ]] || die 65 "path must be absolute: ${raw}" + case "$raw" in + *$'\n'*|*$'\r'*|*$'\0'*) die 65 "path contains control chars" ;; + esac + local canon + canon=$("$READLINK" -f -- "$raw") || die 65 "cannot canonicalize path: ${raw}" + [[ -e $canon ]] || die 65 "path does not exist: ${canon}" + if ! path_allowed "$canon"; then + die 65 "path not in Agor-managed tree (/home/*/agor, /home/*/.agor, /var/agor): ${canon}" + fi + printf '%s' "$canon" +} + +# Home directory validator for add-user. +# The home directory does NOT exist yet at create time, so readlink -f +# cannot canonicalize it. We instead require an exact lexical shape: +# /home/ (no extra components, no '..', no '//') +# /var/agor/ +# The trailing username segment must satisfy the same regex as +# validate_username and must equal the username we are about to create +# (enforced by callers via the same value). +validate_home_dir() { + local dir=$1 + [[ -n $dir ]] || die 65 "empty home dir" + [[ ${#dir} -le 256 ]] || die 65 "home dir too long (>256)" + case "$dir" in + *$'\n'*|*$'\r'*|*$'\0'*) die 65 "home dir contains control chars" ;; + *//*|*/./*|*/../*|*/.|*/..) die 65 "home dir contains '.', '..' or '//': ${dir}" ;; + esac + local prefix tail + case "$dir" in + /home/*) prefix=/home; tail=${dir#/home/} ;; + /var/agor/*) prefix=/var/agor; tail=${dir#/var/agor/} ;; + *) die 65 "home dir not in /home or /var/agor: ${dir}" ;; + esac + # Tail must be a single username component (no slashes). + [[ $tail == */* ]] && die 65 "home dir must be exactly ${prefix}/: ${dir}" + # Reuse the username shape rule on the trailing component. + if ! [[ $tail =~ ^[a-z_][a-z0-9_-]{0,31}$ ]]; then + die 65 "home dir trailing component is not a valid username: ${dir}" + fi +} + +# Password safety for chpasswd stdin. Mirrors assertChpasswdInputSafe() plus +# bounds the length to avoid unbounded reads from a hostile caller. +assert_safe_password() { + local pw=$1 + [[ -n $pw ]] || die 66 "empty password" + [[ ${#pw} -le 256 ]] || die 66 "password too long (>256 bytes)" + case "$pw" in + *$'\n'*|*$'\r'*|*$'\0'*) die 66 "password contains NUL/CR/LF" ;; + *:*) die 66 "password contains ':' (chpasswd field separator)" ;; + esac + # Require printable ASCII 0x20..0x7E. Bash parameter expansion uses the + # current LC_CTYPE for character classes, so pin it to C before stripping. + local old_lc_all=${LC_ALL-} + LC_ALL=C + local stripped=${pw//[[:print:]]/} + LC_ALL=${old_lc_all} + if [[ -n $stripped ]]; then + die 66 "password contains non-printable bytes" + fi +} + +# ---- verbs ------------------------------------------------------------------ + +cmd_add_user() { + # Optional --home flag preserves UnixIntegrationService's homeBase + # configurability without giving useradd a wildcard -d. Order: + # add-user [--home ] + local home_dir="" + if [[ ${1:-} == --home ]]; then + [[ $# -ge 3 ]] || die 64 "usage: add-user [--home ] " + home_dir=$2 + shift 2 + validate_home_dir "$home_dir" + fi + [[ $# -eq 1 ]] || die 64 "usage: add-user [--home ] " + validate_username "$1" + audit add-user "home=${home_dir:-}" "$1" + # Fixed argv: -m create home, -s /bin/bash. `--` prevents any future + # username that looks flag-shaped from being parsed as a flag. When + # --home was given, the validator confirmed it matches /home/ or + # /var/agor/ exactly (no traversal, no symlinks since the dir + # does not yet exist), so it is safe to pass to -d. + if [[ -n $home_dir ]]; then + "$USERADD" -m -d "$home_dir" -s /bin/bash -- "$1" + else + "$USERADD" -m -s /bin/bash -- "$1" + fi +} + +cmd_delete_user() { + # Optional --remove-home flag. Order: verb [--remove-home] + local remove_home=0 + if [[ ${1:-} == --remove-home ]]; then + remove_home=1 + shift || true + fi + [[ $# -eq 1 ]] || die 64 "usage: delete-user [--remove-home] " + validate_username "$1" + audit delete-user "remove_home=${remove_home}" "$1" + if [[ $remove_home -eq 1 ]]; then + "$USERDEL" -r -- "$1" + else + "$USERDEL" -- "$1" + fi +} + +cmd_lock_user() { + [[ $# -eq 1 ]] || die 64 "usage: lock-user " + validate_username "$1" + audit lock-user "$1" + "$USERMOD" -L -- "$1" +} + +cmd_unlock_user() { + [[ $# -eq 1 ]] || die 64 "usage: unlock-user " + validate_username "$1" + audit unlock-user "$1" + "$USERMOD" -U -- "$1" +} + +cmd_add_group() { + [[ $# -eq 1 ]] || die 64 "usage: add-group " + validate_groupname "$1" + audit add-group "$1" + "$GROUPADD" -- "$1" +} + +cmd_delete_group() { + [[ $# -eq 1 ]] || die 64 "usage: delete-group " + validate_groupname "$1" + audit delete-group "$1" + "$GROUPDEL" -- "$1" +} + +cmd_add_to_group() { + [[ $# -eq 2 ]] || die 64 "usage: add-to-group " + validate_username "$1" + validate_groupname "$2" + audit add-to-group "$1" "$2" + # usermod -aG append-group semantics. -- prevents flag-smuggling in user. + "$USERMOD" -aG "$2" -- "$1" +} + +cmd_remove_from_group() { + [[ $# -eq 2 ]] || die 64 "usage: remove-from-group " + validate_username "$1" + validate_groupname "$2" + audit remove-from-group "$1" "$2" + "$GPASSWD" -d -- "$1" "$2" +} + +cmd_set_password() { + [[ $# -eq 1 ]] || die 64 "usage: set-password (password on stdin)" + validate_username "$1" + local user=$1 + # Read at most 257 bytes; anything longer fails validation. This caps the + # amount we are willing to buffer from a possibly-hostile caller. + local pw + if ! IFS= read -r -n 257 pw; then + # Fall through only if read failed AND we got nothing. A non-zero read + # return with content present (EOF without newline) is fine. + [[ -n ${pw:-} ]] || die 66 "could not read password from stdin" + fi + assert_safe_password "$pw" + audit set-password "$user" + # Feed chpasswd via stdin, not argv — password never appears on a command line. + printf '%s:%s\n' "$user" "$pw" | "$CHPASSWD" +} + +cmd_setgid_tree() { + [[ $# -eq 1 ]] || die 64 "usage: setgid-tree " + local canon + canon=$(validate_path "$1") + [[ -d $canon ]] || die 65 "setgid-tree requires a directory: ${canon}" + audit setgid-tree "$canon" + "$FIND" "$canon" -type d -exec chmod g+s {} + +} + +cmd_list_symlinks() { + [[ $# -eq 1 ]] || die 64 "usage: list-symlinks " + local canon + canon=$(validate_path "$1") + [[ -d $canon ]] || die 65 "list-symlinks requires a directory: ${canon}" + audit list-symlinks "$canon" + "$FIND" "$canon" -maxdepth 1 -type l -printf '%f\n' +} + +cmd_prune_all_symlinks() { + [[ $# -eq 1 ]] || die 64 "usage: prune-all-symlinks " + local canon + canon=$(validate_path "$1") + [[ -d $canon ]] || die 65 "prune-all-symlinks requires a directory: ${canon}" + audit prune-all-symlinks "$canon" + "$FIND" "$canon" -maxdepth 1 -type l -delete +} + +cmd_prune_broken_symlinks() { + [[ $# -eq 1 ]] || die 64 "usage: prune-broken-symlinks " + local canon + canon=$(validate_path "$1") + [[ -d $canon ]] || die 65 "prune-broken-symlinks requires a directory: ${canon}" + audit prune-broken-symlinks "$canon" + # Within the wrapper we deliberately use `-exec /usr/bin/test` with an + # absolute path so that even if somehow PATH were subverted, the semantics + # are pinned. + "$FIND" "$canon" -maxdepth 1 -type l ! -exec /usr/bin/test -e {} \; -delete +} + +# ---- dispatch --------------------------------------------------------------- + +# Defensive: if for any reason we are invoked without being root (e.g. the +# sudoers rule was misconfigured), fail loud instead of producing mysterious +# tool errors later. +if [[ $(id -u) -ne 0 ]]; then + die 65 "must be invoked as root (via sudo)" +fi + +if [[ $# -lt 1 ]]; then + die 64 "usage: agor-user-admin [args...]" +fi + +verb=$1; shift + +case "$verb" in + add-user) cmd_add_user "$@" ;; + delete-user) cmd_delete_user "$@" ;; + lock-user) cmd_lock_user "$@" ;; + unlock-user) cmd_unlock_user "$@" ;; + add-group) cmd_add_group "$@" ;; + delete-group) cmd_delete_group "$@" ;; + add-to-group) cmd_add_to_group "$@" ;; + remove-from-group) cmd_remove_from_group "$@" ;; + set-password) cmd_set_password "$@" ;; + setgid-tree) cmd_setgid_tree "$@" ;; + list-symlinks) cmd_list_symlinks "$@" ;; + prune-all-symlinks) cmd_prune_all_symlinks "$@" ;; + prune-broken-symlinks) cmd_prune_broken_symlinks "$@" ;; + *) die 64 "unknown verb: ${verb}" ;; +esac diff --git a/docker/sudoers/tests/agor-user-admin.test.sh b/docker/sudoers/tests/agor-user-admin.test.sh new file mode 100755 index 000000000..71bde5863 --- /dev/null +++ b/docker/sudoers/tests/agor-user-admin.test.sh @@ -0,0 +1,350 @@ +#!/bin/bash +# Test harness for docker/sudoers/agor-user-admin +# ================================================== +# +# Exercises the wrapper's input validators against adversarial arguments +# (flag-smuggling, path escapes, control chars, chpasswd field injection). +# +# Because the wrapper self-checks that it's running as root (the sudoers +# rule enforces that in production), this harness must be executed as +# root. In CI, run it inside a container: +# +# docker run --rm -v "$PWD:/src" -w /src debian:bookworm-slim \ +# bash docker/sudoers/tests/agor-user-admin.test.sh +# +# Environment: +# WRAPPER=/path/to/agor-user-admin # override wrapper under test +# +# The harness DOES NOT actually create users / groups / files. It relies +# on the fact that the validators reject adversarial input with specific +# exit codes (64/65/66) BEFORE the wrapper invokes any real tool. Tests +# that would otherwise mutate system state are asserted via exit code +# + stderr pattern only. + +set -u + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WRAPPER="${WRAPPER:-${SCRIPT_DIR}/../agor-user-admin}" + +if [[ ! -x "$WRAPPER" ]]; then + # Accept non-executable bit (git on some platforms drops +x); run via bash. + if [[ ! -f "$WRAPPER" ]]; then + echo "FATAL: wrapper not found at $WRAPPER" >&2 + exit 2 + fi +fi + +if [[ "$(id -u)" -ne 0 ]]; then + echo "SKIP: agor-user-admin tests require root (run in a container)." >&2 + echo " Usage: docker run --rm -v \"\$PWD:/src\" -w /src debian:bookworm-slim \\" >&2 + echo " bash docker/sudoers/tests/agor-user-admin.test.sh" >&2 + exit 0 +fi + +pass=0 +fail=0 +failures=() + +run_wrapper() { + # Always invoke via bash so a missing +x bit in a fresh checkout isn't a + # test blocker. Captures combined stderr+stdout and exit code. + bash "$WRAPPER" "$@" 2>&1 +} + +run_wrapper_stdin() { + # Same as run_wrapper but with stdin piped in. $1 = stdin, rest = argv. + local stdin=$1 + shift + printf '%s' "$stdin" | bash "$WRAPPER" "$@" 2>&1 +} + +# assert_exit +assert_exit() { + local expected=$1 + local desc=$2 + shift 2 + local out + out=$("$@" || true) + local actual=$? + # $? above is always 0 because of "|| true" — re-run without it for code: + # (We use a subshell to preserve semantics.) + set +e + "$@" >/dev/null 2>&1 + actual=$? + set -e + if [[ "$actual" -eq "$expected" ]]; then + printf ' ok [%d] %s\n' "$expected" "$desc" + pass=$((pass + 1)) + else + printf ' FAIL [exp=%d got=%d] %s\n' "$expected" "$actual" "$desc" + printf ' output: %s\n' "$out" + fail=$((fail + 1)) + failures+=("$desc (exp=$expected got=$actual)") + fi +} + +# assert_stderr_contains +assert_stderr_contains() { + local pattern=$1 + local desc=$2 + shift 2 + local out + out=$("$@" 2>&1 || true) + if grep -qF "$pattern" <<<"$out"; then + printf ' ok [grep %q] %s\n' "$pattern" "$desc" + pass=$((pass + 1)) + else + printf ' FAIL [grep %q] %s\n' "$pattern" "$desc" + printf ' output: %s\n' "$out" + fail=$((fail + 1)) + failures+=("$desc (missing pattern: $pattern)") + fi +} + +echo "== agor-user-admin wrapper tests ==" +echo "wrapper: $WRAPPER" +echo "" + +# ---------------------------------------------------------------------------- +# Dispatch +# ---------------------------------------------------------------------------- +echo "-- dispatch --" +assert_exit 64 "no args → usage error (64)" bash "$WRAPPER" +assert_exit 64 "unknown verb → 64" bash "$WRAPPER" nuke-everything +assert_exit 64 "empty verb → 64" bash "$WRAPPER" "" + +# ---------------------------------------------------------------------------- +# validate_username +# ---------------------------------------------------------------------------- +echo "-- validate_username --" +assert_exit 65 "reject empty username" bash "$WRAPPER" add-user "" +assert_exit 65 "reject uppercase" bash "$WRAPPER" add-user Alice +assert_exit 65 "reject leading digit" bash "$WRAPPER" add-user 1alice +assert_exit 65 "reject leading dash (flag-smuggle)" bash "$WRAPPER" add-user -rf +assert_exit 65 "reject leading double-dash (flag-smuggle)" bash "$WRAPPER" add-user --help +assert_exit 65 "reject semicolon (shell meta)" bash "$WRAPPER" add-user "alice;ls" +assert_exit 65 "reject backtick" bash "$WRAPPER" add-user 'ali`ce`' +assert_exit 65 "reject dollar (variable)" bash "$WRAPPER" add-user 'alice$PWD' +assert_exit 65 "reject space" bash "$WRAPPER" add-user "alice bob" +assert_exit 65 "reject 33-char username (too long)" bash "$WRAPPER" add-user "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +assert_exit 65 "reject root (system deny-list)" bash "$WRAPPER" add-user root +assert_exit 65 "reject agor daemon user" bash "$WRAPPER" add-user agor +assert_exit 65 "reject agor_executor" bash "$WRAPPER" add-user agor_executor + +# delete-user share the same validator +assert_exit 65 "delete-user rejects root" bash "$WRAPPER" delete-user root +assert_exit 65 "delete-user --remove-home rejects root" bash "$WRAPPER" delete-user --remove-home root +assert_exit 64 "delete-user extra arg → 64" bash "$WRAPPER" delete-user alice bob + +# lock/unlock +assert_exit 65 "lock-user rejects root" bash "$WRAPPER" lock-user root +assert_exit 65 "unlock-user rejects root" bash "$WRAPPER" unlock-user root + +# ---------------------------------------------------------------------------- +# validate_groupname +# ---------------------------------------------------------------------------- +echo "-- validate_groupname --" +assert_exit 65 "reject empty group" bash "$WRAPPER" add-group "" +assert_exit 65 "add-group rejects root group" bash "$WRAPPER" add-group root +assert_exit 65 "add-group rejects sudo group" bash "$WRAPPER" add-group sudo +assert_exit 65 "add-group rejects wheel group" bash "$WRAPPER" add-group wheel +assert_exit 65 "add-group rejects agor group" bash "$WRAPPER" add-group agor +assert_exit 65 "add-group rejects flag-shaped name" bash "$WRAPPER" add-group -rf +assert_exit 65 "delete-group rejects root group" bash "$WRAPPER" delete-group root +assert_exit 64 "add-to-group needs two args" bash "$WRAPPER" add-to-group alice +assert_exit 65 "add-to-group rejects root as user" bash "$WRAPPER" add-to-group root developers +assert_exit 65 "add-to-group rejects sudo as group" bash "$WRAPPER" add-to-group alice sudo +assert_exit 65 "remove-from-group rejects root" bash "$WRAPPER" remove-from-group root developers +assert_exit 65 "remove-from-group rejects wheel" bash "$WRAPPER" remove-from-group alice wheel + +# ---------------------------------------------------------------------------- +# validate_path (setgid-tree + symlink verbs) +# ---------------------------------------------------------------------------- +echo "-- validate_path --" +assert_exit 65 "setgid-tree rejects relative path" bash "$WRAPPER" setgid-tree "relative/path" +assert_exit 65 "setgid-tree rejects /etc" bash "$WRAPPER" setgid-tree /etc +assert_exit 65 "setgid-tree rejects /" bash "$WRAPPER" setgid-tree / +assert_exit 65 "setgid-tree rejects /root" bash "$WRAPPER" setgid-tree /root +assert_exit 65 "setgid-tree rejects /var/log" bash "$WRAPPER" setgid-tree /var/log +assert_exit 65 "setgid-tree rejects nonexistent Agor path" \ + bash "$WRAPPER" setgid-tree /home/nobody/agor/does-not-exist +assert_exit 65 "list-symlinks rejects /etc" bash "$WRAPPER" list-symlinks /etc +assert_exit 65 "prune-all-symlinks rejects /tmp" bash "$WRAPPER" prune-all-symlinks /tmp +assert_exit 65 "prune-broken-symlinks rejects /var" bash "$WRAPPER" prune-broken-symlinks /var + +# Escape via .. → readlink -f canonicalizes then checks allowlist +TMP_OUTSIDE=$(mktemp -d) +assert_exit 65 "setgid-tree rejects symlink escape via mktemp target" \ + bash "$WRAPPER" setgid-tree "$TMP_OUTSIDE" +rmdir "$TMP_OUTSIDE" 2>/dev/null || true + +# ---------------------------------------------------------------------------- +# Path inside allowed tree — happy path shape check +# (We don't actually create/mutate; we only verify the wrapper doesn't reject +# a valid Agor-managed directory at validation time. We use /tmp + symlink +# trick to avoid persisting state, or just create a short-lived dir.) +# ---------------------------------------------------------------------------- +echo "-- validate_path (happy path, allowed tree) --" +# Only run happy-path test if /var/agor exists or can be created. +if [[ -d /var/agor ]] || mkdir -p /var/agor 2>/dev/null; then + HAPPY_DIR=/var/agor/wrapper-test-$$ + mkdir -p "$HAPPY_DIR" + # list-symlinks on an empty allowed dir should exit 0 with no output. + if out=$(bash "$WRAPPER" list-symlinks "$HAPPY_DIR" 2>&1); rc=$?; then :; fi + if [[ ${rc:-99} -eq 0 ]]; then + printf ' ok list-symlinks on empty allowed dir exits 0\n' + pass=$((pass + 1)) + else + printf ' FAIL list-symlinks on empty allowed dir — exit=%s output=%s\n' "${rc:-?}" "$out" + fail=$((fail + 1)) + failures+=("list-symlinks happy path") + fi + rmdir "$HAPPY_DIR" 2>/dev/null || true +else + echo " skip /var/agor not writable — happy path test skipped" +fi + +# ---------------------------------------------------------------------------- +# assert_safe_password (stdin to set-password) +# ---------------------------------------------------------------------------- +echo "-- assert_safe_password --" +# We use a username that passes the shape check but will fail at chpasswd +# (user doesn't exist). For rejection tests, we want the wrapper to exit +# 66 BEFORE ever invoking chpasswd — so the test user's existence doesn't +# matter. We pick "testuser_wrapper" which is extremely unlikely to exist. +TEST_USER="testuser_wrapper_$$" +assert_exit 66 "reject empty password" run_wrapper_stdin "" set-password "$TEST_USER" +assert_exit 66 "reject password with LF" run_wrapper_stdin $'abc\ndef' set-password "$TEST_USER" +assert_exit 66 "reject password with CR" run_wrapper_stdin $'abc\rdef' set-password "$TEST_USER" +assert_exit 66 "reject password with colon (chpasswd field injector)" \ + run_wrapper_stdin 'abc:def' set-password "$TEST_USER" +assert_exit 66 "reject password with NUL" run_wrapper_stdin $'abc\x00def' set-password "$TEST_USER" +# Non-printable high byte +assert_exit 66 "reject password with high-bit byte" \ + run_wrapper_stdin $'abc\xffdef' set-password "$TEST_USER" +# 257-byte password (over limit) +BIG=$(printf 'a%.0s' {1..260}) +assert_exit 66 "reject password > 256 bytes" run_wrapper_stdin "$BIG" set-password "$TEST_USER" + +# Username validation happens before stdin read in set-password, so flag-shaped +# username still fails with 65, not 66. +assert_exit 65 "set-password rejects flag-shaped username before reading stdin" \ + run_wrapper_stdin "harmless" set-password -rf + +# ---------------------------------------------------------------------------- +# Flag-smuggling into usermod via group argument +# ---------------------------------------------------------------------------- +echo "-- flag-smuggling regressions --" +assert_exit 65 "add-to-group: flag-shaped group rejected" \ + bash "$WRAPPER" add-to-group alice --foo +assert_exit 65 "add-to-group: flag-shaped user rejected" \ + bash "$WRAPPER" add-to-group --foo developers +assert_exit 65 "remove-from-group: flag-shaped group rejected" \ + bash "$WRAPPER" remove-from-group alice --foo + +# ---------------------------------------------------------------------------- +# Unicode / homoglyph usernames and groupnames +# The shape regex is strictly [a-z_][a-z0-9_-]{0,31}, so every non-ASCII byte +# (Cyrillic lookalikes, full-width digits, combining marks, right-to-left +# overrides, zero-width joiners, etc.) must be rejected at 65. These are not +# hypothetical: NSS/useradd accept many Unicode strings depending on locale, +# and homoglyph attacks on admin UIs have a long history. +# ---------------------------------------------------------------------------- +echo "-- unicode / homoglyph rejection --" +# Cyrillic 'а' (U+0430) looks like ASCII 'a' but is two UTF-8 bytes. +assert_exit 65 "reject Cyrillic homoglyph username (аlice)" \ + bash "$WRAPPER" add-user $'\xd0\xb0lice' +assert_exit 65 "reject Cyrillic homoglyph groupname (аdmins)" \ + bash "$WRAPPER" add-group $'\xd0\xb0dmins' +# Full-width Latin 'ａ' (U+FF41) — three UTF-8 bytes. +assert_exit 65 "reject full-width username (ａlice)" \ + bash "$WRAPPER" add-user $'\xef\xbd\x81lice' +# Zero-width joiner inside otherwise-ASCII string (U+200D). +assert_exit 65 "reject zero-width-joiner inside username" \ + bash "$WRAPPER" add-user $'al\xe2\x80\x8dice' +# Right-to-left override (U+202E) — classic filename-spoofing byte. +assert_exit 65 "reject RTL-override in username" \ + bash "$WRAPPER" add-user $'a\xe2\x80\xaelice' +# BOM prefix (U+FEFF) — invisible but illegal under the ASCII regex. +assert_exit 65 "reject BOM-prefixed username" \ + bash "$WRAPPER" add-user $'\xef\xbb\xbfalice' +# Combining acute accent after 'a' (U+0301). +assert_exit 65 "reject combining-mark username" \ + bash "$WRAPPER" add-user $'a\xcc\x81lice' +# NBSP inside a group name (U+00A0) — looks like a space. +assert_exit 65 "reject NBSP in groupname" \ + bash "$WRAPPER" add-group $'dev\xc2\xa0ops' +# Same classes in add-to-group for both user and group slots. +assert_exit 65 "add-to-group: Cyrillic user rejected" \ + bash "$WRAPPER" add-to-group $'\xd0\xb0lice' developers +assert_exit 65 "add-to-group: Cyrillic group rejected" \ + bash "$WRAPPER" add-to-group alice $'\xd0\xb4evelopers' + +# Non-ASCII in home-dir argument — must also be rejected. +assert_exit 65 "add-user --home rejects Cyrillic path component" \ + bash "$WRAPPER" add-user --home $'/home/\xd0\xb0lice' alice +assert_exit 65 "add-user --home rejects home dir outside allowlist" \ + bash "$WRAPPER" add-user --home /root/alice alice +assert_exit 65 "add-user --home rejects traversal (..)" \ + bash "$WRAPPER" add-user --home /home/alice/../bob alice + +# ---------------------------------------------------------------------------- +# Argv newline / CR / NUL smuggling +# A literal newline in argv has historically bypassed naive regex checks that +# use ^...$ without /s or that rely on line-oriented grep. Bash's [[ =~ ]] with +# our anchors will NOT match across a newline, so these must land at 65. +# ---------------------------------------------------------------------------- +echo "-- argv control-char smuggling --" +assert_exit 65 "reject newline inside username" \ + bash "$WRAPPER" add-user $'alice\nroot' +assert_exit 65 "reject CR inside username" \ + bash "$WRAPPER" add-user $'alice\rroot' +assert_exit 65 "reject tab inside username" \ + bash "$WRAPPER" add-user $'ali\tce' +assert_exit 65 "reject newline inside groupname" \ + bash "$WRAPPER" add-group $'devs\nroot' +# Leading/trailing whitespace — also outside the shape. +assert_exit 65 "reject leading newline in username" \ + bash "$WRAPPER" add-user $'\nalice' +assert_exit 65 "reject trailing newline in username" \ + bash "$WRAPPER" add-user $'alice\n' +# Newline in the --home argument is separately rejected by validate_home_dir. +assert_exit 65 "add-user --home rejects newline in path" \ + bash "$WRAPPER" add-user --home $'/home/alice\nroot' alice +# Newline in a filesystem-verb path (validate_path has an explicit control-char +# branch that beats readlink -f). +assert_exit 65 "setgid-tree rejects newline in path" \ + bash "$WRAPPER" setgid-tree $'/var/agor/wt\nroot' +assert_exit 65 "list-symlinks rejects CR in path" \ + bash "$WRAPPER" list-symlinks $'/var/agor/wt\rroot' + +# Direct NUL in argv: bash generally truncates argv at NUL when using $'\x00', +# because execve's argv is NUL-terminated. We still assert the wrapper does not +# crash/accept when given a name with embedded NUL via printf — if bash strips +# the NUL and everything after, the remaining prefix still must fail the regex +# because it will equal the short prefix only. We test the observable outcome: +# exit 65 regardless of how bash chose to represent the argv slot. +assert_exit 65 "reject NUL-bearing username (observably truncated)" \ + bash "$WRAPPER" add-user $'alice\x00root' + +# ---------------------------------------------------------------------------- +# Audit: every rejection should be silent at auth.info level (we don't log +# failed-validation events — only successful dispatches write to syslog). +# We can't easily assert on syslog from here without a logger; just make sure +# the wrapper doesn't crash when the logger binary is absent (covered by the +# `|| true` in audit()). +# ---------------------------------------------------------------------------- + +echo "" +echo "== summary ==" +echo "passed: $pass" +echo "failed: $fail" +if [[ $fail -gt 0 ]]; then + echo "" + echo "failures:" + for f in "${failures[@]}"; do + echo " - $f" + done + exit 1 +fi +exit 0 diff --git a/packages/core/src/unix/group-manager.test.ts b/packages/core/src/unix/group-manager.test.ts index 64f512b40..0280bfd02 100644 --- a/packages/core/src/unix/group-manager.test.ts +++ b/packages/core/src/unix/group-manager.test.ts @@ -153,33 +153,33 @@ describe('group-manager', () => { describe('UnixGroupCommands', () => { describe('createGroup', () => { - it('generates groupadd command', () => { + it('routes through agor-user-admin wrapper', () => { expect(UnixGroupCommands.createGroup('agor_wt_01234567')).toBe( - 'sudo -n groupadd agor_wt_01234567' + "sudo -n /usr/local/sbin/agor-user-admin add-group 'agor_wt_01234567'" ); }); }); describe('deleteGroup', () => { - it('generates groupdel command', () => { + it('routes through agor-user-admin wrapper', () => { expect(UnixGroupCommands.deleteGroup('agor_wt_01234567')).toBe( - 'sudo -n groupdel agor_wt_01234567' + "sudo -n /usr/local/sbin/agor-user-admin delete-group 'agor_wt_01234567'" ); }); }); describe('addUserToGroup', () => { - it('generates usermod -aG command', () => { + it('routes through agor-user-admin wrapper', () => { expect(UnixGroupCommands.addUserToGroup('alice', 'developers')).toBe( - 'sudo -n usermod -aG developers alice' + "sudo -n /usr/local/sbin/agor-user-admin add-to-group 'alice' 'developers'" ); }); }); describe('removeUserFromGroup', () => { - it('generates gpasswd -d command', () => { + it('routes through agor-user-admin wrapper', () => { expect(UnixGroupCommands.removeUserFromGroup('alice', 'developers')).toBe( - 'sudo -n gpasswd -d alice developers' + "sudo -n /usr/local/sbin/agor-user-admin remove-from-group 'alice' 'developers'" ); }); }); @@ -235,7 +235,7 @@ describe('group-manager', () => { 'sudo -n setfacl -R -m o::rX "/data/project"', 'sudo -n setfacl -R -m m::rwX "/data/project"', 'sudo -n setfacl -R -d -m u::rwX,g:developers:rwX,o::rX,m::rwX "/data/project"', - 'sudo -n find "/data/project" -type d -exec chmod g+s {} +', + "sudo -n /usr/local/sbin/agor-user-admin setgid-tree '/data/project'", ]); }); @@ -248,7 +248,7 @@ describe('group-manager', () => { 'sudo -n setfacl -R -m o::--- "/data/secret"', 'sudo -n setfacl -R -m m::rwX "/data/secret"', 'sudo -n setfacl -R -d -m u::rwX,g:admins:rwX,o::---,m::rwX "/data/secret"', - 'sudo -n find "/data/secret" -type d -exec chmod g+s {} +', + "sudo -n /usr/local/sbin/agor-user-admin setgid-tree '/data/secret'", ]); }); @@ -261,7 +261,7 @@ describe('group-manager', () => { 'sudo -n setfacl -R -m o::rwX "/data/public"', 'sudo -n setfacl -R -m m::rwX "/data/public"', 'sudo -n setfacl -R -d -m u::rwX,g:everyone:rwX,o::rwX,m::rwX "/data/public"', - 'sudo -n find "/data/public" -type d -exec chmod g+s {} +', + "sudo -n /usr/local/sbin/agor-user-admin setgid-tree '/data/public'", ]); }); }); diff --git a/packages/core/src/unix/group-manager.ts b/packages/core/src/unix/group-manager.ts index 299c5995c..73f776af2 100644 --- a/packages/core/src/unix/group-manager.ts +++ b/packages/core/src/unix/group-manager.ts @@ -13,6 +13,8 @@ import { formatShortId } from '../lib/ids.js'; import type { RepoID, UUID, WorktreeID } from '../types/index.js'; +import { escapeShellArg } from './run-as-user.js'; +import { AGOR_USER_ADMIN } from './wrapper-constants.js'; /** * Generate Unix group name for a worktree @@ -115,40 +117,46 @@ export const AGOR_USERS_GROUP = 'agor_users'; */ export const UnixGroupCommands = { /** - * Create a new Unix group + * Create a new Unix group (via agor-user-admin wrapper) * * @param groupName - Name of the group to create * @returns Command string with sudo */ - createGroup: (groupName: string) => `sudo -n groupadd ${groupName}`, + createGroup: (groupName: string) => + `sudo -n ${AGOR_USER_ADMIN} add-group ${escapeShellArg(groupName)}`, /** - * Delete a Unix group + * Delete a Unix group (via agor-user-admin wrapper) * * @param groupName - Name of the group to delete * @returns Command string with sudo */ - deleteGroup: (groupName: string) => `sudo -n groupdel ${groupName}`, + deleteGroup: (groupName: string) => + `sudo -n ${AGOR_USER_ADMIN} delete-group ${escapeShellArg(groupName)}`, /** - * Add user to a Unix group + * Add user to a Unix group (via agor-user-admin wrapper). + * + * Underlying shape: `usermod -aG -- `. * * @param username - Unix username to add * @param groupName - Group to add user to * @returns Command string with sudo */ addUserToGroup: (username: string, groupName: string) => - `sudo -n usermod -aG ${groupName} ${username}`, + `sudo -n ${AGOR_USER_ADMIN} add-to-group ${escapeShellArg(username)} ${escapeShellArg(groupName)}`, /** - * Remove user from a Unix group + * Remove user from a Unix group (via agor-user-admin wrapper). + * + * Underlying shape: `gpasswd -d -- `. * * @param username - Unix username to remove * @param groupName - Group to remove user from * @returns Command string with sudo */ removeUserFromGroup: (username: string, groupName: string) => - `sudo -n gpasswd -d ${username} ${groupName}`, + `sudo -n ${AGOR_USER_ADMIN} remove-from-group ${escapeShellArg(username)} ${escapeShellArg(groupName)}`, /** * Check if a group exists (read-only, no sudo needed) @@ -233,9 +241,13 @@ export const UnixGroupCommands = { // DEFAULT ACLs for new files/dirs (inherit these permissions) // IMPORTANT: Include m::rwX to ensure mask allows group access on new files `sudo -n setfacl -R -d -m u::rwX,g:${groupName}:rwX,${othersAcl},m::rwX "${path}"`, - // Set setgid bit on directories only (new files inherit group ownership) - // Uses sudo for find traversal since chgrp can invalidate ACL cache - `sudo -n find "${path}" -type d -exec chmod g+s {} +`, + // Set setgid bit on directories only (new files inherit group ownership). + // Routed through the agor-user-admin wrapper — the sudoers policy no + // longer grants a wildcard `find *` entry, which was a root-shell + // path (find -exec runs arbitrary commands). The wrapper validates + // the path against the Agor-managed tree allowlist and pins the + // -type d -exec chmod g+s {} + argv. + `sudo -n ${AGOR_USER_ADMIN} setgid-tree ${escapeShellArg(path)}`, ]; }, diff --git a/packages/core/src/unix/index.ts b/packages/core/src/unix/index.ts index c359ebabd..79f8d3eba 100644 --- a/packages/core/src/unix/index.ts +++ b/packages/core/src/unix/index.ts @@ -30,3 +30,5 @@ export * from './unix-integration-service.js'; export * from './user-env-file.js'; // Unix user management export * from './user-manager.js'; +// Shared constants for the privileged-operations wrapper +export * from './wrapper-constants.js'; diff --git a/packages/core/src/unix/symlink-manager.test.ts b/packages/core/src/unix/symlink-manager.test.ts index de8728711..3ef135b45 100644 --- a/packages/core/src/unix/symlink-manager.test.ts +++ b/packages/core/src/unix/symlink-manager.test.ts @@ -114,9 +114,9 @@ describe('symlink-manager', () => { }); describe('listSymlinks', () => { - it('generates find command for symlinks', () => { + it('routes through agor-user-admin wrapper', () => { expect(SymlinkCommands.listSymlinks('/home/alice/agor/worktrees')).toBe( - `find "/home/alice/agor/worktrees" -maxdepth 1 -type l -printf '%f\\n'` + "/usr/local/sbin/agor-user-admin list-symlinks '/home/alice/agor/worktrees'" ); }); }); @@ -151,18 +151,18 @@ describe('symlink-manager', () => { }); describe('removeAllSymlinks', () => { - it('generates find -delete command', () => { + it('routes through agor-user-admin wrapper', () => { expect(SymlinkCommands.removeAllSymlinks('/home/alice/agor/worktrees')).toBe( - 'find "/home/alice/agor/worktrees" -maxdepth 1 -type l -delete' + "/usr/local/sbin/agor-user-admin prune-all-symlinks '/home/alice/agor/worktrees'" ); }); }); describe('removeBrokenSymlinks', () => { - it('generates find command for broken symlinks', () => { + it('routes through agor-user-admin wrapper', () => { const cmd = SymlinkCommands.removeBrokenSymlinks('/home/alice/agor/worktrees'); expect(cmd).toBe( - `find "/home/alice/agor/worktrees" -maxdepth 1 -type l ! -exec test -e {} \\; -delete` + "/usr/local/sbin/agor-user-admin prune-broken-symlinks '/home/alice/agor/worktrees'" ); }); }); diff --git a/packages/core/src/unix/symlink-manager.ts b/packages/core/src/unix/symlink-manager.ts index d97ded8b3..d591ef143 100644 --- a/packages/core/src/unix/symlink-manager.ts +++ b/packages/core/src/unix/symlink-manager.ts @@ -7,7 +7,9 @@ * @see context/guides/rbac-and-unix-isolation.md */ +import { escapeShellArg } from './run-as-user.js'; import { AGOR_HOME_BASE, AGOR_WORKTREES_DIR } from './user-manager.js'; +import { AGOR_USER_ADMIN } from './wrapper-constants.js'; /** * Get the symlink path for a worktree in a user's home @@ -83,12 +85,17 @@ export const SymlinkCommands = { readSymlink: (linkPath: string) => `readlink "${linkPath}"`, /** - * List all symlinks in a directory + * List all symlinks in a directory (via agor-user-admin wrapper). + * + * Routes through the wrapper because the caller often runs as the Agor + * daemon, which no longer has a direct `sudo find *` grant. The wrapper + * validates that dirPath is inside the Agor-managed tree and runs + * `find -maxdepth 1 -type l -printf '%f\n'` as root. * * @param dirPath - Directory to list * @returns Command string (outputs symlink names, one per line) */ - listSymlinks: (dirPath: string) => `find "${dirPath}" -maxdepth 1 -type l -printf '%f\\n'`, + listSymlinks: (dirPath: string) => `${AGOR_USER_ADMIN} list-symlinks ${escapeShellArg(dirPath)}`, /** * Create symlink with proper ownership @@ -116,21 +123,22 @@ export const SymlinkCommands = { }, /** - * Remove all symlinks in a directory + * Remove all symlinks in a directory (via agor-user-admin wrapper). * * @param dirPath - Directory to clean * @returns Command string */ - removeAllSymlinks: (dirPath: string) => `find "${dirPath}" -maxdepth 1 -type l -delete`, + removeAllSymlinks: (dirPath: string) => + `${AGOR_USER_ADMIN} prune-all-symlinks ${escapeShellArg(dirPath)}`, /** - * Remove broken symlinks in a directory + * Remove broken symlinks in a directory (via agor-user-admin wrapper). * * @param dirPath - Directory to clean * @returns Command string */ removeBrokenSymlinks: (dirPath: string) => - `find "${dirPath}" -maxdepth 1 -type l ! -exec test -e {} \\; -delete`, + `${AGOR_USER_ADMIN} prune-broken-symlinks ${escapeShellArg(dirPath)}`, } as const; /** diff --git a/packages/core/src/unix/unix-integration-service.ts b/packages/core/src/unix/unix-integration-service.ts index 4f6ee9c1b..02bb0996b 100644 --- a/packages/core/src/unix/unix-integration-service.ts +++ b/packages/core/src/unix/unix-integration-service.ts @@ -28,7 +28,6 @@ import { } from './group-manager.js'; import { getWorktreeSymlinkPath, SymlinkCommands } from './symlink-manager.js'; import { - AGOR_DEFAULT_SHELL, AGOR_HOME_BASE, generateUnixUsername, getUserHomeDir, @@ -848,10 +847,18 @@ export class UnixIntegrationService { if (!exists) { console.log(`[UnixIntegration] Creating Unix user: ${unixUsername}`); - // Pass homeBase to ensure home directory is created in the configured location - await this.executor.exec( - UnixUserCommands.createUser(unixUsername, AGOR_DEFAULT_SHELL, this.config.homeBase) - ); + // Only pass an explicit --home to the wrapper when config.homeBase + // diverges from the wrapper's default (AGOR_HOME_BASE === '/home'). + // For the default case we let useradd pick up the system default, + // which is what we want on Debian/Alpine/etc. The wrapper validates + // that the supplied path matches exactly `${homeBase}/${username}` + // inside /home or /var/agor. + const configuredHomeBase = this.config.homeBase; + const homeDir = + configuredHomeBase && configuredHomeBase !== AGOR_HOME_BASE + ? getUserHomeDir(unixUsername, configuredHomeBase) + : undefined; + await this.executor.exec(UnixUserCommands.createUser(unixUsername, homeDir)); // Setup ~/agor/worktrees directory await this.executor.execAll( @@ -919,7 +926,7 @@ export class UnixIntegrationService { try { // SECURITY: Use execWithInput to pass password via stdin, not command line - const cmd = UnixUserCommands.setPasswordCommand(); + const cmd = UnixUserCommands.setPasswordCommand(user.unix_username); const input = UnixUserCommands.formatPasswordInput(user.unix_username, plaintextPassword); await this.executor.execWithInput(cmd, { input }); console.log(`[UnixIntegration] Synced password for ${user.unix_username}`); diff --git a/packages/core/src/unix/user-manager.test.ts b/packages/core/src/unix/user-manager.test.ts index ceda62d74..3c39b99a4 100644 --- a/packages/core/src/unix/user-manager.test.ts +++ b/packages/core/src/unix/user-manager.test.ts @@ -211,50 +211,81 @@ describe('user-manager', () => { }); describe('createUser', () => { - it('generates useradd command with defaults', () => { - const cmd = UnixUserCommands.createUser('alice'); - expect(cmd).toBe('sudo -n useradd -m -d "/home/alice" -s "/bin/bash" "alice"'); + it('routes through agor-user-admin add-user', () => { + // Every privileged user op goes through /usr/local/sbin/agor-user-admin + // under the hardened sudoers. Custom shell is no longer exposed; it + // was unused in production and widened the wrapper's attack surface. + expect(UnixUserCommands.createUser('alice')).toBe( + "sudo -n /usr/local/sbin/agor-user-admin add-user 'alice'" + ); }); - it('uses custom shell and home base', () => { - const cmd = UnixUserCommands.createUser('alice', '/bin/zsh', '/users'); - expect(cmd).toBe('sudo -n useradd -m -d "/users/alice" -s "/bin/zsh" "alice"'); - }); - }); - - describe('createUserWithId', () => { - it('generates useradd with UID', () => { - const cmd = UnixUserCommands.createUserWithId('alice', 1001); - expect(cmd).toBe('sudo -n useradd -m -d "/home/alice" -s "/bin/bash" -u 1001 "alice"'); + it('passes --home when a non-default home directory is requested', () => { + // homeBase configurability is preserved via the wrapper's --home flag, + // which re-validates the path shape server-side. + expect(UnixUserCommands.createUser('alice', '/var/agor/alice')).toBe( + "sudo -n /usr/local/sbin/agor-user-admin add-user --home '/var/agor/alice' 'alice'" + ); }); - it('includes GID when provided', () => { - const cmd = UnixUserCommands.createUserWithId('alice', 1001, 1001); - expect(cmd).toBe( - 'sudo -n useradd -m -d "/home/alice" -s "/bin/bash" -u 1001 -g 1001 "alice"' + it('single-quotes home dirs with embedded single quotes', () => { + // Not a legal path in production, but the escape helper must still + // produce a shell-safe argv even for pathological callers. + expect(UnixUserCommands.createUser('alice', "/var/agor/a'lice")).toBe( + "sudo -n /usr/local/sbin/agor-user-admin add-user --home '/var/agor/a'\\''lice' 'alice'" ); }); }); describe('deleteUser', () => { - it('generates userdel command (keep home)', () => { - expect(UnixUserCommands.deleteUser('alice')).toBe('sudo -n userdel "alice"'); + it('routes through agor-user-admin delete-user', () => { + expect(UnixUserCommands.deleteUser('alice')).toBe( + "sudo -n /usr/local/sbin/agor-user-admin delete-user 'alice'" + ); }); }); describe('deleteUserWithHome', () => { - it('generates userdel -r command', () => { - expect(UnixUserCommands.deleteUserWithHome('alice')).toBe('sudo -n userdel -r "alice"'); + it('routes through agor-user-admin delete-user --remove-home', () => { + expect(UnixUserCommands.deleteUserWithHome('alice')).toBe( + "sudo -n /usr/local/sbin/agor-user-admin delete-user --remove-home 'alice'" + ); }); }); describe('lockUser / unlockUser', () => { - it('generates lock command', () => { - expect(UnixUserCommands.lockUser('alice')).toBe('sudo -n usermod -L "alice"'); + it('routes lock through agor-user-admin lock-user', () => { + expect(UnixUserCommands.lockUser('alice')).toBe( + "sudo -n /usr/local/sbin/agor-user-admin lock-user 'alice'" + ); }); - it('generates unlock command', () => { - expect(UnixUserCommands.unlockUser('alice')).toBe('sudo -n usermod -U "alice"'); + it('routes unlock through agor-user-admin unlock-user', () => { + expect(UnixUserCommands.unlockUser('alice')).toBe( + "sudo -n /usr/local/sbin/agor-user-admin unlock-user 'alice'" + ); + }); + }); + + describe('setPasswordCommand / formatPasswordInput', () => { + it('returns wrapper argv with username and stdin-only password', () => { + expect(UnixUserCommands.setPasswordCommand('alice')).toEqual([ + 'sudo', + '-n', + '/usr/local/sbin/agor-user-admin', + 'set-password', + 'alice', + ]); + // The wrapper composes the `user:password` record itself; callers + // pipe only the raw password. + expect(UnixUserCommands.formatPasswordInput('alice', 'hunter2')).toBe('hunter2'); + }); + + it('rejects chpasswd-unsafe password via formatPasswordInput', () => { + expect(() => UnixUserCommands.formatPasswordInput('alice', 'has\nnewline')).toThrow( + /newline/ + ); + expect(() => UnixUserCommands.formatPasswordInput('al:ice', 'ok')).toThrow(/":"/); }); }); diff --git a/packages/core/src/unix/user-manager.ts b/packages/core/src/unix/user-manager.ts index 1b15c2015..e1c2c05dd 100644 --- a/packages/core/src/unix/user-manager.ts +++ b/packages/core/src/unix/user-manager.ts @@ -11,6 +11,12 @@ import { execSync } from 'node:child_process'; import type { UnixUserMode } from '../config/types.js'; import { formatShortId } from '../lib/ids.js'; import type { UserID, UUID } from '../types/index.js'; +import { escapeShellArg } from './run-as-user.js'; +import { AGOR_USER_ADMIN } from './wrapper-constants.js'; + +// Re-export so call-sites can `import { AGOR_USER_ADMIN } from './user-manager.js'` +// during the transition. New code should import from './wrapper-constants.js' directly. +export { AGOR_USER_ADMIN }; /** * Default home directory base for Agor users @@ -102,9 +108,24 @@ export function assertChpasswdInputSafe(username: string, password: string): voi if (typeof password !== 'string' || password.length === 0) { throw new Error('Refusing to sync password: password is empty'); } + // Mirror docker/sudoers/agor-user-admin's assert_safe_password so we fail + // fast in Node rather than letting the wrapper reject with a less helpful + // error. The two checks must stay in sync — relaxing one without the other + // creates a silent gap. if (/[\r\n\0]/.test(password)) { throw new Error('Refusing to sync password: password contains newline or NUL byte'); } + if (password.includes(':')) { + throw new Error('Refusing to sync password: password contains ":" (chpasswd field separator)'); + } + if (Buffer.byteLength(password, 'utf8') > 256) { + throw new Error('Refusing to sync password: password exceeds 256 bytes'); + } + // Printable ASCII 0x20..0x7E only — matches wrapper's [[:print:]] check + // under LC_ALL=C. + if (/[^\x20-\x7e]/.test(password)) { + throw new Error('Refusing to sync password: password contains non-printable bytes'); + } } /** @@ -130,9 +151,13 @@ export function getUserWorktreesDir(username: string, homeBase: string = AGOR_HO } /** - * Unix user management commands (to be executed via sudo) + * Unix user management commands (to be executed via sudo). * - * These are shell command strings for privileged user operations. + * These are shell command strings. Every privileged operation routes through + * the root-owned {@link AGOR_USER_ADMIN} wrapper, which validates verbs and + * arguments before invoking the underlying tool. The sudoers file grants + * NOPASSWD on that single wrapper path — not on useradd/usermod/gpasswd/ + * chpasswd/find directly. See docker/sudoers/agor-user-admin. */ export const UnixUserCommands = { /** @@ -144,38 +169,25 @@ export const UnixUserCommands = { userExists: (username: string) => `id "${username}" > /dev/null 2>&1`, /** - * Create a new Unix user with home directory + * Create a new Unix user with home directory. * - * @param username - Unix username to create - * @param shell - Login shell (default: /bin/bash) - * @param homeBase - Base directory for home (default: /home) - * @returns Command string - */ - createUser: ( - username: string, - shell: string = AGOR_DEFAULT_SHELL, - homeBase: string = AGOR_HOME_BASE - ) => `sudo -n useradd -m -d "${homeBase}/${username}" -s "${shell}" "${username}"`, - - /** - * Create user with specific UID/GID + * Fixed shape: `useradd -m [-d ] -s /bin/bash -- `. * - * @param username - Unix username - * @param uid - User ID - * @param gid - Group ID (optional, defaults to uid) - * @param shell - Login shell - * @param homeBase - Home directory base + * The wrapper's `--home` flag preserves UnixIntegrationService's + * `homeBase` configurability without giving useradd a wildcard `-d`: + * the wrapper validates that the supplied home dir matches exactly + * `/home/` or `/var/agor/` (no traversal, no extra path + * components). When `homeDir` is omitted, useradd falls back to the + * system default HOME base (typically `/home`). + * + * @param username - Unix username to create + * @param homeDir - Optional explicit home directory. Must equal + * `${homeBase}/${username}` for the wrapper to accept it. * @returns Command string */ - createUserWithId: ( - username: string, - uid: number, - gid?: number, - shell: string = AGOR_DEFAULT_SHELL, - homeBase: string = AGOR_HOME_BASE - ) => { - const gidArg = gid !== undefined ? `-g ${gid}` : ''; - return `sudo -n useradd -m -d "${homeBase}/${username}" -s "${shell}" -u ${uid} ${gidArg} "${username}"`; + createUser: (username: string, homeDir?: string) => { + const homePart = homeDir ? `--home ${escapeShellArg(homeDir)} ` : ''; + return `sudo -n ${AGOR_USER_ADMIN} add-user ${homePart}${escapeShellArg(username)}`; }, /** @@ -184,45 +196,55 @@ export const UnixUserCommands = { * @param username - Unix username to delete * @returns Command string */ - deleteUser: (username: string) => `sudo -n userdel "${username}"`, + deleteUser: (username: string) => + `sudo -n ${AGOR_USER_ADMIN} delete-user ${escapeShellArg(username)}`, /** - * Get command array for setting Unix user password via chpasswd + * Get command array for setting a Unix user's password via the wrapper. * - * SECURITY: This returns a command array to be used with execWithInput(). - * The password MUST be passed via stdin (not command-line arguments) to avoid: - * 1. Command injection vulnerabilities (shell metacharacters in password) - * 2. Password exposure in process listings (ps aux) - * 3. Password exposure in shell history + * SECURITY: Used with execWithInput(). The password MUST be passed via + * stdin (not argv) to avoid: + * 1. Command injection (shell metacharacters in password) + * 2. Exposure in process listings (ps aux) + * 3. Exposure in shell history * - * Format for stdin: "username:password\n" + * The wrapper reads the password from stdin (no newline required), runs + * `chpasswd` internally as `:`, and only supports + * usernames that pass its own validator. * - * @returns Command array for execWithInput: ['chpasswd'] + * @param username - Unix username whose password to set + * @returns Command array for execWithInput * * @example * ```ts - * const cmd = UnixUserCommands.setPasswordCommand(); - * await executor.execWithInput(cmd, { input: `${username}:${password}\n` }); + * const cmd = UnixUserCommands.setPasswordCommand(user.unix_username); + * const input = UnixUserCommands.formatPasswordInput(user.unix_username, password); + * await executor.execWithInput(cmd, { input }); * ``` */ - setPasswordCommand: (): string[] => { - return ['sudo', '-n', '/usr/sbin/chpasswd']; - }, + setPasswordCommand: (username: string): string[] => [ + 'sudo', + '-n', + AGOR_USER_ADMIN, + 'set-password', + username, + ], /** - * Format stdin input for chpasswd command + * Format stdin input for the set-password verb. * - * Validates inputs via {@link assertChpasswdInputSafe} to prevent a caller - * from injecting extra `username:password` records into chpasswd's stdin. + * Validates inputs via {@link assertChpasswdInputSafe} for defense-in-depth; + * the wrapper re-validates too. * * @param username - Unix username * @param password - Plaintext password to set - * @returns Formatted stdin input: "username:password\n" + * @returns The raw password (no username prefix, no trailing newline). + * The wrapper composes the `user:password` record internally. * @throws Error if username or password contain chpasswd-unsafe characters */ formatPasswordInput: (username: string, password: string): string => { assertChpasswdInputSafe(username, password); - return `${username}:${password}\n`; + return password; }, /** @@ -231,7 +253,8 @@ export const UnixUserCommands = { * @param username - Unix username to delete * @returns Command string */ - deleteUserWithHome: (username: string) => `sudo -n userdel -r "${username}"`, + deleteUserWithHome: (username: string) => + `sudo -n ${AGOR_USER_ADMIN} delete-user --remove-home ${escapeShellArg(username)}`, /** * Lock a Unix user account (disable login) @@ -239,7 +262,8 @@ export const UnixUserCommands = { * @param username - Unix username * @returns Command string */ - lockUser: (username: string) => `sudo -n usermod -L "${username}"`, + lockUser: (username: string) => + `sudo -n ${AGOR_USER_ADMIN} lock-user ${escapeShellArg(username)}`, /** * Unlock a Unix user account @@ -247,7 +271,8 @@ export const UnixUserCommands = { * @param username - Unix username * @returns Command string */ - unlockUser: (username: string) => `sudo -n usermod -U "${username}"`, + unlockUser: (username: string) => + `sudo -n ${AGOR_USER_ADMIN} unlock-user ${escapeShellArg(username)}`, /** * Get user's UID diff --git a/packages/core/src/unix/wrapper-constants.ts b/packages/core/src/unix/wrapper-constants.ts new file mode 100644 index 000000000..ce08ce4c4 --- /dev/null +++ b/packages/core/src/unix/wrapper-constants.ts @@ -0,0 +1,14 @@ +/** + * Shared constants for the privileged-operations wrapper. + * + * The wrapper at this absolute path replaces the wildcard NOPASSWD sudoers + * rules that previously exposed useradd/userdel/usermod/gpasswd/groupadd/ + * groupdel/chpasswd/find directly to the daemon. All Node-side callers that + * shell out to it must reference this constant rather than hard-coding the + * path so the location stays in lockstep with `docker/sudoers/agor-user-admin` + * and the sudoers rule. + * + * @see docker/sudoers/agor-user-admin + * @see docker/sudoers/agor-daemon.sudoers + */ +export const AGOR_USER_ADMIN = '/usr/local/sbin/agor-user-admin'; diff --git a/packages/executor/src/commands/unix.ts b/packages/executor/src/commands/unix.ts index d03ee2319..e0e1d4f06 100644 --- a/packages/executor/src/commands/unix.ts +++ b/packages/executor/src/commands/unix.ts @@ -21,6 +21,7 @@ import { existsSync } from 'node:fs'; import { promisify } from 'node:util'; import type { RepoID, WorktreeID } from '@agor/core/types'; import { + AGOR_USER_ADMIN, AGOR_USERS_GROUP, assertChpasswdInputSafe, generateRepoGroupName, @@ -741,17 +742,21 @@ export async function handleUnixSyncUser( } assertChpasswdInputSafe(unixUsername, password); - // Use chpasswd with stdin for security (password not in process list) + // Route through the agor-user-admin wrapper rather than hitting + // /usr/sbin/chpasswd directly. The wrapper takes the username as an + // argv verb-arg and the password via stdin (no username:password + // record on the wire) — the sudoers policy only grants NOPASSWD on + // the wrapper path, not on chpasswd itself. const { spawn } = await import('node:child_process'); await new Promise((resolve, reject) => { - const proc = spawn('sudo', ['-n', '/usr/sbin/chpasswd'], { + const proc = spawn('sudo', ['-n', AGOR_USER_ADMIN, 'set-password', unixUsername], { stdio: ['pipe', 'pipe', 'pipe'], }); - proc.stdin.write(`${unixUsername}:${password}\n`); + proc.stdin.write(password); proc.stdin.end(); proc.on('close', (code) => { if (code === 0) resolve(); - else reject(new Error(`chpasswd exited with code ${code}`)); + else reject(new Error(`agor-user-admin set-password exited with code ${code}`)); }); proc.on('error', reject); });