Skip to content

feat(pam): support requesting access from the CLI on the approval gate#293

Open
bernie-g wants to merge 3 commits into
pam-revampfrom
bernie/pam-286-technical-plan-access-request-implementation
Open

feat(pam): support requesting access from the CLI on the approval gate#293
bernie-g wants to merge 3 commits into
pam-revampfrom
bernie/pam-286-technical-plan-access-request-implementation

Conversation

@bernie-g

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

Copy link
Copy Markdown
Contributor

Description 📣

Restores the ability to request access from the CLI against the new PAM model. When infisical pam access <path> hits the approval gate (PAM_APPROVAL_REQUIRED) on a gated account, the CLI now offers to submit an access request for that account path in an interactive terminal (and prints clear guidance otherwise), calling the new POST /v1/pam/access-requests endpoint. Also removes the legacy approval-workflow code that was orphaned by the launch-path refactor.

Type ✨

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

Tests 🛠️

Requires the matching backend branch. Steps:

# As a user without an active grant, on a gated account:
infisical pam access <folder>/<account>
# -> prompts "Request access now?"; on yes, submits the request
# After an approver approves, re-run:
infisical pam access <folder>/<account>
# -> session launches

@linear

linear Bot commented Jul 2, 2026

Copy link
Copy Markdown

PAM-286

@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-293-feat-pam-support-requesting-access-from-the-cli-on-the-approva

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

Also removes the dead StartRDPLocalProxy entry (superseded by startRDPProxy in the PAM revamp). Redis proxy and the legacy approval workflow are left untouched.
@bernie-g bernie-g force-pushed the bernie/pam-286-technical-plan-access-request-implementation branch from 1825baa to d4de1d7 Compare July 2, 2026 16:21
…code

- send justification as reason (API strips the unknown note key)
- convert Go durations to milliseconds before sending; npm ms can't parse compound formats like 2h30m
- exit non-zero when the approval gate is hit in a non-interactive terminal
@bernie-g bernie-g marked this pull request as ready for review July 2, 2026 23:17
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR restores CLI-side access-request submission when infisical pam access hits the PAM_APPROVAL_REQUIRED gate, and removes the now-orphaned StartRDPLocalProxy entry-point from the legacy approval workflow. The new handleApprovalRequired helper detects the named API error, prompts interactively in a TTY, and calls the new POST /v1/pam/access-requests endpoint.

  • packages/pam/local/access.go: adds handleApprovalRequired which intercepts PAM_APPROVAL_REQUIRED, offers a TTY confirm prompt, and submits an access request via the new API function.
  • packages/api/api.go + model.go: adds CallPAMCreateAccessRequest and its request/response types, following existing API helper conventions.
  • packages/pam/local/rdp-proxy.go: removes the legacy StartRDPLocalProxy function that relied on the old approval workflow.

Confidence Score: 3/5

Safe to merge for the happy path, but the interactive error-handling diverges from existing conventions in a way that silently changes exit-code behavior on Ctrl+C.

The new handleApprovalRequired helper treats promptui.ErrInterrupt (Ctrl+C during the confirm prompt) identically to ErrAbort (pressing 'n'), logging a benign message and returning with exit code 0. The legacy HandleApprovalWorkflow it parallels explicitly routes non-abort errors through util.HandleError, which exits with code 1. Any script or automation that wraps infisical pam access and checks the exit code will silently see success when the user interrupted the approval prompt. Additionally, the expired branch is driven by a substring match on the backend error message, making it brittle to future message rewording.

packages/pam/local/access.go — specifically the promptui error-handling block inside handleApprovalRequired and the expired-detection logic.

Important Files Changed

Filename Overview
packages/pam/local/access.go Adds handleApprovalRequired to intercept PAM_APPROVAL_REQUIRED errors and offer interactive access-request submission; Ctrl+C during the confirm prompt silently exits 0 (diverging from the legacy handler) and expired detection relies on server message substring matching.
packages/api/api.go Adds CallPAMCreateAccessRequest following the same pattern as other PAM API helpers; implementation is straightforward and consistent with existing conventions.
packages/api/model.go Adds PAMCreateAccessRequestBody and PAMCreateAccessRequestResponse structs; modelling is correct and consistent with existing response shapes.
packages/pam/local/rdp-proxy.go Removes the legacy StartRDPLocalProxy entry-point that used the old approval workflow; remaining RDP proxy helpers are unchanged and correct.

Reviews (1): Last reviewed commit: "fix(pam): align access request payload w..." | Re-trigger Greptile

Comment thread packages/pam/local/access.go
Comment thread packages/pam/local/access.go Outdated
- detect an expired grant via the PAM_GRANT_EXPIRED error name instead of
  substring-matching the backend error message
- treat Ctrl+C (promptui.ErrInterrupt) at the request prompt as a non-zero
  exit rather than a graceful decline
@bernie-g bernie-g requested a review from x032205 July 3, 2026 00:16
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