fix(02-03): scrub PKCE and OAuth form fields by name
Value-based replace can miss a code_verifier when an authorization code is a substring of it, which 502'd MCP token capture. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -403,3 +403,42 @@ func TestScrubShortNumericIDsDoNotCorruptPaths(t *testing.T) {
|
|||||||
t.Fatalf("substring replace leaked: %s", body)
|
t.Fatalf("substring replace leaked: %s", body)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestScrubFormFieldDespiteSubstringSecrets(t *testing.T) {
|
||||||
|
store, err := OpenStore("")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
code := "overlapSECRET99"
|
||||||
|
verifier := "xx" + code + "yyPKCEverifierValue"
|
||||||
|
store.Set("oauth:code", code)
|
||||||
|
step := Step{
|
||||||
|
ID: "20",
|
||||||
|
Request: Request{
|
||||||
|
Method: "POST",
|
||||||
|
Path: "/oauth/mcp/token",
|
||||||
|
Body: Body("grant_type=authorization_code&code=" + code + "&code_verifier=" + verifier),
|
||||||
|
},
|
||||||
|
Response: Response{Status: 200, Body: Body(`{"ok":true}`)},
|
||||||
|
Capture: []CaptureRule{
|
||||||
|
{From: "request.form", Name: "code_verifier", As: "pkce:mcp", Category: "pkce"},
|
||||||
|
{From: "request.form", Name: "code", As: "oauth:code", Category: "oauth_code"},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
if err := CaptureStep(store, &step); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := ScrubStep(store, &step); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
got := string(step.Request.Body)
|
||||||
|
if strings.Contains(got, verifier) || strings.Contains(got, code) {
|
||||||
|
t.Fatalf("form still live: %s", got)
|
||||||
|
}
|
||||||
|
if !strings.Contains(got, "code_verifier={{pkce:mcp}}") {
|
||||||
|
t.Fatalf("verifier placeholder: %s", got)
|
||||||
|
}
|
||||||
|
if !strings.Contains(got, "code={{oauth:code}}") {
|
||||||
|
t.Fatalf("code placeholder: %s", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -448,6 +448,15 @@ func ScrubStep(store *Store, step *Step) error {
|
|||||||
step.Request.Query = replaceAll(step.Request.Query, pairs)
|
step.Request.Query = replaceAll(step.Request.Query, pairs)
|
||||||
step.Request.Headers = scrubMap(step.Request.Headers, pairs)
|
step.Request.Headers = scrubMap(step.Request.Headers, pairs)
|
||||||
step.Request.Body = Body(replaceAll(string(step.Request.Body), pairs))
|
step.Request.Body = Body(replaceAll(string(step.Request.Body), pairs))
|
||||||
|
for _, rule := range step.Capture {
|
||||||
|
if strings.TrimSpace(rule.From) != "request.form" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if rule.Name == "" || rule.As == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
step.Request.Body = Body(scrubFormField(string(step.Request.Body), rule.Name, rule.As))
|
||||||
|
}
|
||||||
step.Response.Headers = scrubMap(step.Response.Headers, pairs)
|
step.Response.Headers = scrubMap(step.Response.Headers, pairs)
|
||||||
step.Response.Body = Body(replaceAll(string(step.Response.Body), pairs))
|
step.Response.Body = Body(replaceAll(string(step.Response.Body), pairs))
|
||||||
return rejectUnclassifiedCredentials(*step)
|
return rejectUnclassifiedCredentials(*step)
|
||||||
@@ -510,6 +519,14 @@ func phpJSONEscape(s string) string {
|
|||||||
return strings.ReplaceAll(s, "/", `\/`)
|
return strings.ReplaceAll(s, "/", `\/`)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func scrubFormField(body, name, as string) string {
|
||||||
|
if name == "" || as == "" || body == "" {
|
||||||
|
return body
|
||||||
|
}
|
||||||
|
re := regexp.MustCompile(`(?i)(^|&)(` + regexp.QuoteMeta(name) + `=)[^&]*`)
|
||||||
|
return re.ReplaceAllString(body, `${1}${2}{{`+as+`}}`)
|
||||||
|
}
|
||||||
|
|
||||||
func replaceIsolated(s, old, neu string) string {
|
func replaceIsolated(s, old, neu string) string {
|
||||||
if old == "" || s == "" {
|
if old == "" || s == "" {
|
||||||
return s
|
return s
|
||||||
|
|||||||
Reference in New Issue
Block a user