From b0a8de7bf65cdf78ec185be7b28739c309d698c9 Mon Sep 17 00:00:00 2001 From: Metal <911jason@163.com> Date: Thu, 8 Oct 2026 04:28:15 +0800 Subject: [PATCH] fix(redact): reset redaction store between command runs (#5386) * fix(redact): reset redaction store between command runs cli.Command() may be invoked more than once in the same process (e.g. when syft is embedded as a library). The clio initializer calls internal/redact.Set on every command execution, but the redact store is process-global and Set panics when a store already exists, so the second invocation dies with "replace existing redaction store (probably unintentional)". Add redact.Reset() to clear the previous run's store and call it in the initializer before Set. The double-Set guard in Set is kept: an unexpected second Set within a single run still panics. Add TestAppClioSetupConfigInitializerCanRunMultipleTimes which runs the initializer twice; it panics with the exact reported message on the old code and passes with the fix. Fixes #2285 Signed-off-by: JasonMetal <935216773@qq.com> * fix(redact): release the redact store at the end of each run Clear the global redact store in the post-run hook instead of resetting it before every `Set`. The double-`Set` panic keeps its meaning: a store is only present while a run is in flight, so a second run starting mid-run still panics rather than splitting secrets across two stores and leaking the ones in the dropped store. Sequential runs in the same process work since the previous run releases its store on the way out. A run only releases the store it set, so it never clears a store owned by someone else. Signed-off-by: Alex Goodman --------- Signed-off-by: JasonMetal <935216773@qq.com> Signed-off-by: Alex Goodman Co-authored-by: Alex Goodman --- cmd/syft/cli/cli_test.go | 25 ++++++++++++++++ cmd/syft/internal/clio_setup_config.go | 10 ++++++- cmd/syft/internal/clio_setup_config_test.go | 32 +++++++++++++++++++++ internal/redact/redact.go | 8 ++++++ 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 cmd/syft/cli/cli_test.go create mode 100644 cmd/syft/internal/clio_setup_config_test.go diff --git a/cmd/syft/cli/cli_test.go b/cmd/syft/cli/cli_test.go new file mode 100644 index 000000000..f74b70d16 --- /dev/null +++ b/cmd/syft/cli/cli_test.go @@ -0,0 +1,25 @@ +package cli + +import ( + "io" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/anchore/clio" + "github.com/anchore/syft/internal/redact" +) + +func TestCommandCanRunMultipleTimes(t *testing.T) { + // https://github.com/anchore/syft/issues/2285 + t.Cleanup(redact.Reset) + + for i := 0; i < 2; i++ { + _, cmd := create(clio.Identification{Name: "syft"}, io.Discard) + cmd.SetArgs([]string{"cataloger", "list", "-o", "json"}) + require.NoError(t, cmd.Execute()) + + // the store is released at the end of each run so the next one can set its own + require.Nil(t, redact.Get()) + } +} diff --git a/cmd/syft/internal/clio_setup_config.go b/cmd/syft/internal/clio_setup_config.go index f2a665d12..8a9dacc4c 100644 --- a/cmd/syft/internal/clio_setup_config.go +++ b/cmd/syft/internal/clio_setup_config.go @@ -41,6 +41,8 @@ func AppClioSetupConfig(id clio.Identification, out io.Writer) *clio.SetupConfig stereoscope.SetBus(state.Bus) bus.Set(state.Bus) + // the redact store is process-global and only cleared when a run ends (see the post-run below), so + // setting it while another run is still in flight panics rather than splitting secrets across stores. redact.Set(state.RedactStore) log.Set(state.Logger) @@ -48,8 +50,14 @@ func AppClioSetupConfig(id clio.Identification, out io.Writer) *clio.SetupConfig return nil }, ). - WithPostRuns(func(_ *clio.State, _ error) { + WithPostRuns(func(state *clio.State, _ error) { stereoscope.Cleanup() //nolint:staticcheck // we don't have access to the image object here + + // release the redact store now that nothing in this run can report anymore, so a later run in the same + // process (e.g. syft embedded as a library) can set its own. Only release the store this run set. + if redact.Get() == state.RedactStore { + redact.Reset() + } }) return clioCfg } diff --git a/cmd/syft/internal/clio_setup_config_test.go b/cmd/syft/internal/clio_setup_config_test.go new file mode 100644 index 000000000..722732446 --- /dev/null +++ b/cmd/syft/internal/clio_setup_config_test.go @@ -0,0 +1,32 @@ +package internal + +import ( + "io" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/anchore/clio" + "github.com/anchore/go-logger/adapter/discard" + gologgerredact "github.com/anchore/go-logger/adapter/redact" + "github.com/anchore/syft/internal/redact" +) + +func TestAppClioSetupConfigInitializerPanicsWhileRunInFlight(t *testing.T) { + t.Cleanup(redact.Reset) + + cfg := AppClioSetupConfig(clio.Identification{Name: "syft"}, io.Discard) + require.Len(t, cfg.Initializers, 1) + + newState := func() *clio.State { + return &clio.State{Logger: discard.New(), RedactStore: gologgerredact.NewStore()} + } + + require.NoError(t, cfg.Initializers[0](newState())) + + // a second run starting before the first has finished must not silently replace the store, otherwise + // secrets added to the first store would no longer be redacted + require.PanicsWithValue(t, "replace existing redaction store (probably unintentional)", func() { + _ = cfg.Initializers[0](newState()) + }) +} diff --git a/internal/redact/redact.go b/internal/redact/redact.go index 3bb76e269..57ad1c897 100644 --- a/internal/redact/redact.go +++ b/internal/redact/redact.go @@ -13,6 +13,14 @@ func Set(s redact.Store) { store = s } +// Reset clears the global redaction store. It must only be called once a command +// run has fully finished (nothing left to Add or Apply), so that a later run can +// Set its own store. Calling it while a run is in flight would let a second Set +// split secrets across two stores and leak the ones in the dropped store. +func Reset() { + store = nil +} + func Get() redact.Store { return store }