From da71e0b1c175b5d3f3a743ed2b88d5518b5de651 Mon Sep 17 00:00:00 2001 From: Andrzej Stencel Date: Wed, 18 Dec 2024 10:16:11 +0100 Subject: [PATCH] Prevent leaking secrets when logging component model (#6329) * feat: Prevent leaking secrets when logging component model * add changelog entry * test: add test checking that secrets are not leaked when components are marhsaled to JSON (cherry picked from commit d359ccd2f4b3c01cd1f62e161b5256728446a480) --- ...-secrets-when-logging-component-model.yaml | 32 +++++++++++++++++ .../application/coordinator/coordinator.go | 1 + pkg/component/component.go | 34 +++++++++++++++++++ pkg/component/component_test.go | 26 ++++++++++++++ 4 files changed, 93 insertions(+) create mode 100644 changelog/fragments/1734098868-prevent-leaking-secrets-when-logging-component-model.yaml diff --git a/changelog/fragments/1734098868-prevent-leaking-secrets-when-logging-component-model.yaml b/changelog/fragments/1734098868-prevent-leaking-secrets-when-logging-component-model.yaml new file mode 100644 index 00000000000..1a8c13ff8ba --- /dev/null +++ b/changelog/fragments/1734098868-prevent-leaking-secrets-when-logging-component-model.yaml @@ -0,0 +1,32 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: enhancement + +# Change summary; a 80ish characters long description of the change. +summary: Prevent leaking secrets when logging the component model + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: elastic-agent + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +#pr: https://github.com/owner/repo/1234 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +#issue: https://github.com/owner/repo/1234 diff --git a/internal/pkg/agent/application/coordinator/coordinator.go b/internal/pkg/agent/application/coordinator/coordinator.go index eb2b8055821..4a6dabd10c0 100644 --- a/internal/pkg/agent/application/coordinator/coordinator.go +++ b/internal/pkg/agent/application/coordinator/coordinator.go @@ -1268,6 +1268,7 @@ func (c *Coordinator) refreshComponentModel(ctx context.Context) (err error) { } c.logger.Info("Updating running component model") + c.logger.With("components", model.Components).Debug("Updating running component model") c.runtimeMgr.Update(model) return nil } diff --git a/pkg/component/component.go b/pkg/component/component.go index 06a9820d8ec..21e87ea5233 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -5,6 +5,7 @@ package component import ( + "encoding/json" "errors" "fmt" "sort" @@ -173,6 +174,39 @@ func (c Component) MarshalYAML() (interface{}, error) { return c, nil } +func (c *Component) MarshalJSON() ([]byte, error) { + marshalableComponent := struct { + ID string `json:"ID"` + InputType string `json:"InputType"` + OutputType string `json:"OutputType"` + ErrMsg string `json:"ErrMsg,omitempty"` + Units []struct { + ID string `json:"ID"` + ErrMsg string `json:"ErrMsg,omitempty"` + } `json:"Units"` + }{ + ID: c.ID, + InputType: c.InputType, + OutputType: c.OutputType, + } + if c.Err != nil { + marshalableComponent.ErrMsg = c.Err.Error() + } + for i := range c.Units { + marshalableComponent.Units = append(marshalableComponent.Units, struct { + ID string `json:"ID"` + ErrMsg string `json:"ErrMsg,omitempty"` + }{ + ID: c.Units[i].ID, + }) + if c.Units[i].Err != nil { + marshalableComponent.Units[i].ErrMsg = c.Units[i].Err.Error() + } + } + + return json.Marshal(marshalableComponent) +} + // Type returns the type of the component. func (c *Component) Type() string { if c.InputSpec != nil { diff --git a/pkg/component/component_test.go b/pkg/component/component_test.go index 8ddbf65f931..995b223d91d 100644 --- a/pkg/component/component_test.go +++ b/pkg/component/component_test.go @@ -59,6 +59,32 @@ func TestComponentMarshalError(t *testing.T) { require.Contains(t, string(outData), "test error value") } +func TestMarshalJSON(t *testing.T) { + testComponent := Component{ + ID: "test-device", + Err: testErr{data: "test error value"}, + Units: []Unit{ + { + ID: "test-unit", + Config: &proto.UnitExpectedConfig{ + Source: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "api_key": structpb.NewStringValue("test-api-key"), + }, + }, + }, + }, + }, + } + + marshaledBytes, err := testComponent.MarshalJSON() + require.NoError(t, err) + marshaledJsonString := string(marshaledBytes) + assert.Contains(t, marshaledJsonString, "test error value") + assert.Contains(t, marshaledJsonString, "test-unit") + assert.NotContains(t, marshaledJsonString, "test-api-key") +} + func TestToComponents(t *testing.T) { linuxAMD64Platform := PlatformDetail{ Platform: Platform{