From 05ea1334030d510278f3436ae0dc348d2fdbf307 Mon Sep 17 00:00:00 2001
From: rzisholz <rzisholz@paloaltonetworks.com>
Date: Thu, 3 Sep 2026 17:54:11 +0300
Subject: [PATCH 1/5] CP-25960, CP-25964: agent-side hardening from the Conjur
 auth SCR
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CP-25960 (AG1, Medium): service discovery's identity/discoverycontext/
secrets_manager API hosts were 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 that host with no allowlist — an
SSRF-shaped hole if the discovery response is ever tampered with. Mirrors
finding R2 already fixed on the discoverycontext-regional-resources side
(token.py's iss-host allowlist). Constrain each discovered host to a known
CyberArk root domain, or to the same host we already made the successful,
TLS-authenticated discovery call to (covers the ARK_DISCOVERY_API dev/CI
override and same-origin test mocks without a per-env map). A dropped host
is treated the same as one absent from the response.

CP-25964 (A1, Low): the authn-jwt exchange error included up to 4KiB of
Conjur's response body, which propagates to a Kubernetes Pod Event
(pkg/agent/run.go's PushingErr notification) — readable by anyone with `get
events` in the namespace. The body can contain Conjur policy structure,
service IDs and host identities. Log it at V(2) instead; the returned error
(and therefore the Event) now carries only the status code and the existing
troubleshooting hint.

SCR: https://ca-il-confluence.il.cyber-ark.com/pages/viewpage.action?pageId=710861530
---
 internal/cyberark/client_test.go              |  3 +-
 internal/cyberark/conjur/conjur.go            | 17 +++--
 internal/cyberark/conjur/conjur_test.go       | 12 +++-
 .../cyberark/servicediscovery/discovery.go    | 70 +++++++++++++++++++
 .../servicediscovery/discovery_test.go        | 46 ++++++++++++
 internal/envelope/keyfetch/client_test.go     |  6 +-
 pkg/testutil/envtest.go                       |  5 +-
 7 files changed, 145 insertions(+), 14 deletions(-)

diff --git a/internal/cyberark/client_test.go b/internal/cyberark/client_test.go
index 9b64d543..06b187a6 100644
--- a/internal/cyberark/client_test.go
+++ b/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{
diff --git a/internal/cyberark/conjur/conjur.go b/internal/cyberark/conjur/conjur.go
index 08fc464b..64333a42 100644
--- a/internal/cyberark/conjur/conjur.go
+++ b/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 {
diff --git a/internal/cyberark/conjur/conjur_test.go b/internal/cyberark/conjur/conjur_test.go
index 8cab98e4..8069a4c4 100644
--- a/internal/cyberark/conjur/conjur_test.go
+++ b/internal/cyberark/conjur/conjur_test.go
@@ -213,7 +213,14 @@ 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()
 
@@ -221,5 +228,6 @@ func TestAuthenticateRequest_ExchangeErrorIncludesConjurResponseBody(t *testing.
 	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)")
 }
diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go
index bfbd66dd..c1bd68fc 100644
--- a/internal/cyberark/servicediscovery/discovery.go
+++ b/internal/cyberark/servicediscovery/discovery.go
@@ -9,11 +9,13 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"strings"
 	"sync"
 	"time"
 
 	arkapi "github.com/jetstack/preflight/internal/cyberark/api"
 	"github.com/jetstack/preflight/pkg/version"
+	"k8s.io/klog/v2"
 )
 
 const (
@@ -40,6 +42,66 @@ 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.
+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",
+}
+
+// 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 +257,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
diff --git a/internal/cyberark/servicediscovery/discovery_test.go b/internal/cyberark/servicediscovery/discovery_test.go
index 23c2f1b6..ec9afc8f 100644
--- a/internal/cyberark/servicediscovery/discovery_test.go
+++ b/internal/cyberark/servicediscovery/discovery_test.go
@@ -50,6 +50,52 @@ 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)
+	})
+
 	for name, testSpec := range tests {
 		t.Run(name, func(t *testing.T) {
 			logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
diff --git a/internal/envelope/keyfetch/client_test.go b/internal/envelope/keyfetch/client_test.go
index f8406ff0..b7f9f282 100644
--- a/internal/envelope/keyfetch/client_test.go
+++ b/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,
diff --git a/pkg/testutil/envtest.go b/pkg/testutil/envtest.go
index 9a49588a..3b65a200 100644
--- a/pkg/testutil/envtest.go
+++ b/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,

From c50afe12792c71c02ed5fe4858775104262be304 Mon Sep 17 00:00:00 2001
From: rzisholz <rzisholz@paloaltonetworks.com>
Date: Thu, 3 Sep 2026 18:24:03 +0300
Subject: [PATCH 2/5] CP-25960: add missing gov-cloud domains to the discovery
 host allowlist

The allowlist added in the previous commit was copied from the identity
authorizer Lambda's local commercial-only clone of everest_env_utils'
ROOT_DOMAIN map, not the real package. The real map (everest_env_utils_cyberark,
v2.0.117) has 8 more entries for the GOV_* environments
(*-cyberarkgov.com/.cloud) that the clone omits. Without them, any gov-cloud
tenant's agent would have every discovery-derived host rejected, breaking
identity discovery entirely (fatal, since it's required unconditionally).
---
 .../cyberark/servicediscovery/discovery.go    | 15 ++++++++++++
 .../servicediscovery/discovery_test.go        | 24 +++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go
index c1bd68fc..eab2b9c1 100644
--- a/internal/cyberark/servicediscovery/discovery.go
+++ b/internal/cyberark/servicediscovery/discovery.go
@@ -51,6 +51,12 @@ const (
 // 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",
@@ -63,6 +69,15 @@ var allowedRootDomains = []string{
 	"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
diff --git a/internal/cyberark/servicediscovery/discovery_test.go b/internal/cyberark/servicediscovery/discovery_test.go
index ec9afc8f..89520118 100644
--- a/internal/cyberark/servicediscovery/discovery_test.go
+++ b/internal/cyberark/servicediscovery/discovery_test.go
@@ -96,6 +96,30 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) {
 		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)

From 26fe262ae582aed3e825f158dc61bbbcec1fad59 Mon Sep 17 00:00:00 2001
From: rzisholz <rzisholz@paloaltonetworks.com>
Date: Thu, 3 Sep 2026 18:37:53 +0300
Subject: [PATCH 3/5] CP-25960: fix gci import ordering

CI's verify job failed: gci requires third-party imports (k8s.io/klog)
grouped separately from and before this module's own imports, per
.golangci.yaml's [standard, default, localmodule] section order.
---
 internal/cyberark/servicediscovery/discovery.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go
index eab2b9c1..623f99a0 100644
--- a/internal/cyberark/servicediscovery/discovery.go
+++ b/internal/cyberark/servicediscovery/discovery.go
@@ -13,9 +13,10 @@ import (
 	"sync"
 	"time"
 
+	"k8s.io/klog/v2"
+
 	arkapi "github.com/jetstack/preflight/internal/cyberark/api"
 	"github.com/jetstack/preflight/pkg/version"
-	"k8s.io/klog/v2"
 )
 
 const (

From c7993a98a5aa1d45fe7ab66167a2dd01ee5dc28e Mon Sep 17 00:00:00 2001
From: rzisholz <rzisholz@paloaltonetworks.com>
Date: Sun, 6 Sep 2026 14:08:59 +0300
Subject: [PATCH 4/5] CP-25960: require HTTPS on discovery-derived hosts

Per the pentest finding CP-23593 ("Broken Trust Chain in Tenant Discovery
Bootstrap"), the domain allowlist alone doesn't require HTTPS -- a discovery
response could point identity/discoverycontext/secrets_manager at a plain
HTTP endpoint on an allowlisted-looking host.

This also closes the loopback-attacker shape of the "same host as
discoveryHost" escape hatch in isAllowedServiceHost: an attacker-controlled
ARK_DISCOVERY_API pointing at e.g. 127.0.0.1 (the PoC in CP-23593) can't
present a certificate this client's TLS verification will accept, so
requiring https closes exactly the vector the PoC demonstrated even for the
same-host case.

Does not touch the discovery bootstrap call's own base URL (ARK_DISCOVERY_API
itself) -- that override is deliberately left alone for dev/CI use, per AG1's
original writeup.
---
 .../cyberark/servicediscovery/discovery.go    | 14 +++++-
 .../servicediscovery/discovery_test.go        | 46 +++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go
index 623f99a0..304e4e72 100644
--- a/internal/cyberark/servicediscovery/discovery.go
+++ b/internal/cyberark/servicediscovery/discovery.go
@@ -100,8 +100,9 @@ func isAllowedServiceHost(host, discoveryHost string) bool {
 	return false
 }
 
-// sanitizeServiceAPI returns rawAPI unchanged if its host is allowed, or ""
-// (treated the same as "service not present in the response") if not.
+// sanitizeServiceAPI returns rawAPI unchanged if its scheme is https and 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 ""
@@ -111,6 +112,15 @@ func sanitizeServiceAPI(ctx context.Context, serviceName, rawAPI, discoveryHost
 		klog.FromContext(ctx).Info("dropping unparseable service discovery API URL", "service", serviceName, "api", rawAPI)
 		return ""
 	}
+	if u.Scheme != "https" {
+		// Rejecting plain HTTP also closes the loopback-attacker shape of
+		// the isAllowedServiceHost "same host as discoveryHost" case: a
+		// same-host rogue endpoint (e.g. an attacker-controlled
+		// ARK_DISCOVERY_API pointing at 127.0.0.1) can't present a
+		// certificate this client's TLS verification will accept.
+		klog.FromContext(ctx).Info("dropping non-HTTPS service discovery API URL", "service", serviceName, "scheme", u.Scheme)
+		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 ""
diff --git a/internal/cyberark/servicediscovery/discovery_test.go b/internal/cyberark/servicediscovery/discovery_test.go
index 89520118..9443b2c8 100644
--- a/internal/cyberark/servicediscovery/discovery_test.go
+++ b/internal/cyberark/servicediscovery/discovery_test.go
@@ -96,6 +96,52 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) {
 		assert.Equal(t, "", services.SecretsManager.API)
 	})
 
+	t.Run("plain-HTTP identity host is rejected even though the hostname is allowlisted", func(t *testing.T) {
+		logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+		ctx := klog.NewContext(t.Context(), logger)
+
+		httpClient := MockDiscoveryServer(t, Services{
+			Identity: ServiceEndpoint{
+				API: "http://ajp5871.id.integration-cyberark.cloud",
+			},
+			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("plain-HTTP secrets_manager and discovery_context hosts 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: "http://venafi-test.inventory.integration-cyberark.cloud",
+			},
+			SecretsManager: ServiceEndpoint{
+				API: "http://venafi-test.secretsmgr.integration-cyberark.cloud",
+			},
+		})
+
+		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)

From 6fea54238f32302f3ced68fcc6e1e53bcdeb5176 Mon Sep 17 00:00:00 2001
From: rzisholz <rzisholz@paloaltonetworks.com>
Date: Tue, 8 Sep 2026 13:30:32 +0300
Subject: [PATCH 5/5] CP-25960: address review feedback from wallrj-cyberark

- Case-insensitive host comparison (net/url doesn't lowercase the host,
  only the scheme -- verified). A discovery response with any uppercase in
  an otherwise-legitimate hostname was being dropped, with identity_administration
  hitting the misleading "suspended tenant" error as a result.
- Distinguish "identity_administration absent from the response" from
  "present but rejected by the allowlist" in the fatal error -- same
  underlying bug, different accurate message.
- keyfetch.FetchKey now rejects an empty DiscoveryContext.API explicitly
  (mirrors cyberark.NewDatauploadClient's existing check on the same
  field), instead of surfacing url.JoinPath's silent empty-string fallout
  as an opaque "unsupported protocol scheme \"\"".
- keyfetch.FetchKey no longer embeds the JWKS endpoint's raw error body in
  the returned error -- same leak class as CP-25964 (conjur.go's authn-jwt
  exchange), against the discoverycontext host instead of the Conjur host.
  Logged at V(2) instead. This was flagged as in-scope-but-outside-the-diff
  since keyfetch/client.go wasn't otherwise touched by CP-25960; fixed here
  rather than deferred, since it's the same mechanical change as CP-25964.
- Test placeholders that are never dialed (Identity.API in several test
  setups) now use a loopback literal instead of a name under a real,
  resolvable CyberArk zone -- preserves the hermeticity property those
  values are supposed to have.
- Corrected the isAllowedServiceHost doc comment: ARK_DISCOVERY_API isn't
  guaranteed HTTPS, and the same-host escape hatch being hostname-only
  (port-unconstrained) is real -- both are actually fixed by CP-26002's
  restructuring (already open as a stacked PR), not by a partial patch
  here, since a host:port comparison alone would break every test relying
  on same-host-different-port mocks without that same rework.

Not addressed here, tracked separately as a new subtask: the tenant-scoping
residual on the allowlist itself (cyberark.cloud admits every tenant's
host, so a tampered response can redirect the SA-token POST to a different
tenant's secretsmgr/id host) -- a real design decision on whether to add
tenant-id pinning, not a mechanical fix.
---
 internal/cyberark/client_test.go              |  6 ++-
 .../cyberark/servicediscovery/discovery.go    | 37 ++++++++++++++-----
 .../servicediscovery/discovery_test.go        | 27 ++++++++++++++
 internal/envelope/keyfetch/client.go          | 20 +++++++++-
 internal/envelope/keyfetch/client_test.go     | 28 ++++++++++++--
 pkg/testutil/envtest.go                       |  9 +++--
 6 files changed, 109 insertions(+), 18 deletions(-)

diff --git a/internal/cyberark/client_test.go b/internal/cyberark/client_test.go
index 06b187a6..b52e3507 100644
--- a/internal/cyberark/client_test.go
+++ b/internal/cyberark/client_test.go
@@ -40,8 +40,10 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) {
 	discoveryContextAPI, _ := dataupload.MockDataUploadServer(t)
 
 	// Unused by the Conjur path, but service discovery requires it to be set.
-	// Never dialed, so it just needs a host servicediscovery's allowlist accepts.
-	const identitySrv = "https://identity.example.integration-cyberark.cloud"
+	// Never dialed, so a loopback address is fine — see pkg/testutil/envtest.go's
+	// identical const for why this is preferred over a real, resolvable
+	// CyberArk zone name.
+	const identitySrv = "https://127.0.0.1:1"
 
 	httpClient := servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{
 		Identity: servicediscovery.ServiceEndpoint{
diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go
index 304e4e72..b6e27809 100644
--- a/internal/cyberark/servicediscovery/discovery.go
+++ b/internal/cyberark/servicediscovery/discovery.go
@@ -82,16 +82,26 @@ var allowedRootDomains = []string{
 }
 
 // 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.
+// allowedRootDomains — or is exactly discoveryHost, the host the discovery
+// request was addressed to. The latter matters for ARK_DISCOVERY_API-
+// overridden (dev/CI/test) discovery endpoints: whatever host that override
+// already points at is at least as trusted as the discovery call itself.
+//
+// This comparison is hostname-only: discoveryHost carries no port, and
+// ARK_DISCOVERY_API is not guaranteed to be HTTPS or to be the host the
+// request actually landed on after redirects — the follow-up in CP-26002
+// (validating ARK_DISCOVERY_API itself against allowedRootDomains, removing
+// this whole escape hatch) is the actual fix for both gaps; a host:port
+// comparison here alone would break every test that currently relies on
+// same-host-different-port mocks without that same rework.
 func isAllowedServiceHost(host, discoveryHost string) bool {
-	if host == discoveryHost {
+	// DNS is case-insensitive and net/url doesn't normalise host case (it
+	// lowercases the scheme but not the host), so a discovery response with
+	// any uppercase in a legitimate hostname must still match here.
+	if strings.EqualFold(host, discoveryHost) {
 		return true
 	}
+	host = strings.ToLower(host)
 	for _, root := range allowedRootDomains {
 		if host == root || strings.HasSuffix(host, "."+root) {
 			return true
@@ -287,6 +297,7 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error
 	// 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.
+	rawIdentityAPI := identityAPI
 	identityAPI = sanitizeServiceAPI(ctx, IdentityServiceName, identityAPI, u.Hostname())
 	discoveryContextAPI = sanitizeServiceAPI(ctx, DiscoveryContextServiceName, discoveryContextAPI, u.Hostname())
 	secretsManagerAPI = sanitizeServiceAPI(ctx, SecretsManagerServiceName, secretsManagerAPI, u.Hostname())
@@ -296,8 +307,16 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error
 	// tenant, so callers may rely on it being non-empty without checking it
 	// themselves again.
 	if identityAPI == "" {
-		return nil, "", fmt.Errorf("didn't find %s in service discovery response, "+
-			"which may indicate a suspended tenant; unable to detect CyberArk Identity API URL", IdentityServiceName)
+		if rawIdentityAPI == "" {
+			return nil, "", fmt.Errorf("didn't find %s in service discovery response, "+
+				"which may indicate a suspended tenant; unable to detect CyberArk Identity API URL", IdentityServiceName)
+		}
+		// The response did name an identity_administration endpoint, but its
+		// host isn't on our allowlist — a distinct, more actionable failure
+		// than "suspended tenant" (see sanitizeServiceAPI's Info log for
+		// which host was rejected and why).
+		return nil, "", fmt.Errorf("%s endpoint %q is not on an allowed CyberArk domain over HTTPS; refusing to use it",
+			IdentityServiceName, rawIdentityAPI)
 	}
 	// discoveryContextAPI and secretsManagerAPI are deliberately not required
 	// here, unlike identityAPI above: not every caller needs both, and
diff --git a/internal/cyberark/servicediscovery/discovery_test.go b/internal/cyberark/servicediscovery/discovery_test.go
index 9443b2c8..61fe7787 100644
--- a/internal/cyberark/servicediscovery/discovery_test.go
+++ b/internal/cyberark/servicediscovery/discovery_test.go
@@ -70,6 +70,11 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) {
 		services, _, err := client.DiscoverServices(ctx)
 		require.Error(t, err)
 		assert.Nil(t, services)
+		// The error must say the host was rejected, not the unrelated
+		// "suspended tenant" message reserved for a genuinely absent
+		// identity_administration entry (see the no-identity-in-response
+		// case in the tests map above, which still gets that message).
+		assert.Contains(t, err.Error(), "not on an allowed CyberArk domain")
 	})
 
 	t.Run("secrets_manager and discovery_context hosts outside the allowed CyberArk domains are dropped, not fatal", func(t *testing.T) {
@@ -166,6 +171,28 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) {
 		assert.Equal(t, "https://venafi-test.secretsmgr.dev-cyberarkgov.com", services.SecretsManager.API)
 	})
 
+	t.Run("uppercase host is still accepted (net/url doesn't lowercase the host)", 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.Integration-CyberArk.Cloud",
+			},
+			DiscoveryContext: ServiceEndpoint{
+				API: mockDiscoveryContextAPIURL,
+			},
+			SecretsManager: ServiceEndpoint{
+				API: mockSecretsManagerAPIURL,
+			},
+		})
+
+		client := New(httpClient, MockDiscoverySubdomain)
+		services, _, err := client.DiscoverServices(ctx)
+		require.NoError(t, err)
+		assert.Equal(t, "https://AJP5871.ID.Integration-CyberArk.Cloud", services.Identity.API)
+	})
+
 	for name, testSpec := range tests {
 		t.Run(name, func(t *testing.T) {
 			logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
diff --git a/internal/envelope/keyfetch/client.go b/internal/envelope/keyfetch/client.go
index 370c61f7..cbbadeb1 100644
--- a/internal/envelope/keyfetch/client.go
+++ b/internal/envelope/keyfetch/client.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/rsa"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -118,6 +119,16 @@ func (c *Client) FetchKey(ctx context.Context) (PublicKey, error) {
 		return PublicKey{}, fmt.Errorf("failed to get services from discovery client: %w", err)
 	}
 
+	if services.DiscoveryContext.API == "" {
+		// Mirrors cyberark.NewDatauploadClient's check on the same field —
+		// without this, url.JoinPath("", ...) silently returns a relative
+		// path and the request below fails with the opaque
+		// "unsupported protocol scheme \"\"" instead of naming the real cause
+		// (service discovery didn't return a discoverycontext endpoint, e.g.
+		// because its host isn't on the allowed CyberArk domain list).
+		return PublicKey{}, errors.New("service discovery returned an empty discovery API")
+	}
+
 	endpoint, err := url.JoinPath(services.DiscoveryContext.API, "discovery-context/jwks")
 	if err != nil {
 		return PublicKey{}, fmt.Errorf("failed to construct endpoint URL: %w", err)
@@ -143,8 +154,15 @@ func (c *Client) FetchKey(ctx context.Context) (PublicKey, error) {
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
+		// The response body isn't included in the returned error — same leak
+		// class as CP-25964 (conjur.go's authn-jwt exchange error), just
+		// against the discoverycontext host instead. This error doesn't
+		// currently reach a Pod Event (only postData failures do, via
+		// eventf), but keep the body out of it so that stays true if the
+		// call sites ever change.
 		body, _ := io.ReadAll(io.LimitReader(resp.Body, maxResponseBytes))
-		return PublicKey{}, fmt.Errorf("unexpected status code %d from %s: %s", resp.StatusCode, endpoint, string(body))
+		logger.V(2).Info("unexpected status code fetching JWKS", "statusCode", resp.StatusCode, "endpoint", endpoint, "body", string(body))
+		return PublicKey{}, fmt.Errorf("unexpected status code %d from %s", resp.StatusCode, endpoint)
 	}
 
 	body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBytes))
diff --git a/internal/envelope/keyfetch/client_test.go b/internal/envelope/keyfetch/client_test.go
index b7f9f282..d1b88849 100644
--- a/internal/envelope/keyfetch/client_test.go
+++ b/internal/envelope/keyfetch/client_test.go
@@ -32,7 +32,10 @@ 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.integration-cyberark.cloud",
+			// Never dialed, so a loopback address is fine — passes the
+			// allowlist via the same-host-as-discovery escape hatch rather
+			// than depending on a real, resolvable CyberArk zone name.
+			API: "https://127.0.0.1:1",
 		},
 		DiscoveryContext: servicediscovery.ServiceEndpoint{
 			IsActive: true,
@@ -288,7 +291,9 @@ func TestClient_FetchKey(t *testing.T) {
 				IsActive: true,
 				Type:     "main",
 				// Unused by the Conjur path, but service discovery requires it.
-				API: "https://identity.example.integration-cyberark.cloud",
+				// Never dialed, so a loopback address is fine — see the
+				// comment on the identical field above.
+				API: "https://127.0.0.1:1",
 			},
 			DiscoveryContext: servicediscovery.ServiceEndpoint{
 				IsActive: true,
@@ -334,7 +339,9 @@ func TestClient_FetchKey(t *testing.T) {
 				IsActive: true,
 				Type:     "main",
 				// Unused by the Conjur path, but service discovery requires it.
-				API: "https://identity.example.integration-cyberark.cloud",
+				// Never dialed, so a loopback address is fine — see the
+				// comment on the identical field above.
+				API: "https://127.0.0.1:1",
 			},
 			SecretsManager: servicediscovery.ServiceEndpoint{
 				IsActive: true,
@@ -449,6 +456,21 @@ func TestClient_FetchKey(t *testing.T) {
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "no valid RSA keys found")
 	})
+
+	t.Run("empty discoverycontext API from discovery fails with a clear error, not an opaque one", func(t *testing.T) {
+		// An empty jwksServerURL makes DiscoveryContext.API "" in the
+		// discovery response (mainActiveAPI requires a non-empty api field,
+		// same as a genuinely absent/inactive service) — e.g. because its
+		// host wasn't on the allowed CyberArk domain list. Without the
+		// explicit check this fails with url.JoinPath("", ...) producing a
+		// relative path and client.Do erroring with the opaque
+		// `unsupported protocol scheme ""`.
+		client, _ := testClientSetup(t, "")
+		_, err := client.FetchKey(t.Context())
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "service discovery returned an empty discovery API")
+	})
 }
 
 // TestClient_FetchKey_UsernamePasswordAuth covers the legacy auth path, which
diff --git a/pkg/testutil/envtest.go b/pkg/testutil/envtest.go
index 3b65a200..226da2d2 100644
--- a/pkg/testutil/envtest.go
+++ b/pkg/testutil/envtest.go
@@ -290,9 +290,12 @@ 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. Never dialed, so it just needs a host
-			// servicediscovery's allowlist accepts.
-			API: "https://identity.example.integration-cyberark.cloud",
+			// the Conjur path itself. Never dialed, so a loopback address is
+			// fine — it passes the allowlist via the same-host-as-discovery
+			// escape hatch (MockDiscoveryServer's ARK_DISCOVERY_API is also
+			// 127.0.0.1) without depending on a real, resolvable CyberArk
+			// zone name for something this test never intends to reach.
+			API: "https://127.0.0.1:1",
 		},
 		DiscoveryContext: servicediscovery.ServiceEndpoint{
 			API: discoveryContextAPI,