Skip to content

feat: revamp PAM product#296

Open
bernie-g wants to merge 22 commits into
mainfrom
pam-revamp
Open

feat: revamp PAM product#296
bernie-g wants to merge 22 commits into
mainfrom
pam-revamp

Conversation

@bernie-g

@bernie-g bernie-g commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

CLI counterpart of the PAM product revamp in Infisical/infisical#7145.

sheensantoscapadngan and others added 22 commits June 17, 2026 04:37
- Wire Windows/RDP case in StartPAMAccess to use RDPProxyServer
- Use gateway disconnect detection so CLI exits on server-side session termination
Wire SSH account type into the new path-based `pam access` flow.
Starts a local TCP proxy and prints connection info with hint
commands, matching the database proxy pattern.
Match the database banner's `if username != ""` check for consistency.
* feat(pam): add Kubernetes CLI access

Wire KubernetesProxyServer into the new path-based pam access command,
extract kubeconfig setup into a reusable SetupKubeconfig method, and
add a Kubernetes-specific session banner.

* fix(pam): remove misleading password injection copy from CLI banner

* chore(pam): delete unused StartKubernetesLocalProxy

---------

Co-authored-by: saif <[email protected]>
…276)

Switch access.go to use CallPAMAccessWithMFA instead of direct
CallPAMAccess. This adds interactive reason prompting (catches
PAM_REASON_REQUIRED, prompts with PromptForReason) and MFA handling
(catches SESSION_MFA_REQUIRED, opens browser, polls) to the new
path-based access flow for all account types.

Co-authored-by: saif <[email protected]>
The database, redis, and kubernetes local PAM proxies created their TCP
listener with an empty host (":0" / ":<port>"), which Go resolves to all
interfaces. These proxies are only meant to be used by the client on the
same machine, so they should not be reachable from elsewhere.

Bind the listeners to 127.0.0.1 so they accept local connections only,
and advertise 127.0.0.1 in the printed connection strings and kubeconfig
so the client connects to exactly the address the proxy listens on. This
matches the ssh and rdp proxies, which already bind loopback.
…k-bind

fix(pam): bind local proxies to loopback
feat(pam): add RDP CLI access and recording chunk batching
feat(pam): add --target flag to select a host on pam access
* feat(pam): add AWS IAM CLI access

Write temporary STS credentials to ~/.aws/credentials under a named
profile (infisical-pam/<folder>/<account>). Credentials are cleaned up
on Ctrl+C or session expiry. Follows the same pattern as Kubernetes
kubeconfig management.

* fix(pam): handle stale AWS credentials profile from previous crash

Use cfg.Section() instead of cfg.NewSection() so that a leftover
profile from a killed session is silently overwritten instead of
causing a fatal error. Also always chmod credentials file to 0600
after write (not just when creating it), and guard against negative
remaining duration from clock skew.

* fix(pam): check credential expiry before writing to disk

Move the expiry guard before the file write so expired credentials
are never written to ~/.aws/credentials.

* fix(pam): end backend session on CLI cleanup

Call the session termination API before removing the local AWS
credentials profile, so the session is marked as ended on the backend.

* fix(pam): use user-facing terminate endpoint for AWS IAM session cleanup

The /end endpoint requires gateway auth. Use /terminate (JWT auth)
instead since the CLI authenticates as a user, not a gateway.

* revert(pam): remove session end call from CLI cleanup

The /end endpoint requires gateway auth and the /terminate endpoint
marks the session as "Terminated" (admin action) instead of "Ended"
(normal close). Neither is appropriate for a user-initiated Ctrl+C.

---------

Co-authored-by: saif <[email protected]>
@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-296-feat-revamp-pam-product

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

@gitguardian

gitguardian Bot commented Jul 3, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
34068408 Triggered Generic Database Assignment 4c35f70 packages/pam/local/access.go View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR revamps local PAM access around a single path-based command. The main changes are:

  • Adds pam access <path> as the unified CLI entrypoint.
  • Adds path, account type, relay host, and target host fields to the PAM API models.
  • Routes new PAM sessions by returned account type.
  • Adds AWS IAM credential-file support.
  • Moves proxy startup into shared account-specific handlers and binds several proxies to loopback.

Confidence Score: 4/5

The changed PAM access flow and AWS credential lifecycle need fixes before merging.

  • Approval-protected PAM accounts can exit without offering the approval request flow.
  • Overlapping AWS IAM sessions for the same PAM path can overwrite and remove each other's credentials.
  • The loopback proxy changes and model additions look contained.

packages/pam/local/access.go, packages/pam/local/aws-iam-access.go

Security Review

The AWS IAM credential writer uses a path-only profile name. Overlapping sessions for the same account can overwrite and remove each other's AWS credentials.

Important Files Changed

Filename Overview
packages/api/model.go Adds new path-based PAM request and response fields while keeping legacy fields.
packages/cmd/pam.go Replaces resource-specific PAM subcommands with a unified path-based access command.
packages/pam/local/access.go Adds centralized PAM access routing, but the new error path skips approval-request handling.
packages/pam/local/aws-iam-access.go Adds AWS IAM credential writing and cleanup, with a profile collision issue for overlapping sessions.
packages/pam/local/base-proxy.go Updates shared disconnect handling and keeps MFA/reason handling in the common access helper.
packages/pam/local/database-proxy.go Removes old database access orchestration and binds the database proxy to loopback.
packages/pam/local/kubernetes-proxy.go Moves kubeconfig setup into a reusable method and binds the Kubernetes proxy to loopback.
packages/pam/local/rdp-proxy.go Switches RDP connection shutdown handling to shared disconnect channels.
packages/pam/local/redis-proxy.go Updates Redis connection output and listener binding to loopback.
packages/pam/local/ssh-proxy.go Removes legacy SSH client launch logic and leaves proxy serving for the new access flow.

Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment on lines +71 to +74
TargetHost: targetHost,
}, true)
if err != nil {
util.HandleError(err, "Failed to create PAM session")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Approval Flow Is Skipped

When the access API returns the existing approval-policy error, the new unified command sends it straight to util.HandleError instead of starting the approval request flow. Approval-protected PAM accounts that worked through the old entrypoints now exit without giving the user a way to request access.

}

folder, account := parsePath(path)
profileName := fmt.Sprintf("infisical-pam/%s/%s", folder, account)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security AWS Profile Name Collides

This profile name is based only on the PAM path, so two overlapping sessions for the same AWS account write the same [infisical-pam/<folder>/<account>] section. The later session overwrites the earlier keys, and when either session exits removeAWSProfile deletes the shared section, breaking the other active AWS session.

section.Key("aws_secret_access_key").SetValue(secretAccessKey)
section.Key("aws_session_token").SetValue(sessionToken)

if err := cfg.SaveTo(credFilePath); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: AWS credentials can be exposed while being written

On a multi-user host, another local user can read the newly issued STS credentials if this creates ~/.aws/credentials under a normal 022 umask, or if the existing credentials file is group/world-readable. SaveTo writes the secret values before the later Chmod; create/open the credentials file with 0600 before writing, or write to a 0600 temp file and rename it into place.

@veria-ai

veria-ai Bot commented Jul 3, 2026

Copy link
Copy Markdown

PR overview

This pull request revamps the PAM product and includes logic for local AWS IAM access that writes newly issued STS credentials into the user’s AWS credentials file.

There is one open security issue involving how AWS credentials are written to disk. On multi-user systems, the credentials file may be readable by other local users before permissions are tightened, which could expose newly issued STS secrets. No issues have been addressed yet, so the PR still needs a fix to create or replace the credentials file with restrictive permissions before writing secrets.

Open issues (1)

Fixed/addressed: 0 · PR risk: 7/10

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.

5 participants