carlos / platform Public

reqrec: mask long runs and slash-split base64, drop bare query components, escape controls (final review C1, I4)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev 82040d4041e5f41cb7403bca9dbd41ba006eee1b parent 7129128
2 files changed, +198 −32
  • internal/reqrec/redact.go +143 −32
  • internal/reqrec/redact_test.go +55 −0
diff --git a/internal/reqrec/redact.go b/internal/reqrec/redact.go
index 341b396f1..661260c7c 100644
--- a/internal/reqrec/redact.go
+++ b/internal/reqrec/redact.go
@@ -7,6 +7,7 @@
package reqrec
import (
+ "math/bits"
"net/url"
"strings"
"unicode/utf8"
@@ -38,27 +39,30 @@ func isHexByte(c byte) bool {
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).
-func maskable(run string) bool {
- if len(run) < minRun {
- return false
- }
- letter, digit, hex := false, false, true
- for i := 0; i < len(run); i++ {
- c := run[i]
- switch {
- case c >= '0' && c <= '9':
- digit = true
- case c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z':
- letter = true
- }
- if !isHexByte(c) {
- hex = false
+// isBase64Byte is the standard base64 alphabet, which has '/' in it.
+func isBase64Byte(c byte) bool { return isRunByte(c) || c == '/' }
+
+// The character classes a run can mix, as bits.
+const (
+ classUpper = 1 << iota
+ classLower
+ classDigit
+ classAll = classUpper | classLower | classDigit
+)
+
+func classesOf(s string) int {
+ var c int
+ for i := 0; i < len(s); i++ {
+ switch b := s[i]; {
+ case b >= 'A' && b <= 'Z':
+ c |= classUpper
+ case b >= 'a' && b <= 'z':
+ c |= classLower
+ case b >= '0' && b <= '9':
+ c |= classDigit
}
}
- return letter && digit || hex
+ return c
}
// maskableCompound reports whether a maximal run of [A-Za-z0-9+=_-] is a
@@ -119,14 +123,114 @@ func maskMatching(s string, in func(byte) bool, test func(string) bool) 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.
+// maskSplitTokens masks standard base64 tokens that a '/' has cut into short
+// path segments. Within each run of [A-Za-z0-9+=/] it finds the stretches of
+// consecutive segments that each mix at least two of upper case, lower case
+// and digits, and masks a stretch as one span when, joined, it holds all
+// three and at least minRun characters. A segment of one class ("reset",
+// "2023") or an empty one ("//") ends a stretch, so the path words around a
+// token are kept; Title Case words hold no digit, so a path of them is kept.
+func maskSplitTokens(s string) string {
+ var b strings.Builder
+ last := 0
+ for i := 0; i < len(s); {
+ if !isBase64Byte(s[i]) {
+ i++
+ continue
+ }
+ j := i
+ for j < len(s) && isBase64Byte(s[j]) {
+ j++
+ }
+ // The run's segments as [start, end) offsets into s.
+ var segs [][2]int
+ for p := i; p <= j; {
+ q := p
+ for q < j && s[q] != '/' {
+ q++
+ }
+ segs = append(segs, [2]int{p, q})
+ p = q + 1
+ }
+ mixed := func(k int) (int, bool) {
+ c := classesOf(s[segs[k][0]:segs[k][1]])
+ return c, bits.OnesCount(uint(c)) >= 2
+ }
+ for a := 0; a < len(segs); {
+ if _, ok := mixed(a); !ok {
+ a++
+ continue
+ }
+ z, cls, n := a, 0, 0
+ for z < len(segs) {
+ c, ok := mixed(z)
+ if !ok {
+ break
+ }
+ cls |= c
+ n += segs[z][1] - segs[z][0]
+ z++
+ }
+ if cls == classAll && n >= minRun {
+ b.WriteString(s[last:segs[a][0]])
+ b.WriteString(MaskToken)
+ last = segs[z-1][1]
+ }
+ a = z
+ }
+ i = j
+ }
+ if last == 0 {
+ return s
+ }
+ b.WriteString(s[last:])
+ return b.String()
+}
+
+// maskRuns masks secret-looking tokens in s. The rule leans to masking: a
+// masked word costs a little evidence, a kept token is a secret in a log.
+// Three passes, in order:
+//
+// 1. Each compound of [A-Za-z0-9+=_-] is judged whole (maskableCompound),
+// which catches UUIDs and base64url strings that a - or _ would
+// otherwise split into short pieces.
+// 2. Standard base64 split into path segments at '/' is judged joined
+// (maskSplitTokens).
+// 3. Every run of [A-Za-z0-9+=] of minRun or more characters is masked,
+// whatever it holds: mixed case with no digit, digits only, hex beside
+// a non-hex letter. Words that long are rare in a path; ids, hashes and
+// tokens are common. This also masks a long token inside a hyphenated
+// name (application-<hash>.js) and leaves the name.
func maskRuns(s string) string {
s = maskMatching(s, isCompoundByte, maskableCompound)
- return maskMatching(s, isRunByte, maskable)
+ s = maskSplitTokens(s)
+ return maskMatching(s, isRunByte, func(run string) bool { return len(run) >= minRun })
+}
+
+// isControl reports whether r is a C0 control, DEL or a C1 control: the
+// characters a terminal acts on rather than prints.
+func isControl(r rune) bool { return r < 0x20 || r == 0x7f || r >= 0x80 && r <= 0x9f }
+
+// escapeControls writes every control character in s as \xNN (its code
+// point in hex), so request text in a record cannot forge a log line or
+// drive the terminal of whoever prints it. JSON alone does not do this:
+// it escapes controls on the way into a chunk and decoding restores them.
+func escapeControls(s string) string {
+ if strings.IndexFunc(s, isControl) < 0 {
+ return s
+ }
+ const hexDigits = "0123456789abcdef"
+ var b strings.Builder
+ for _, r := range s {
+ if isControl(r) {
+ b.WriteString(`\x`)
+ b.WriteByte(hexDigits[r>>4])
+ b.WriteByte(hexDigits[r&0xf])
+ continue
+ }
+ b.WriteRune(r)
+ }
+ return b.String()
}
// truncate cuts s to at most n bytes without splitting a rune. s must already
@@ -141,21 +245,25 @@ func truncate(s string, n int) string {
return s[:n]
}
-// clean makes s valid UTF-8, masks secret-looking runs and caps the length.
+// clean makes s valid UTF-8, masks secret-looking runs, escapes controls and
+// caps the length. The cap comes last so escaping cannot push s past it.
func clean(s string, max int) string {
s = strings.ToValidUTF8(s, "�")
- return truncate(maskRuns(s), max)
+ return truncate(escapeControls(maskRuns(s)), max)
}
// MaskPath returns a request path that is safe to store: valid UTF-8, with
-// token-like runs masked, and at most MaxPath bytes.
+// token-like runs masked and controls escaped, and at most MaxPath bytes.
func MaskPath(p string) string {
return clean(p, MaxPath)
}
// QueryNames returns the parameter names of a raw query string, comma-joined,
-// in first-seen order and without repeats. Values are never kept. A name that
-// would push the result past MaxQueryNames bytes is dropped whole.
+// in first-seen order and without repeats. Values are never kept. A component
+// with no '=' (?hunter2) is left out: it has no name, and what it carries may
+// be the value itself. Names are masked and their controls escaped like a
+// path. A name that would push the result past MaxQueryNames bytes is dropped
+// whole.
func QueryNames(rawQuery string) string {
if rawQuery == "" {
return ""
@@ -163,12 +271,15 @@ func QueryNames(rawQuery string) string {
seen := map[string]bool{}
var out strings.Builder
for _, pair := range strings.FieldsFunc(rawQuery, func(r rune) bool { return r == '&' || r == ';' }) {
- name, _, _ := strings.Cut(pair, "=")
+ name, _, ok := strings.Cut(pair, "=")
+ if !ok {
+ continue
+ }
if un, err := url.QueryUnescape(name); err == nil {
name = un
}
name = strings.ToValidUTF8(name, "�")
- name = maskRuns(name)
+ name = escapeControls(maskRuns(name))
if name == "" || seen[name] {
continue
}
diff --git a/internal/reqrec/redact_test.go b/internal/reqrec/redact_test.go
index 938c067be..ea3d12b45 100644
--- a/internal/reqrec/redact_test.go
+++ b/internal/reqrec/redact_test.go
@@ -21,6 +21,15 @@ func TestMaskPath(t *testing.T) {
"/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",
+ // Final review C1: a long run with no digit, standard base64 split
+ // into short pieces at '/', and long hex beside a non-hex letter.
+ "/reset/rKpWmTzQvLsNxYhJbCfDaGeS": "/reset/[masked]",
+ "/reset/Xq3vB9kLmN2pQr7s/T1uVwYz4aBcDeFg": "/reset/[masked]",
+ "/zabcdefabcdefabcdefabcdefabcdefab": "/[masked]",
+ "/a/Xq3vB9kL+mN2pQr7s/T1uVwYz4aBcDeFg=/b.js": "/a/[masked]/b.js",
+ // Title-case segments are words, not a split token.
+ "/en/Products/iPhone/Accessories/Cases": "/en/Products/iPhone/Accessories/Cases",
+ "/api/v2/users/12345/orders/67890": "/api/v2/users/12345/orders/67890",
}
for in, want := range cases {
if got := MaskPath(in); got != want {
@@ -43,6 +52,12 @@ func TestQueryNames(t *testing.T) {
if got := QueryNames(""); got != "" {
t.Errorf("got %q", got)
}
+ // A bare component has no name to keep: it may be the value itself.
+ for in, want := range map[string]string{"hunter2": "", "a=1&hunter2&b=": "a,b", "hunter2;x=1": "x"} {
+ if got := QueryNames(in); got != want {
+ t.Errorf("QueryNames(%q) = %q, want %q", in, got, want)
+ }
+ }
many := strings.Repeat("abcdefgh=1&", 2000)
if got := QueryNames(many); len(got) > MaxQueryNames {
t.Errorf("not capped: %d bytes", len(got))
@@ -79,6 +94,8 @@ func TestFragmentReferer(t *testing.T) {
"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]",
+ "https://shop.example.com/r?hunter2": "https://shop.example.com/r",
+ "https://shop.example.com/r?a=1&hunter2": "https://shop.example.com/r?a",
}
for in, want := range cases {
got, ok := Fragment(SourceHeader, "Referer", in)
@@ -121,3 +138,41 @@ func TestFragmentEmptyName(t *testing.T) {
t.Errorf("blank-named query fragment kept: %q", got)
}
}
+
+// TestControlsEscaped: request text a record keeps never carries a C0
+// control, DEL or a C1 control, which a terminal would act on when the record
+// is printed. Each becomes \xNN.
+func TestControlsEscaped(t *testing.T) {
+ bad := func(s string) bool {
+ for _, r := range s {
+ if r < 0x20 || r == 0x7f || r >= 0x80 && r <= 0x9f {
+ return true
+ }
+ }
+ return false
+ }
+ if got, want := MaskPath("/x\x1b[2J\nforged\r\x00\x7f\u009b"), `/x\x1b[2J\x0aforged\x0d\x00\x7f\x9b`; got != want {
+ t.Errorf("MaskPath = %q, want %q", got, want)
+ }
+ if got, want := QueryNames("a%0Ab=1&c%1b=2"), `a\x0ab,c\x1b`; got != want {
+ t.Errorf("QueryNames = %q, want %q", got, want)
+ }
+ for _, in := range []struct {
+ src Source
+ name, val string
+ }{
+ {SourceQuery, "q", "x\ny\x1bz"},
+ {SourceHeader, "User-Agent", "ua\r\n\x9b"},
+ {SourceHeader, "Referer", "https://h.example/p%0a%1b?x%0d=1"},
+ {SourcePath, "", "/p\x07"},
+ } {
+ got, ok := Fragment(in.src, in.name, in.val)
+ if !ok || bad(got) || !strings.Contains(got, `\x`) {
+ t.Errorf("Fragment(%s, %q, %q) = %q, %v", in.src, in.name, in.val, got, ok)
+ }
+ }
+ // Escaping happens before the cap, so a path of controls is still capped.
+ if got := MaskPath(strings.Repeat("\x1b", 1000)); len(got) > MaxPath || bad(got) {
+ t.Errorf("escaped path %d bytes, controls=%v", len(got), bad(got))
+ }
+}