Skip to content

Commit 3e095a2

Browse files
alnrclaude
andcommitted
fix: redact the rate-limit header from Playwright traces
Sending the header from the browser puts it somewhere it was never exposed before. Traces capture complete request headers — the same property that made them useful for diagnosing the 429 — and CI uploads them as a build artifact of a public repository. GitHub masks secrets in workflow logs but not inside artifacts, so without this the change would have published the token that exempts CI from Ory Network's rate limits. Playwright has no redaction option: TracingStartOptions carries only name, title, screenshots, snapshots, live and sources. The archive is therefore rewritten once Tracing().Stop() has written it, replacing the value in every entry. The JSON-escaped spelling is replaced too, since the trace stores headers as JSON string values and a token containing a quote or backslash would otherwise sit there in a form a raw byte comparison misses. If the archive cannot be rewritten it is deleted rather than left in place. Losing one diagnostic is the far cheaper failure. Verified end to end: a login run with a dummy ORY_RATE_LIMIT_HEADER produces a trace holding 131 occurrences of the header name — which is also the first direct confirmation that the browser now sends it — zero occurrences of the value, and 139 placeholders. TestRedactInZip covers the rewriting itself, including the escaped spelling and the unreadable archive, and needs no network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tg5VWFUb7824qkrooUdvBA
1 parent 0858d99 commit 3e095a2

2 files changed

Lines changed: 203 additions & 1 deletion

File tree

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright © 2026 Ory Corp
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package testhelpers
5+
6+
import (
7+
"archive/zip"
8+
"encoding/json"
9+
"io"
10+
"os"
11+
"path/filepath"
12+
"testing"
13+
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/require"
16+
)
17+
18+
// TestRedactInZip pins the guarantee the Playwright traces depend on: this
19+
// repository is public and CI uploads the traces as an artifact, so the
20+
// rate-limit header value must not survive anywhere inside one.
21+
func TestRedactInZip(t *testing.T) {
22+
const secret = `s3cret"value\with-escapes`
23+
24+
writeArchive := func(t *testing.T, entries map[string]string) string {
25+
path := filepath.Join(t.TempDir(), "trace.zip")
26+
f, err := os.Create(path)
27+
require.NoError(t, err)
28+
defer f.Close()
29+
30+
w := zip.NewWriter(f)
31+
for name, content := range entries {
32+
e, err := w.Create(name)
33+
require.NoError(t, err)
34+
_, err = e.Write([]byte(content))
35+
require.NoError(t, err)
36+
}
37+
require.NoError(t, w.Close())
38+
return path
39+
}
40+
41+
readArchive := func(t *testing.T, path string) map[string]string {
42+
r, err := zip.OpenReader(path)
43+
require.NoError(t, err)
44+
defer r.Close()
45+
46+
out := make(map[string]string, len(r.File))
47+
for _, f := range r.File {
48+
src, err := f.Open()
49+
require.NoError(t, err)
50+
content, err := io.ReadAll(src)
51+
require.NoError(t, err)
52+
require.NoError(t, src.Close())
53+
out[f.Name] = string(content)
54+
}
55+
return out
56+
}
57+
58+
// The trace stores headers as JSON string values, so the secret appears in
59+
// its escaped spelling rather than verbatim.
60+
escaped, err := json.Marshal(secret)
61+
require.NoError(t, err)
62+
jsonEncoded := string(escaped)
63+
64+
t.Run("case=removes the secret in both spellings", func(t *testing.T) {
65+
path := writeArchive(t, map[string]string{
66+
"trace.network": `{"headers":[{"name":"Ory-RateLimit-Action","value":` + jsonEncoded + `}]}`,
67+
"trace.trace": "prefix " + secret + " suffix",
68+
"resources/1": "a response body mentioning nothing",
69+
})
70+
71+
require.NoError(t, redactInZip(path, secret))
72+
73+
for name, content := range readArchive(t, path) {
74+
assert.NotContains(t, content, secret, "%s still holds the raw secret", name)
75+
assert.NotContains(t, content, jsonEncoded[1:len(jsonEncoded)-1], "%s still holds the escaped secret", name)
76+
}
77+
})
78+
79+
t.Run("case=leaves the rest of the trace intact", func(t *testing.T) {
80+
path := writeArchive(t, map[string]string{
81+
"trace.trace": "keep me " + secret + " keep me too",
82+
"resources/1": "untouched",
83+
})
84+
85+
require.NoError(t, redactInZip(path, secret))
86+
87+
got := readArchive(t, path)
88+
assert.Equal(t, "keep me "+redactedPlaceholder+" keep me too", got["trace.trace"])
89+
assert.Equal(t, "untouched", got["resources/1"])
90+
})
91+
92+
t.Run("case=no configured secret leaves the archive alone", func(t *testing.T) {
93+
path := writeArchive(t, map[string]string{"trace.trace": "verbatim"})
94+
95+
require.NoError(t, redactInZip(path, ""))
96+
97+
assert.Equal(t, "verbatim", readArchive(t, path)["trace.trace"])
98+
})
99+
100+
t.Run("case=an unreadable archive is an error, so the caller can delete it", func(t *testing.T) {
101+
path := filepath.Join(t.TempDir(), "not-a-zip.zip")
102+
require.NoError(t, os.WriteFile(path, []byte("definitely not a zip"), 0o600))
103+
104+
assert.Error(t, redactInZip(path, secret))
105+
})
106+
}

cmd/cloudx/testhelpers/testhelpers.go

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package testhelpers
55

66
import (
7+
"archive/zip"
8+
"bytes"
79
"context"
810
"encoding/json"
911
"fmt"
@@ -307,6 +309,100 @@ func NewPage(t testing.TB, browser playwright.Browser) playwright.Page {
307309
return page
308310
}
309311

312+
// stopTracing writes the trace of the login flow and strips the rate-limit
313+
// header value out of it.
314+
//
315+
// A trace records complete request headers — this repository is public and CI
316+
// uploads the traces as a build artifact, where GitHub's secret masking does not
317+
// reach. The header that exempts CI from Ory Network's rate limits therefore
318+
// must not survive into one. Playwright offers no redaction option, so the
319+
// archive is rewritten after it has been written.
320+
//
321+
// If it cannot be rewritten the trace is deleted: losing a diagnostic is the
322+
// cheaper failure by far.
323+
func stopTracing(t testing.TB, page playwright.Page) {
324+
path := filepath.Join(tracesDir, fmt.Sprintf("%s.zip", t.Name()))
325+
if err := page.Context().Tracing().Stop(path); err != nil {
326+
t.Logf("tracing stop error: %+v", err)
327+
return
328+
}
329+
330+
_, secret, ok := client.RateLimitHeader()
331+
if !ok {
332+
return
333+
}
334+
if err := redactInZip(path, secret); err != nil {
335+
t.Logf("could not redact %s, removing it: %+v", path, err)
336+
require.NoError(t, os.Remove(path))
337+
}
338+
}
339+
340+
const redactedPlaceholder = "[redacted]"
341+
342+
// redactInZip rewrites every entry of the zip archive at path, replacing each
343+
// occurrence of secret with a placeholder.
344+
//
345+
// The JSON-escaped spelling is replaced as well, because the trace stores
346+
// headers as JSON string values and a secret containing a quote or backslash
347+
// would otherwise appear there in a form the raw comparison does not match.
348+
func redactInZip(path, secret string) error {
349+
if secret == "" {
350+
return nil
351+
}
352+
353+
needles := [][]byte{[]byte(secret)}
354+
if escaped, err := json.Marshal(secret); err == nil {
355+
if inner := escaped[1 : len(escaped)-1]; !bytes.Equal(inner, []byte(secret)) {
356+
needles = append(needles, inner)
357+
}
358+
}
359+
360+
r, err := zip.OpenReader(path)
361+
if err != nil {
362+
return err
363+
}
364+
365+
tmp, err := os.CreateTemp(filepath.Dir(path), "trace-*.zip")
366+
if err != nil {
367+
_ = r.Close()
368+
return err
369+
}
370+
defer os.Remove(tmp.Name()) // no-op once the rename below succeeded
371+
372+
err = func() error {
373+
w := zip.NewWriter(tmp)
374+
for _, f := range r.File {
375+
src, err := f.Open()
376+
if err != nil {
377+
return err
378+
}
379+
content, err := io.ReadAll(src)
380+
_ = src.Close()
381+
if err != nil {
382+
return err
383+
}
384+
for _, needle := range needles {
385+
content = bytes.ReplaceAll(content, needle, []byte(redactedPlaceholder))
386+
}
387+
dst, err := w.Create(f.Name)
388+
if err != nil {
389+
return err
390+
}
391+
if _, err := dst.Write(content); err != nil {
392+
return err
393+
}
394+
}
395+
return w.Close()
396+
}()
397+
_ = r.Close()
398+
_ = tmp.Close()
399+
if err != nil {
400+
return err
401+
}
402+
403+
return os.Rename(tmp.Name(), path)
404+
}
405+
310406
// submitPasswordForm submits the filled-in login form and fails immediately if
311407
// the server refused the request outright.
312408
//
@@ -353,7 +449,7 @@ func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, emai
353449
}))
354450
defer func() {
355451
r := recover()
356-
_ = page.Context().Tracing().Stop(filepath.Join(tracesDir, fmt.Sprintf("%s.zip", t.Name())))
452+
stopTracing(t, page)
357453
if r != nil {
358454
panic(r)
359455
}

0 commit comments

Comments
 (0)