Skip to content

feat: add a reason field before PAM account access #189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
carlosmonastyrski wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add a reason field before PAM account access #189

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d72335b
Add a reason field before PAM account access
carlosmonastyrski Apr 20, 2026
5e5cab9
Address Claude-bot comments
carlosmonastyrski Apr 20, 2026
700752c
Address PR comments
carlosmonastyrski Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/api/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,7 @@ type PAMAccessRequest struct {
AccountName string `json:"accountName,omitempty"`
ProjectId string `json:"projectId,omitempty"`
Comment thread
carlosmonastyrski marked this conversation as resolved.
MfaSessionId string `json:"mfaSessionId,omitempty"`
Comment thread
carlosmonastyrski marked this conversation as resolved.
Reason string `json:"reason,omitempty"`
}
Comment thread
carlosmonastyrski marked this conversation as resolved.

type PAMAccessResponse struct {
Expand Down
33 changes: 33 additions & 0 deletions packages/cmd/pam.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,31 @@
package cmd

import (
"os"
"time"

pam "github.com/Infisical/infisical-merge/packages/pam/local"
"github.com/Infisical/infisical-merge/packages/util"
"github.com/mattn/go-isatty"
"github.com/rs/zerolog/log"
"github.com/spf13/cobra"
)

func resolveReason(cmd *cobra.Command) string {
if cmd.Flags().Changed("reason") {
reason, _ := cmd.Flags().GetString("reason")
return reason
}
if !isatty.IsTerminal(os.Stdin.Fd()) {
return ""
}
reason, err := pam.PromptForReason(false)
if err != nil {
return ""
}
return reason

Check failure on line 26 in packages/cmd/pam.go

View check run for this annotation

Claude / Claude Code Review

Double reason prompt when account policy requires reason 

When a user runs any PAM command interactively without `--reason`, `resolveReason()` eagerly prompts for an optional reason upfront; if the user presses Enter to skip, an empty reason is sent to the API. If the account policy mandates a reason, the server returns `PAM_REASON_REQUIRED` and `CallPAMAccessWithMFA` then prompts a second time with a mandatory "Reason for access" prompt — the user is asked the same question twice with no explanation for why the first answer was insufficient.
Comment thread
carlosmonastyrski marked this conversation as resolved.
}

var pamCmd = &cobra.Command{
Use: "pam",
Short: "PAM-related commands",
Expand Down Expand Up @@ -72,6 +89,8 @@
util.HandleError(err, "Unable to parse port flag")
}

reason := resolveReason(cmd)

log.Debug().Msg("PAM Database Access: Trying to fetch secrets using logged in details")

loggedInUserDetails, err := util.GetCurrentLoggedInUserDetails(true)
Expand All @@ -92,6 +111,7 @@
pam.StartDatabaseLocalProxy(loggedInUserDetails.UserCredentials.JTWToken, pam.PAMAccessParams{
ResourceName: resourceName,
AccountName: accountName,
Reason: reason,
}, projectID, durationStr, port)
},
}
Expand Down Expand Up @@ -194,6 +214,8 @@
projectID = workspaceFile.WorkspaceId
}

reason := resolveReason(cmd)

log.Debug().Msg("PAM SSH: Trying to fetch credentials using logged in details")

loggedInUserDetails, err := util.GetCurrentLoggedInUserDetails(true)
Expand All @@ -214,6 +236,7 @@
pam.StartSSHLocalProxy(loggedInUserDetails.UserCredentials.JTWToken, pam.PAMAccessParams{
ResourceName: resourceName,
AccountName: accountName,
Reason: reason,
}, projectID, durationStr, options)
}

Expand Down Expand Up @@ -273,6 +296,8 @@
projectID = workspaceFile.WorkspaceId
}

reason := resolveReason(cmd)

log.Debug().Msg("PAM Kubernetes Access: Trying to fetch credentials using logged in details")

loggedInUserDetails, err := util.GetCurrentLoggedInUserDetails(true)
Expand All @@ -293,6 +318,7 @@
pam.StartKubernetesLocalProxy(loggedInUserDetails.UserCredentials.JTWToken, pam.PAMAccessParams{
ResourceName: resourceName,
AccountName: accountName,
Reason: reason,
}, projectID, durationStr, port)
},
}
Expand Down Expand Up @@ -352,6 +378,8 @@
util.HandleError(err, "Unable to parse port flag")
}

reason := resolveReason(cmd)

log.Debug().Msg("PAM Redis Access: Trying to fetch secrets using logged in details")

loggedInUserDetails, err := util.GetCurrentLoggedInUserDetails(true)
Expand All @@ -372,6 +400,7 @@
pam.StartRedisLocalProxy(loggedInUserDetails.UserCredentials.JTWToken, pam.PAMAccessParams{
ResourceName: resourceName,
AccountName: accountName,
Reason: reason,
}, projectID, durationStr, port)
},
}
Expand All @@ -384,6 +413,7 @@
pamDbAccessCmd.Flags().String("duration", "1h", "Duration for database access session (e.g., '1h', '30m', '2h30m')")
pamDbAccessCmd.Flags().Int("port", 0, "Port for the local database proxy server (0 for auto-assign)")
pamDbAccessCmd.Flags().String("project-id", "", "Project ID of the account to access")
pamDbAccessCmd.Flags().String("reason", "", "Reason for accessing the account (stored on the session for audit purposes)")
pamDbAccessCmd.MarkFlagRequired("resource")
pamDbAccessCmd.MarkFlagRequired("account")

Expand All @@ -393,6 +423,7 @@
cmd.Flags().String("account", "", "Name of the account within the resource")
cmd.Flags().String("duration", "1h", "Duration for SSH access session (e.g., '1h', '30m', '2h30m')")
cmd.Flags().String("project-id", "", "Project ID of the account to access")
cmd.Flags().String("reason", "", "Reason for accessing the account (stored on the session for audit purposes)")
cmd.MarkFlagRequired("resource")
cmd.MarkFlagRequired("account")
}
Expand All @@ -413,6 +444,7 @@
pamKubernetesAccessCmd.Flags().String("duration", "1h", "Duration for kubernetes access session (e.g., '1h', '30m', '2h30m')")
pamKubernetesAccessCmd.Flags().Int("port", 0, "Port for the local kubernetes proxy server (0 for auto-assign)")
pamKubernetesAccessCmd.Flags().String("project-id", "", "Project ID of the account to access")
pamKubernetesAccessCmd.Flags().String("reason", "", "Reason for accessing the account (stored on the session for audit purposes)")
pamKubernetesAccessCmd.MarkFlagRequired("resource")
pamKubernetesAccessCmd.MarkFlagRequired("account")

Expand All @@ -423,6 +455,7 @@
pamRedisAccessCmd.Flags().String("duration", "1h", "Duration for Redis access session (e.g., '1h', '30m', '2h30m')")
pamRedisAccessCmd.Flags().Int("port", 0, "Port for the local Redis proxy server (0 for auto-assign)")
pamRedisAccessCmd.Flags().String("project-id", "", "Project ID of the account to access")
pamRedisAccessCmd.Flags().String("reason", "", "Reason for accessing the account (stored on the session for audit purposes)")
Comment thread
carlosmonastyrski marked this conversation as resolved.
Outdated
pamRedisAccessCmd.MarkFlagRequired("resource")
pamRedisAccessCmd.MarkFlagRequired("account")

Expand Down
47 changes: 45 additions & 2 deletions packages/pam/local/base-proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"fmt"
"io"
"net"
"os"
"slices"
"strconv"
"strings"
Expand All @@ -21,12 +22,14 @@
"github.com/Infisical/infisical-merge/packages/util"
"github.com/go-resty/resty/v2"
"github.com/manifoldco/promptui"
"github.com/mattn/go-isatty"
"github.com/rs/zerolog/log"
)

type PAMAccessParams struct {
ResourceName string
AccountName string
Reason string
}

// GetDisplayName returns a user-friendly display name for the access params
Expand All @@ -38,12 +41,13 @@
func (p PAMAccessParams) ToAPIRequest(projectID, duration string) api.PAMAccessRequest {
return api.PAMAccessRequest{
Duration: duration,
ResourceName: p.ResourceName,
AccountName: p.AccountName,
ProjectId: projectID,
Reason: p.Reason,
}
}

Check failure on line 50 in packages/pam/local/base-proxy.go

View check run for this annotation

Claude / Claude Code Review

Reason not included in approval request payload 

The `Reason` field is not propagated to the approval request payload. When an account is protected by an approval policy, `HandleApprovalWorkflow` calls `accessParams.ToApprovalRequestData(durationStr)`, which omits the user's stated reason entirely — approvers and audit logs for the approval workflow will never see why access was requested, defeating a core purpose of the new feature.
Comment thread
carlosmonastyrski marked this conversation as resolved.
// ToApprovalRequestData converts PAMAccessParams to api.PAMAccessApprovalRequestPayloadRequestData
func (p PAMAccessParams) ToApprovalRequestData(duration string) api.PAMAccessApprovalRequestPayloadRequestData {
return api.PAMAccessApprovalRequestPayloadRequestData{
Expand Down Expand Up @@ -313,17 +317,56 @@
}
}

const reasonRequiredErrorName = "PAM_REASON_REQUIRED"

func PromptForReason(required bool) (string, error) {
label := "Reason for access"
prompt := promptui.Prompt{
Label: label,
Validate: func(input string) error {
if required && strings.TrimSpace(input) == "" {
return fmt.Errorf("a reason is required")
}
return nil
},
}
result, err := prompt.Run()
if err != nil {
return "", err
}
return strings.TrimSpace(result), nil
}

// CallPAMAccessWithMFA attempts to access a PAM account and handles MFA if required
// This is a shared function used by all PAM proxies
func CallPAMAccessWithMFA(httpClient *resty.Client, pamRequest api.PAMAccessRequest) (api.PAMAccessResponse, error) {
// Initial request
pamResponse, err := api.CallPAMAccess(httpClient, pamRequest)
if err != nil {
// Check if MFA is required
if apiErr, ok := err.(*api.APIError); ok {
// Reason required by account policy
if apiErr.Name == reasonRequiredErrorName {
if !isatty.IsTerminal(os.Stdin.Fd()) {
return api.PAMAccessResponse{}, fmt.Errorf(
"a reason is required to access this account — pass one with --reason")
}
log.Info().Msg("A reason is required to access this account.")
reason, promptErr := PromptForReason(true)
if promptErr != nil {
return api.PAMAccessResponse{}, fmt.Errorf("reason prompt cancelled: %w", promptErr)
}
pamRequest.Reason = reason
pamResponse, err = api.CallPAMAccess(httpClient, pamRequest)
if err != nil {
return api.PAMAccessResponse{}, err
Comment thread
carlosmonastyrski marked this conversation as resolved.
Outdated
}
return pamResponse, nil
}

// MFA required
if apiErr.Name == "SESSION_MFA_REQUIRED" {
// Extract MFA details from error
if details, ok := apiErr.Details.(map[string]interface{}); ok {

Check failure on line 369 in packages/pam/local/base-proxy.go

View check run for this annotation

Claude / Claude Code Review

MFA flow bypassed after reason-required retry 

The MFA flow is bypassed when an account requires both a reason and MFA. In `CallPAMAccessWithMFA`, after the user provides a reason (PAM_REASON_REQUIRED branch), the retry calls `api.CallPAMAccess` directly and returns any error raw — so if the server then responds with SESSION_MFA_REQUIRED, the CLI returns that error to the user instead of completing the MFA flow. Any account that enforces both a reason policy and MFA will be inaccessible via the interactive prompt flow; the fix is to call `Ca
Comment thread
carlosmonastyrski marked this conversation as resolved.
mfaSessionId, _ := details["mfaSessionId"].(string)
mfaMethod, _ := details["mfaMethod"].(string)

Expand All @@ -347,7 +390,7 @@
}
}
}
// Return original error if not MFA-related
// Return original error if not MFA/reason-related
return api.PAMAccessResponse{}, err
}

Expand Down
Loading