fix(executor): mask the encodings of the token as the official runner does
The runner registers, besides a secret, its base64 at the three byte offsets (a 'user:' or 'us:' prefix shifts the encoding), and its JSON, URI and command-line escapes (HostContext.cs:103-113). The masker now covers the base64 forms and the JSON and URL escapes; the XML, PowerShell and expression-string forms are not needed for a token of base64url characters.
- commit
- e1ff0f27454c25e3f167cf4ec67298d6920d5dae
- author
- brice · · 2026-10-13 04:07 UTC
- parent
- f355cc5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +65 -3
M internal/executor/masker.go +23 -3
| @@ -3,6 +3,9 @@ | |||
| 3 | 3 | package executor | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "encoding/base64" | |
| 7 | + | "encoding/json" | |
| 8 | + | "net/url" | |
| 6 | 9 | "slices" | |
| 7 | 10 | "strings" | |
| 8 | 11 | "time" | |
| @@ -10,8 +13,10 @@ import ( | |||
| 10 | 13 | ||
| 11 | 14 | // masker replaces secret values by *** in text that leaves the executor | |
| 12 | 15 | // (HostContext.SecretMasker). A value is masked as is and in the forms the | |
| 13 | - | // official runner registers for it (HostContext.cs:103-113, ValueEncoders). | |
| 14 | - | // Longer forms are replaced first. | |
| 16 | + | // official runner registers for it (HostContext.cs:103-113, ValueEncoders): its | |
| 17 | + | // base64 at the three byte offsets a prefix of one or two bytes can give | |
| 18 | + | // ("user:" + secret is base64-encoded as a whole), and its JSON and URI | |
| 19 | + | // escapes. Longer forms are replaced first. | |
| 15 | 20 | type masker struct{ values []string } | |
| 16 | 21 | ||
| 17 | 22 | const maskedValue = "***" | |
| @@ -36,7 +41,22 @@ func newMasker(secrets ...string) *masker { | |||
| 36 | 41 | } | |
| 37 | 42 | ||
| 38 | 43 | // secretEncodings are the other forms of v that the runner masks. | |
| 39 | - | func secretEncodings(string) []string { return nil } | |
| 44 | + | func secretEncodings(v string) []string { | |
| 45 | + | var out []string | |
| 46 | + | b := []byte(v) | |
| 47 | + | for shift := range 3 { | |
| 48 | + | if len(b) > shift { | |
| 49 | + | out = append(out, base64.StdEncoding.EncodeToString(b[shift:])) | |
| 50 | + | } else if shift == 0 { | |
| 51 | + | out = append(out, base64.StdEncoding.EncodeToString(b)) | |
| 52 | + | } | |
| 53 | + | } | |
| 54 | + | if j, err := json.Marshal(v); err == nil && len(j) > 2 { | |
| 55 | + | out = append(out, string(j[1:len(j)-1])) | |
| 56 | + | } | |
| 57 | + | out = append(out, url.QueryEscape(v), url.PathEscape(v)) | |
| 58 | + | return out | |
| 59 | + | } | |
| 40 | 60 | ||
| 41 | 61 | // mask returns s with every secret replaced. | |
| 42 | 62 | func (m *masker) mask(s string) string { | |
| @@ -3,6 +3,9 @@ | |||||
| 3 | package executor | 3 | package executor | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "encoding/base64" | |||
| 7 | + | "encoding/json" | |||
| 8 | + | "net/url" | |||
| 6 | "slices" | 9 | "slices" | ||
| 7 | "strings" | 10 | "strings" | ||
| 8 | "time" | 11 | "time" | ||
| @@ -10,8 +13,10 @@ import ( | |||||
| 10 | 13 | ||||
| 11 | // masker replaces secret values by *** in text that leaves the executor | 14 | // masker replaces secret values by *** in text that leaves the executor | ||
| 12 | // (HostContext.SecretMasker). A value is masked as is and in the forms the | 15 | // (HostContext.SecretMasker). A value is masked as is and in the forms the | ||
| 13 | - | // official runner registers for it (HostContext.cs:103-113, ValueEncoders). | 16 | + | // official runner registers for it (HostContext.cs:103-113, ValueEncoders): its |
| 14 | - | // Longer forms are replaced first. | 17 | + | // base64 at the three byte offsets a prefix of one or two bytes can give |
| 18 | + | // ("user:" + secret is base64-encoded as a whole), and its JSON and URI | |||
| 19 | + | // escapes. Longer forms are replaced first. | |||
| 15 | type masker struct{ values []string } | 20 | type masker struct{ values []string } | ||
| 16 | 21 | ||||
| 17 | const maskedValue = "***" | 22 | const maskedValue = "***" | ||
| @@ -36,7 +41,22 @@ func newMasker(secrets ...string) *masker { | |||||
| 36 | } | 41 | } | ||
| 37 | 42 | ||||
| 38 | // secretEncodings are the other forms of v that the runner masks. | 43 | // secretEncodings are the other forms of v that the runner masks. | ||
| 39 | - | func secretEncodings(string) []string { return nil } | 44 | + | func secretEncodings(v string) []string { |
| 45 | + | var out []string | |||
| 46 | + | b := []byte(v) | |||
| 47 | + | for shift := range 3 { | |||
| 48 | + | if len(b) > shift { | |||
| 49 | + | out = append(out, base64.StdEncoding.EncodeToString(b[shift:])) | |||
| 50 | + | } else if shift == 0 { | |||
| 51 | + | out = append(out, base64.StdEncoding.EncodeToString(b)) | |||
| 52 | + | } | |||
| 53 | + | } | |||
| 54 | + | if j, err := json.Marshal(v); err == nil && len(j) > 2 { | |||
| 55 | + | out = append(out, string(j[1:len(j)-1])) | |||
| 56 | + | } | |||
| 57 | + | out = append(out, url.QueryEscape(v), url.PathEscape(v)) | |||
| 58 | + | return out | |||
| 59 | + | } | |||
| 40 | 60 | ||||
| 41 | // mask returns s with every secret replaced. | 61 | // mask returns s with every secret replaced. | ||
| 42 | func (m *masker) mask(s string) string { | 62 | func (m *masker) mask(s string) string { | ||
M internal/executor/masker_test.go +42 -0
| @@ -3,6 +3,7 @@ | |||
| 3 | 3 | package executor | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "encoding/base64" | |
| 6 | 7 | "strings" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "time" | |
| @@ -14,6 +15,47 @@ func tokenContexts() string { | |||
| 14 | 15 | return strings.Replace(testGitHubContext("main"), `"github":{`, `"github":{"token":"`+testToken+`",`, 1) | |
| 15 | 16 | } | |
| 16 | 17 | ||
| 18 | + | func TestMaskerForms(t *testing.T) { | |
| 19 | + | m := newMasker(testToken) | |
| 20 | + | for _, tc := range []struct{ name, text string }{ | |
| 21 | + | {"plain", "x " + testToken + " y"}, | |
| 22 | + | {"base64", "Authorization: Basic " + base64.StdEncoding.EncodeToString([]byte(testToken))}, | |
| 23 | + | {"base64, one byte of prefix", base64.StdEncoding.EncodeToString([]byte("u" + testToken))}, | |
| 24 | + | {"base64, two bytes of prefix", base64.StdEncoding.EncodeToString([]byte("us" + testToken))}, | |
| 25 | + | {"base64, three bytes of prefix", base64.StdEncoding.EncodeToString([]byte("usr" + testToken))}, | |
| 26 | + | } { | |
| 27 | + | if got := m.mask(tc.text); strings.Contains(got, testToken) || got == tc.text || !strings.Contains(got, "***") { | |
| 28 | + | t.Errorf("%s: %q -> %q", tc.name, tc.text, got) | |
| 29 | + | } | |
| 30 | + | if !m.has(tc.text) { | |
| 31 | + | t.Errorf("%s: has() is false", tc.name) | |
| 32 | + | } | |
| 33 | + | } | |
| 34 | + | // base64 at the three offsets: the secret's own text is found in every form | |
| 35 | + | for shift := 0; shift < 3; shift++ { | |
| 36 | + | prefix := strings.Repeat("p", shift) | |
| 37 | + | enc := base64.StdEncoding.EncodeToString([]byte(prefix + testToken)) | |
| 38 | + | if got := m.mask("[" + enc + "]"); strings.Contains(got, enc[4:len(enc)-4]) { | |
| 39 | + | t.Errorf("shift %d: %q", shift, got) | |
| 40 | + | } | |
| 41 | + | } | |
| 42 | + | for _, tc := range []string{"", "nothing here", "ghs_other"} { | |
| 43 | + | if m.has(tc) { | |
| 44 | + | t.Errorf("%q must not be masked", tc) | |
| 45 | + | } | |
| 46 | + | } | |
| 47 | + | if !newMasker().has("") == false || newMasker().mask("a") != "a" { | |
| 48 | + | t.Error("an empty masker changes text") | |
| 49 | + | } | |
| 50 | + | // special characters: the JSON and URL forms | |
| 51 | + | s := newMasker(`a"b/c d`) | |
| 52 | + | for _, text := range []string{`a"b/c d`, `a\"b/c d`, `a%22b%2Fc+d`, `a%22b%2Fc%20d`} { | |
| 53 | + | if got := s.mask(text); strings.Contains(got, "b") { | |
| 54 | + | t.Errorf("%q -> %q", text, got) | |
| 55 | + | } | |
| 56 | + | } | |
| 57 | + | } | |
| 58 | + | ||
| 17 | 59 | // The name of a step that holds the token is masked before it reaches the | |
| 18 | 60 | // server (StepStarted and StepCompleted). | |
| 19 | 61 | func TestStepNameIsMasked(t *testing.T) { | |
| @@ -3,6 +3,7 @@ | |||||
| 3 | package executor | 3 | package executor | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "encoding/base64" | |||
| 6 | "strings" | 7 | "strings" | ||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "time" | 9 | "time" | ||
| @@ -14,6 +15,47 @@ func tokenContexts() string { | |||||
| 14 | return strings.Replace(testGitHubContext("main"), `"github":{`, `"github":{"token":"`+testToken+`",`, 1) | 15 | return strings.Replace(testGitHubContext("main"), `"github":{`, `"github":{"token":"`+testToken+`",`, 1) | ||
| 15 | } | 16 | } | ||
| 16 | 17 | ||||
| 18 | + | func TestMaskerForms(t *testing.T) { | |||
| 19 | + | m := newMasker(testToken) | |||
| 20 | + | for _, tc := range []struct{ name, text string }{ | |||
| 21 | + | {"plain", "x " + testToken + " y"}, | |||
| 22 | + | {"base64", "Authorization: Basic " + base64.StdEncoding.EncodeToString([]byte(testToken))}, | |||
| 23 | + | {"base64, one byte of prefix", base64.StdEncoding.EncodeToString([]byte("u" + testToken))}, | |||
| 24 | + | {"base64, two bytes of prefix", base64.StdEncoding.EncodeToString([]byte("us" + testToken))}, | |||
| 25 | + | {"base64, three bytes of prefix", base64.StdEncoding.EncodeToString([]byte("usr" + testToken))}, | |||
| 26 | + | } { | |||
| 27 | + | if got := m.mask(tc.text); strings.Contains(got, testToken) || got == tc.text || !strings.Contains(got, "***") { | |||
| 28 | + | t.Errorf("%s: %q -> %q", tc.name, tc.text, got) | |||
| 29 | + | } | |||
| 30 | + | if !m.has(tc.text) { | |||
| 31 | + | t.Errorf("%s: has() is false", tc.name) | |||
| 32 | + | } | |||
| 33 | + | } | |||
| 34 | + | // base64 at the three offsets: the secret's own text is found in every form | |||
| 35 | + | for shift := 0; shift < 3; shift++ { | |||
| 36 | + | prefix := strings.Repeat("p", shift) | |||
| 37 | + | enc := base64.StdEncoding.EncodeToString([]byte(prefix + testToken)) | |||
| 38 | + | if got := m.mask("[" + enc + "]"); strings.Contains(got, enc[4:len(enc)-4]) { | |||
| 39 | + | t.Errorf("shift %d: %q", shift, got) | |||
| 40 | + | } | |||
| 41 | + | } | |||
| 42 | + | for _, tc := range []string{"", "nothing here", "ghs_other"} { | |||
| 43 | + | if m.has(tc) { | |||
| 44 | + | t.Errorf("%q must not be masked", tc) | |||
| 45 | + | } | |||
| 46 | + | } | |||
| 47 | + | if !newMasker().has("") == false || newMasker().mask("a") != "a" { | |||
| 48 | + | t.Error("an empty masker changes text") | |||
| 49 | + | } | |||
| 50 | + | // special characters: the JSON and URL forms | |||
| 51 | + | s := newMasker(`a"b/c d`) | |||
| 52 | + | for _, text := range []string{`a"b/c d`, `a\"b/c d`, `a%22b%2Fc+d`, `a%22b%2Fc%20d`} { | |||
| 53 | + | if got := s.mask(text); strings.Contains(got, "b") { | |||
| 54 | + | t.Errorf("%q -> %q", text, got) | |||
| 55 | + | } | |||
| 56 | + | } | |||
| 57 | + | } | |||
| 58 | + | ||||
| 17 | // The name of a step that holds the token is masked before it reaches the | 59 | // The name of a step that holds the token is masked before it reaches the | ||
| 18 | // server (StepStarted and StepCompleted). | 60 | // server (StepStarted and StepCompleted). | ||
| 19 | func TestStepNameIsMasked(t *testing.T) { | 61 | func TestStepNameIsMasked(t *testing.T) { | ||