Skip to content

feat(baseline): named sudo users (key-only NOPASSWD accounts) - #23

Merged
thiras merged 2 commits into
mainfrom
feat/baseline-named-sudo-users
Jul 11, 2026
Merged

thiras merged 2 commits into
mainfrom
feat/baseline-named-sudo-users

Conversation

@thiras

@thiras thiras commented Jul 11, 2026 •

Copy link
Copy Markdown
Contributor

What & why

Adds baseline_sudo_users to the baseline role: a list of {name, keys:[...]} entries, each becoming a distinct login account (own username, home, key) with sudo. Today the role supports exactly one admin account plus extra authorized keys on that same account (ssh_admin_extra_pubkeys); this grants teammates their own accounts instead of sharing keys on the admin.

Each account is created key-only, exactly like the admin: member of sudo, a NOPASSWD sudoers.d drop-in, and a locked password (login by key only).

Implementation

  • New roles/baseline/tasks/sudo_users.yml, wired via include_tasks from main.yml after the admin keys and before the firewall/DevSec hardening (so a listed operator can log in the moment hardening lands). Kept in its own file so molecule can exercise this container-safe block on its own — the rest of baseline is real-host-only.
  • Mirrors the admin path's hard-won reasoning:
    • sudoers.d #includedir filename hazard (./~ silently skipped → silent lockout) → same regex_replace('[^A-Za-z0-9_-]', '_') sanitize; the rule inside still names the real user.
    • visudo -cf content validation.
    • authorized_key check-mode hard-fail → guarded with when: not ansible_check_mode (these accounts aren't lockout-critical — the play always connects as the admin — so no getent-probe needed).
  • Fail-loud precondition assert (hardening from review) refuses two silent-lockout input shapes before any account is touched: an entry with no keys, and names that sanitize to a colliding drop-in filename (including vs the admin's own drop-in).

Docs & config

  • defaults/main.yml: baseline_sudo_users: [] with a comment distinguishing it from ssh_admin_extra_pubkeys.
  • README.md: table row + a sentence under the admin step.
  • inventory/group_vars/all.yml: commented example (nothing enabled).

Test coverage (molecule, runs in CI)

Two named users driven through converge → idempotence → verify:

  • molecule-operator (simple name, one key) and alice.smith (a . in the name — verifies the drop-in lands at the sanitized /etc/sudoers.d/alice_smith; two keys exercise the subelements fan-out).
  • Asserts: accounts exist, are in sudo, drop-ins are 0440 containing NOPASSWD:ALL, keys installed, and passwords locked. alice.smith is seeded with a * disabled-password sentinel so password_lock must convert it to !* — a non-vacuous lock assertion.

Validation

  • make lint-ansible → pass (production profile)
  • make molecule → pass, idempotence successful, new assertions ran
  • make security (KICS) → HIGH: 0; no new findings from these changes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added support for configuring additional named, SSH key-only sudo accounts.
    • Each account receives separate login access, passwordless sudo privileges, and locked password authentication.
    • Added validation for account configuration, unique sudoers entries, and sudoers syntax.
  • Documentation
    • Documented the new account configuration and usage guidance.
  • Tests
    • Added coverage verifying account creation, sudo access, password locking, SSH keys, permissions, and username sanitization.

Add `baseline_sudo_users`: a list of `{name, keys:[...]}` entries, each
becoming a distinct login account (own username, home, key) created
key-only exactly like the admin — member of `sudo`, a NOPASSWD sudoers.d
drop-in, and a locked password. This grants teammates their OWN accounts
rather than sharing keys on the admin via `ssh_admin_extra_pubkeys`.

The block lives in its own `tasks/sudo_users.yml` (included after the
admin keys, before the firewall/DevSec hardening) so molecule can exercise
this container-safe subset on its own; the rest of baseline is host-only.

Mirrors the admin path's hard-won reasoning: the sudoers.d `.`/`~`
filename sanitize, `visudo -cf` validation, and a check-mode guard on the
key install. A fail-loud precondition assert refuses two silent-lockout
input shapes: an entry with no keys, and names that sanitize to a
colliding drop-in filename (including vs the admin's own).

Molecule coverage (two users incl. a dotted name + multi-key, non-vacuous
password-lock assertion) plus README, defaults, and group_vars example.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 11, 2026 20:46
@coderabbitai

coderabbitai Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@thiras, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d28e3f10-8198-442f-a7ed-312163a799dc

📥 Commits

Reviewing files that changed from the base of the PR and between 17eb94c and 854b91f.

📒 Files selected for processing (1)
  • ansible/roles/baseline/tasks/sudo_users.yml
📝 Walkthrough

Walkthrough

The baseline role adds baseline_sudo_users for distinct key-only sudo accounts, creates per-user sudoers entries and SSH keys, locks passwords, and adds Molecule configuration and assertions for two example users.

Changes

Named sudo user accounts

Layer / File(s) Summary
Configuration and role wiring
ansible/roles/baseline/defaults/main.yml, ansible/roles/baseline/tasks/main.yml, ansible/roles/baseline/README.md, ansible/inventory/group_vars/all.yml
Defines and documents baseline_sudo_users, provides commented inventory guidance, and conditionally includes the new task file.
Sudo user provisioning
ansible/roles/baseline/tasks/sudo_users.yml
Validates user definitions and sanitized filenames, creates sudo users, installs validated NOPASSWD drop-ins, locks passwords, and installs SSH authorized keys.
Molecule exercise and verification
ansible/molecule/default/converge.yml, ansible/molecule/default/verify.yml
Configures molecule-operator and alice.smith, then verifies account membership, password locks, sudoers files, modes, rules, and SSH keys.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Baseline as baseline role
  participant Users as sudo_users.yml
  participant System as Linux accounts and sudoers
  participant SSH as authorized_keys
  Baseline->>Users: include named-user tasks
  Users->>System: create sudo users
  Users->>System: install and validate NOPASSWD rules
  Users->>System: lock local passwords
  Users->>SSH: install configured public keys
  System-->>Baseline: expose account and sudoers state
  SSH-->>Baseline: expose authorized key state
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding named, key-only sudo users with NOPASSWD access to the baseline role.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/baseline-named-sudo-users

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the baseline_sudo_users feature to the baseline Ansible role, allowing the creation of distinct, key-only named sudo accounts with passwordless sudo access. The changes include task definitions, documentation updates, and Molecule test coverage. The review feedback highlights potential failures during dry-runs (--check mode) on fresh hosts. Specifically, the reviewer suggests probing existing users with getent to conditionally run the password locking and authorized keys installation tasks in check mode, which prevents execution failures while still enabling configuration drift reporting for existing users.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ansible/roles/baseline/tasks/sudo_users.yml
Comment thread ansible/roles/baseline/tasks/sudo_users.yml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
ansible/roles/baseline/tasks/sudo_users.yml (1)

27-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider validating name is non-empty in the assert.

The assert catches keyless entries and filename collisions, but an entry with a missing or empty name would pass this check and fail later at the user module step. While the failure would be loud (not silent), it slightly undermines the assert's stated design goal of catching all misconfiguration "before any account is touched." Adding a name check would make the fail message more informative and consistent with the assert's purpose.

♻️ Optional: add name validation to the assert
     that:
+      - named_users_without_name | length == 0
       - keyless_named_users | length == 0
       - sudoers_dropin_names | length == (sudoers_dropin_names | unique | length)
     fail_msg: >-
       baseline_sudo_users is misconfigured (validated before any account is created).
+      Entries lacking a non-empty `name`: {{ named_users_without_name }}.
       Entries lacking a non-empty `keys` list: {{ keyless_named_users }}.
       Sanitized /etc/sudoers.d/ filenames must be unique (including vs the admin
       drop-in), else one grant silently overwrites another -> silent sudo lockout;
       got {{ sudoers_dropin_names }}.
   vars:
+    named_users_without_name: >-
+      {{ (baseline_sudo_users | rejectattr('name', 'defined')
+          | map(attribute='name') | list)
+         + (baseline_sudo_users | selectattr('name', 'defined')
+            | selectattr('name', 'falsy') | map(attribute='name') | list) }}
     keyless_named_users: >-
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ansible/roles/baseline/tasks/sudo_users.yml` around lines 27 - 50, Update the
“Assert each named sudo user” task to reject entries whose name is missing or
empty before account creation. Add a name-validation condition and include the
affected entries in fail_msg, alongside keyless_named_users and
sudoers_dropin_names, while preserving the existing key and filename uniqueness
checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ansible/roles/baseline/tasks/sudo_users.yml`:
- Around line 27-50: Update the “Assert each named sudo user” task to reject
entries whose name is missing or empty before account creation. Add a
name-validation condition and include the affected entries in fail_msg,
alongside keyless_named_users and sudoers_dropin_names, while preserving the
existing key and filename uniqueness checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1e4e0e50-f1c0-4e65-8915-8f3d48692409

📥 Commits

Reviewing files that changed from the base of the PR and between b26ced6 and 17eb94c.

📒 Files selected for processing (7)
  • ansible/inventory/group_vars/all.yml
  • ansible/molecule/default/converge.yml
  • ansible/molecule/default/verify.yml
  • ansible/roles/baseline/README.md
  • ansible/roles/baseline/defaults/main.yml
  • ansible/roles/baseline/tasks/main.yml
  • ansible/roles/baseline/tasks/sudo_users.yml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support in the baseline Ansible role for provisioning multiple distinct, key-only sudo-enabled operator accounts (each with its own username/home/authorized_keys) instead of sharing keys on the single admin account.

Changes:

  • Introduces baseline_sudo_users and a dedicated sudo_users.yml task file to create per-operator accounts, sudoers drop-ins, password locks, and authorized keys.
  • Wires the new task block into roles/baseline/tasks/main.yml (runs after admin key setup and before firewall/hardening).
  • Extends Molecule coverage plus docs/defaults/inventory examples for the new variable.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ansible/roles/baseline/tasks/sudo_users.yml New task block to assert input, create named sudo users, write sudoers drop-ins, lock passwords, and install authorized keys.
ansible/roles/baseline/tasks/main.yml Includes the new sudo_users.yml block when baseline_sudo_users is non-empty, positioned before firewall/hardening.
ansible/roles/baseline/README.md Documents baseline_sudo_users and clarifies distinction from ssh_admin_extra_pubkeys.
ansible/roles/baseline/defaults/main.yml Adds baseline_sudo_users: [] with documentation of expected shape/behavior.
ansible/molecule/default/converge.yml Configures baseline_sudo_users test data and runs tasks_from: sudo_users for container-safe Molecule coverage.
ansible/molecule/default/verify.yml Verifies the two named users exist, are in sudo, have correct sudoers drop-ins, keys installed, and passwords locked.
ansible/inventory/group_vars/all.yml Adds a commented example showing how to configure baseline_sudo_users.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ansible/roles/baseline/tasks/sudo_users.yml Outdated
Harden the baseline_sudo_users precondition assert (PR review): also reject
a scalar-string `keys` (truthy, so it slipped past the keyless check and
would mis-iterate in subelements AFTER accounts are already changed) and a
missing/empty `name`, so all malformed input fails before any account is
touched.

Read `keys` via map('list') + the `superset` test rather than
map(attribute='keys'): `keys` collides with the dict .keys() method, so
attribute access on an entry that omits `keys` (a `key:` typo) returns the
bound method (truthy) and silently escaped the keyless check. Builtin-only —
community.general (json_query) stays intentionally absent per galaxy.yml.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thiras
thiras merged commit 8fcbb5e into main Jul 11, 2026
9 checks passed
@thiras
thiras deleted the feat/baseline-named-sudo-users branch July 11, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants