fix(executor): track original masks apart from their encodings
AddValue skips a value only when it was added before as an original. Testing the combined set let add-mask of the URI form of an earlier mask return early, leaving the encodings of the new value unmasked.
- commit
- f0e28f8d2514007781c75cfa6e5871e39f314011
- author
- brice · · 2026-10-14 02:40 UTC
- parent
- e59b2f0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +36 -5
M internal/executor/masker.go +11 -5
| @@ -30,9 +30,14 @@ import ( | |||
| 30 | 30 | // determines. actions/runner only registers the full forms; the truncated ones | |
| 31 | 31 | // are an addition of ermite. | |
| 32 | 32 | type masker struct { | |
| 33 | - | mu sync.RWMutex | |
| 34 | - | values map[string]struct{} | |
| 35 | - | list []string | |
| 33 | + | mu sync.RWMutex | |
| 34 | + | // originals are the values passed to add (SecretMasker.m_originalValueSecrets); | |
| 35 | + | // values are the forms to look for: the originals and their encodings. A | |
| 36 | + | // value that is the encoding of an earlier one is still a new original: its | |
| 37 | + | // own encodings must be registered (AddValue tests the originals only). | |
| 38 | + | originals map[string]struct{} | |
| 39 | + | values map[string]struct{} | |
| 40 | + | list []string | |
| 36 | 41 | } | |
| 37 | 42 | ||
| 38 | 43 | const maskedValue = "***" | |
| @@ -43,7 +48,7 @@ const maskedValue = "***" | |||
| 43 | 48 | const minBase64Tail = 8 | |
| 44 | 49 | ||
| 45 | 50 | func newMasker(secrets ...string) *masker { | |
| 46 | - | m := &masker{values: map[string]struct{}{}} | |
| 51 | + | m := &masker{originals: map[string]struct{}{}, values: map[string]struct{}{}} | |
| 47 | 52 | for _, s := range secrets { | |
| 48 | 53 | m.add(s) | |
| 49 | 54 | } | |
| @@ -57,7 +62,7 @@ func (m *masker) add(value string) { | |||
| 57 | 62 | return | |
| 58 | 63 | } | |
| 59 | 64 | m.mu.RLock() | |
| 60 | - | _, known := m.values[value] | |
| 65 | + | _, known := m.originals[value] | |
| 61 | 66 | m.mu.RUnlock() | |
| 62 | 67 | if known { | |
| 63 | 68 | return | |
| @@ -65,6 +70,7 @@ func (m *masker) add(value string) { | |||
| 65 | 70 | forms := append([]string{value}, secretEncodings(value)...) | |
| 66 | 71 | m.mu.Lock() | |
| 67 | 72 | defer m.mu.Unlock() | |
| 73 | + | m.originals[value] = struct{}{} | |
| 68 | 74 | for _, f := range forms { | |
| 69 | 75 | if f == "" { | |
| 70 | 76 | continue | |
| @@ -30,9 +30,14 @@ import ( | |||||
| 30 | // determines. actions/runner only registers the full forms; the truncated ones | 30 | // determines. actions/runner only registers the full forms; the truncated ones | ||
| 31 | // are an addition of ermite. | 31 | // are an addition of ermite. | ||
| 32 | type masker struct { | 32 | type masker struct { | ||
| 33 | - | mu sync.RWMutex | 33 | + | mu sync.RWMutex |
| 34 | - | values map[string]struct{} | 34 | + | // originals are the values passed to add (SecretMasker.m_originalValueSecrets); |
| 35 | - | list []string | 35 | + | // values are the forms to look for: the originals and their encodings. A |
| 36 | + | // value that is the encoding of an earlier one is still a new original: its | |||
| 37 | + | // own encodings must be registered (AddValue tests the originals only). | |||
| 38 | + | originals map[string]struct{} | |||
| 39 | + | values map[string]struct{} | |||
| 40 | + | list []string | |||
| 36 | } | 41 | } | ||
| 37 | 42 | ||||
| 38 | const maskedValue = "***" | 43 | const maskedValue = "***" | ||
| @@ -43,7 +48,7 @@ const maskedValue = "***" | |||||
| 43 | const minBase64Tail = 8 | 48 | const minBase64Tail = 8 | ||
| 44 | 49 | ||||
| 45 | func newMasker(secrets ...string) *masker { | 50 | func newMasker(secrets ...string) *masker { | ||
| 46 | - | m := &masker{values: map[string]struct{}{}} | 51 | + | m := &masker{originals: map[string]struct{}{}, values: map[string]struct{}{}} |
| 47 | for _, s := range secrets { | 52 | for _, s := range secrets { | ||
| 48 | m.add(s) | 53 | m.add(s) | ||
| 49 | } | 54 | } | ||
| @@ -57,7 +62,7 @@ func (m *masker) add(value string) { | |||||
| 57 | return | 62 | return | ||
| 58 | } | 63 | } | ||
| 59 | m.mu.RLock() | 64 | m.mu.RLock() | ||
| 60 | - | _, known := m.values[value] | 65 | + | _, known := m.originals[value] |
| 61 | m.mu.RUnlock() | 66 | m.mu.RUnlock() | ||
| 62 | if known { | 67 | if known { | ||
| 63 | return | 68 | return | ||
| @@ -65,6 +70,7 @@ func (m *masker) add(value string) { | |||||
| 65 | forms := append([]string{value}, secretEncodings(value)...) | 70 | forms := append([]string{value}, secretEncodings(value)...) | ||
| 66 | m.mu.Lock() | 71 | m.mu.Lock() | ||
| 67 | defer m.mu.Unlock() | 72 | defer m.mu.Unlock() | ||
| 73 | + | m.originals[value] = struct{}{} | |||
| 68 | for _, f := range forms { | 74 | for _, f := range forms { | ||
| 69 | if f == "" { | 75 | if f == "" { | ||
| 70 | continue | 76 | continue | ||
M internal/executor/masker_parity_test.go +25 -0
| @@ -105,3 +105,28 @@ func TestMaskerAddAndMerge(t *testing.T) { | |||
| 105 | 105 | t.Error(got) | |
| 106 | 106 | } | |
| 107 | 107 | } | |
| 108 | + | ||
| 109 | + | // SecretMasker.AddValue skips a value only when it was added before as an | |
| 110 | + | // original, not when it is already one of the forms of another value: the | |
| 111 | + | // encodings of "one%20two" are masked after "one two" was added, although | |
| 112 | + | // "one%20two" itself is the URI form of "one two" (GO-001). | |
| 113 | + | func TestMaskerAddsEncodingsOfAValueThatIsAnEncodingOfAnother(t *testing.T) { | |
| 114 | + | m := newMasker() | |
| 115 | + | m.add("one two") | |
| 116 | + | m.add("one%20two") | |
| 117 | + | for _, text := range []string{"one two", "one%20two", "one%2520two"} { | |
| 118 | + | if got := m.mask(text); got != "***" { | |
| 119 | + | t.Errorf("mask(%q) = %q", text, got) | |
| 120 | + | } | |
| 121 | + | } | |
| 122 | + | // the same original twice is still a no-op | |
| 123 | + | m.add("one%20two") | |
| 124 | + | if n := len(m.list); n == 0 { | |
| 125 | + | t.Fatal("empty") | |
| 126 | + | } | |
| 127 | + | before := len(m.list) | |
| 128 | + | m.add("one%20two") | |
| 129 | + | if len(m.list) != before { | |
| 130 | + | t.Error("an original added twice grew the list") | |
| 131 | + | } | |
| 132 | + | } | |
| @@ -105,3 +105,28 @@ func TestMaskerAddAndMerge(t *testing.T) { | |||||
| 105 | t.Error(got) | 105 | t.Error(got) | ||
| 106 | } | 106 | } | ||
| 107 | } | 107 | } | ||
| 108 | + | ||||
| 109 | + | // SecretMasker.AddValue skips a value only when it was added before as an | |||
| 110 | + | // original, not when it is already one of the forms of another value: the | |||
| 111 | + | // encodings of "one%20two" are masked after "one two" was added, although | |||
| 112 | + | // "one%20two" itself is the URI form of "one two" (GO-001). | |||
| 113 | + | func TestMaskerAddsEncodingsOfAValueThatIsAnEncodingOfAnother(t *testing.T) { | |||
| 114 | + | m := newMasker() | |||
| 115 | + | m.add("one two") | |||
| 116 | + | m.add("one%20two") | |||
| 117 | + | for _, text := range []string{"one two", "one%20two", "one%2520two"} { | |||
| 118 | + | if got := m.mask(text); got != "***" { | |||
| 119 | + | t.Errorf("mask(%q) = %q", text, got) | |||
| 120 | + | } | |||
| 121 | + | } | |||
| 122 | + | // the same original twice is still a no-op | |||
| 123 | + | m.add("one%20two") | |||
| 124 | + | if n := len(m.list); n == 0 { | |||
| 125 | + | t.Fatal("empty") | |||
| 126 | + | } | |||
| 127 | + | before := len(m.list) | |||
| 128 | + | m.add("one%20two") | |||
| 129 | + | if len(m.list) != before { | |||
| 130 | + | t.Error("an original added twice grew the list") | |||
| 131 | + | } | |||
| 132 | + | } | |||