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 }