Skip to content

feat: enhance macOS 14+ compatibility with sudo_local and add pam_reattach support - #21

Merged
artginzburg merged 4 commits into
artginzburg:mainfrom
ben-ranford:feature/issue15-macos14-pamlocalsupport
Sep 16, 2025
Merged

feat: enhance macOS 14+ compatibility with sudo_local and add pam_reattach support#21
artginzburg merged 4 commits into
artginzburg:mainfrom
ben-ranford:feature/issue15-macos14-pamlocalsupport

Conversation

@ben-ranford

Copy link
Copy Markdown
Contributor

Overview

This PR enhances macOS 14+ support and pam_reattach support while maintaining backward compatibility with previous versions.

Changes

  • Use /etc/pam.d/sudo_local for macOS 14+ - Follows the modern recommended modular approach rather than modifying system files.
  • Upgrades existing installations if a compatible macOS version is present at next run whether immediate or on system restart.
  • tmux/screen support - Includes pam_reattach integration via --with-reattach flag.
  • Added --migrate flag to perform a manual migration.
  • Refactored into modular functions and kept legacy/modern approaches separate.
  • ver bump to 0.5
  • /nit changed exit code behaviour when options are invalid to 1 from 0

Caveats

  • We tradeoff some potential compatibility with other unix-like systems in exchange for performing version checks and using macOS specific pathing/tools.
  • If tmp is full, the installer will fail with a clear error.
  • Migration is automatic if a compatible macOS version is present at next run whether immediate or on system restart...
  • /nit Removed configurable backup extension, as this wasn't used anywhere and was a hardcoded global var.

Compatibility

  • Full backward compatibility with v0.4 in terms of script behaviour.
  • macOS 13 and below: unchanged script behavior.
  • macOS 14+: secure sudo_local approach which survives system updates.
    • could update the documentation to this effect as technically the brew service step during install should no longer be required on macOS 14 and above?
    • could add removal of the service as a migration step if desired.

Fixes #15.

Copilot AI review requested due to automatic review settings August 24, 2025 10:02

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

This PR enhances macOS compatibility by implementing a dual approach for TouchID sudo configuration: using the modern /etc/pam.d/sudo_local file for macOS 14+ and maintaining backward compatibility with the legacy /etc/pam.d/sudo modification for older versions. The changes include automatic migration from legacy configurations, pam_reattach support for GUI session reattachment, and improved error handling.

  • Implements macOS version detection to choose appropriate configuration method
  • Adds automatic migration from legacy TouchID configurations
  • Introduces pam_reattach support via --with-reattach flag for tmux/screen compatibility

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
sudo-touchid.sh Complete refactor with modular functions, macOS version detection, and dual installation methods
README.md Updated documentation to reflect new features and installation approaches

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread sudo-touchid.sh Outdated
Comment thread sudo-touchid.sh Outdated
Comment thread sudo-touchid.sh Outdated
Comment thread sudo-touchid.sh
…pport

- Enhance macOS 14+ support using /etc/pam.d/sudo_local instead of modifying system-managed sudo file
- Add pam_reattach.so support for GUI session reattachment in tmux/screen sessions
- Implement automatic detection and migration from legacy configurations on next run provided a compatible macOS version is installed
- Add --migrate and --with-reattach options
- Update documentation with version-specific behavior and migration details
- Bump version to 0.5
@ben-ranford

Copy link
Copy Markdown
Contributor Author

keen to understand if this fits the appetite for a sonoma+ patch and migration given it includes more niceties than the sonoma/ branch. it's also a lot heavier so i understand if there's simply not the desire to bring it back in. happy to simplify or adapt otherwise if there are bits both unwanted and wanted (:

@DamienCassou

Copy link
Copy Markdown

It's sad that this PR doesn't get any interest.

@ben-ranford

Copy link
Copy Markdown
Contributor Author

It's sad that this PR doesn't get any interest.

i'm honestly tempted to start a fork. i'm not sure this project is maintained anymore ):

@artginzburg

Copy link
Copy Markdown
Owner

I am sorry, this one was a busy month. I'll merge this today.
Thanks a lot for the PR, I fairly have no idea how you held yourself from starting a fork :)

Afterwards, I think maybe the script should fail (stop execution) when running --with-reattach but missing the pam_reattach.so. Or it could run the brew install automatically.

Adding --with-watchid should now also be straightforward.

@ben-ranford

Copy link
Copy Markdown
Contributor Author

no worries @artginzburg - i've pushed a commit for the reattach item. happy to do the with-watchid one too if desired? let me know 😄

@artginzburg

Copy link
Copy Markdown
Owner

I think we should skip pam_watchid for now:

We could include /usr/local/lib/pam/pam_watchid.so.2 as the search path, and tell the user to refer to https://github.com/Logicer16/pam-watchid if not installed. This fork seems to be the most active now, but they're not on Homebrew. Also, I don't like including the version inside the search path.

Moreover, they have a custom install script, which makes sudo-touchid useless for installing pam_watchid. Apparently that script doesn't care about uninstallation, which could become the only reason for including pam_watchid with sudo-touchid.

@artginzburg
artginzburg merged commit aaa05c5 into artginzburg:main Sep 16, 2025
1 check passed
@artginzburg artginzburg mentioned this pull request Sep 17, 2025
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.

[feature req] use pam_service of sudoers to keep touchid after system upgrading

4 participants