Skip to content

Conversation

@fangpenlin
Copy link
Contributor

@fangpenlin fangpenlin commented Dec 23, 2025

Context

ref: https://linear.app/infisical/issue/PAM-42/add-redis-account-type-for-pam

Screenshots

Steps to verify the change

To test this feature, one needs to run a Redis server locally. First, let's see the example redis config file:

# Redis configuration file with multiple users

# Default user (admin) - requires password
requirepass admin123

# ACL Users configuration
# Format: user <username> on <password> <permissions> <keys>
# 
# Example users:
# 1. admin - full access
user admin on +@all ~* &* >admin123

# 2. appuser - read and write access
user appuser on +@read +@write ~* >apppass456

# 3. readonly - read-only access
user readonly on +@read ~* >readonly789

# 4. testuser - full access for testing
user testuser on +@all ~* >testpass321

# 5. developer - read, write, and some admin commands
user developer on +@read +@write +@keyspace +@string ~* >devpass999

# Bind to all interfaces
bind 0.0.0.0

# Enable protected mode (optional, but recommended)
protected-mode yes

With that, we can first have a redis server running without TLS:

version: '3.8'

services:
  redis:
    image: redis:7-alpine
    container_name: redis
    restart: always
    ports:
      - "6378:6379"
    command: redis-server /usr/local/etc/redis/redis.conf
    volumes:
      - redis-non-tls-data:/data
      - ./redis.conf:/usr/local/etc/redis/redis.conf:ro
    healthcheck:
      test: ["CMD", "redis-cli", "-a", "admin123", "ping"]
      interval: 10s
      timeout: 3s
      retries: 5

volumes:
  redis-non-tls-data:

For the TLS version, you can first generate the certs with:

#!/bin/bash

mkdir -p certs && \
openssl genrsa -out certs/ca.key 4096 && \
openssl req -x509 -new -nodes -sha256 -key certs/ca.key -days 3650 -subj "/O=Redis Test/CN=Redis Test CA" -out certs/ca.crt && \
openssl genrsa -out certs/redis.key 2048 && \
openssl req -new -sha256 -key certs/redis.key -subj "/O=Redis Test/CN=Redis Server" -out certs/redis.csr && \
openssl x509 -req -sha256 -in certs/redis.csr -CA certs/ca.crt -CAkey certs/ca.key -CAcreateserial -days 365 \
  -extfile <(printf "[san]\nsubjectAltName=IP:192.168.50.215") -extensions san \
  -out certs/redis.crt && \
rm -f certs/redis.csr certs/ca.srl  # Clean up temporary files (use -f to avoid error if files don't exist)

then with the compose:

version: '3.8'

services:
  redis-tls:
    image: redis:7-alpine
    ports:
      - "6377:6379"  # Host port 6378 maps to container TLS port 6379
    command:
      - redis-server
      - /usr/local/etc/redis/redis.conf
      - --port 0                  # Disable non-TLS port
      - --tls-port 6379           # Enable TLS on the standard Redis port inside container
      - --tls-cert-file /certs/redis.crt
      - --tls-key-file /certs/redis.key
      - --tls-ca-cert-file /certs/ca.crt
      - --tls-auth-clients no
    volumes:
      - redis-data:/data
      - ./redis.conf:/usr/local/etc/redis/redis.conf:ro
      - ./certs:/certs:ro         # Mount your certs directory (read-only)
    healthcheck:
      test: ["CMD", "redis-cli", "-a", "admin123", "--tls", "ping"]  # Update healthcheck for TLS (adjust certs if needed for internal check)
      interval: 10s
      timeout: 3s
      retries: 5

volumes:
  redis-data:

the testing process is pretty much largely the same as other PAM resources.

  1. Create the Redis resource - go to PAM resource page and create a Redis server resource pointing to the redis server you want to test
  2. Create a Redis account with credentials defined in the config file for the redis resource you want to test
  3. Run CLI in branch feature: add redis resource access for PAM cli#95 with the cmd generated for accessing the account
  4. Run redis -u <infisical cli proxy_url> to test it. Some corner cases, such as pub/sub cmds, monitors are a bit special in terms of how our proxy handles them.
  5. Kill the access cli process, check the session logs and see if they are uploaded

Type

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

@maidul98
Copy link
Collaborator

maidul98 commented Dec 23, 2025

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

Status Scanner 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.

@fangpenlin fangpenlin marked this pull request as ready for review December 26, 2025 01:53
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 26, 2025

Greptile Summary

This PR adds Redis resource support to the PAM system following the established architecture for other gateway-based resources like PostgreSQL and MySQL. The implementation includes backend factory pattern integration, schemas, types, frontend forms, and API route updates. The backend implementation is solid with proper hostname validation, SSL/TLS support through gateway proxying, and appropriate error handling. However, there is a maintainability concern: the RedisAccountForm reuses SqlAccountFields component designed for SQL databases, creating semantic coupling despite identical credential structure. This should use a dedicated component or explicitly named shared component to maintain clarity and future flexibility.

Key changes:

  • New Redis resource factory with connection validation, credential authentication, and SSL/TLS support
  • Redis resource/account type definitions and Zod schemas with proper credential sanitization
  • Frontend forms for Redis resource and account management with SSL configuration UI
  • Integration into PAM account/resource/session routers and discriminated unions
  • Gateway-based access for Redis similar to PostgreSQL/MySQL with proper credential handling

Confidence Score: 4/5

  • Safe to merge with minor maintainability concern about component reuse in the frontend account form
  • The PR implements Redis PAM support following established patterns with solid backend implementation including proper hostname validation, SSL/TLS handling, and security practices. Backend code quality is high with appropriate error handling and connection management. Primary concern is the semantic coupling in RedisAccountForm which imports SqlAccountFields designed for SQL databases. While the credential structure is identical and functional, this violates the separation of concerns principle and could cause confusion or maintainability issues if resource types diverge in the future. This is a design/clarity issue rather than a functional bug, and does not affect security or stability. Security review found no vulnerabilities - user input (hostname) is properly validated via verifyHostInputValidity, SSL certificates are handled correctly, and audit logging tracks session activity appropriately.
  • frontend/src/pages/pam/PamAccountsPage/components/PamAccountForm/RedisAccountForm.tsx - Consider using a dedicated Redis account fields component or renaming SqlAccountFields to reflect its broader applicability

Important Files Changed

Filename Overview
backend/src/ee/services/pam-resource/redis/redis-resource-factory.ts New Redis resource factory implementation. Properly validates connection with hostname verification, authenticates credentials, and handles SSL/TLS configuration through gateway proxy. Hostname validation via verifyHostInputValidity prevents DNS-based manipulation attacks. No rotation support is intentional (marked as TODO). Code quality is high with proper error handling and connection lifecycle management.
backend/src/ee/services/pam-resource/redis/redis-resource-schemas.ts Schema definitions for Redis resource and accounts. Properly defines connection details (host, port, SSL options) and credentials (username, password). Sanitization schemas correctly redact password fields. No issues identified.
frontend/src/pages/pam/PamAccountsPage/components/PamAccountForm/RedisAccountForm.tsx Redis account form component. Issue: Imports and uses SqlAccountFields which is designed for SQL databases, creating semantic coupling and maintainability concerns. While credentials structure is identical (username/password), component reuse should be explicit and properly named. Should use dedicated Redis account fields component or rename shared component to reflect intent.
backend/src/ee/routes/v1/pam-account-routers/pam-account-router.ts Account router properly registers Redis account schema and response types. Includes Redis in gateway-based resources list and discriminated union for access response. Integration is consistent with other resource types.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

25 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@maidul98 maidul98 requested a review from x032205 January 5, 2026 03:54
Copy link
Contributor

@x032205 x032205 left a comment

Choose a reason for hiding this comment

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

Works great, just a few comments!

Also sent some feedback in DMs regarding these concepts:

  • Adding accounts without username/password for unauthorized connections
  • Formatting session logs to show commands and outputs in a neater format

@fangpenlin fangpenlin force-pushed the PAM-42-add-redis-pam branch from 7a87fdc to 1fe781a Compare January 6, 2026 00:10
@fangpenlin fangpenlin force-pushed the PAM-42-add-redis-pam branch from f4348e8 to d8db03d Compare January 6, 2026 18:13
@fangpenlin fangpenlin merged commit f297b33 into main Jan 6, 2026
11 of 12 checks passed
@fangpenlin fangpenlin deleted the PAM-42-add-redis-pam branch January 6, 2026 20:50
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.

4 participants