amadan

carlos / platform Public

reqrec: mask compound tokens and UUIDs, strip Referer queries, widen secret names

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
claude 6f58b0a0851084856916d6dd55481c06cda701c1 parent afde4ae
2 files changed, +139 −8
  • internal/reqrec/redact.go +81 −8
  • internal/reqrec/redact_test.go +58 −0

CI cancelled — superseded by a newer push — run details

CI log 
=== gate ===
.amadan/test-gate.sh

amadan: job cancelled — a newer push superseded it
diff --git a/internal/reqrec/redact.go b/internal/reqrec/redact.go
index 3a17945c2..341b396f1 100644
--- a/internal/reqrec/redact.go
+++ b/internal/reqrec/redact.go
@@ -36,6 +36,8 @@ func isHexByte(c byte) bool {
return c >= '0' && c <= '9' || c >= 'a' && c <= 'f' || c >= 'A' && c <= 'F'
}
+func isCompoundByte(c byte) bool { return isRunByte(c) || c == '-' || c == '_' }
+
// maskable reports whether a maximal run of [A-Za-z0-9+=] is long enough and
// mixed enough to be a secret: either it holds a letter and a digit, or it is
// all hex digits (a long digit string or a hash with no digit in it).
@@ -59,22 +61,51 @@ func maskable(run string) bool {
return letter && digit || hex
}
-// maskRuns replaces every maskable run in s with MaskToken. Any byte outside
-// [A-Za-z0-9+=] ends a run, which is what keeps hyphenated slugs and file
-// extensions out of it.
-func maskRuns(s string) string {
+// maskableCompound reports whether a maximal run of [A-Za-z0-9+=_-] is a
+// token that happens to contain - or _: a UUID, or a base64url string. With
+// the - and _ removed it must have at least minRun characters and be either
+// all hex with a digit, or mixed case with a digit. Requiring mixed case is
+// what lets a plain lower-case slug such as "export-attendees-2023" through.
+func maskableCompound(run string) bool {
+ var n int
+ var upper, lower, digit, hex bool
+ hex = true
+ for i := 0; i < len(run); i++ {
+ c := run[i]
+ if c == '-' || c == '_' {
+ continue
+ }
+ n++
+ switch {
+ case c >= '0' && c <= '9':
+ digit = true
+ case c >= 'a' && c <= 'z':
+ lower = true
+ case c >= 'A' && c <= 'Z':
+ upper = true
+ }
+ if !isHexByte(c) {
+ hex = false
+ }
+ }
+ return n >= minRun && digit && (hex || upper && lower)
+}
+
+// maskMatching replaces each maximal run of bytes accepted by in that the test
+// accepts with MaskToken.
+func maskMatching(s string, in func(byte) bool, test func(string) bool) string {
var b strings.Builder
last := 0
for i := 0; i < len(s); {
- if !isRunByte(s[i]) {
+ if !in(s[i]) {
i++
continue
}
j := i
- for j < len(s) && isRunByte(s[j]) {
+ for j < len(s) && in(s[j]) {
j++
}
- if maskable(s[i:j]) {
+ if test(s[i:j]) {
b.WriteString(s[last:i])
b.WriteString(MaskToken)
last = j
@@ -88,6 +119,16 @@ func maskRuns(s string) string {
return b.String()
}
+// maskRuns masks secret-looking tokens in s in two passes. First it judges
+// each whole compound of [A-Za-z0-9+=_-], which catches UUIDs and base64url
+// strings that a - or _ would otherwise split into short pieces. Then it
+// judges each run of [A-Za-z0-9+=] on its own, so a long token inside a
+// hyphenated name (application-<hash>.js) is masked and the name is not.
+func maskRuns(s string) string {
+ s = maskMatching(s, isCompoundByte, maskableCompound)
+ return maskMatching(s, isRunByte, maskable)
+}
+
// truncate cuts s to at most n bytes without splitting a rune. s must already
// be valid UTF-8.
func truncate(s string, n int) string {
@@ -167,6 +208,7 @@ var safeHeaders = []string{"User-Agent", "Referer", "Content-Type", "Accept", "A
var secretNames = []string{
"password", "passwd", "pwd", "token", "secret", "key", "session",
"auth", "sig", "signature", "credential", "otp", "code",
+ "pass", "pw", "pin", "jwt", "bearer", "ticket", "cvv", "cvc", "card",
}
// IsSecretName reports whether a parameter or field name looks like it holds a
@@ -189,7 +231,9 @@ func Fragment(src Source, name, value string) (string, bool) {
switch src {
case SourcePath:
case SourceQuery, SourceBody:
- if IsSecretName(name) {
+ // A fragment with no name cannot be judged, and a form body or query
+ // string passed whole would carry its secrets with it.
+ if strings.TrimSpace(name) == "" || IsSecretName(name) {
return "", false
}
case SourceHeader:
@@ -203,9 +247,38 @@ func Fragment(src Source, name, value string) (string, bool) {
if !ok {
return "", false
}
+ if strings.EqualFold(name, "Referer") {
+ return referer(value)
+ }
default:
// Cookies, and any source this code does not know about.
return "", false
}
return clean(value, MaxFragment), true
}
+
+// referer reduces a Referer value to its scheme, host and masked path, with
+// the query cut down to parameter names: a referring page's query string often
+// holds a password or an OAuth code. Userinfo and fragment are dropped. A value
+// that does not parse is omitted.
+func referer(value string) (string, bool) {
+ u, err := url.Parse(strings.TrimSpace(value))
+ if err != nil {
+ return "", false
+ }
+ var b strings.Builder
+ if u.Scheme != "" {
+ b.WriteString(u.Scheme)
+ b.WriteString(":")
+ }
+ if u.Host != "" {
+ b.WriteString("//")
+ b.WriteString(u.Host)
+ }
+ b.WriteString(u.Path)
+ if names := QueryNames(u.RawQuery); names != "" {
+ b.WriteString("?")
+ b.WriteString(names)
+ }
+ return clean(b.String(), MaxFragment), true
+}
diff --git a/internal/reqrec/redact_test.go b/internal/reqrec/redact_test.go
index d0478f1ee..938c067be 100644
--- a/internal/reqrec/redact_test.go
+++ b/internal/reqrec/redact_test.go
@@ -14,6 +14,13 @@ func TestMaskPath(t *testing.T) {
"/i/eyJhbGciOiJIUzI1NiJ9xQ2w9pLmNb7Ks4Ua/x.png": "/i/[masked]/x.png",
"/wp-login.php": "/wp-login.php",
"/short1234abcd": "/short1234abcd",
+ // Compounds: runs joined by - or _ are judged as one token.
+ "/r/Xq3vB9kLmN2pQr7sT1uVw-Yz4aBcDeFgHiJkLmNoPq": "/r/[masked]",
+ "/r/Xq3vB9kLmN2pQr7sT1uV_wYz4aBcDeFgHiJkLmNoPq": "/r/[masked]",
+ "/reset/3f9a0c1d-2e4b-5a69-7887-766554433221": "/reset/[masked]",
+ "/f/3f9a0c1d-2e4b-5a69-7887-766554433221.png": "/f/[masked].png",
+ "/blog/export-attendees-from-all-events-2023": "/blog/export-attendees-from-all-events-2023",
+ "/blog/Export-Attendees-From-All-Events": "/blog/Export-Attendees-From-All-Events",
}
for in, want := range cases {
if got := MaskPath(in); got != want {
@@ -63,3 +70,54 @@ func TestFragment(t *testing.T) {
t.Errorf("user-agent fragment = %q, %v", got, ok)
}
}
+
+func TestFragmentReferer(t *testing.T) {
+ cases := map[string]string{
+ "https://shop.example.com/login?password=hunter2": "https://shop.example.com/login?password",
+ "https://app.example.com/cb?code=4%2F0AfJohXn&state=xyz": "https://app.example.com/cb?code,state",
+ "https://shop.example.com/items/42": "https://shop.example.com/items/42",
+ "https://shop.example.com/items#access_token=abc": "https://shop.example.com/items",
+ "https://user:hunter2@shop.example.com/items?a=1": "https://shop.example.com/items?a",
+ "https://shop.example.com/reset/3f9a0c1d2e4b5a6978877665": "https://shop.example.com/reset/[masked]",
+ }
+ for in, want := range cases {
+ got, ok := Fragment(SourceHeader, "Referer", in)
+ if !ok || got != want {
+ t.Errorf("Referer %q = %q, %v; want %q", in, got, ok, want)
+ }
+ if strings.Contains(got, "hunter2") || strings.Contains(got, "AfJohXn") {
+ t.Errorf("Referer %q leaked a value: %q", in, got)
+ }
+ }
+ if got, ok := Fragment(SourceHeader, "referer", "http://%zz/\x7f"); ok {
+ t.Errorf("unparseable referer kept: %q", got)
+ }
+}
+
+func TestIsSecretNameBroad(t *testing.T) {
+ for _, name := range []string{"pass", "pw", "pin", "jwt", "bearer", "ticket", "cvv", "cvc", "card", "PASSWORD", "reset_token"} {
+ if !IsSecretName(name) {
+ t.Errorf("%q not treated as secret", name)
+ }
+ if _, ok := Fragment(SourceBody, name, "x"); ok {
+ t.Errorf("body field %q kept", name)
+ }
+ }
+ for _, name := range []string{"q", "page", "name"} {
+ if IsSecretName(name) {
+ t.Errorf("%q treated as secret", name)
+ }
+ }
+}
+
+func TestFragmentEmptyName(t *testing.T) {
+ if got, ok := Fragment(SourceBody, "", "name=bob&password=hunter2"); ok {
+ t.Errorf("unnamed body fragment kept: %q", got)
+ }
+ if got, ok := Fragment(SourceQuery, "", "token=x"); ok {
+ t.Errorf("unnamed query fragment kept: %q", got)
+ }
+ if got, ok := Fragment(SourceQuery, " ", "token=x"); ok {
+ t.Errorf("blank-named query fragment kept: %q", got)
+ }
+}