Skip to content

feat: ssh pam exec and sftp#5825

Merged
sheensantoscapadngan merged 4 commits into
mainfrom
feat/show-ssh-pam-exec
Apr 1, 2026
Merged

feat: ssh pam exec and sftp#5825
sheensantoscapadngan merged 4 commits into
mainfrom
feat/show-ssh-pam-exec

Conversation

@sheensantoscapadngan
Copy link
Copy Markdown
Member

@sheensantoscapadngan sheensantoscapadngan commented Mar 26, 2026

Context

This PR makes it so that SSH PAM exec and sftp are displayed in session logs

Relevant PR: Infisical/cli#161

Screenshots

Steps to verify the change

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Updated CLAUDE.md files (if needed)
  • Read the contributing guide

@maidul98
Copy link
Copy Markdown
Collaborator

maidul98 commented Mar 26, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 26, 2026

Greptile Summary

This PR improves PAM session terminal replay to support SSH PAM exec and SFTP session types by detecting whether terminal input is echoed in output and adjusting which event types are displayed accordingly. When input is echoed (interactive PTY sessions), only output events are shown to avoid duplication; when input is not echoed (exec/SFTP), both input and output events are included.

Key changes:

  • Introduces isPrintableText to filter binary/non-printable data from base64-decoded terminal events.
  • Introduces decodeEventData as a shared decoding helper.
  • Introduces isInputEchoedInOutput which uses a heuristic substring match to detect echo behaviour.
  • Updates aggregateTerminalEvents to include input events for non-echoed sessions and correctly fall back when outputEvents is empty.

Two issues identified:

  • The fallback block (lines 78–80) for "SFTP with only input messages" is dead code — it can never be reached given the guarantees established by isInputEchoedInOutput.
  • The echo detection heuristic uses allOutput.includes(inputText), which is a plain substring match and can produce false positives when a short command name (e.g., ls, id, pwd) appears anywhere in the terminal output for reasons unrelated to echo, causing input events to be silently dropped from the display.

Confidence Score: 4/5

Safe to merge with a minor logic concern in the echo-detection heuristic that could cause input events to be hidden in edge cases.

The change is a well-scoped frontend utility improvement. Neither consumer of aggregateTerminalEvents uses the eventType field, so the hardcoded 'output' is harmless. The dead fallback is benign. The false-positive risk in the substring echo check is real but limited to short, common command names and unlikely to cause user-visible problems in most PAM exec sessions. No security, data-loss, or breaking API concerns.

frontend/src/pages/pam/PamSessionsByIDPage/components/terminal-utils.ts — echo detection heuristic at lines 58–61.

Important Files Changed

Filename Overview
frontend/src/pages/pam/PamSessionsByIDPage/components/terminal-utils.ts Refactors terminal event aggregation to support SSH PAM exec and SFTP sessions by detecting whether input is echoed in output and conditionally including input events; introduces a dead fallback block and a substring-match heuristic susceptible to false positives for short command names.

Reviews (1): Last reviewed commit: "feat: show ssh pam exec and sftp" | Re-trigger Greptile

Comment thread frontend/src/pages/pam/PamSessionsByIDPage/components/terminal-utils.ts Outdated
Comment thread frontend/src/pages/pam/PamSessionsByIDPage/components/terminal-utils.ts Outdated
@sheensantoscapadngan sheensantoscapadngan changed the title feat: show ssh pam exec and sftp feat: ssh pam exec and sftp Mar 26, 2026
@sheensantoscapadngan sheensantoscapadngan merged commit a1879ca into main Apr 1, 2026
11 checks passed
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.

3 participants