Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion hack/govulncheck.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ set -o errexit
# - https://pkg.go.dev/vuln/GO-2026-4340 - Handshake messages may be processed at the incorrect encryption level in crypto/tls
# - https://pkg.go.dev/vuln/GO-2025-4175 - Improper application of excluded DNS name constraints when verifying wildcard names in crypto/x509
# - https://pkg.go.dev/vuln/GO-2025-4155 - Excessive resource consumption when printing error string for host certificate validation in crypto/x509
KNOWN_VULNS_PATTERN="GO-2025-3547|GO-2025-3521|GO-2025-4240|GO-2026-4341|GO-2026-4340|GO-2025-4175|GO-2025-4155"
# - https://pkg.go.dev/vuln/GO-2026-4337 - During session resumption in crypto/tls, if the underlying Config has its ClientCAs or RootCAs fields mutated between the initial handshake and the resumed handshake, the resumed handshake may succeed when it should have failed.
KNOWN_VULNS_PATTERN="GO-2025-3547|GO-2025-3521|GO-2025-4240|GO-2026-4341|GO-2026-4340|GO-2025-4175|GO-2025-4155|GO-2026-4337"

GOVULNCHECK_BIN="${1:-}"
OUTPUT_DIR="${2:-}"
Expand Down
44 changes: 25 additions & 19 deletions pkg/controller/external_secrets/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,25 +110,31 @@ const (
controllerDeploymentAssetName = "external-secrets/resources/deployment_external-secrets.yml"
certControllerDeploymentAssetName = "external-secrets/resources/deployment_external-secrets-cert-controller.yml"
webhookDeploymentAssetName = "external-secrets/resources/deployment_external-secrets-webhook.yml"
controllerRoleLeaderElectionAssetName = "external-secrets/resources/role_external-secrets-leaderelection.yml"
controllerRoleBindingLeaderElectionAssetName = "external-secrets/resources/rolebinding_external-secrets-leaderelection.yml"
webhookTLSSecretAssetName = "external-secrets/resources/secret_external-secrets-webhook.yml"
bitwardenServiceAssetName = "external-secrets/resources/service_bitwarden-sdk-server.yml"
webhookServiceAssetName = "external-secrets/resources/service_external-secrets-webhook.yml"
metricsServiceAssetName = "external-secrets/resources/service_external-secrets-metrics.yml"
certControllerMetricsServiceAssetName = "external-secrets/resources/service_external-secrets-cert-controller-metrics.yml"
controllerServiceAccountAssetName = "external-secrets/resources/serviceaccount_external-secrets.yml"
bitwardenServiceAccountAssetName = "external-secrets/resources/serviceaccount_bitwarden-sdk-server.yml"
certControllerServiceAccountAssetName = "external-secrets/resources/serviceaccount_external-secrets-cert-controller.yml"
webhookServiceAccountAssetName = "external-secrets/resources/serviceaccount_external-secrets-webhook.yml"
validatingWebhookExternalSecretCRDAssetName = "external-secrets/resources/validatingwebhookconfiguration_externalsecret-validate.yml"
validatingWebhookSecretStoreCRDAssetName = "external-secrets/resources/validatingwebhookconfiguration_secretstore-validate.yml"
denyAllNetworkPolicyAssetName = "external-secrets/networkpolicy_deny-all.yaml"
allowMainControllerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-main-controller-traffic.yaml"
allowWebhookTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-and-webhook-traffic.yaml"
allowCertControllerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-cert-controller-traffic.yaml"
allowBitwardenServerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-bitwarden-sever.yaml"
allowDnsTrafficAsserName = "external-secrets/networkpolicy_allow-dns.yaml"

// Container names for each component deployment
controllerContainerName = "external-secrets"
webhookContainerName = "webhook"
certControllerContainerName = "cert-controller"
bitwardenContainerName = "bitwarden-sdk-server"
Comment on lines +115 to +118
Copy link
Contributor

@bharath-b-rh bharath-b-rh Feb 19, 2026

Choose a reason for hiding this comment

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

This doesn't look right. These consts can be defined in this file along with others after line 77 with proper comment.

controllerRoleLeaderElectionAssetName = "external-secrets/resources/role_external-secrets-leaderelection.yml"
controllerRoleBindingLeaderElectionAssetName = "external-secrets/resources/rolebinding_external-secrets-leaderelection.yml"
webhookTLSSecretAssetName = "external-secrets/resources/secret_external-secrets-webhook.yml"
bitwardenServiceAssetName = "external-secrets/resources/service_bitwarden-sdk-server.yml"
webhookServiceAssetName = "external-secrets/resources/service_external-secrets-webhook.yml"
metricsServiceAssetName = "external-secrets/resources/service_external-secrets-metrics.yml"
certControllerMetricsServiceAssetName = "external-secrets/resources/service_external-secrets-cert-controller-metrics.yml"
controllerServiceAccountAssetName = "external-secrets/resources/serviceaccount_external-secrets.yml"
bitwardenServiceAccountAssetName = "external-secrets/resources/serviceaccount_bitwarden-sdk-server.yml"
certControllerServiceAccountAssetName = "external-secrets/resources/serviceaccount_external-secrets-cert-controller.yml"
webhookServiceAccountAssetName = "external-secrets/resources/serviceaccount_external-secrets-webhook.yml"
validatingWebhookExternalSecretCRDAssetName = "external-secrets/resources/validatingwebhookconfiguration_externalsecret-validate.yml"
validatingWebhookSecretStoreCRDAssetName = "external-secrets/resources/validatingwebhookconfiguration_secretstore-validate.yml"
denyAllNetworkPolicyAssetName = "external-secrets/networkpolicy_deny-all.yaml"
allowMainControllerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-main-controller-traffic.yaml"
allowWebhookTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-and-webhook-traffic.yaml"
allowCertControllerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-cert-controller-traffic.yaml"
allowBitwardenServerTrafficAssetName = "external-secrets/networkpolicy_allow-api-server-egress-for-bitwarden-sever.yaml"
allowDnsTrafficAsserName = "external-secrets/networkpolicy_allow-dns.yaml"
)

var (
Expand Down
51 changes: 43 additions & 8 deletions pkg/controller/external_secrets/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ func (r *Reconciler) removeTrustedCAVolumeMount(container *corev1.Container) {

// applyUserDeploymentConfigs updates the deployment resource spec with user specified configurations.
func (r *Reconciler) applyUserDeploymentConfigs(deployment *appsv1.Deployment, esc *operatorv1alpha1.ExternalSecretsConfig, assetName string) error {
componentName, err := getComponentNameFromAsset(assetName)
componentName, containerName, err := getComponentNameFromAsset(assetName)
if err != nil {
return err
}
Expand All @@ -672,25 +672,60 @@ func (r *Reconciler) applyUserDeploymentConfigs(deployment *appsv1.Deployment, e
if i.DeploymentConfigs != nil && i.DeploymentConfigs.RevisionHistoryLimit != nil {
deployment.Spec.RevisionHistoryLimit = i.DeploymentConfigs.RevisionHistoryLimit
}

// Apply OverrideEnv if set
if len(i.OverrideEnv) > 0 {
for j := range deployment.Spec.Template.Spec.Containers {
if deployment.Spec.Template.Spec.Containers[j].Name == containerName {
mergeEnvVars(&deployment.Spec.Template.Spec.Containers[j], i.OverrideEnv)
break
}
}
// Apply to all init containers in the deployment without name filtering,
for j := range deployment.Spec.Template.Spec.InitContainers {
mergeEnvVars(&deployment.Spec.Template.Spec.InitContainers[j], i.OverrideEnv)
}
}
Comment on lines +676 to +688
Copy link

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

OverrideEnv is applied to all init containers—potential secret leakage.
OverrideEnv is documented as applying to the component container; pushing the same env into every init container can expose secrets or alter init behavior unexpectedly. Consider restricting to the target container, or explicitly extending the API/CRD docs and validating this behavior.

🛠️ Option: restrict overrideEnv to the target container only
-            // Apply to all init containers in the deployment without name filtering,
-            for j := range deployment.Spec.Template.Spec.InitContainers {
-                mergeEnvVars(&deployment.Spec.Template.Spec.InitContainers[j], i.OverrideEnv)
-            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Apply OverrideEnv if set
if len(i.OverrideEnv) > 0 {
for j := range deployment.Spec.Template.Spec.Containers {
if deployment.Spec.Template.Spec.Containers[j].Name == containerName {
mergeEnvVars(&deployment.Spec.Template.Spec.Containers[j], i.OverrideEnv)
break
}
}
// Apply to all init containers in the deployment without name filtering,
for j := range deployment.Spec.Template.Spec.InitContainers {
mergeEnvVars(&deployment.Spec.Template.Spec.InitContainers[j], i.OverrideEnv)
}
}
// Apply OverrideEnv if set
if len(i.OverrideEnv) > 0 {
for j := range deployment.Spec.Template.Spec.Containers {
if deployment.Spec.Template.Spec.Containers[j].Name == containerName {
mergeEnvVars(&deployment.Spec.Template.Spec.Containers[j], i.OverrideEnv)
break
}
}
}
🤖 Prompt for AI Agents
In `@pkg/controller/external_secrets/deployments.go` around lines 676 - 688, The
current code applies OverrideEnv to every init container
(deployment.Spec.Template.Spec.InitContainers), risking secret leakage; change
the second loop to only merge when the init container's Name matches the target
containerName (i.e., check InitContainers[j].Name == containerName before
calling mergeEnvVars) or remove applying OverrideEnv to init containers entirely
if the intent is to only target the component container; update any related
comments to reflect the new behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

@siddhibhor-56 I think we should consider, though I had suggested we consider init containers.

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

break
}
}

return nil
}

// getComponentNameFromAsset maps asset file names to ComponentName enum values.
func getComponentNameFromAsset(assetName string) (operatorv1alpha1.ComponentName, error) {
// mergeEnvVars merges user-defined environment variables into a container, User-defined values take precedence over existing values.
func mergeEnvVars(container *corev1.Container, overrideEnv []corev1.EnvVar) {
if container.Env == nil {
container.Env = []corev1.EnvVar{}
}

for _, override := range overrideEnv {
found := false
for i, existing := range container.Env {
if existing.Name == override.Name {
container.Env[i] = override // User-defined value takes precedence
found = true
break
}
}
if !found {
container.Env = append(container.Env, override)
}
}
}

// getComponentNameFromAsset maps asset file names to ComponentName enum values and container names.
func getComponentNameFromAsset(assetName string) (operatorv1alpha1.ComponentName, string, error) {
switch assetName {
case controllerDeploymentAssetName:
return operatorv1alpha1.CoreController, nil
return operatorv1alpha1.CoreController, controllerContainerName, nil
case webhookDeploymentAssetName:
return operatorv1alpha1.Webhook, nil
return operatorv1alpha1.Webhook, webhookContainerName, nil
case certControllerDeploymentAssetName:
return operatorv1alpha1.CertController, nil
return operatorv1alpha1.CertController, certControllerContainerName, nil
case bitwardenDeploymentAssetName:
return operatorv1alpha1.BitwardenSDKServer, nil
return operatorv1alpha1.BitwardenSDKServer, bitwardenContainerName, nil
default:
return "", fmt.Errorf("unknown deployment asset name: %s", assetName)
return "", "", fmt.Errorf("unknown deployment asset name: %s", assetName)
}
}
Loading