From 834152573966d725b6d6e6cf7aacbd889282fd16 Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Mon, 27 Jul 2026 18:21:37 +0530 Subject: [PATCH 1/5] Remove service token from secret_path for remote ES --- internal/pkg/api/handleCheckin.go | 13 +++- internal/pkg/policy/parsed_policy.go | 6 ++ internal/pkg/policy/parsed_policy_test.go | 29 ++++++++ .../remote_es_policy_with_secrets.json | 69 +++++++++++++++++++ 4 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 internal/pkg/policy/testdata/remote_es_policy_with_secrets.json diff --git a/internal/pkg/api/handleCheckin.go b/internal/pkg/api/handleCheckin.go index 3af0613d92..0ab50d00e9 100644 --- a/internal/pkg/api/handleCheckin.go +++ b/internal/pkg/api/handleCheckin.go @@ -903,13 +903,20 @@ func processPolicy(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, a } data := model.ClonePolicyData(pp.Policy.Data) - for _, policyOutput := range data.Outputs { + for name, policyOutput := range data.Outputs { // NOTE: Not sure if output secret keys collected here include new entries, but they are collected for completeness + outputType, _ := policyOutput["type"].(string) ks, err := secret.ProcessOutputSecret(policyOutput, secretValues) if err != nil { - return nil, fmt.Errorf("failed to process output secret for output %q: %w", policyOutput["name"], err) + return nil, fmt.Errorf("failed to process output secret for output %q: %w", name, err) + } + for _, key := range ks { + // Remote ES service_token is stripped in Prepare; omit from secret_paths + if outputType == policy.OutputTypeRemoteElasticsearch && key == policy.FieldOutputServiceToken { + continue + } + pp.SecretKeys = append(pp.SecretKeys, "outputs."+name+"."+key) } - pp.SecretKeys = append(pp.SecretKeys, ks...) } // Iterate through the policy outputs and prepare them for _, policyOutput := range pp.Outputs { diff --git a/internal/pkg/policy/parsed_policy.go b/internal/pkg/policy/parsed_policy.go index d1d1bc0ad3..a5ed3aa565 100644 --- a/internal/pkg/policy/parsed_policy.go +++ b/internal/pkg/policy/parsed_policy.go @@ -77,11 +77,17 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa return nil, err } for name, policyOutput := range p.Data.Outputs { + outputType, _ := policyOutput[FieldOutputType].(string) ks, err := secret.ProcessOutputSecret(policyOutput, secretValues) if err != nil { return nil, fmt.Errorf("failed to replace secrets in output section of policy '%s': %w", name, err) } for _, key := range ks { + // Do not advertise remote ES service_token in secret_paths; Prepare(...) deletes it + // before the policy is sent to agents + if outputType == OutputTypeRemoteElasticsearch && key == FieldOutputServiceToken { + continue + } secretKeys = append(secretKeys, "outputs."+name+"."+key) } } diff --git a/internal/pkg/policy/parsed_policy_test.go b/internal/pkg/policy/parsed_policy_test.go index 9fc8ab6583..f4ab18699e 100644 --- a/internal/pkg/policy/parsed_policy_test.go +++ b/internal/pkg/policy/parsed_policy_test.go @@ -31,6 +31,9 @@ var logstashOutputPolicy string //go:embed testdata/remote_es_policy.json var testPolicyRemoteES string +//go:embed testdata/remote_es_policy_with_secrets.json +var testPolicyRemoteESWithSecrets string + //go:embed testdata/policy_with_secrets_mixed.json var policyWithSecretsMixed string @@ -111,6 +114,32 @@ func TestNewParsedPolicyRemoteES(t *testing.T) { require.Equal(t, "remote", pp.Default.Name) } +// TestNewParsedPolicyRemoteESServiceTokenSecret ensures that a remote Elasticsearch +// output with secrets.service_token: +// 1. Resolves the service_token value (needed by fleet-server to create API keys) +// 2. Does NOT list outputs..service_token in SecretKeys — that field is +// stripped before the policy is sent to agents +func TestNewParsedPolicyRemoteESServiceTokenSecret(t *testing.T) { + var m model.Policy + var d model.PolicyData + err := json.Unmarshal([]byte(testPolicyRemoteESWithSecrets), &d) + require.NoError(t, err) + m.Data = &d + + bulker := ftesting.NewMockBulk() + pp, err := NewParsedPolicy(t.Context(), bulker, m) + require.NoError(t, err) + + remote := pp.Policy.Data.Outputs["OUTPUT_ID"] + require.Equal(t, "SERVICE_TOKEN_ID_value", remote["service_token"]) + require.Equal(t, "SSL_KEY_ID_value", remote["ssl"].(map[string]any)["key"]) + _, hasSecrets := remote["secrets"] + require.False(t, hasSecrets, "secrets map should be removed after processing") + + require.NotContains(t, pp.SecretKeys, "outputs.OUTPUT_ID.service_token") + require.Contains(t, pp.SecretKeys, "outputs.OUTPUT_ID.ssl.key") +} + // TestParsedPolicyMixedSecretsReplacement tests that secrets specified in a policy // using either the `secrets...id:` format or the // `: $co.elastic.secret{}` format are both replaced correctly. diff --git a/internal/pkg/policy/testdata/remote_es_policy_with_secrets.json b/internal/pkg/policy/testdata/remote_es_policy_with_secrets.json new file mode 100644 index 0000000000..d4f01d75e7 --- /dev/null +++ b/internal/pkg/policy/testdata/remote_es_policy_with_secrets.json @@ -0,0 +1,69 @@ +{ + "id": "remote-with-secrets", + "revision": 1, + "outputs": { + "default": { + "type": "elasticsearch", + "hosts": [ + "https://local.es.example:443" + ] + }, + "OUTPUT_ID": { + "type": "remote_elasticsearch", + "hosts": [ + "https://remote.es.example:443" + ], + "service_token": null, + "secrets": { + "service_token": { + "id": "SERVICE_TOKEN_ID" + }, + "ssl": { + "key": { + "id": "SSL_KEY_ID" + } + } + } + } + }, + "output_permissions": { + "default": { + "_fallback": { + "cluster": ["monitor"], + "indices": [ + { + "names": ["logs-*", "metrics-*"], + "privileges": ["auto_configure", "create_doc"] + } + ] + } + }, + "OUTPUT_ID": { + "_fallback": { + "cluster": ["monitor"], + "indices": [ + { + "names": ["logs-*", "metrics-*"], + "privileges": ["auto_configure", "create_doc"] + } + ] + } + } + }, + "inputs": [], + "secret_references": [ + {"id": "SERVICE_TOKEN_ID"}, + {"id": "SSL_KEY_ID"} + ], + "agent": { + "monitoring": { + "enabled": true, + "use_output": "OUTPUT_ID", + "logs": true, + "metrics": true + } + }, + "fleet": { + "hosts": ["http://localhost:8220"] + } +} From 5f4c9d220935c786b512339abca0ca69a525673a Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 31 Jul 2026 17:16:52 +0530 Subject: [PATCH 2/5] add additional guard --- internal/pkg/policy/parsed_policy.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/internal/pkg/policy/parsed_policy.go b/internal/pkg/policy/parsed_policy.go index a5ed3aa565..344046eddf 100644 --- a/internal/pkg/policy/parsed_policy.go +++ b/internal/pkg/policy/parsed_policy.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "go.elastic.co/apm/v2" @@ -82,15 +83,23 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa if err != nil { return nil, fmt.Errorf("failed to replace secrets in output section of policy '%s': %w", name, err) } + for _, key := range ks { - // Do not advertise remote ES service_token in secret_paths; Prepare(...) deletes it - // before the policy is sent to agents - if outputType == OutputTypeRemoteElasticsearch && key == FieldOutputServiceToken { - continue - } secretKeys = append(secretKeys, "outputs."+name+"."+key) } + + // Do not advertise remote ES service_token in secret_paths; Prepare(...) deletes it + // before the policy is sent to agents + if _, ok := policyOutput[FieldOutputServiceToken]; ok { + if outputType == OutputTypeRemoteElasticsearch { + prefixed := "outputs." + name + "." + FieldOutputServiceToken + secretKeys = slices.DeleteFunc(secretKeys, func(key string) bool { + return key == prefixed + }) + } + } } + defaultName, err := findDefaultOutputName(p.Data.Outputs) if err != nil { return nil, err From f0b0822c18960817d790bba8c503f36bc21ee5ba Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 31 Jul 2026 17:29:56 +0530 Subject: [PATCH 3/5] fix it --- internal/pkg/policy/parsed_policy.go | 13 +--- internal/pkg/policy/parsed_policy_test.go | 29 -------- .../remote_es_policy_with_secrets.json | 69 ------------------- 3 files changed, 1 insertion(+), 110 deletions(-) delete mode 100644 internal/pkg/policy/testdata/remote_es_policy_with_secrets.json diff --git a/internal/pkg/policy/parsed_policy.go b/internal/pkg/policy/parsed_policy.go index 344046eddf..5681ce7310 100644 --- a/internal/pkg/policy/parsed_policy.go +++ b/internal/pkg/policy/parsed_policy.go @@ -9,7 +9,6 @@ import ( "encoding/json" "errors" "fmt" - "slices" "go.elastic.co/apm/v2" @@ -78,7 +77,7 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa return nil, err } for name, policyOutput := range p.Data.Outputs { - outputType, _ := policyOutput[FieldOutputType].(string) + ks, err := secret.ProcessOutputSecret(policyOutput, secretValues) if err != nil { return nil, fmt.Errorf("failed to replace secrets in output section of policy '%s': %w", name, err) @@ -88,16 +87,6 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa secretKeys = append(secretKeys, "outputs."+name+"."+key) } - // Do not advertise remote ES service_token in secret_paths; Prepare(...) deletes it - // before the policy is sent to agents - if _, ok := policyOutput[FieldOutputServiceToken]; ok { - if outputType == OutputTypeRemoteElasticsearch { - prefixed := "outputs." + name + "." + FieldOutputServiceToken - secretKeys = slices.DeleteFunc(secretKeys, func(key string) bool { - return key == prefixed - }) - } - } } defaultName, err := findDefaultOutputName(p.Data.Outputs) diff --git a/internal/pkg/policy/parsed_policy_test.go b/internal/pkg/policy/parsed_policy_test.go index f4ab18699e..9fc8ab6583 100644 --- a/internal/pkg/policy/parsed_policy_test.go +++ b/internal/pkg/policy/parsed_policy_test.go @@ -31,9 +31,6 @@ var logstashOutputPolicy string //go:embed testdata/remote_es_policy.json var testPolicyRemoteES string -//go:embed testdata/remote_es_policy_with_secrets.json -var testPolicyRemoteESWithSecrets string - //go:embed testdata/policy_with_secrets_mixed.json var policyWithSecretsMixed string @@ -114,32 +111,6 @@ func TestNewParsedPolicyRemoteES(t *testing.T) { require.Equal(t, "remote", pp.Default.Name) } -// TestNewParsedPolicyRemoteESServiceTokenSecret ensures that a remote Elasticsearch -// output with secrets.service_token: -// 1. Resolves the service_token value (needed by fleet-server to create API keys) -// 2. Does NOT list outputs..service_token in SecretKeys — that field is -// stripped before the policy is sent to agents -func TestNewParsedPolicyRemoteESServiceTokenSecret(t *testing.T) { - var m model.Policy - var d model.PolicyData - err := json.Unmarshal([]byte(testPolicyRemoteESWithSecrets), &d) - require.NoError(t, err) - m.Data = &d - - bulker := ftesting.NewMockBulk() - pp, err := NewParsedPolicy(t.Context(), bulker, m) - require.NoError(t, err) - - remote := pp.Policy.Data.Outputs["OUTPUT_ID"] - require.Equal(t, "SERVICE_TOKEN_ID_value", remote["service_token"]) - require.Equal(t, "SSL_KEY_ID_value", remote["ssl"].(map[string]any)["key"]) - _, hasSecrets := remote["secrets"] - require.False(t, hasSecrets, "secrets map should be removed after processing") - - require.NotContains(t, pp.SecretKeys, "outputs.OUTPUT_ID.service_token") - require.Contains(t, pp.SecretKeys, "outputs.OUTPUT_ID.ssl.key") -} - // TestParsedPolicyMixedSecretsReplacement tests that secrets specified in a policy // using either the `secrets...id:` format or the // `: $co.elastic.secret{}` format are both replaced correctly. diff --git a/internal/pkg/policy/testdata/remote_es_policy_with_secrets.json b/internal/pkg/policy/testdata/remote_es_policy_with_secrets.json deleted file mode 100644 index d4f01d75e7..0000000000 --- a/internal/pkg/policy/testdata/remote_es_policy_with_secrets.json +++ /dev/null @@ -1,69 +0,0 @@ -{ - "id": "remote-with-secrets", - "revision": 1, - "outputs": { - "default": { - "type": "elasticsearch", - "hosts": [ - "https://local.es.example:443" - ] - }, - "OUTPUT_ID": { - "type": "remote_elasticsearch", - "hosts": [ - "https://remote.es.example:443" - ], - "service_token": null, - "secrets": { - "service_token": { - "id": "SERVICE_TOKEN_ID" - }, - "ssl": { - "key": { - "id": "SSL_KEY_ID" - } - } - } - } - }, - "output_permissions": { - "default": { - "_fallback": { - "cluster": ["monitor"], - "indices": [ - { - "names": ["logs-*", "metrics-*"], - "privileges": ["auto_configure", "create_doc"] - } - ] - } - }, - "OUTPUT_ID": { - "_fallback": { - "cluster": ["monitor"], - "indices": [ - { - "names": ["logs-*", "metrics-*"], - "privileges": ["auto_configure", "create_doc"] - } - ] - } - } - }, - "inputs": [], - "secret_references": [ - {"id": "SERVICE_TOKEN_ID"}, - {"id": "SSL_KEY_ID"} - ], - "agent": { - "monitoring": { - "enabled": true, - "use_output": "OUTPUT_ID", - "logs": true, - "metrics": true - } - }, - "fleet": { - "hosts": ["http://localhost:8220"] - } -} From 2563ebbf2178b493813f6565da46bda23d319c45 Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 31 Jul 2026 17:30:02 +0530 Subject: [PATCH 4/5] fix it --- internal/pkg/api/handleCheckin.go | 20 +++-- internal/pkg/api/handleCheckin_test.go | 113 +++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/internal/pkg/api/handleCheckin.go b/internal/pkg/api/handleCheckin.go index 0ab50d00e9..e390fb5a5c 100644 --- a/internal/pkg/api/handleCheckin.go +++ b/internal/pkg/api/handleCheckin.go @@ -905,16 +905,11 @@ func processPolicy(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, a data := model.ClonePolicyData(pp.Policy.Data) for name, policyOutput := range data.Outputs { // NOTE: Not sure if output secret keys collected here include new entries, but they are collected for completeness - outputType, _ := policyOutput["type"].(string) ks, err := secret.ProcessOutputSecret(policyOutput, secretValues) if err != nil { return nil, fmt.Errorf("failed to process output secret for output %q: %w", name, err) } for _, key := range ks { - // Remote ES service_token is stripped in Prepare; omit from secret_paths - if outputType == policy.OutputTypeRemoteElasticsearch && key == policy.FieldOutputServiceToken { - continue - } pp.SecretKeys = append(pp.SecretKeys, "outputs."+name+"."+key) } } @@ -925,6 +920,21 @@ func processPolicy(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, a policyOutput.Name, err) } } + + // Do not advertise remote ES service_token in secret_paths once Prepare(...) has + // deleted it from the policy sent to agents. Use pp.Outputs for type because + // Prepare rewrites data.Outputs type to elasticsearch. + for name, out := range pp.Outputs { + if out.Type != policy.OutputTypeRemoteElasticsearch { + continue + } + if _, ok := data.Outputs[name][policy.FieldOutputServiceToken]; !ok { + prefixed := "outputs." + name + "." + policy.FieldOutputServiceToken + pp.SecretKeys = slices.DeleteFunc(pp.SecretKeys, func(key string) bool { + return key == prefixed + }) + } + } // Prepare OTel exporters from the information in outputs. if err := prepareOTelExporters(data.Outputs, data.Exporters); err != nil { return nil, fmt.Errorf("failed to prepare OTel exporters: %w", err) diff --git a/internal/pkg/api/handleCheckin_test.go b/internal/pkg/api/handleCheckin_test.go index e9d2fb407c..d5d2850408 100644 --- a/internal/pkg/api/handleCheckin_test.go +++ b/internal/pkg/api/handleCheckin_test.go @@ -1506,3 +1506,116 @@ func TestProcessPolicyDetails(t *testing.T) { assert.Empty(t, opts) }) } + +// TestProcessPolicyRemoteESServiceTokenSecretPaths ensures secret_path does not +// contain service_token for remote ES output as they are stripped before sending the policy to the agents + +func TestProcessPolicyRemoteESServiceTokenSecretPaths(t *testing.T) { + logger := testlog.SetLogger(t) + + const policyPayload = `{ + "id": "remote-with-secrets", + "revision": 1, + "outputs": { + "default": { + "type": "elasticsearch", + "hosts": ["https://local.es.example:443"] + }, + "OUTPUT_ID": { + "type": "remote_elasticsearch", + "hosts": ["https://remote.es.example:443"], + "service_token": null, + "secrets": { + "service_token": {"id": "SERVICE_TOKEN_ID"}, + "ssl": {"key": {"id": "SSL_KEY_ID"}} + } + } + }, + "output_permissions": { + "default": { + "_fallback": { + "cluster": ["monitor"], + "indices": [{"names": ["logs-*", "metrics-*"], "privileges": ["auto_configure", "create_doc"]}] + } + }, + "OUTPUT_ID": { + "_fallback": { + "cluster": ["monitor"], + "indices": [{"names": ["logs-*", "metrics-*"], "privileges": ["auto_configure", "create_doc"]}] + } + } + }, + "inputs": [], + "secret_references": [ + {"id": "SERVICE_TOKEN_ID"}, + {"id": "SSL_KEY_ID"} + ], + "agent": { + "monitoring": { + "enabled": true, + "use_output": "OUTPUT_ID", + "logs": true, + "metrics": true + } + }, + "fleet": { + "hosts": ["http://localhost:8220"] + } +}` + + var d model.PolicyData + err := json.Unmarshal([]byte(policyPayload), &d) + require.NoError(t, err) + + bulker := ftesting.NewMockBulk() + pp, err := policy.NewParsedPolicy(t.Context(), bulker, model.Policy{ + PolicyID: "policy1", + RevisionIdx: 1, + Data: &d, + }) + require.NoError(t, err) + + defaultOut := pp.Outputs["default"] + remoteOut := pp.Outputs["OUTPUT_ID"] + require.NotNil(t, defaultOut.Role) + require.NotNil(t, remoteOut.Role) + + outputBulker := ftesting.NewMockBulk() + bulker.On("CreateAndGetBulker", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(outputBulker, false, nil) + + defaultKey := bulk.APIKey{ID: "default-id", Key: "default-key"} + remoteKey := bulk.APIKey{ID: "remote-id", Key: "remote-key"} + agent := &model.Agent{ + ESDocument: model.ESDocument{Id: "agent1"}, + Outputs: map[string]*model.PolicyOutput{ + "default": { + APIKey: defaultKey.Agent(), + APIKeyID: defaultKey.ID, + PermissionsHash: defaultOut.Role.Sha2, + Type: policy.OutputTypeElasticsearch, + }, + "OUTPUT_ID": { + APIKey: remoteKey.Agent(), + APIKeyID: remoteKey.ID, + PermissionsHash: remoteOut.Role.Sha2, + Type: policy.OutputTypeRemoteElasticsearch, + }, + }, + } + + action, err := processPolicy(t.Context(), logger, bulker, agent, pp) + require.NoError(t, err) + + pc, err := action.Data.AsActionPolicyChange() + require.NoError(t, err) + + assert.NotContains(t, pc.Policy.SecretPaths, "outputs.OUTPUT_ID.service_token") + assert.Contains(t, pc.Policy.SecretPaths, "outputs.OUTPUT_ID.ssl.key") + + remotePolicy, ok := pc.Policy.Outputs["OUTPUT_ID"].(map[string]any) + require.True(t, ok) + _, hasServiceToken := remotePolicy["service_token"] + assert.False(t, hasServiceToken, "service_token should be deleted by Prepare before delivery to agents") + assert.Equal(t, policy.OutputTypeElasticsearch, remotePolicy["type"]) +} From 367b614730a1106bc915625a6e7647713929abef Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 31 Jul 2026 17:32:18 +0530 Subject: [PATCH 5/5] parsed policy revert --- internal/pkg/policy/parsed_policy.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/pkg/policy/parsed_policy.go b/internal/pkg/policy/parsed_policy.go index 5681ce7310..d1d1bc0ad3 100644 --- a/internal/pkg/policy/parsed_policy.go +++ b/internal/pkg/policy/parsed_policy.go @@ -77,18 +77,14 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa return nil, err } for name, policyOutput := range p.Data.Outputs { - ks, err := secret.ProcessOutputSecret(policyOutput, secretValues) if err != nil { return nil, fmt.Errorf("failed to replace secrets in output section of policy '%s': %w", name, err) } - for _, key := range ks { secretKeys = append(secretKeys, "outputs."+name+"."+key) } - } - defaultName, err := findDefaultOutputName(p.Data.Outputs) if err != nil { return nil, err