Merge pull request 'fix(executor): mask base64 of a secret followed by other bytes' (#8) from fix/mask-base64-tail into main

commit: cbb8ad605a21ab39fd7a02ba2e5b3f54eb3da368
author: bnema · 9d ago · 2026-10-13 04:42 UTC
merge of: fc8849e + e66e40e · diff against the first parent
signature: SSH UnverifiedSHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivA The key is unknown
refs: tag: v0.1.0

2 files · +54 -4

M internal/executor/masker.go +19 -4


@@ -17,10 +17,21 @@ import (
17	17		// base64 at the three byte offsets a prefix of one or two bytes can give
18	18		// ("user:" + secret is base64-encoded as a whole), and its JSON and URI
19	19		// escapes. Longer forms are replaced first.
	20	+	//
	21	+	// Bytes that follow the secret change the last, incomplete group of its base64
	22	+	// (`echo "$T" \| base64` appends a newline), so each base64 form is also
	23	+	// registered without that group: only the characters the secret alone
	24	+	// determines. actions/runner (ValueEncoders.Base64StringEscapeShift) only
	25	+	// registers the full forms; the truncated ones are an addition.
20	26		type masker struct{ values []string }
21	27
22	28		const maskedValue = "***"
23	29
	30	+	// minBase64Tail is the shortest base64 form, in characters, registered once its
	31	+	// incomplete last group is removed: a shorter one (3 bytes of the secret)
	32	+	// would mask ordinary text.
	33	+	const minBase64Tail = 8
	34	+
24	35		func newMasker(secrets ...string) *masker {
25	36		seen := map[string]bool{}
26	37		var vals []string

@@ -45,10 +56,14 @@ func secretEncodings(v string) []string {
45	56		var out []string
46	57		b := []byte(v)
47	58		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))
	59	+	if len(b) <= shift && shift != 0 {
	60	+	continue
	61	+	}
	62	+	sub := b[shift:]
	63	+	out = append(out, base64.StdEncoding.EncodeToString(sub))
	64	+	// Whole groups only: 3 bytes give 4 characters that no later byte changes.
	65	+	if n := len(sub) / 3 * 4; n >= minBase64Tail {
	66	+	out = append(out, base64.StdEncoding.EncodeToString(sub[:len(sub)/3*3]))
52	67		}
53	68		}
54	69		if j, err := json.Marshal(v); err == nil && len(j) > 2 {

 // base64 at the three byte offsets a prefix of one or two bytes can give
 // ("user:" + secret is base64-encoded as a whole), and its JSON and URI
 // escapes. Longer forms are replaced first.
+//
+// Bytes that follow the secret change the last, incomplete group of its base64
+// (`echo "$T" | base64` appends a newline), so each base64 form is also
+// registered without that group: only the characters the secret alone
+// determines. actions/runner (ValueEncoders.Base64StringEscapeShift) only
+// registers the full forms; the truncated ones are an addition.
 type masker struct{ values []string }
 const maskedValue = "***"
+// minBase64Tail is the shortest base64 form, in characters, registered once its
+// incomplete last group is removed: a shorter one (3 bytes of the secret)
+// would mask ordinary text.
+const minBase64Tail = 8
 func newMasker(secrets ...string) *masker {
 	seen := map[string]bool{}
 	var vals []string
 	var out []string
 	b := []byte(v)
 	for shift := range 3 {
-		if len(b) > shift {
+		if len(b) <= shift && shift != 0 {
-			out = append(out, base64.StdEncoding.EncodeToString(b[shift:]))
+			continue
-		} else if shift == 0 {
+		}
-			out = append(out, base64.StdEncoding.EncodeToString(b))
+		sub := b[shift:]
+		out = append(out, base64.StdEncoding.EncodeToString(sub))
+		// Whole groups only: 3 bytes give 4 characters that no later byte changes.
+		if n := len(sub) / 3 * 4; n >= minBase64Tail {
+			out = append(out, base64.StdEncoding.EncodeToString(sub[:len(sub)/3*3]))
 		}
 	}
 	if j, err := json.Marshal(v); err == nil && len(j) > 2 {

M internal/executor/masker_test.go +35 -0


@@ -56,6 +56,41 @@ func TestMaskerForms(t *testing.T) {
56	56		}
57	57		}
58	58
	59	+	// Bytes after the secret change the end of its base64 (`echo "$T" \| base64`
	60	+	// appends a newline): the part the secret alone determines is still masked, at
	61	+	// the three offsets and whatever the length of the secret.
	62	+	func TestMaskerBase64FollowedByOtherBytes(t *testing.T) {
	63	+	for _, secret := range []string{testToken, testToken + "4", testToken + "45", "ghs_0123456789abcdef"} {
	64	+	m := newMasker(secret)
	65	+	for shift := 0; shift < 3; shift++ {
	66	+	prefix := strings.Repeat("p", shift)
	67	+	for _, suffix := range []string{"\n", "!", "xy", " and more text"} {
	68	+	enc := base64.StdEncoding.EncodeToString([]byte(prefix + secret + suffix))
	69	+	text := "out: " + enc + " :end"
	70	+	got := m.mask(text)
	71	+	if !m.has(text) \|\| got == text \|\| !strings.Contains(got, "***") {
	72	+	t.Errorf("secret %q, prefix %q, suffix %q: %q -> %q", secret, prefix, suffix, text, got)
	73	+	continue
	74	+	}
	75	+	// the groups the secret alone determines are gone
	76	+	if strings.Contains(got, enc[4:12]) {
	77	+	t.Errorf("secret %q, prefix %q, suffix %q: %q still has the secret", secret, prefix, suffix, got)
	78	+	}
	79	+	}
	80	+	}
	81	+	}
	82	+	// the exact case of the review
	83	+	m := newMasker(testToken)
	84	+	if got := m.mask("Z2hzX3Rlc3R0b2tlbjEyMwo="); got == "Z2hzX3Rlc3R0b2tlbjEyMwo=" {
	85	+	t.Errorf("echo $T \| base64 is not masked: %q", got)
	86	+	}
	87	+	// a short secret does not mask the base64 of a few common bytes
	88	+	short := newMasker("abcd")
	89	+	if got := short.mask("YWJj"); got != "YWJj" {
	90	+	t.Errorf("a short tail is masked: %q", got)
	91	+	}
	92	+	}
	93	+
59	94		// The name of a step that holds the token is masked before it reaches the
60	95		// server (StepStarted and StepCompleted).
61	96		func TestStepNameIsMasked(t *testing.T) {


@@ -56,6 +56,41 @@ func TestMaskerForms(t *testing.T) {
56	}	56		}
57	}	57		}
58		58
		59	+	// Bytes after the secret change the end of its base64 (`echo "$T" \| base64`
		60	+	// appends a newline): the part the secret alone determines is still masked, at
		61	+	// the three offsets and whatever the length of the secret.
		62	+	func TestMaskerBase64FollowedByOtherBytes(t *testing.T) {
		63	+	for _, secret := range []string{testToken, testToken + "4", testToken + "45", "ghs_0123456789abcdef"} {
		64	+	m := newMasker(secret)
		65	+	for shift := 0; shift < 3; shift++ {
		66	+	prefix := strings.Repeat("p", shift)
		67	+	for _, suffix := range []string{"\n", "!", "xy", " and more text"} {
		68	+	enc := base64.StdEncoding.EncodeToString([]byte(prefix + secret + suffix))
		69	+	text := "out: " + enc + " :end"
		70	+	got := m.mask(text)
		71	+	if !m.has(text) \|\| got == text \|\| !strings.Contains(got, "***") {
		72	+	t.Errorf("secret %q, prefix %q, suffix %q: %q -> %q", secret, prefix, suffix, text, got)
		73	+	continue
		74	+	}
		75	+	// the groups the secret alone determines are gone
		76	+	if strings.Contains(got, enc[4:12]) {
		77	+	t.Errorf("secret %q, prefix %q, suffix %q: %q still has the secret", secret, prefix, suffix, got)
		78	+	}
		79	+	}
		80	+	}
		81	+	}
		82	+	// the exact case of the review
		83	+	m := newMasker(testToken)
		84	+	if got := m.mask("Z2hzX3Rlc3R0b2tlbjEyMwo="); got == "Z2hzX3Rlc3R0b2tlbjEyMwo=" {
		85	+	t.Errorf("echo $T \| base64 is not masked: %q", got)
		86	+	}
		87	+	// a short secret does not mask the base64 of a few common bytes
		88	+	short := newMasker("abcd")
		89	+	if got := short.mask("YWJj"); got != "YWJj" {
		90	+	t.Errorf("a short tail is masked: %q", got)
		91	+	}
		92	+	}
		93	+
59	// The name of a step that holds the token is masked before it reaches the	94		// The name of a step that holds the token is masked before it reaches the
60	// server (StepStarted and StepCompleted).	95		// server (StepStarted and StepCompleted).
61	func TestStepNameIsMasked(t *testing.T) {	96		func TestStepNameIsMasked(t *testing.T) {