Skip to content

feat(proxy): Add support for secret update using SSE#169

Open
adilsitos wants to merge 21 commits intomainfrom
adilsitos/feat/ENG-4525
Open

feat(proxy): Add support for secret update using SSE#169
adilsitos wants to merge 21 commits intomainfrom
adilsitos/feat/ENG-4525

Conversation

@adilsitos
Copy link
Copy Markdown

Description 📣

Allow support to update secrets using SSE (server sent events)

Type ✨

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

Tests 🛠️

  • Created a new secret management project
  • Create a universal identity machine policy
  • Start the proxy pointing to the server (default port is 8080)
  • Create some secrets using the interface
  • Try to fetch using the API (check the bash code below)
  • Update the secret using the interface
  • Check the SSE connection was opened (check the logs)
  • Try to fetch it again (using the API)
  • Check the request was cached.
curl --location 'http://localhost:8081/api/v4/secrets/test?type=shared&projectId=66b56e25-7db3-49c3-8014-f96407852d54&environment=prod' \
--header 'Authorization: Bearer tokwn'

@linear
Copy link
Copy Markdown

linear bot commented Mar 31, 2026

@adilsitos adilsitos changed the title Adilsitos/feat/eng 4525 feat(proxy): Add support for secret update using SSE Mar 31, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 31, 2026

Greptile Summary

This PR adds demand-driven SSE (Server-Sent Events) support to the Infisical proxy, allowing real-time cache invalidation when secrets are created, updated, or deleted. When --use-sse is enabled, the static periodic refresh loop is disabled and replaced by per-project SSE connections that are opened lazily on the first secret request. On a mutation event, affected cache entries are collected, purged, and immediately re-fetched using the original per-user tokens — correctly addressing the authorization concern from the previous review cycle.

Key changes:

  • New packages/proxy/sse.go: SSEManager, SSEAuthState, connectSSEForProject, parseSSEStream, handleSSEEvent, and refetchSecretsAfterSSEEvent implement the full SSE lifecycle with exponential-backoff retry, re-authentication, and cache resync on reconnect.
  • packages/proxy/cache.go: New CollectAndPurgeByMutation method reads cached entries before evicting them so they can be replayed with the original tokens.
  • packages/proxy/resync.go: StartBackgroundLoops gains a sseEnabled flag; when true the secrets-refresh ticker channel is replaced with a nil (never-firing) channel.
  • packages/cmd/proxy.go: Three new flags (--use-sse, --client-id, --client-secret), SSEManager initialisation, and a refactor of the cache-serving path into helper functions.

Issues found:

  • SSEAuthState is never pre-authenticated, so the very first SSE connection per project always fails with 401 before the retry loop obtains a token and reconnects, creating a window in which early events can be missed.
  • The bufio.Scanner in parseSSEStream uses the default 64 KB line-size cap; large payloads (e.g., bulk import mutations) can trigger a scan error, force a reconnect, and silently drop the oversized event.
  • EnsureSubscription and its call-site in proxy.go both log the same message, producing duplicate log entries on every secret request.
  • No documentation exists for the new flags or the SSE operating mode.

Confidence Score: 3/5

Safe to merge with caveats — the authorization fix is solid, but the missing initial authentication creates a guaranteed-failure first connection per project and a small event-loss window that should be addressed before production use.

The core authorization concern from the previous review cycle has been properly resolved. However, a P1 logic issue remains: SSEAuthState starts with an empty token, making every project's first SSE connection guaranteed to receive a 401. While the retry loop recovers, it introduces a window where early mutation events can be silently dropped, leaving the cache stale. The 64 KB scanner line-limit is also a real reliability gap for large batch events. Together these two issues are material enough to warrant fixes before merge.

packages/proxy/sse.go — initial token not set in SSEAuthState and default scanner line-size limit both need attention.

Important Files Changed

Filename Overview
packages/proxy/sse.go New file implementing SSE-based cache invalidation. Key issues: SSEAuthState starts with an empty token so the first connection for every project always fails with 401 before retrying; default scanner line limit can silently drop large events.
packages/cmd/proxy.go Adds --use-sse, --client-id, --client-secret flags and wires up SSEManager/SSEAuthState. Cache-serving logic refactored into helper functions. No documentation exists for the new flags yet.
packages/proxy/cache.go Adds CollectedCacheEntry type and CollectAndPurgeByMutation method to support SSE-triggered refetch. Logic mirrors the existing PurgeByMutation but additionally reads the full entry before eviction so it can be replayed. No issues found.
packages/proxy/resync.go StartBackgroundLoops updated to accept sseEnabled flag. When SSE is active, the static-secrets refresh ticker is replaced with a nil channel so its case never fires. Clean and correct implementation.
.gitignore Adds the compiled infisical binary to .gitignore. Correct and necessary.

Reviews (2): Last reviewed commit: "change to use httpclient" | Re-trigger Greptile

@adilsitos
Copy link
Copy Markdown
Author

@greptile check the comments added and the changes. See if the problems still make sense. Update the summary

@adilsitos adilsitos requested a review from varonix0 April 1, 2026 13:16
@adilsitos
Copy link
Copy Markdown
Author

The E2E test will only work when the other PR gets merged, since this depends on a change there.

@adilsitos adilsitos force-pushed the adilsitos/feat/ENG-4525 branch from 810bd46 to 4f6ea97 Compare April 6, 2026 13:02
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

@adilsitos adilsitos force-pushed the adilsitos/feat/ENG-4525 branch from 5afe349 to 5c181a2 Compare April 9, 2026 13:40
@adilsitos adilsitos force-pushed the adilsitos/feat/ENG-4525 branch from a3ae7cc to 807aa40 Compare April 9, 2026 16:43
proxyStartCmd.Flags().Bool("use-sse", false, "Enable SSE (Server-Sent Events) mode for real-time cache invalidation. When enabled, the static secrets refresh loop is disabled and --client-id/--client-secret are required.")
proxyStartCmd.Flags().String("client-id", "", "Universal auth client ID for SSE (env: INFISICAL_UNIVERSAL_AUTH_CLIENT_ID)")
proxyStartCmd.Flags().String("client-secret", "", "Machine identity client secret for universal auth (env: INFISICAL_UNIVERSAL_AUTH_CLIENT_SECRET)")
proxyStartCmd.Flags().Bool("event-subscription-enabled", false, "Enable Event Subscription mode for real-time cache invalidation. When enabled, the static secrets refresh loop is disabled. If event subscriptions are unavailable, the proxy will fall back to a polling mechanism. `--client id` and `--client-secret` are required when this is set to true ")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would be in favor of just --enable-event-subscriptions, seems cleaner?


if config.UseSSE {
args = append(args, "--use-sse")
args = append(args, "--event-subscription-enabled")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

@varonix0
Copy link
Copy Markdown
Member

varonix0 commented Apr 9, 2026

@codex can you please re-review this PR

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 807aa406b9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

go m.attemptSSEReconnection(projectId, environmentSlug)
}

go startProjectPollingLoop(pollCtx, m.cache, m.domainURL, m.resyncHttpClient, projectId, environmentSlug, m.pollingFallbackInterval, retrySSE)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Poll every cached environment during SSE fallback

transitionToPolling starts fallback polling for only one environmentSlug per project, but SSE subscriptions are managed per-project and static refresh is disabled when SSE mode is on. If a project has cached secrets in multiple environments (for example dev and prod) and SSE remains unavailable, only the selected environment is refreshed while the others can stay stale indefinitely.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This makes sense, but it would change the way how the pooling work (even the one that already exist). With this change, it will track all the environments+projects that the user requests (like happens on the general pooling)

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.

2 participants