feature(pam): windows RDP support and S3 recording overrides by bernie-g · Pull Request #6936 · Infisical/infisical

bernie-g · 2026-06-19T00:46:00Z

Context

Adds Windows RDP support to the PAM revamp and enables account-level S3 recording config overrides. Includes CORS probe after S3 config save, CLI session activation on creation, and cleanup fixes for the RDP session handler.
https://linear.app/infisical/issue/PAM-263/add-windows-account-rdp-web-and-cli

Type

Feature

Checklist

Title follows the conventional commit format
Tested locally
Updated docs (if needed)
Updated CLAUDE.md files (if needed)
Read the contributing guide

linear · 2026-06-19T00:46:04Z

PAM-237

infisical-review-police · 2026-06-19T00:46:05Z

💬 Discussion in Slack: #pr-review-infisical-6936-feat-pam-add-windows-rdp-support

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

Wire Windows/RDP account type into the c5 PAM model: - Backend: Windows account schemas, RDP session handler, legacy alias resolution, session activation fix, S3 recording config resolution from template settings - Frontend: RDP web access, WASM-based session recording playback, account type resolution, S3 recording configuration in template settings

…tivation - Account-level recording settings (S3 bucket/region/keyPrefix override) - CORS probe URL returned after S3 config save, frontend warns on misconfigured CORS - Bucket change warning alert on template and account sheets - Trim whitespace on S3 config fields to prevent validation failures - Activate CLI sessions immediately on creation instead of waiting for first client connection

- Remove duplicate recording connection picker from template settings tab - Remove recording enabled toggle (recording is always on) - Rename storage backend label to Internal Database - Add docs link to CORS warning notification - Fix double cleanup in RDP session handler

…entity functions

…ad maps.ts

…vents

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d913e7ac2e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

greptile-apps · 2026-06-19T20:36:06Z

Greptile Summary

This PR adds Windows RDP session support to the PAM revamp and enables per-account S3 recording config overrides, alongside a WASM-backed RDP replay player, a CORS probe after S3 config saves, and immediate CLI session activation.

Windows RDP: new PamAccountType.Windows replaces ActiveDirectory; a TCP-proxy session handler bridges the browser WebSocket to the local relay, and a new RDP replay player decodes and renders recordings via an IronRDP WASM decoder.
Account-level S3 override: accounts can now supply their own recordingSettings.s3Config (bucket/region/key prefix) that overrides the template's S3 config; the backend validates the config against the AWS app connection and returns a presigned CORS probe URL so the UI can warn about misconfigured bucket CORS policies.
Session lifecycle: CLI sessions are now activated immediately on creation; RDP web sessions defer activation to the gateway's credential-exchange callback rather than the web access service.

Confidence Score: 3/5

Not safe to merge without addressing the recording connection fallback and the unvalidated region input.

Three defects make the new RDP/S3 path unreliable in common configurations: (1) resolveRecordingConfig ignores templateRecordingConnectionId, so Windows accounts that don't override the connection at the account level will silently fail every recording chunk upload; (2) handleRdpSession skips onCleanup on pre-connection TCP failure, leaving sessions in an unresolved state; (3) the user-supplied S3 region is only type-cast rather than validated against the AWSRegion enum, letting arbitrary strings reach AWS endpoint construction. The frontend replay player and migration are clean, but the core recording and session-lifecycle paths need fixes before this is production-ready.

pam-recording-chunk-service.ts (missing template connection ID fallback), pam-validators.ts (unvalidated region), and pam-rdp-session-handler.ts (missing onCleanup on pre-connect failure) are the three files that need changes before merging.

Security Review

Unvalidated region in AWS endpoint construction (pam-validators.ts, pam-recording-chunk-service.ts): s3Config.region is accepted as a free-form string (validated only with z.string().min(1)) and passed directly to getAwsConnectionConfig via a type-cast to AWSRegion. An attacker-supplied region string could influence the AWS endpoint the server contacts during provider.validateConfig(...) and subsequent chunk operations — an SSRF-class risk. Recommend replacing the type-cast with z.nativeEnum(AWSRegion) validation in PamRecordingS3ConfigSchema.

Important Files Changed

Filename	Overview
backend/src/ee/services/pam-session-recording/pam-recording-chunk-service.ts	Implements account-level S3 config resolution for recording chunks, but `resolveRecordingConfig` only reads `account.recordingConnectionId` (account-level column) and does not fall back to `account.templateRecordingConnectionId`, breaking recordings for accounts that rely on the template's connection.
backend/src/ee/services/pam-web-access/rdp/pam-rdp-session-handler.ts	New RDP TCP-proxy session handler; post-connection teardown paths call `onCleanup` correctly, but pre-connection failure (initial TCP error or connect timeout) omits the `onCleanup` call before rejecting.
backend/src/ee/services/pam/pam-validators.ts	Adds `validateRecordingS3Config` and `mintCorsProbeUrl`; user-supplied `region` string is only type-cast to `AWSRegion` rather than validated against the enum, allowing arbitrary strings to reach AWS endpoint construction.
backend/src/ee/services/pam-account/pam-account-service.ts	Adds account-level `recordingSettings` S3 override support for create and update flows, with CORS probe URL generation; update path silently skips S3 validation if no connection ID can be resolved.
backend/src/ee/services/pam-session/pam-session-service.ts	CLI session now activated immediately on creation; recording backend is resolved from template settings; connection details are normalized before being returned to the gateway.
backend/src/ee/services/pam-web-access/pam-web-access-service.ts	Routes Windows accounts to the new RDP protocol and defers session activation to the gateway for RDP; clean separation from SSH/Postgres web access path.
backend/src/db/migrations/20260619144500_pam-account-recording-settings.ts	Additive migration adds nullable `recordingSettings` JSONB column to `pam_accounts`; guarded with `hasColumn` checks in both up and down.
frontend/src/pages/pam/PamSessionsPage/components/RdpReplayView/rdpReplayPlayer.ts	New WASM-backed RDP session replay player; RAF-driven playback with buffering, append streaming, and dirty-rect blitting; thorough lifecycle and dispose handling.
frontend/src/pages/pam/PamAccountsPage/components/AccountDetailSheet.tsx	Adds account-level S3 recording override UI with CORS probe on save and inline warning about bucket migration; straightforward form extension.
backend/src/ee/services/pam-account-template/pam-account-template-service.ts	Template create/update now validates S3 config against the AWS connection and returns a CORS probe URL; logic mirrors the account service correctly.

_{Reviews (1): Last reviewed commit: "chore(pam): fix lint and prettier format..." | Re-trigger Greptile}

…te recording connection fallback

…call onCleanup on pre-connect failures

veria-ai · 2026-06-19T20:59:43Z

+  connectionId: string,
+  s3Config: { bucket: string; region: string; keyPrefix?: string }
+): Promise<TPamRecordingResolvedConfig> => {
+  const raw = await appConnectionDAL.findById(connectionId);


Medium: Unauthorized app connection use

recordingConnectionId is only checked by org membership before this helper decrypts and uses the connection's AWS credentials. A PAM account/template editor who knows another AWS app connection ID in the org can configure S3 recording with it and make the backend perform S3 Head/Put/Delete operations and mint recording URLs without having Connect permission to that connection or confirming the connection belongs to the PAM project. Validate the connection the same way other app-connection consumers do: require AWS, allow only org-scoped or same-project connections, and enforce the caller's app-connection connect permission before decrypting or using the credentials.

veria-ai · 2026-06-19T20:59:53Z

PR overview

This pull request adds Windows RDP support for PAM and introduces S3 recording override configuration, including validation around the AWS connection used for recording storage.

There is one open security issue: S3 recording configuration can reference an AWS app connection based only on organization membership, rather than enforcing the expected connection permission and project-scope checks. In practice, a PAM account or template editor with knowledge of another connection ID in the same organization could cause backend S3 operations to run through that connection. No issues have been addressed yet, so the PR still needs a permission/scope validation fix before this area is safe to merge.

Open issues (1)

Medium: Unauthorized app connection use — backend/src/ee/services/pam/pam-validators.ts:85

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

bernie-g added 2 commits June 19, 2026 10:22

bernie-g force-pushed the PAM-237-RDP branch from fc8b7d6 to 1304fe6 Compare June 19, 2026 17:15

bernie-g changed the title ~~feat(pam): add Windows RDP support~~ feat(pam): Windows RDP support and S3 recording overrides Jun 19, 2026

bernie-g added 4 commits June 19, 2026 15:56

refactor(pam): remove Active Directory account type, migrate to Windows

770ff6a

refactor(pam): remove resolveAccountType and resolvePamAccountType id…

0ea409f

…entity functions

refactor(pam): make sanitizedCredentials username required, remove de…

7a3b1aa

…ad maps.ts

merge pam-revamp-c5-andrey into PAM-237-RDP

89dabba

bernie-g changed the title ~~feat(pam): Windows RDP support and S3 recording overrides~~ feature(pam): Windows RDP support and S3 recording overrides Jun 19, 2026

fix(pam): use raw table name in AD migration, type session playback e…

d913e7a

…vents

bernie-g changed the title ~~feature(pam): Windows RDP support and S3 recording overrides~~ feature(pam): windows RDP support and S3 recording overrides Jun 19, 2026

bernie-g marked this pull request as ready for review June 19, 2026 20:28

fix(pam): add activateSession to session DAL Pick type

b445031

chatgpt-codex-connector Bot reviewed Jun 19, 2026

View reviewed changes

Comment thread backend/src/ee/services/pam-session-recording/pam-recording-chunk-service.ts Outdated

Comment thread backend/src/ee/services/pam-account-template/pam-account-template-service.ts

chore(pam): fix lint and prettier formatting

70c3773

greptile-apps Bot reviewed Jun 19, 2026

View reviewed changes

bernie-g added 2 commits June 19, 2026 16:37

fix(pam): lint fixes, validate S3 config on template save, use templa…

b3e3f26

…te recording connection fallback

fix(pam): validate S3 region enum, require connection for S3 config, …

b81443b

…call onCleanup on pre-connect failures

bernie-g requested a review from x032205 June 19, 2026 20:46

chore(pam): fix generated schema field ordering

9e16124

veria-ai Bot reviewed Jun 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(pam): windows RDP support and S3 recording overrides#6936

feature(pam): windows RDP support and S3 recording overrides#6936
bernie-g wants to merge 13 commits into
pam-revamp-c5-andreyfrom
PAM-237-RDP

bernie-g commented Jun 19, 2026 •

edited

Loading

Uh oh!

linear Bot commented Jun 19, 2026

Uh oh!

infisical-review-police Bot commented Jun 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 19, 2026

Greptile Summary

Security Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

veria-ai Bot Jun 19, 2026

Uh oh!

veria-ai Bot commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bernie-g commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Type

Checklist

Uh oh!

linear Bot commented Jun 19, 2026

Uh oh!

infisical-review-police Bot commented Jun 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 19, 2026

Greptile Summary

Confidence Score: 3/5

Security Review

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

veria-ai Bot Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

veria-ai Bot commented Jun 19, 2026

PR overview

Open issues (1)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bernie-g commented Jun 19, 2026 •

edited

Loading