Skip to content

feat(secret-sync): add Infisical-to-Infisical secret sync#5743

Merged
IgorHorta merged 64 commits intomainfrom
igor/eng-4703-implement-infisical-to-in-infisical-secret-sync
Mar 20, 2026
Merged

feat(secret-sync): add Infisical-to-Infisical secret sync#5743
IgorHorta merged 64 commits intomainfrom
igor/eng-4703-implement-infisical-to-in-infisical-secret-sync

Conversation

@IgorHorta
Copy link
Copy Markdown
Contributor

@IgorHorta IgorHorta commented Mar 18, 2026

Context

Implements a new External Infisical secret sync that allows syncing secrets from one Infisical instance to another (e.g., cloud → self-hosted, or between two self-hosted deployments).

This includes:

  • A new External Infisical App Connection using Universal Auth (Machine Identity) — authenticates to a remote Infisical instance via instanceUrl + clientId + clientSecret
  • A new External Infisical Secret Sync destination — syncs secrets to a target project/environment/path on the remote instance
  • Error handling with retryable/non-retryable status detection for external HTTP calls
  • Daily retry mechanism — failed External Infisical syncs are re-enqueued once per day via the DailyResourceCleanUp queue, since remote instances may be temporarily unreachable for extended periods (longer than the normal BullMQ retry window). New providers can opt into this behavior by adding to SECRET_SYNC_DAILY_RETRY_DESTINATIONS in secret-sync-maps.ts
  • Documentation with step-by-step UI guide and API examples

Closes ENG-4703

Screenshots

image

Steps to verify the change

  1. Create an External Infisical App Connection pointing to a remote Infisical instance (requires a Machine Identity with Universal Auth on the remote)
  2. Add the Machine Identity to the target project on the remote instance with at least Member role
  3. Create an External Infisical Secret Sync selecting project / environment / secret path on the remote
  4. Trigger a sync and verify secrets appear on the remote instance
  5. To test daily retry: set DAILY_RESOURCE_CLEAN_UP_DEVELOPMENT_MODE=true, force a sync failure, and confirm it gets re-enqueued within 5 minutes

Type

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

Checklist

@linear
Copy link
Copy Markdown

linear bot commented Mar 18, 2026

@maidul98
Copy link
Copy Markdown
Collaborator

maidul98 commented Mar 18, 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.

Introduce External Infisical as a new app connection type with complete
service implementation, including credential validation, enums, type
definitions, and schema mappings.
Register External Infisical connection router and integrate service into
the app connection factory. Add identityUaDAL dependency to support
credential management.
Implement External Infisical as a new secret sync destination with
complete service implementation, schemas, queue handlers, and API routes.
Register sync router and integrate into secret sync factory.
Add frontend components and forms for External Infisical app connection
and secret sync configuration, including type definitions, API hooks,
form validation schemas, and integration logo. Update default on-prem
license features to enable audit logs for self-hosted deployments.
@IgorHorta IgorHorta force-pushed the igor/eng-4703-implement-infisical-to-in-infisical-secret-sync branch from 700a088 to 6d9670b Compare March 18, 2026 00:33
@IgorHorta
Copy link
Copy Markdown
Contributor Author

@greptile review this please!

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 18, 2026

Greptile Summary

This PR implements a new External Infisical Secret Sync — allowing secrets to be synced between Infisical instances (e.g., cloud → self-hosted) using Machine Identity (Universal Auth). It includes a new app connection type, a dedicated secret sync destination, a daily-retry mechanism for failed syncs via the DailyResourceCleanUp queue, full frontend support, and documentation.

The architecture is well-structured and consistent with how other sync destinations are implemented in the codebase. Key concerns from earlier review rounds (SSRF via instanceUrl, multiple token fetches, unsafe non-null assertions) have been addressed — the token-fetch issue is resolved by the shared getRemoteContext helper, and identityUaDAL is now guarded with an explicit InternalServerError.

Notable issues found:

  • Path traversal via projectId (external-infisical-connection-router.ts): The projectId route parameter used in the outbound URL path lacks UUID validation. A caller can supply an encoded path segment to probe arbitrary endpoints on the remote Infisical instance. Adding .uuid() validation is a one-line fix.
  • Silent error swallowing (external-infisical-connection-service.ts): Both listProjects and getEnvironmentFolderTree catch all errors and return empty results. For listProjects in particular there is no UX fallback, so users with invalid/revoked credentials will see an empty project dropdown with no explanation.
  • Missing structured log context (external-infisical-connection-service.ts): Error log calls omit connectionId/projectId, making them hard to correlate in cloud log systems.
  • Fragile string matching (secretSyncs.ts): destinationName === "Infisical" couples UI label logic to the display name constant; a name change would silently produce wrong labels.
  • Previously flagged: race condition in queueDailyRetryForFailedSyncs (broad filter used for the update rather than the IDs from the initial find), and the duplicate DEFAULT_SECRET_SYNC_RETRY_CONFIG constant.

Confidence Score: 3/5

  • The PR is mostly safe but has a path traversal vulnerability and error-swallowing UX issues that should be addressed before merge.
  • Score reflects: (1) an unvalidated path parameter that enables URL path traversal on the remote instance, (2) silent error suppression that could confuse users, (3) the previously flagged race condition in queueDailyRetryForFailedSyncs (update filter is not scoped to the initially retrieved IDs) which can leave syncs stuck in Pending permanently. The core sync and auth logic is sound.
  • backend/src/server/routes/v1/app-connection-routers/external-infisical-connection-router.ts (path traversal), backend/src/services/app-connection/external-infisical/external-infisical-connection-service.ts (silent failures), backend/src/services/secret-sync/secret-sync-queue.ts (race condition in daily retry)

Important Files Changed

Filename Overview
backend/src/services/app-connection/external-infisical/external-infisical-connection-fns.ts New file implementing External Infisical connection logic. Uses blockLocalAndPrivateIpAddresses for SSRF protection and properly validates credentials. Checks for self-instance targeting via identityUaDAL.findOne. Uses native regex .replace(/\/$/, "") (flagged in previous threads). Overall flow is correct.
backend/src/services/app-connection/external-infisical/external-infisical-connection-service.ts Service layer for External Infisical connection. Both listProjects and getEnvironmentFolderTree silently swallow all errors and return empty results, which leaves users with an empty UI and no diagnostic feedback. Logging calls are missing structured context fields (connectionId, actor).
backend/src/services/secret-sync/external-infisical/external-infisical-sync-fns.ts Sync logic correctly resolves the previously flagged multiple-token-fetch issue by using a shared getRemoteContext. Retryable vs non-retryable errors are correctly classified. Batch operations (create/update/delete) are properly sequenced and all wrapped in withExternalInfisicalErrorHandling.
backend/src/server/routes/v1/app-connection-routers/external-infisical-connection-router.ts New router for External Infisical connection endpoints. The projectId path parameter in the environment-folder-tree route is not validated as a UUID, allowing path traversal in the outbound URL constructed from it on the remote instance.
backend/src/services/secret-sync/secret-sync-queue.ts Adds queueDailyRetryForFailedSyncs for retrying failed External Infisical syncs. The DEFAULT_SECRET_SYNC_RETRY_CONFIG duplicate and the race condition in queueDailyRetryForFailedSyncs (update uses broad filter instead of IDs from the initial find) were raised in previous review threads.
backend/src/services/secret-sync/secret-sync-maps.ts Adds ExternalInfisical to all sync maps and introduces SECRET_SYNC_DAILY_RETRY_DESTINATIONS constant. Changes are correct and consistent across all map entries.
backend/src/services/app-connection/app-connection-fns.ts Integrates ExternalInfisical into the app connection validation dispatch map. The previous unsafe non-null assertion concern is addressed — the code now throws an InternalServerError when identityUaDAL is missing.
backend/src/services/resource-cleanup/resource-cleanup-queue.ts Wires secretSyncQueue.queueDailyRetryForFailedSyncs into the daily cleanup job. Changes are minimal and correctly threaded through the DI graph.
frontend/src/helpers/secretSyncs.ts Adds ExternalInfisical to the sync map. Uses hardcoded string comparison destinationName === "Infisical" to differentiate UI labels, which is brittle and will silently break if the display name changes.

Last reviewed commit: "fix(ui): add Externa..."

… and add daily retry destinations

Configure ExternalInfisical sync type to use default retry behavior and register it as
a daily retry destination, enabling automatic re-attempts of failed syncs.
…l syncs

Implement a new daily retry queue job that processes failed External Infisical syncs.
This job retrieves syncs that need retry and re-executes them to improve reliability
for external Infisical integrations.
…leanup job

Wire the new secretSync daily retry queue into the daily resource cleanup job,
ensuring failed syncs are retried as part of the system's regular cleanup cadence.
…ntation

Add comprehensive documentation for the External Infisical app connection and
sync integration, including setup instructions and configuration details.
@IgorHorta IgorHorta force-pushed the igor/eng-4703-implement-infisical-to-in-infisical-secret-sync branch from 6d9670b to c91144c Compare March 18, 2026 00:44
… job

Add isDailyResourceCleanUpDevelopmentMode flag support to run the daily secret
sync retry job every 5 minutes in development mode instead of only at midnight.
This enables faster testing and iteration during development.
Add comprehensive API documentation for the new Infisical-to-Infisical secret
sync integration (External Infisical). Includes 9 endpoint references (list,
get-by-id, get-by-name, create, update, delete, sync-secrets, import-secrets,
remove-secrets) organized under the Infisical nav group in docs.json.
Update the Project field tooltip to clarify that the machine identity must be
added as a member to target projects for the sync to work correctly.
…type only

Only expose Secret Manager projects when listing remote Infisical projects for sync destination, preventing selection of non-compatible project types.
… API query param

Move the filtering logic from client-side to the remote Infisical API layer by passing
`type=secret-manager` as a query parameter instead of filtering after the response arrives.
This removes the need for the `type` field in the response schema and simplifies the mapping logic.

No behavioral change — results are identical but the API call is more efficient.
…deletion

Add canRemoveSecretsOnDeletion property to External Infisical sync configuration.
This allows users to opt into automatic secret deletion when the sync destination
is removed, providing safer cleanup of synced secrets.
…wsing

Replace the conditional FilterableSelect + Input fallback pattern with a unified
SecretPathInput component that derives child folder names dynamically from the
remote Infisical folder tree. This simplifies the component logic and improves
the UX for navigating destination folder paths.

Changes:
- Add folderNames prop to SecretPathInput to accept external folder suggestions,
  allowing callers to override the internal hook-based folder discovery
- Fix bug where selecting a subfolder didn't show its children by appending
  trailing "/" on selection
- Simplify ExternalInfisicalSyncFields by replacing dual-path logic with a
  single SecretPathInput, computing childFolderNamesAtPath from the current
  browsed path in the remote folder tree
- Remove unused type field from TRemoteInfisicalProject (matches backend fix)
…sync error messages

In withExternalInfisicalErrorHandling, the message field was set to
parseSyncErrorMessage(err), which returns a pre-serialized JSON string.
When the queue later called parseSyncErrorMessage on the outer
SecretSyncError object, it resulted in re-serialization of that string,
causing double-nested JSON in the audit log.

Now we extract the plain human-readable error message directly:
err.response?.data?.message ?? err.message. This ensures the message
stored in the audit log is a readable string, not nested JSON.

Also removes the now-unused parseSyncErrorMessage import from the
external-infisical-sync-fns module.
…eSyncErrorMessage

Previously, when handling a SecretSyncError, the error field was set to
parseSyncErrorMessage(err.error) which returns a string. When this string was
passed to JSON.stringify, it was escaped into a nested JSON string, resulting
in double-serialization.

The fix extracts the inner error directly as an object:
- For AxiosError: extract response.data (object)
- For other errors: extract .message (string)

This ensures JSON.stringify serializes the error field cleanly without
intermediate stringification.
…eSyncErrorMessage

Previously, when handling a SecretSyncError, the error field was set to
parseSyncErrorMessage(err.error) which returns a string. When this string was
passed to JSON.stringify, it was escaped into a nested JSON string, resulting
in double-serialization.

The fix extracts the inner error directly as an object:
- For AxiosError: extract response.data (object)
- For other errors: extract .message (string)

This ensures JSON.stringify serializes the error field cleanly without
intermediate stringification.
…c with key schema

When an Infisical-to-Infisical sync had a key schema configured, applying the schema
would prefix the secret key. On the destination Infisical instance, the synced secret
with the prefixed key would match the sync rule again, triggering another sync cycle
that would apply the schema again, creating an infinite loop.

The fix disables key schema support for Infisical-to-Infisical syncs entirely:
- Pass raw secretMap (not schemaSecretMap) to ExternalInfisicalSyncFns
- Set supportsKeySchema: false in sync configuration and schemas
- This prevents the prefixed key from triggering downstream syncs
victorvhs017
victorvhs017 previously approved these changes Mar 20, 2026
Copy link
Copy Markdown
Contributor

@victorvhs017 victorvhs017 left a comment

Choose a reason for hiding this comment

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

Great work!!

@IgorHorta IgorHorta merged commit d3c0aa4 into main Mar 20, 2026
12 of 13 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