fix: block disabling autologin when password is unusable#19
fix: block disabling autologin when password is unusable#19assisted-by-ai wants to merge 4 commits into
Conversation
Instead of merely warning after the fact, the script now checks password usability BEFORE disabling autologin: - Locked/disabled password (CLI mode): refuses with an error and tells the user to run 'pwchange' first. - Locked/disabled password (interactive mode): launches an inline password-change prompt; only proceeds to disable autologin once a valid password is confirmed. - Empty password: still warns (non-blocking) since login is technically possible, just insecure. This prevents users from being locked out of their accounts after disabling autologin, which was the root cause of the sysmaint login failure reported by users. https://claude.ai/code/session_01NAtpA8bBuvZfeu1PeytGRs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ee89a35b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| check_password_usability "${user}" || true | ||
| pass_status="$?" |
There was a problem hiding this comment.
block_on_unusable_password tries to inspect check_password_usability via check_password_usability "${user}" || true and then reads $?, but that construct always leaves $? as 0 (from true). As a result, the pass_status == 1 and pass_status == 2 branches never run, so locked/disabled passwords are not blocked and empty-password warnings are skipped, defeating the commit's core safety behavior in both CLI and interactive disable flows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 0255836. Changed to pass_status=0; check_password_usability "${user}" || pass_status="$?" which correctly captures the non-zero exit codes.
Generated by Claude Code
The || true idiom always sets $? to 0, so pass_status was never 1 or 2 and the locked/disabled password check was dead code. Use pass_status=0 with || pass_status="$?" to capture the real exit code. https://claude.ai/code/session_01NAtpA8bBuvZfeu1PeytGRs
Drop check_password_usability, block_on_unusable_password, and pwchange_for_user. Instead, warn_on_unusable_password returns 1 when the password is locked/disabled, and callers refuse to proceed, telling the user to run pwchange themselves. Same protection, much less code, no new features. https://claude.ai/code/session_01NAtpA8bBuvZfeu1PeytGRs
Move the "please run pwchange" message into the function itself so callers just check the return code. No duplicated message at each call site. https://claude.ai/code/session_01NAtpA8bBuvZfeu1PeytGRs
2 participants
Summary
warn_on_unusable_passwordnow returns 1 when the account password is locked or disabledpwchangefirst -- autologin stays enabled so they are not locked outBuilds on the warning-only fix (commit ae4eb79). That version warned after the fact but still disabled autologin, leaving the user locked out. This version refuses to disable autologin until the password is usable.
Test plan
autologinchange -d sysmaintwith a locked sysmaint password -- should refuse with errorautologinchangeinteractively, disable sysmaint autologin with locked password -- should refuseautologinchangeinteractively, disable sysmaint autologin with a valid password set -- should proceed normallyautologinchange -d userwith locked password -- should refuseautologinchangeinteractively, disable autologin for a regular user with locked password -- should refusehttps://claude.ai/code/session_01NAtpA8bBuvZfeu1PeytGRs