fix(auth): capture PAM messages and set TTY item#100
Conversation
This commit addresses missing context and leaked state in PAM auth. 1. Captures `pam.ErrorMsg` and `pam.TextInfo` messages during conversation to provide context when `tx.Authenticate` or `tx.AcctMgmt` fail. 2. Extracts the terminal via `/proc/self/fd/0` and supplies it to PAM via `tx.SetItem(pam.Tty, ...)`. 3. Adds `defer tx.End()` to correctly finalize the transaction. Co-authored-by: arran4 <111667+arran4@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
This commit fixes a linting error reported by errcheck in `golangci-lint` by explicitly ignoring the returned error from `tx.End()`. Co-authored-by: arran4 <111667+arran4@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the PAM authenticator by capturing informational and error messages from the PAM transaction to provide more detailed error feedback. It also introduces TTY detection and ensures proper cleanup of the PAM transaction. Feedback suggests improving the TTY detection logic to be less brittle, refining the message collection to avoid empty strings, and clearing the message buffer between different PAM phases to prevent cross-contamination of error messages.
This commit clears the collected `pamMsgs` after a successful `Authenticate` phase so that unrelated or informational messages do not incorrectly leak into the error generated if `tx.AcctMgmt` subsequently fails. Co-authored-by: arran4 <111667+arran4@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
Fixes an issue where PAM auth failures were returning generic error messages without logging actual PAM module rejections, and improves transaction handling.
PR created automatically by Jules for task 14443616570265861596 started by @arran4