feat: Support recursive synchronization of nested secret directories #4504

prayanshchh · 2025-09-09T13:27:42Z

Description 📣

This PR introduces recursive synchronization of nested secret directories, adds support for syncing multiple folders, and introduces a recursive option to include subfolders automatically.

A new join table: secret_sync_folders

Columns:
secretSyncId → references secret_syncs.id.
folderId → references secret_folders.id.
createdAt / updatedAt timestamps.
Composite primary key: (secretSyncId, folderId).

issue: #4402

Type ✨

Tests 🛠️

no tests (will write)

Screencast.from.2025-09-07.11-45-13.webm

# Here's some code block to paste some code snippets

[] I have read the contributing guide, agreed and acknowledged the code of conduct. 📝

maidul98 · 2025-09-09T13:28:24Z

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

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

greptile-apps

Greptile Summary

This PR implements recursive synchronization of nested secret directories, addressing issue #4402 where users previously had to create separate sync configurations for each directory in complex project structures. The change introduces a fundamental architectural shift from a one-to-one relationship between secret syncs and folders to a many-to-many relationship.

Key Changes:

Database Schema Migration: A new join table secret_sync_folders is created with a composite primary key (secretSyncId, folderId) to establish many-to-many relationships between secret syncs and folders. The existing folderId column in secret_syncs is removed and data is migrated to the new join table.
Backend Service Layer: The secret sync service is extensively modified to handle arrays of folder IDs instead of single folder IDs. A new recursive parameter enables automatic discovery and inclusion of nested subdirectories through a Common Table Expression (CTE) in the folder DAL.
API Layer: Both create and update endpoints now accept an optional recursive boolean parameter, allowing users to opt into recursive synchronization behavior.
Frontend Interface: A new checkbox is added to the secret sync creation form, enabling users to toggle recursive syncing of subfolders. The form validation and permission checking are updated to handle the recursive parameter.
Queue Processing: The secret sync queue is updated to process multiple folders simultaneously instead of single folders, with corresponding changes to secret and import retrieval logic.

The implementation maintains backward compatibility by making the recursive parameter optional (defaulting to false) and normalizing single folder operations to work with the new array-based structure. The recursive folder discovery uses a PostgreSQL CTE to efficiently traverse folder hierarchies and return all descendant folders for a given path.

Confidence score: 2/5

This PR introduces complex changes with significant potential for runtime errors and data consistency issues
Score lowered due to unsafe database queries, type safety concerns, and incomplete error handling in critical paths
Pay close attention to secret-sync-dal.ts, secret-sync-service.ts, and the database migration file

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 -** For better readability, consider splitting complex expressions into multiple variables. ([link](https://app.greptile.com/review/custom-context?memory=891fb737-20dd-4e83-a0f7-907a2d3aa5ad))
**Context -** Use 'email auth' instead of 'direct credential login' in documentation to provide clearer terminology for users. ([link](https://app.greptile.com/review/custom-context?memory=8067f41a-5f47-4fe6-8155-04af79d986ba))

<sub>12 files reviewed, no 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_4504)</sub>

greptile-apps · 2025-09-09T13:29:29Z

frontend/src/components/secret-syncs/forms/SecretSyncSourceFields.tsx

+    <Controller
+      name="recursive" 
+      control={control}
+      defaultValue={false}
+      render={({ field: { value, onChange } }) => (
+        <div className="flex items-center gap-2">
+          <span className="text-sm text-bunker-200">Sync subfolders recursively</span>
+          <Checkbox
+            id="recursive"
+            isChecked={value}
+            onCheckedChange={onChange}
+          />
+        </div>
+      )}
+    />


style: Consider adding a tooltip or help text to explain what 'recursive' means and its implications for users unfamiliar with the concept

backend/src/services/secret-folder/secret-folder-dal.ts

greptile-apps · 2025-09-09T13:29:31Z

backend/src/services/secret-sync/secret-sync-dal.ts

        // TODO (scott): replace with cached folder path once implemented
-        const [folderWithPath] = secretSync.folderId
+        const folderWithPath = secretSync.folderId
          ? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, [secretSync.folderId])


logic: Inconsistent array wrapping - should use secretSync.folderId directly since it's already an array from the aggregated query

Suggested change

? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, [secretSync.folderId])

? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, secretSync.folderId)

greptile-apps · 2025-09-09T13:29:32Z

backend/src/db/migrations/20250828110158_add_folderIds_to_secret_syncs.ts

+import { TableName } from "@app/db/schemas";
+
+const TABLE = TableName.SecretSync;
+const JOIN_TABLE = "secret_sync_folders";


style: The JOIN_TABLE constant should use TableName enum for consistency with codebase patterns. Currently the table name is hardcoded as string.

greptile-apps · 2025-09-09T13:29:33Z

backend/src/db/migrations/20250828110158_add_folderIds_to_secret_syncs.ts

+    await knex.schema.createTable(JOIN_TABLE, (t) => {
+      t.uuid("secretSyncId").notNullable().references("id").inTable(TABLE).onDelete("CASCADE");
+
+      t.uuid("folderId").notNullable().references("id").inTable("secret_folders").onDelete("CASCADE");


style: Using hardcoded table name 'secret_folders' instead of TableName.SecretFolder enum value creates inconsistency with codebase patterns.

greptile-apps · 2025-09-09T13:29:37Z

backend/src/services/secret-sync/secret-sync-service.ts

          ? subject(ProjectPermissionSub.SecretSyncs, {
              environment: secretSync.environment.slug,
-              secretPath: secretSync.folder.path
+              secretPath: Array.isArray(secretSync.folder) ? secretSync.folder.map((f: { path: string }) => f.path) : []


logic: The permission check assumes folder is always an array when using Array.isArray(), but the fallback [] may not provide proper permission validation for non-array cases.

greptile-apps · 2025-09-09T13:29:40Z

backend/src/services/secret-sync/secret-sync-queue.ts

    });

-    const secrets = await secretV2BridgeDAL.findByFolderId({ folderId });
+    const secrets = await secretV2BridgeDAL.findByFolderIds({ folderIds: folderId });


logic: Parameter name mismatch: passing folderId to a function expecting folderIds array. This suggests folderId should be an array or wrapped in an array.

Suggested change

const secrets = await secretV2BridgeDAL.findByFolderIds({ folderIds: folderId });

const secrets = await secretV2BridgeDAL.findByFolderIds({ folderIds: Array.isArray(folderId) ? folderId : [folderId] });

greptile-apps · 2025-09-09T13:29:41Z

backend/src/services/secret-sync/secret-sync-queue.ts

    if (!includeImports) return secretMap;

-    const secretImports = await secretImportDAL.find({ folderId, isReplication: false });
+    const secretImports = await secretImportDAL.findByFolderIds(folderId);


logic: Inconsistent parameter passing: folderId passed directly to findByFolderIds() which expects an array parameter.

Suggested change

const secretImports = await secretImportDAL.findByFolderIds(folderId);

const secretImports = await secretImportDAL.findByFolderIds(Array.isArray(folderId) ? folderId : [folderId]);

backend/src/services/secret-sync/secret-sync-schemas.ts

prayanshchh · 2025-09-12T15:36:17Z

I will do the changes by greptile, can someone have a look at the approach taken? @maidul98 @vmatsiiako

prayanshchh added 6 commits September 7, 2025 00:28

feat: recursive nature added

b2d43ea

linting

027ac89

remove console.log

0851a1f

address greptile comments

bf6d3bf

lint

e9c409b

add recursive to update

35ebe95

greptile-apps bot reviewed Sep 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Support recursive synchronization of nested secret directories #4504

feat: Support recursive synchronization of nested secret directories #4504

Uh oh!

prayanshchh commented Sep 9, 2025

Uh oh!

maidul98 commented Sep 9, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Sep 9, 2025

Uh oh!

Uh oh!

greptile-apps bot Sep 9, 2025

Uh oh!

greptile-apps bot Sep 9, 2025

Uh oh!

greptile-apps bot Sep 9, 2025

Uh oh!

greptile-apps bot Sep 9, 2025

Uh oh!

greptile-apps bot Sep 9, 2025

Uh oh!

greptile-apps bot Sep 9, 2025

Uh oh!

Uh oh!

Uh oh!

prayanshchh commented Sep 12, 2025

Uh oh!

Uh oh!

	? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, [secretSync.folderId])
	? await folderDAL.findSecretPathByFolderIds(secretSync.projectId, secretSync.folderId)

	const secrets = await secretV2BridgeDAL.findByFolderIds({ folderIds: folderId });
	const secrets = await secretV2BridgeDAL.findByFolderIds({ folderIds: Array.isArray(folderId) ? folderId : [folderId] });

	const secretImports = await secretImportDAL.findByFolderIds(folderId);
	const secretImports = await secretImportDAL.findByFolderIds(Array.isArray(folderId) ? folderId : [folderId]);

feat: Support recursive synchronization of nested secret directories #4504

Are you sure you want to change the base?

feat: Support recursive synchronization of nested secret directories #4504

Uh oh!

Conversation

prayanshchh commented Sep 9, 2025

Description 📣

Type ✨

Tests 🛠️

Uh oh!

maidul98 commented Sep 9, 2025

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

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Summary

Confidence score: 2/5

Context used:

Objective

What to Flag

What NOT to Flag

Detection Pattern

Review Message Template

Uh oh!

greptile-apps bot Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

prayanshchh commented Sep 12, 2025

Uh oh!

Uh oh!