Skip to content

sec(sudoers): collapse root wildcards behind agor-user-admin wrapper#1040

Open
mistercrunch wants to merge 2 commits into
mainfrom
sec-sudoers-wrapper
Open

sec(sudoers): collapse root wildcards behind agor-user-admin wrapper#1040
mistercrunch wants to merge 2 commits into
mainfrom
sec-sudoers-wrapper

Conversation

@mistercrunch
Copy link
Copy Markdown
Member

@mistercrunch mistercrunch commented Apr 19, 2026

Summary

Replaces five wildcard NOPASSWD sudoers rules (useradd *, userdel *, usermod *, chpasswd, groupadd * / groupdel * / gpasswd *, find *) with a single audited wrapper at /usr/local/sbin/agor-user-admin and one sudoers line:

agor ALL=(root) NOPASSWD: /usr/local/sbin/agor-user-admin

Each wildcard was a well-known root-shell path under sudo (flag-smuggling, hashed-password injection, find -exec). Funneling through one validated entry point closes those avenues while keeping the legitimate functionality the daemon needs.

What's in the wrapper

Fixed verb set: add-user, delete-user [--remove-home], lock-user, unlock-user, add-group, delete-group, add-to-group, remove-from-group, set-password, setgid-tree, list-symlinks, prune-all-symlinks, prune-broken-symlinks.

Validators (re-run inside the wrapper, even though Node-side validators stay as defense-in-depth):

  • Usernames / groupnames — ^[a-z_][a-z0-9_-]{0,31}$ + system-account deny-list (root, daemon, sudo, wheel, agor, agor_executor, …)
  • Paths — readlink -f canonicalization + allowlist (/home/*/agor/*, /home/*/.agor/*, /var/agor/*); symlink escapes resolve and fail
  • Passwords (chpasswd stdin) — no NUL/CR/LF/":" (chpasswd field separator), printable ASCII only, ≤256 bytes
  • Flag-smuggling — every shell-out uses -- end-of-options
  • Audit — every successful invocation logged via logger -t agor-user-admin (auth.info)

Node-side wiring

  • packages/core/src/unix/{user,group,symlink}-manager.ts route through AGOR_USER_ADMIN
  • packages/executor/src/commands/unix.ts (syncUnixPassword) replaces direct sudo -n /usr/sbin/chpasswd spawn with sudo -n agor-user-admin set-password <user>
  • apps/agor-cli/src/commands/admin/ensure-user.ts and docker/docker-entrypoint-postgres.sh updated
  • docker/Dockerfile installs the wrapper root-owned, mode 0755

Tests

  • Unit tests updated for new command shapes (group-manager, symlink-manager, user-manager)
  • New bash test harness at docker/sudoers/tests/agor-user-admin.test.sh exercises validators against adversarial inputs (flag-shaped names, path escapes, CRLF/:/non-printable passwords, system-account deny-list)

Test plan

  • pnpm check passes (typecheck + lint + build) — verified locally
  • pnpm --filter @agor/core test src/unix/ — 198/198 passing locally
  • Run bash docker/sudoers/tests/agor-user-admin.test.sh inside a root container (e.g. docker run --rm -v "$PWD:/src" -w /src debian:bookworm-slim bash docker/sudoers/tests/agor-user-admin.test.sh)
  • Manual smoke: sudo -n /usr/local/sbin/agor-user-admin should exit 64 with usage; sudo -n /usr/local/sbin/agor-user-admin add-user root should exit 65
  • Postgres profile entrypoint creates alice/bob via wrapper
  • Existing strict/insulated mode flows (user creation, password sync, worktree group setup, symlink pruning) still work end-to-end

🤖 Generated with Claude Code

rusackas pushed a commit that referenced this pull request Apr 19, 2026
@mistercrunch @claude
sec(sudoers): address Codex review — restore homeBase, dedupe shq(), …
db829f1 
…tighten validators

Addresses findings from Codex review of PR #1040:

1. BLOCKER: home-base regression
   - Add optional `--home <dir>` flag to wrapper's add-user verb, with a
     shape-only validator (`validate_home_dir`) that requires exact
     /home/<user> or /var/agor/<user> form (no traversal, no control
     chars, ≤256 bytes). readlink -f is skipped here because the dir
     does not exist at useradd time.
   - Thread `homeBase` through `UnixUserCommands.createUser(username,
     homeDir?)` and both callers (unix-integration-service,
     ensure-user CLI) so the existing `homeBase` config (defaults to
     /home but can be overridden to /var/agor) is preserved.

2. shq() duplication → shared escapeShellArg
   - Extract AGOR_USER_ADMIN constant to new
     packages/core/src/unix/wrapper-constants.ts so every module that
     shells out to the wrapper imports from one place.
   - Drop the local shq() copies in user-manager.ts, group-manager.ts,
     and symlink-manager.ts; import canonical escapeShellArg from
     run-as-user.ts.

3. Node-side password validator weaker than wrapper
   - Strengthen assertChpasswdInputSafe to reject ":" (chpasswd field
     separator), >256-byte inputs, and non-printable bytes — matching
     the wrapper's assert_safe_password so defense-in-depth holds at
     both layers.

4. Wrapper test harness misses adversarial classes
   - Add Unicode/homoglyph coverage (Cyrillic 'а', full-width 'ａ',
     zero-width joiner, RTL-override, BOM, combining marks, NBSP) for
     usernames, groupnames, and --home paths.
   - Add argv control-char coverage (newline, CR, tab, leading/
     trailing newline) for usernames, groupnames, --home paths, and
     filesystem-verb paths.
   - Add NUL-bearing argv case as an observable-outcome test.

Tests: user-manager unit tests extended for new createUser signature
(58 passing). All 200 unix tests pass. pnpm check green.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rusackas pushed a commit that referenced this pull request Apr 19, 2026
@mistercrunch @claude
sec(sudoers): address Codex review — restore homeBase, dedupe shq(), …
20a19de 
…tighten validators

Addresses findings from Codex review of PR #1040:

1. BLOCKER: home-base regression
   - Add optional `--home <dir>` flag to wrapper's add-user verb, with a
     shape-only validator (`validate_home_dir`) that requires exact
     /home/<user> or /var/agor/<user> form (no traversal, no control
     chars, ≤256 bytes). readlink -f is skipped here because the dir
     does not exist at useradd time.
   - Thread `homeBase` through `UnixUserCommands.createUser(username,
     homeDir?)` and both callers (unix-integration-service,
     ensure-user CLI) so the existing `homeBase` config (defaults to
     /home but can be overridden to /var/agor) is preserved.

2. shq() duplication → shared escapeShellArg
   - Extract AGOR_USER_ADMIN constant to new
     packages/core/src/unix/wrapper-constants.ts so every module that
     shells out to the wrapper imports from one place.
   - Drop the local shq() copies in user-manager.ts, group-manager.ts,
     and symlink-manager.ts; import canonical escapeShellArg from
     run-as-user.ts.

3. Node-side password validator weaker than wrapper
   - Strengthen assertChpasswdInputSafe to reject ":" (chpasswd field
     separator), >256-byte inputs, and non-printable bytes — matching
     the wrapper's assert_safe_password so defense-in-depth holds at
     both layers.

4. Wrapper test harness misses adversarial classes
   - Add Unicode/homoglyph coverage (Cyrillic 'а', full-width 'ａ',
     zero-width joiner, RTL-override, BOM, combining marks, NBSP) for
     usernames, groupnames, and --home paths.
   - Add argv control-char coverage (newline, CR, tab, leading/
     trailing newline) for usernames, groupnames, --home paths, and
     filesystem-verb paths.
   - Add NUL-bearing argv case as an observable-outcome test.

Tests: user-manager unit tests extended for new createUser signature
(58 passing). All 200 unix tests pass. pnpm check green.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rusackas rusackas force-pushed the sec-sudoers-wrapper branch from db829f1 to 20a19de Compare April 19, 2026 16:59
mistercrunch and others added 2 commits May 3, 2026 20:07
@mistercrunch @claude
sec(sudoers): collapse root wildcards behind agor-user-admin wrapper
a461c89 
Replaces five wildcard NOPASSWD rules (useradd *, userdel *, usermod *,
chpasswd, groupadd *, groupdel *, gpasswd *, find *) with a single
audited wrapper at /usr/local/sbin/agor-user-admin and one sudoers line:

  agor ALL=(root) NOPASSWD: /usr/local/sbin/agor-user-admin

The wrapper exposes a fixed verb set (add-user, delete-user, lock-user,
unlock-user, add-group, delete-group, add-to-group, remove-from-group,
set-password, setgid-tree, list-symlinks, prune-all-symlinks,
prune-broken-symlinks) and re-validates every argument:

- usernames/groupnames: ^[a-z_][a-z0-9_-]{0,31}$ + system-account deny-list
- paths: readlink -f canonicalization + allowlist (/home/*/agor/*,
  /home/*/.agor/*, /var/agor/*) — symlink escapes resolve and fail
- passwords (chpasswd stdin): no NUL/CR/LF/":" injection, printable
  ASCII only, ≤256 bytes
- end-of-options (--) on every shell-out to defeat flag-smuggling

Every successful invocation is audited to syslog (logger -t
agor-user-admin). Node-side validators in user-manager.ts remain as
defense-in-depth.

Node callers (user-manager, group-manager, symlink-manager,
unix-integration-service, executor/commands/unix, ensure-user CLI) and
the postgres entrypoint route through the wrapper. Dockerfile installs
it root-owned 0755. Tests updated for new command shapes; bash test
harness exercises validators against adversarial inputs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mistercrunch @claude
sec(sudoers): address Codex review — restore homeBase, dedupe shq(), …
e279a24 
…tighten validators

Addresses findings from Codex review of PR #1040:

1. BLOCKER: home-base regression
   - Add optional `--home <dir>` flag to wrapper's add-user verb, with a
     shape-only validator (`validate_home_dir`) that requires exact
     /home/<user> or /var/agor/<user> form (no traversal, no control
     chars, ≤256 bytes). readlink -f is skipped here because the dir
     does not exist at useradd time.
   - Thread `homeBase` through `UnixUserCommands.createUser(username,
     homeDir?)` and both callers (unix-integration-service,
     ensure-user CLI) so the existing `homeBase` config (defaults to
     /home but can be overridden to /var/agor) is preserved.

2. shq() duplication → shared escapeShellArg
   - Extract AGOR_USER_ADMIN constant to new
     packages/core/src/unix/wrapper-constants.ts so every module that
     shells out to the wrapper imports from one place.
   - Drop the local shq() copies in user-manager.ts, group-manager.ts,
     and symlink-manager.ts; import canonical escapeShellArg from
     run-as-user.ts.

3. Node-side password validator weaker than wrapper
   - Strengthen assertChpasswdInputSafe to reject ":" (chpasswd field
     separator), >256-byte inputs, and non-printable bytes — matching
     the wrapper's assert_safe_password so defense-in-depth holds at
     both layers.

4. Wrapper test harness misses adversarial classes
   - Add Unicode/homoglyph coverage (Cyrillic 'а', full-width 'ａ',
     zero-width joiner, RTL-override, BOM, combining marks, NBSP) for
     usernames, groupnames, and --home paths.
   - Add argv control-char coverage (newline, CR, tab, leading/
     trailing newline) for usernames, groupnames, --home paths, and
     filesystem-verb paths.
   - Add NUL-bearing argv case as an observable-outcome test.

Tests: user-manager unit tests extended for new createUser signature
(58 passing). All 200 unix tests pass. pnpm check green.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mistercrunch mistercrunch force-pushed the sec-sudoers-wrapper branch from 20a19de to e279a24 Compare May 3, 2026 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mistercrunch