Skip to content

feat: add cli support postgres pam#266

Open
sheensantoscapadngan wants to merge 2 commits into
pam-revampfrom
feat/add-cli-support-postgres-pam
Open

feat: add cli support postgres pam#266
sheensantoscapadngan wants to merge 2 commits into
pam-revampfrom
feat/add-cli-support-postgres-pam

Conversation

@sheensantoscapadngan

@sheensantoscapadngan sheensantoscapadngan commented Jun 16, 2026

Copy link
Copy Markdown
Member

Description 📣

image

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-266-feat-add-cli-support-postgres-pam

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

@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors the PAM CLI from separate resource/account sub-commands (pam db access, pam ssh access, etc.) to a single path-based entry point (pam access <path>), with initial support for Postgres account type routing.

  • packages/pam/local/access.go (new): implements StartPAMAccess, which calls the API with a path, inspects AccountType in the response, and dispatches to startPostgresProxy; all other account types respond with a "not yet supported" message.
  • packages/cmd/pam.go: replaces the five resource-type sub-command trees with a single pamAccessCmd taking a positional path argument.
  • packages/api/model.go: adds Path and AccountType fields to the PAM request/response structs.

Confidence Score: 2/5

The new unified access path skips the interactive MFA and reason-required handling that the old per-resource commands relied on, making PAM accounts with those policies inaccessible; additionally the relay host from the API response is dialed directly without validation.

The new StartPAMAccess function calls api.CallPAMAccess directly rather than CallPAMAccessWithMFA, which means any account protected by an MFA policy or a reason-required policy will fail with a raw API error instead of an interactive prompt — a functional regression for a significant slice of users. On top of that, response.RelayHost is used as the verbatim TCP dial target, which is a real SSRF vector given that an attacker with a valid token or control over the API response can redirect outbound connections to internal addresses on the user's machine.

packages/pam/local/access.go needs the most attention — both the missing MFA/reason flow and the relay-host SSRF concern originate there.

Security Review

  • SSRF via relayHost (packages/pam/local/access.go): response.RelayHost from the Infisical API response is used as-is as the TCP dial target for the relay connection. A compromised API token or server could supply an internal address, causing the CLI to initiate connections to private network resources from the user's machine.

Important Files Changed

Filename Overview
packages/pam/local/access.go New file implementing the unified PAM access entry point; bypasses MFA/reason-required interactive flows, SSRF via relay host from API response, and a potential listener leak on metadata failure
packages/cmd/pam.go Replaces resource/account-based sub-commands with a single path-based pam access <path> command; reason flag no longer prompts interactively (deferred to access.go flow)
packages/api/model.go Adds Path and AccountType fields to PAM request/response structs; reorders RelayHost to appear earlier in PAMAccessResponse — no functional issues
packages/pam/local/base-proxy.go Minor cleanup: moves reasonRequiredErrorName constant, updates comments to mark CallPAMAccessWithMFA as legacy, removes inline comments — no logic changes
packages/pam/local/database-proxy.go Removes the old StartDatabaseLocalProxy function and dead imports; remaining Start/Run/handleConnection logic is unchanged

Reviews (1): Last reviewed commit: "feat: add cli support postgres pam" | Re-trigger Greptile

Comment thread packages/pam/local/access.go
Comment thread packages/pam/local/access.go
Comment thread packages/pam/local/access.go Outdated
Comment thread packages/pam/local/access.go
@veria-ai

veria-ai Bot commented Jun 16, 2026

Copy link
Copy Markdown

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/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.

1 participant