Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Greptile SummaryThis PR adds a new NetScaler PKI sync integration, allowing Infisical to push certificates to Citrix/NetScaler appliances and optionally bind them to an SSL virtual server. It follows the existing App Connection + PKI Sync patterns with proper gateway v2 support, RE2 regex in most server-side code, and thorough documentation. Key issues found:
Confidence Score: 3/5Not safe to merge until the SSRF (missing IP validation on the direct path) and the potential login session-ID mis-parse are resolved. Two P1 issues block merge: (1) a real SSRF attack surface where any org member with connection-creation privileges can point the backend at internal IPs/metadata endpoints without a gateway, and (2) the NITRO API session-ID response key appears to be parsed at the wrong level, which would cause all sync and validation calls to fail in production. The remaining findings are P2 style/policy items that do not block merge on their own. backend/src/services/app-connection/netscaler/netscaler-connection-fns.ts (SSRF), backend/src/services/pki-sync/netscaler/netscaler-pki-sync-fns.ts (session ID parsing + native regex) Important Files Changed
Reviews (1): Last reviewed commit: "Add NetScaler PKI sync" | Re-trigger Greptile |
backend/src/services/app-connection/netscaler/netscaler-connection-fns.ts
Show resolved
Hide resolved
backend/src/services/pki-sync/netscaler/netscaler-pki-sync-fns.ts
Outdated
Show resolved
Hide resolved
frontend/src/components/pki-syncs/forms/schemas/netscaler-pki-sync-destination-schema.ts
Show resolved
Hide resolved
backend/src/services/app-connection/netscaler/netscaler-connection-schemas.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Looks good, just a couple of suggestions:
- the net scaler logo looks a bit squished. Let's see what works best here: if we should make all logos always have the width correct, or if we should find a different logo of a different dimension for netscaler. Feel feel to ignore this comment as well if the effort is not worth it.
- The delete call to sslcertkey takes the following param according to their docs: deletefromdevice and deletecertkeyfilesonremoval, could this come in handy?
Context
This PR contains a new PKI sync dedicated to NetScaler, allowing users to sync their certificates to this platform under the SSL inventory, and allowing them to also bind these to a virtual server as well.
Screenshots
Steps to verify the change
Type
Checklist
type(scope): short description(scope is optional, e.g.,fix: prevent crash on syncorfix(api): handle null response).