| index 2809b8740..01423573f 100644 |
| --- a/internal/edge/carlosjs_test.go |
| +++ b/internal/edge/carlosjs_test.go |
| @@ -5,6 +5,7 @@ import ( |
| "net/http/httptest" |
| "os" |
| "os/exec" |
| + "strconv" |
| "strings" |
| "testing" |
| ) |
| @@ -93,14 +94,21 @@ func TestCarlosJSSanitizer(t *testing.T) { |
| |
| // A 40-char string that matches san()'s email pattern in full: 28 |
| // local-part chars + "@example.com" (12 chars). |
| - email40 := strings.Repeat("a", 28) + "@example.com" |
| + localPart := strings.Repeat("a", 28) |
| + email40 := localPart + "@example.com" |
| if len(email40) != 40 { |
| t.Fatalf("test bug: email40 is %d chars, want 40", len(email40)) |
| } |
| - // Filler for the truncation case must NOT be a valid email local-part |
| - // character (letters/digits/._%+- all qualify), or the greedy email |
| - // regex swallows the filler into the "email" match too. |
| - tail9000 := strings.Repeat("!", 9000-len(email40)) + email40 |
| + // A secret that genuinely straddles the 8192 truncation boundary: 8180 |
| + // filler chars, then the 40-char email (spanning indices 8180-8219, so |
| + // its '@' at 8208 sits past the old naive cut point), then 500 more |
| + // filler chars. Against the pre-fix ordering (slice to 8192 BEFORE |
| + // sanitising) this would truncate mid-local-part with no '@' anywhere |
| + // in the fragment, so the regex can't match and the local part's head |
| + // leaks raw. Filler must not be a valid email local-part character |
| + // (letters/digits/._%+- all qualify) or it gets swallowed into the |
| + // match too. |
| + straddle := strings.Repeat("!", 8180) + email40 + strings.Repeat("!", 500) |
| |
| cases := []string{ |
| "contact me at foo@example.com now", |
| @@ -110,12 +118,18 @@ func TestCarlosJSSanitizer(t *testing.T) { |
| "aB3dE6fG9hJ2kL5mN8pQ1rS4tU7vW+xYzAbCdEfG", // 40-char mixed base64-ish run with '+', no '/' |
| "at Object.render (https://cdn.example.com/static/js/mainChunkVendors12ab/bundleFileName/index.js:2:1)", // stack frame: unchanged |
| "/some/Long/Path1/with/many/segments/here/ok", // ordinary path: unchanged |
| - tail9000, // 9000 chars; the last 40 are an email |
| + straddle, // straddles the 8192 truncation boundary |
| } |
| casesJSON, err := json.Marshal(cases) |
| if err != nil { |
| t.Fatal(err) |
| } |
| + // Proves the pre-cap (san() caps to 16384 chars before running the |
| + // regexes) bounds the base64 rule's cost: without it, this 40,000-char |
| + // run of hex-valid characters is quadratic in the regex engine's |
| + // lookaheads. Timed inside node with Date.now() rather than around the |
| + // whole exec.Command call, which would also cover node's own startup. |
| + timedInput := strings.Repeat("Ab", 20000) |
| |
| script := "var window={addEventListener:function(){}};\n" + |
| "var navigator={sendBeacon:function(){return true;}};\n" + |
| @@ -123,22 +137,38 @@ func TestCarlosJSSanitizer(t *testing.T) { |
| "var document={createElement:function(){return{};},head:{appendChild:function(){}}};\n" + |
| carlosJS + "\n" + |
| "var cases=" + string(casesJSON) + ";\n" + |
| - "process.stdout.write(JSON.stringify(cases.map(function(c){return window.carlos.__san(c);})));\n" |
| + "var results=cases.map(function(c){return window.carlos.__san(c);});\n" + |
| + "var timedInput=" + strconv.Quote(timedInput) + ";\n" + |
| + "var t0=Date.now();\n" + |
| + "var timedResult=window.carlos.__san(timedInput);\n" + |
| + "var ms=Date.now()-t0;\n" + |
| + "process.stdout.write(JSON.stringify({results:results,timedResultLen:timedResult.length,ms:ms}));\n" |
| |
| dir := t.TempDir() |
| file := dir + "/san-check.js" |
| if err := os.WriteFile(file, []byte(script), 0o644); err != nil { |
| t.Fatal(err) |
| } |
| - out, err := exec.Command(nodePath, file).CombinedOutput() |
| + // Output(), not CombinedOutput(): a stderr warning from node must not |
| + // be able to corrupt the JSON on stdout. |
| + out, err := exec.Command(nodePath, file).Output() |
| if err != nil { |
| - t.Fatalf("node: %v\n%s", err, out) |
| + stderr := "" |
| + if ee, ok := err.(*exec.ExitError); ok { |
| + stderr = string(ee.Stderr) |
| + } |
| + t.Fatalf("node: %v\n%s", err, stderr) |
| } |
| |
| - var got []string |
| - if err := json.Unmarshal(out, &got); err != nil { |
| + var res struct { |
| + Results []string `json:"results"` |
| + TimedResultLen int `json:"timedResultLen"` |
| + Ms float64 `json:"ms"` |
| + } |
| + if err := json.Unmarshal(out, &res); err != nil { |
| t.Fatalf("node output not JSON: %v\n%s", err, out) |
| } |
| + got := res.Results |
| if len(got) != len(cases) { |
| t.Fatalf("got %d results, want %d", len(got), len(cases)) |
| } |
| @@ -158,19 +188,29 @@ func TestCarlosJSSanitizer(t *testing.T) { |
| } |
| } |
| |
| - // The 9000-char case: sanitised before truncation (never leaks a raw |
| - // secret straddling the 8192 cut), never over the 8192 cap, and never |
| - // leaves a bare '@' behind even when the redacted tag itself lands |
| - // beyond the final window and is truncated away with it. |
| + // The straddling case: sanitised in full before the final truncation, |
| + // so the complete email is redacted rather than leaking a raw |
| + // fragment. Must fit within the cap, must carry the <email> tag, and |
| + // must show no trace of the raw secret (no '@' and no surviving run |
| + // of the local part's characters). |
| last := got[len(got)-1] |
| if len(last) > 8192 { |
| - t.Errorf("9000-char case: result is %d bytes, want <= 8192", len(last)) |
| + t.Errorf("straddle case: result is %d bytes, want <= 8192", len(last)) |
| } |
| - if !strings.Contains(last, "<email>") && strings.Contains(last, "@") { |
| - tail := last |
| - if len(tail) > 40 { |
| - tail = tail[len(tail)-40:] |
| - } |
| - t.Errorf("9000-char case: raw '@' survived sanitisation+truncation, tail %q", tail) |
| + if !strings.Contains(last, "<email>") { |
| + t.Errorf("straddle case: result lacks <email>: %q", last) |
| + } |
| + if strings.Contains(last, "@") { |
| + t.Errorf("straddle case: raw '@' survived sanitisation+truncation: %q", last) |
| + } |
| + if strings.Contains(last, strings.Repeat("a", 6)) { |
| + t.Errorf("straddle case: a run of the email's local-part characters survived: %q", last) |
| + } |
| + |
| + // The pre-cap keeps a 40,000-char hex-valid run's sanitisation well |
| + // under a second; before the fix this was 7s at 100KB and 28s at |
| + // 200KB on the main thread. |
| + if res.Ms >= 1000 { |
| + t.Errorf("40,000-char case: san() took %vms, want < 1000ms (result len %d)", res.Ms, res.TimedResultLen) |
| } |
| } |
amadan