diff --git a/internal/redact/redact.go b/internal/redact/redact.go index 3bb76e2694d..bfa64beace9 100644 --- a/internal/redact/redact.go +++ b/internal/redact/redact.go @@ -4,12 +4,12 @@ import "github.com/anchore/go-logger/adapter/redact" var store redact.Store +// Set replaces the package-level redaction store. Library consumers that construct and execute a syft +// command more than once within the same process (each call getting its own fresh clio state, and thus +// its own redaction store) will legitimately call Set again; that's expected and simply swaps in the new +// store, consistent with how the sibling bus and log singleton packages behave on repeated Set calls. +// Redactions added to a store before it's replaced no longer apply to output produced afterwards. func Set(s redact.Store) { - if store != nil { - // if someone is trying to set a redaction store and we already have one then something is wrong. The store - // that we're replacing might already have values in it, so we should never replace it. - panic("replace existing redaction store (probably unintentional)") - } store = s } diff --git a/internal/redact/redact_test.go b/internal/redact/redact_test.go new file mode 100644 index 00000000000..074ef996c04 --- /dev/null +++ b/internal/redact/redact_test.go @@ -0,0 +1,34 @@ +package redact + +import ( + "testing" + + gologgerredact "github.com/anchore/go-logger/adapter/redact" + "github.com/stretchr/testify/assert" +) + +// TestSet_ReplacingExistingStoreDoesNotPanic is a regression test for +// https://github.com/anchore/syft/issues/2285: library consumers that +// construct and execute a syft command more than once in the same process +// (e.g. https://github.com/anchore/syft/blob/main/cmd/syft/internal/clio_setup_config.go +// calling redact.Set on every clio initializer run) used to panic on the +// second call, because Set refused to replace an already-set store. +func TestSet_ReplacingExistingStoreDoesNotPanic(t *testing.T) { + orig := store + defer func() { store = orig }() + + Set(gologgerredact.NewStore()) + Add("first-secret") + assert.Equal(t, "prefix ******* suffix", Apply("prefix first-secret suffix")) + + assert.NotPanics(t, func() { + Set(gologgerredact.NewStore()) + }) + + // the replaced store doesn't carry over redactions registered before the swap + assert.Equal(t, "prefix first-secret suffix", Apply("prefix first-secret suffix")) + + // but works normally going forward + Add("second-secret") + assert.Equal(t, "prefix ******* suffix", Apply("prefix second-secret suffix")) +}