Skip to content

Harden the departure and purge flow after the expiry rework #26

Closed

Description

@jonasehrlich

Follow-up to #24, consolidating the remaining safety and reliability work previously tracked across #26 and #27.

Items the departure rework (#25) already landed have been removed. Every independent disablement step is attempted and its error returned, disabled_at is set only on complete success, and the machine-side shadow-expiry check gates permanent deletion alongside the store dates.

Report scheduled-job removal failures instead of swallowing them

Running every independent disablement step and marking disabled_at only on complete success landed with the departure rework. What remains is remove_scheduled_jobs, which still cannot tell a user without a crontab from a crontab it failed to read, and which fails the whole disablement when only the optional atrm is missing.

  • Distinguish “no crontab” from permission, spool, and other crontab -l failures. Do not silently leave scheduled jobs behind.
  • Treat a missing atrm consistently with the optional atq command rather than failing the whole disablement solely because atrm is unavailable.

Reset authorized keys on expiry

With archiving gone, a departed member's authorized_keys remains on disk for the whole retention period. The lockout rests on shadow expiry being honored by whatever starts a session. sshd honors it both under PAM and through its own shadow check, so this is defense in depth, but the keys are the one piece of access material the departure leaves in place.

  • Clear the octosync-managed block during DisableAccount. A later rejoin sync restores it, keeping the departure reversible.
  • Decide what happens to keys outside the managed block. Removing them makes departure irreversible for keys a rejoin cannot restore. Leaving them makes the reset partial.

Resolve account identity before updating authorized_keys

Other mutating handlers cross-check the stored name and UID through resolve_account. UpdateAuthorizedKeys previously looked up the account only by stored UID, so a reassigned UID could cause octosync to write keys into an unrelated account's home.

  • Route key updates through the same name/UID identity resolution.
  • Refuse the update on an identity mismatch.

Pin the authorized_keys parent directory

O_NOFOLLOW protects the final file, but the user-controlled .ssh path can still be replaced between path-based checks and opens or renames. The home and .ssh directories are now pinned before file operations begin.

  • Open and validate the home and .ssh directories once, then perform file operations relative to pinned directory file descriptors.
  • Keep the atomic replacement relative to the pinned directory.
  • Add race-oriented regression tests for replacing or removing .ssh after it is pinned.

Make purge crash-safe

purge_disabled_accounts previously updated each user in memory but saved once after the loop. A crash or SIGTERM mid-batch could leave an account already deleted by userdel while its departure record remained in departed. The next run would resolve the account as Missing, report PurgeOutcome::NoAccount, and retain the departure record, warning on every sync.

Refusing to remove a departure record when an operator deleted the account is correct. Retaining one after octosync's own interrupted purge is not.

  • Persist a deletion marker before userdel so the next run can distinguish interrupted octosync work from an independent account deletion.

Run membership safety guards before mutation and purge

Octosync::sync previously created groups and processed users before would_disable_all_users aborted a mass departure. The guard prevented account disablement but not the preceding partial mutations. Separately, the standalone purge command lacked an equivalent empty-member-list guard, so a mis-scoped installation returning [] could incorrectly satisfy the membership half of purge eligibility.

  • Build the fetched member-ID set immediately after loading the organization and store.
  • If no stored user remains in the fetched membership, abort before creating groups or processing users, without losing the distinction partition_stale_users already makes between an absent member and a current member whose per-user processing failed.
  • Apply the empty-member-list guard to the standalone purge command and to the purge pass in sync, including the case where there are departure records but no active stored users.

Do not tear down a rejoining member whose sync failed

prune_rejoined only removes a departure record when the member lands in new_store.users. A rejoining member whose CreateUser succeeds and restores the account, but whose sync_supplementary_groups then fails, is left out of the new store. Their departure record survives, and the disablement step at the end of the same sync could disable the restored account, kill their processes, and remove their groups because of a transient group failure.

  • Skip unfinished departures whose member is present in org_member_map, mirroring what partition_stale_users already does for active users.

Do not write migration artifacts while loading the store

Loading a v1 users.json immediately copies it to users-v1.json, including during --dry-run. A read or preview should not mutate the data directory.

  • Defer the backup or migration write until the first real save, or adopt users-v2.json for the new schema and leave users.json untouched.
  • Ensure dry-run loading creates no files.
  • Keep rollback behavior explicit and covered by tests.

Read the shadow expiry reentrantly

departed_at records when the member left and disabled_at records when account disablement completed. sp_expire records the account's current state on the machine. Requiring both gates permanent deletion. shadow_expiry_days now reads the shadow entry through the reentrant getspnam_r call.

  • Replace the non-reentrant getspnam used by the shadow-expiry check with getspnam_r.

Activity

  1. jonasehrlich commented on Aug 16, 2026

    @jonasehrlich
    OwnerAuthor

    Updated the checklist to match the issue branch:

    • 57b309c: authorized-key updates validate the stored name and UID
    • ab0bbd4: membership guards run before mutation and purge, and current members are not torn down after an incomplete sync
    • c6cc93d: interrupted purges resume from a persisted in-progress marker
    • 43ed64c: expiry removes the complete authorized_keys file
    • b92dca2: authorized-key operations use pinned home/.ssh descriptors with race regression tests

    The scheduled-job handling and reentrant shadow lookup remain unchecked because their current fixes are not committed yet.

  2. jonasehrlich commented on Aug 16, 2026

    @jonasehrlich
    OwnerAuthor

    The shadow lookup now uses getspnam_r with caller-owned entry and string storage. It retries ERANGE with a larger buffer and propagates other lookup errors. The Linux-only implementation passes an x86_64 Linux cross-build.

    The final shadow-lookup checkbox is now complete. The two scheduled-job items remain open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions