CP-25960: allowlist discovery-derived hosts before authenticating against them

internal/cyberark/client_test.go

@@ -40,7 +40,8 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) {
 	discoveryContextAPI, _ := dataupload.MockDataUploadServer(t)
 
 	// Unused by the Conjur path, but service discovery requires it to be set.
-	const identitySrv = "https://identity.example.invalid"
+	// Never dialed, so it just needs a host servicediscovery's allowlist accepts.
+	const identitySrv = "https://identity.example.integration-cyberark.cloud"
 
 	httpClient := servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{
 		Identity: servicediscovery.ServiceEndpoint{

internal/cyberark/conjur/conjur.go

@@ -85,16 +85,21 @@ func (c *Client) exchange(ctx context.Context) (string, error) {
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != http.StatusOK {
-		// Conjur returns a JSON error body with the actual reason; include a
-		// bounded prefix so the operator doesn't have to go read Conjur's own
-		// audit log to find out why. 401 here most often means the SA token
-		// audience != authenticator audience=conjur.
+		// Conjur returns a JSON error body with the actual reason. It can
+		// contain policy structure, service IDs and host identities, and this
+		// error is surfaced all the way up to a Kubernetes Pod Event
+		// (pkg/agent/run.go's PushingErr notification), which anyone with
+		// `get events` in the namespace can read — so the body is logged at
+		// V(2) for an operator to go find, not embedded in the returned
+		// error. 401 here most often means the SA token audience != the
+		// authenticator's audience=conjur.
 		errBody, _ := io.ReadAll(io.LimitReader(resp.Body, 4*1024))
 		// Drain the rest so the connection can be reused, bounded so a
 		// misbehaving server can't make this read unboundedly.
 		_, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 1024*1024))
-		return "", fmt.Errorf("authn-jwt exchange rejected (%d): %s; verify service_id, the authenticator is enabled, and the SA token audience is 'conjur'",
-			resp.StatusCode, strings.TrimSpace(string(errBody)))
+		klog.FromContext(ctx).V(2).Info("authn-jwt exchange rejected", "statusCode", resp.StatusCode, "body", strings.TrimSpace(string(errBody)))
+		return "", fmt.Errorf("authn-jwt exchange rejected (%d); verify service_id, the authenticator is enabled, and the SA token audience is 'conjur' (run with -v=2 to see Conjur's response body)",
+			resp.StatusCode)
 	}
 	body, err := io.ReadAll(io.LimitReader(resp.Body, 64*1024))
 	if err != nil {

internal/cyberark/conjur/conjur_test.go

@@ -213,13 +213,21 @@ func TestInvalidate_ForcesReexchange(t *testing.T) {
 	require.Equal(t, 2, exchanges, "Invalidate must force the next AuthenticateRequest call to re-exchange")
 }
 
-func TestAuthenticateRequest_ExchangeErrorIncludesConjurResponseBody(t *testing.T) {
+// This error propagates all the way up to a Kubernetes Pod Event
+// (pkg/agent/run.go's PushingErr notification), readable by anyone with `get
+// events` in the namespace — so Conjur's response body (which can contain
+// policy structure, service IDs and host identities) must not appear in it.
+// The body is still logged at V(2) for an operator to go find, but that's
+// exercised via the "authn-jwt exchange rejected" log line, not asserted
+// here — ktesting has no easy log-buffer assertion in this codebase.
+func TestAuthenticateRequest_ExchangeErrorOmitsConjurResponseBody(t *testing.T) {
 	srv, httpClient := MockConjurExchangeServerStatusBody(t, http.StatusUnauthorized, []byte(`{"error":{"message":"CONJ00001E Invalid JWT token"}}`))
 	defer srv.Close()
 
 	c := New(httpClient, srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"})
 	req, _ := http.NewRequestWithContext(t.Context(), http.MethodGet, "https://example.com/x", nil)
 	_, err := c.AuthenticateRequest(req)
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "CONJ00001E Invalid JWT token")
+	require.NotContains(t, err.Error(), "CONJ00001E Invalid JWT token")
+	require.Contains(t, err.Error(), "authn-jwt exchange rejected (401)")
 }

internal/cyberark/servicediscovery/discovery.go

@@ -9,9 +9,12 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"strings"
 	"sync"
 	"time"
 
+	"k8s.io/klog/v2"
+
 	arkapi "github.com/jetstack/preflight/internal/cyberark/api"
 	"github.com/jetstack/preflight/pkg/version"
 )
@@ -40,6 +43,81 @@ const (
 	maxDiscoverBodySize = 2 * 1024 * 1024
 )
 
+// allowedRootDomains are the only root domains a discovery response is
+// allowed to point us at for identity/discoverycontext/secrets_manager.
+// Without this, mainActiveAPI's ep.API is trusted verbatim from the response
+// body and handed straight to the Conjur/Identity clients, which then POST
+// the agent's SA token (or username/password) to it — an SSRF-shaped hole if
+// the response is ever tampered with. Mirrors the per-env ROOT_DOMAIN
+// allowlist already enforced on the discoverycontext-regional-resources side
+// (token.py, for the JWT `iss` host) — copied by value here since these
+// domains rarely change and the agent has no access to that env-keyed map.
+//
+// Source of truth is the `everest_env_utils` package's ROOT_DOMAIN map
+// (published to Artifactory as everest_env_utils_cyberark, v2.0.117 as of
+// 2026-09-03), not the Lambda's local commercial-only clone — that clone
+// omits the GOV_* environments entirely, which would have made this
+// allowlist silently break every gov-cloud tenant's agent.
+var allowedRootDomains = []string{
+	"cyberark.cloud",
+	"cyberark-everest-dev.com",
+	"cyberark-everest-test.com",
+	"cyberark-everest-stage.com",
+	"sandbox-cyberark.cloud",
+	"integration-cyberark.cloud",
+	"pt-cyberark.cloud",
+	"cyberark-everest-integdev.cloud",
+	"cyberark-everest-preinteg.cloud",
+	"cyberark-everest-perf.cloud",
+	"cyberark-everest-pre-prod.cloud",
+	// Gov-cloud environments.
+	"dev-cyberarkgov.com",
+	"test-cyberarkgov.com",
+	"stage-cyberarkgov.com",
+	"integdev-cyberarkgov.cloud",
+	"integration-cyberarkgov.cloud",
+	"preprod-cyberarkgov.cloud",
+	"dum-preprod-cyberarkgov.cloud",
+	"cyberarkgov.cloud",
+}
+
+// isAllowedServiceHost reports whether host is, or is a subdomain of, one of
+// allowedRootDomains — or is exactly discoveryHost, the host we just made a
+// successful, TLS-authenticated discovery call to. The latter matters for
+// ARK_DISCOVERY_API-overridden (dev/CI/test) discovery endpoints: whatever
+// host that override already points at is exactly as trusted as the
+// discovery call itself, so a service response pointing back at that same
+// host can't be a new SSRF target.
+func isAllowedServiceHost(host, discoveryHost string) bool {
+	if host == discoveryHost {
+		return true
+	}
+	for _, root := range allowedRootDomains {
+		if host == root || strings.HasSuffix(host, "."+root) {
+			return true
+		}
+	}
+	return false
+}
+
+// sanitizeServiceAPI returns rawAPI unchanged if its host is allowed, or ""
+// (treated the same as "service not present in the response") if not.
+func sanitizeServiceAPI(ctx context.Context, serviceName, rawAPI, discoveryHost string) string {
+	if rawAPI == "" {
+		return ""
+	}
+	u, err := url.Parse(rawAPI)
+	if err != nil || u.Hostname() == "" {
+		klog.FromContext(ctx).Info("dropping unparseable service discovery API URL", "service", serviceName, "api", rawAPI)
+		return ""
+	}
+	if !isAllowedServiceHost(u.Hostname(), discoveryHost) {
+		klog.FromContext(ctx).Info("dropping service discovery API URL outside the allowed CyberArk domains", "service", serviceName, "host", u.Hostname())
+		return ""
+	}
+	return rawAPI
+}
+
 // Client is a Golang client for interacting with the CyberArk Discovery Service. It allows
 // users to fetch URLs for various APIs available in CyberArk. This client is specialised to
 // fetch only API endpoints, since only API endpoints are required by the Venafi Kubernetes Agent currently.
@@ -195,6 +273,14 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error
 		}
 	}
 
+	// Drop any of the three API URLs whose host isn't one of the CyberArk
+	// domains we actually trust, before anything downstream authenticates
+	// against it. A dropped URL is treated exactly like one absent from the
+	// response — see the required/optional distinction below.
+	identityAPI = sanitizeServiceAPI(ctx, IdentityServiceName, identityAPI, u.Hostname())
+	discoveryContextAPI = sanitizeServiceAPI(ctx, DiscoveryContextServiceName, discoveryContextAPI, u.Hostname())
+	secretsManagerAPI = sanitizeServiceAPI(ctx, SecretsManagerServiceName, secretsManagerAPI, u.Hostname())
+
 	// identityAPI is required unconditionally, unlike discoveryContextAPI and
 	// secretsManagerAPI below: it's present and active for every healthy
 	// tenant, so callers may rely on it being non-empty without checking it

internal/cyberark/servicediscovery/discovery_test.go

@@ -50,6 +50,76 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) {
 		},
 	}
 
+	t.Run("identity API host outside the allowed CyberArk domains is rejected", func(t *testing.T) {
+		logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+		ctx := klog.NewContext(t.Context(), logger)
+
+		httpClient := MockDiscoveryServer(t, Services{
+			Identity: ServiceEndpoint{
+				API: "https://ajp5871.id.attacker.example",
+			},
+			DiscoveryContext: ServiceEndpoint{
+				API: mockDiscoveryContextAPIURL,
+			},
+			SecretsManager: ServiceEndpoint{
+				API: mockSecretsManagerAPIURL,
+			},
+		})
+
+		client := New(httpClient, MockDiscoverySubdomain)
+		services, _, err := client.DiscoverServices(ctx)
+		require.Error(t, err)
+		assert.Nil(t, services)
+	})
+
+	t.Run("secrets_manager and discovery_context hosts outside the allowed CyberArk domains are dropped, not fatal", func(t *testing.T) {
+		logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+		ctx := klog.NewContext(t.Context(), logger)
+
+		httpClient := MockDiscoveryServer(t, Services{
+			Identity: ServiceEndpoint{
+				API: mockIdentityAPIURL,
+			},
+			DiscoveryContext: ServiceEndpoint{
+				API: "https://venafi-test.inventory.attacker.example",
+			},
+			SecretsManager: ServiceEndpoint{
+				API: "https://venafi-test.secretsmgr.attacker.example",
+			},
+		})
+
+		client := New(httpClient, MockDiscoverySubdomain)
+		services, _, err := client.DiscoverServices(ctx)
+		require.NoError(t, err)
+		assert.Equal(t, mockIdentityAPIURL, services.Identity.API)
+		assert.Equal(t, "", services.DiscoveryContext.API)
+		assert.Equal(t, "", services.SecretsManager.API)
+	})
+
+	t.Run("gov-cloud root domains are accepted", func(t *testing.T) {
+		logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+		ctx := klog.NewContext(t.Context(), logger)
+
+		httpClient := MockDiscoveryServer(t, Services{
+			Identity: ServiceEndpoint{
+				API: "https://ajp5871.id.cyberarkgov.cloud",
+			},
+			DiscoveryContext: ServiceEndpoint{
+				API: "https://venafi-test.inventory.integration-cyberarkgov.cloud",
+			},
+			SecretsManager: ServiceEndpoint{
+				API: "https://venafi-test.secretsmgr.dev-cyberarkgov.com",
+			},
+		})
+
+		client := New(httpClient, MockDiscoverySubdomain)
+		services, _, err := client.DiscoverServices(ctx)
+		require.NoError(t, err)
+		assert.Equal(t, "https://ajp5871.id.cyberarkgov.cloud", services.Identity.API)
+		assert.Equal(t, "https://venafi-test.inventory.integration-cyberarkgov.cloud", services.DiscoveryContext.API)
+		assert.Equal(t, "https://venafi-test.secretsmgr.dev-cyberarkgov.com", services.SecretsManager.API)
+	})
+
 	for name, testSpec := range tests {
 		t.Run(name, func(t *testing.T) {
 			logger := ktesting.NewLogger(t, ktesting.DefaultConfig)

internal/envelope/keyfetch/client_test.go

@@ -32,7 +32,7 @@ func testClientSetup(t *testing.T, jwksServerURL string) (*Client, cyberark.Clie
 			IsActive: true,
 			Type:     "main",
 			// Unused by the Conjur path, but service discovery requires it.
-			API: "https://identity.example.invalid",
+			API: "https://identity.example.integration-cyberark.cloud",
 		},
 		DiscoveryContext: servicediscovery.ServiceEndpoint{
 			IsActive: true,
@@ -288,7 +288,7 @@ func TestClient_FetchKey(t *testing.T) {
 				IsActive: true,
 				Type:     "main",
 				// Unused by the Conjur path, but service discovery requires it.
-				API: "https://identity.example.invalid",
+				API: "https://identity.example.integration-cyberark.cloud",
 			},
 			DiscoveryContext: servicediscovery.ServiceEndpoint{
 				IsActive: true,
@@ -334,7 +334,7 @@ func TestClient_FetchKey(t *testing.T) {
 				IsActive: true,
 				Type:     "main",
 				// Unused by the Conjur path, but service discovery requires it.
-				API: "https://identity.example.invalid",
+				API: "https://identity.example.integration-cyberark.cloud",
 			},
 			SecretsManager: servicediscovery.ServiceEndpoint{
 				IsActive: true,

pkg/testutil/envtest.go

@@ -290,8 +290,9 @@ func FakeCyberArk(t testing.TB) (httpClient *http.Client, jwtFilePath string) {
 		Identity: servicediscovery.ServiceEndpoint{
 			// Required unconditionally by DiscoverServices, present for every
 			// healthy tenant — see servicediscovery/discovery.go. Unused by
-			// the Conjur path itself.
-			API: "https://identity.example.invalid",
+			// the Conjur path itself. Never dialed, so it just needs a host
+			// servicediscovery's allowlist accepts.
+			API: "https://identity.example.integration-cyberark.cloud",
 		},
 		DiscoveryContext: servicediscovery.ServiceEndpoint{
 			API: discoveryContextAPI,