Skip to content

Conversation

DanielHougaard
Copy link
Member

Description 📣

This PR adds a redis app connection and a Redis secret rotation. The rotation follows the issuer pattern for issuance of new credential sets. Todo: Write documentation

Type ✨

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

@DanielHougaard DanielHougaard self-assigned this Sep 20, 2025
@maidul98
Copy link
Collaborator

maidul98 commented Sep 20, 2025

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

security/snyk check is complete. No issues have been found. (View Details)

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.

Greptile Summary

This PR introduces comprehensive Redis support to Infisical by implementing both Redis app connections and secret rotation capabilities. The implementation follows the established "issuer pattern" for credential rotation, which generates new Redis user credentials before revoking old ones to maintain service continuity.

Backend Implementation:

  • Adds Redis as a new AppConnection type with username/password authentication and SSL support
  • Implements Redis credential rotation factory that manages Redis ACL users with configurable permission scopes
  • Creates comprehensive schemas for validation, CRUD operations, and generated credentials
  • Follows the established secret rotation v2 architecture with dedicated routers, services, and type definitions

Frontend Implementation:

  • Adds Redis connection form with tabbed interface for configuration and SSL settings
  • Implements parameter fields for Redis ACL permission scopes and password generation requirements
  • Creates review and credential viewing components following existing UI patterns
  • Integrates Redis into all relevant form schemas and component mappings

Integration Points:

  • Updates all central type registries and union types to include Redis
  • Extends app connection maps, rotation maps, and helper functions
  • Adds Redis to routing systems and service factories
  • Maintains consistency with existing database rotation patterns (PostgreSQL, MySQL, etc.)

The implementation is architecturally sound and follows established patterns throughout the codebase, enabling users to create Redis connections and configure automatic credential rotations with customizable ACL permissions and password requirements.

Confidence score: 3/5

  • This PR requires careful review due to potential security vulnerabilities in connection handling and missing implementation details
  • Score lowered due to security concerns around user input processing for URLs, missing timeout configurations, and incomplete error handling in critical connection logic
  • Pay close attention to redis-connection-fns.ts and AppConnectionForm.tsx for security and completeness issues

Context used:

Rule - # Greptile Code Review Prompt: OR Query Safety Check (knex.js)

Objective

Flag database queries that use or conditions without proper grouping, which can break outer filters and cause unintended data exposure.

What to Flag

Look for query builder patterns where or methods are called directly on the main query object without being wrapped in a subquery or callback function.

Flag these patterns:

  • .orWhere(), .orWhereRaw(), .orWhereILike(), .orWhereIn(), etc. called directly on the main query
  • Multiple chained or conditions without proper grouping
  • Any or condition that could bypass security filters or WHERE clauses applied elsewhere

Examples to FLAG:

// ❌ DANGEROUS - or conditions break outer filters
query.where('status', 'active')
  .orWhere('name', 'like', '%search%')
  .orWhere('email', 'like', '%search%');

// ❌ DANGEROUS - mixed with other conditions
query.where('tenantId', userId)
  .where('deleted_at', null)
  .orWhere('name', 'like', '%search%')
  .orWhereRaw('email ilike ?', ['%search%']);

What NOT to Flag

Do NOT flag or conditions that are properly grouped within a callback function or subquery.

Examples that are SAFE:

// ✅ SAFE - or conditions grouped in callback
query.where('status', 'active')
  .where((qb) => {
    qb.where('name', 'like', '%search%')
      .orWhere('email', 'like', '%search%');
  });

// ✅ SAFE - explicit subquery grouping
query.where('tenantId', userId)
  .where('deleted_at', null)
  .where(function() {
    this.orWhere('name', 'like', '%search%')
        .orWhere('email', 'like', '%search%');
  });

Detection Pattern

Flag any line containing:

  • .orWhere*() methods called directly on a query object
  • NOT preceded by .where(( or .where(function
  • NOT inside a callback function block

Review Message Template

When flagging, use this message:

⚠️ **Unsafe OR Query Detected**

This query uses `or` conditions directly on the main query object, which can bypass outer filters and security constraints.

**Issue:** OR conditions at the query root level can break tenant isolation, permission checks, or other important filters.

**Fix:** Wrap OR conditions in a grouped WHERE clause:
```javascript
// Instead of this:
query.where('important_filter', value)
  .orWhere('field1', condition)
  .orWhere('field2', condition);

// Do this:
query.where('important_filter', value)
  .where((qb) => {
    qb.where('field1', condition)
      .orWhere('field2', condition);
  });

Security Impact: High - Could expose unauthorized data


## Additional Context
This pattern is particularly dangerous in multi-tenant applications, permission systems, or any query with security-critical WHERE clauses. Always ensure OR conditions are logically grouped to maintain the integrity of outer security filters. ([link](https://app.greptile.com/review/custom-context?memory=c4ca0367-148d-42b9-bcbd-958caf88aa07))
**Rule -** Add validation for string fields that have database column length limits to prevent insertion failures. For username template fields, add a 255 character limit validation. ([link](https://app.greptile.com/review/custom-context?memory=ab5293aa-4452-426c-a274-f034c06d0066))
**Context -** When using parameters in API calls, ensure they are utilized effectively; if not, consider removing them. ([link](https://app.greptile.com/review/custom-context?memory=aaee2f92-bc4e-4567-a347-3f4f0314b498))
**Context -** Follow a specific logging pattern for easier querying in cloud services, such as including the username in the format '[username=${username}]'. ([link](https://app.greptile.com/review/custom-context?memory=6f482b78-d4c6-4848-8389-31a942780af3))

<sub>47 files reviewed, 10 comments</sub>

<sub>[Edit Code Review Bot Settings](https://app.greptile.com/review/github) | [Greptile](https://greptile.com?utm_source=greptile_expert&utm_medium=github&utm_campaign=code_reviews&utm_content=infisical_4564)</sub>

Copy link
Contributor

@scott-ray-wilson scott-ray-wilson left a comment

Choose a reason for hiding this comment

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

working well! couple of comments/suggestions

Copy link
Contributor

@scott-ray-wilson scott-ray-wilson left a comment

Choose a reason for hiding this comment

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

LGTM!

@DanielHougaard DanielHougaard merged commit be324a7 into main Sep 23, 2025
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