[Graphite MQ] Draft PR GROUP:spec_3f3909 (PRs 42)

internal/client/diagnostics.go

@@ -57,19 +57,6 @@ var sensitiveHeaderSuffixes = []string{
 	"-token",
 }
 
-// sensitiveJSONKeys lists JSON field names that should be redacted.
-var sensitiveJSONKeys = map[string]bool{
-	"password":      true,
-	"secret":        true,
-	"token":         true,
-	"access_token":  true,
-	"refresh_token": true,
-	"api_key":       true,
-	"apikey":        true,
-	"private_key":   true,
-	"client_secret": true,
-}
-
 // isSensitiveHeader returns true if the header key matches a sensitive pattern.
 func isSensitiveHeader(key string) bool {
 	lower := strings.ToLower(key)
@@ -106,10 +93,12 @@ func redactJSON(v interface{}, depth int) interface{} {
 	}
 	switch val := v.(type) {
 	case map[string]interface{}:
+		// Which values are hidden, and why: see redact.go.
+		credentialPair := isCredentialPair(val)
 		out := make(map[string]interface{}, len(val))
 		for k, child := range val {
-			if sensitiveJSONKeys[strings.ToLower(k)] {
-				out[k] = "[REDACTED]"
+			if hidesValue(k, child) || (credentialPair && k == "value" && canHoldSecret(child)) {
+				out[k] = redacted
 			} else {
 				out[k] = redactJSON(child, depth+1)
 			}
@@ -205,7 +194,7 @@ func (c *DebugClient) Do(req *http.Request) (*http.Response, error) {
 	if req.Body != nil {
 		bodyData, restored := readAndRestoreBody(req.Body)
 		req.Body = restored
-		fmt.Fprintf(c.Stderr, "[DEBUG] Request Body:\n  %s\n", redactBody(bodyData))
+		fmt.Fprintf(c.Stderr, "[DEBUG] Request Body:\n  %s\n", redactRequestBody(req.URL.Path, bodyData))
 	}
 
 	// Execute
@@ -241,7 +230,7 @@ func (c *DryRunClient) Do(req *http.Request) (*http.Response, error) {
 	fmt.Fprintf(c.Stderr, "[DRY-RUN] Headers:\n%s", formatHeaders(redactHeaders(req.Header)))
 	if req.Body != nil {
 		bodyData, _ := readAndRestoreBody(req.Body)
-		fmt.Fprintf(c.Stderr, "[DRY-RUN] Body:\n  %s\n", redactBody(bodyData))
+		fmt.Fprintf(c.Stderr, "[DRY-RUN] Body:\n  %s\n", redactRequestBody(req.URL.Path, bodyData))
 	}
 	fmt.Fprintf(c.Stderr, "[DRY-RUN] Network call skipped.\n")
 

internal/client/redact.go

@@ -0,0 +1,137 @@
+// This file is not generated by Speakeasy. It decides which values --dry-run
+// and --debug hide when they print request and response bodies.
+//
+// The generated rule hid a fixed list of lowercase names: password, secret,
+// token, api_key and a few more. The Voiceflow API names its fields in
+// camelCase, so most of the secrets it carries printed in full: apiKeySecret,
+// keySecret, secretKey, oauthSecretKey, webhookSecret, webhookSecretKey and
+// authHeaderValue in integration credentials, a secret's value in secret
+// create and set-value, and Authorization values in API tool and MCP server
+// headers. Coding agents keep stderr in their transcripts, so a dry run or a
+// debug run leaked them.
+//
+// The rule is fitted to a survey of all 868 property names in the OpenAPI spec
+// (.speakeasy/out.openapi.yaml):
+//
+//   - Names are compared without case or separators, so apiKey, api_key and
+//     API-KEY are one name. A name that ends in secret, secretkey, password,
+//     token, apikey or privatekey holds a secret, which covers every name the
+//     generated list had. Counts such as maxTokens end in "tokens", and
+//     identifiers such as accountSid, apiKeySid, keyId and accessTokenID end in
+//     none of them, so they stay visible.
+//   - credentials is hidden whole. It holds an integration's secrets, some
+//     under names, such as authHeaderValue, that no general rule would catch.
+//   - A {key, value} pair, the shape of API tool and MCP server headers and of
+//     query parameters, hides its value when the key names a credential, such
+//     as Authorization or X-Api-Key.
+//   - The secret endpoints carry the secret in generic fields: defaultValue on
+//     create and value on set-value. Those are hidden on requests to them.
+//
+// Booleans, numbers and null are never hidden: hasPassword says whether a
+// password is set, not what it is.
+
+package client
+
+import (
+	"encoding/json"
+	"strings"
+	"unicode"
+)
+
+// redacted replaces every hidden value.
+const redacted = "[REDACTED]"
+
+// secretNameSuffixes end the normalized name of every field holding a secret.
+var secretNameSuffixes = []string{"secret", "secretkey", "password", "token", "apikey", "privatekey"}
+
+// credentialContainers are hidden whole, whatever they hold.
+var credentialContainers = map[string]bool{"credential": true, "credentials": true}
+
+// secretEndpointFields hold the secret itself on the secret endpoints.
+var secretEndpointFields = []string{"defaultValue", "value"}
+
+// normalizeName lowercases a field name and drops separators.
+func normalizeName(name string) string {
+	return strings.Map(func(r rune) rune {
+		switch r {
+		case '_', '-', '.', ' ':
+			return -1
+		}
+		return unicode.ToLower(r)
+	}, name)
+}
+
+// namesASecret reports whether a field of this name holds a secret.
+func namesASecret(name string) bool {
+	normalized := normalizeName(name)
+	if credentialContainers[normalized] {
+		return true
+	}
+	for _, suffix := range secretNameSuffixes {
+		if strings.HasSuffix(normalized, suffix) {
+			return true
+		}
+	}
+	return false
+}
+
+// canHoldSecret reports whether a decoded JSON value could be a secret: a
+// boolean, a number or null cannot.
+func canHoldSecret(value interface{}) bool {
+	switch value.(type) {
+	case bool, float64, json.Number, nil:
+		return false
+	}
+	return true
+}
+
+// hidesValue reports whether redactJSON hides the value of field name.
+func hidesValue(name string, value interface{}) bool {
+	return canHoldSecret(value) && namesASecret(name)
+}
+
+// isCredentialPair reports whether obj is a {key, value} pair, such as a header
+// or a query parameter, whose key names a credential.
+func isCredentialPair(obj map[string]interface{}) bool {
+	key, ok := obj["key"].(string)
+	if !ok {
+		return false
+	}
+	if _, hasValue := obj["value"]; !hasValue {
+		return false
+	}
+	return isSensitiveHeader(key) || namesASecret(key)
+}
+
+// isSecretEndpoint reports whether path is one of the secret endpoints, such
+// as /v2/stable/secret or /v1/stable/secret/{secretID}/value.
+func isSecretEndpoint(path string) bool {
+	for _, segment := range strings.Split(path, "/") {
+		if segment == "secret" {
+			return true
+		}
+	}
+	return false
+}
+
+// redactRequestBody is redactBody for a request to path. On the secret
+// endpoints it also hides the generic fields that carry the secret itself.
+func redactRequestBody(path string, body []byte) string {
+	if !isSecretEndpoint(path) {
+		return redactBody(body)
+	}
+	var fields map[string]interface{}
+	if err := json.Unmarshal(body, &fields); err != nil {
+		return redactBody(body)
+	}
+	for _, name := range secretEndpointFields {
+		if value, ok := fields[name]; ok && canHoldSecret(value) {
+			fields[name] = redacted
+		}
+	}
+	hidden, err := json.Marshal(fields)
+	if err != nil {
+		return redactBody(body)
+	}
+	return redactBody(hidden)
+}

internal/client/redact_test.go

@@ -0,0 +1,112 @@
+package client
+
+import (
+	"strings"
+	"testing"
+)
+
+// The names come from a survey of the OpenAPI spec; see redact.go.
+func TestNamesASecret(t *testing.T) {
+	secrets := []string{
+		// integration credentials
+		"apiKey", "apiKeySecret", "keySecret", "secretKey", "oauthSecretKey", "webhookSecret", "webhookSecretKey", "credentials",
+		// common spellings, whatever the case or separator
+		"password", "secret", "token", "accessToken", "refresh_token", "clientSecret", "client_secret", "API-KEY", "privateKey",
+	}
+	for _, name := range secrets {
+		if !namesASecret(name) {
+			t.Errorf("namesASecret(%q) = false, want true", name)
+		}
+	}
+
+	visible := []string{
+		// identifiers that sit next to secrets
+		"accountSid", "apiKeySid", "keyId", "clientId", "appId", "accessTokenID", "authHeaderName", "secretIDs", "secretNameToId",
+		// token counts
+		"maxTokens", "queryTokens", "answerTokens", "tokens",
+		// ordinary fields
+		"key", "value", "name", "sessionID", "authType", "translationKey", "s3Key",
+	}
+	for _, name := range visible {
+		if namesASecret(name) {
+			t.Errorf("namesASecret(%q) = true, want false", name)
+		}
+	}
+}
+
+// The generated rule hid exactly these names. None may become visible.
+func TestEveryNameTheGeneratedListHidStaysHidden(t *testing.T) {
+	for _, name := range []string{"password", "secret", "token", "access_token", "refresh_token", "api_key", "apikey", "private_key", "client_secret"} {
+		if !namesASecret(name) {
+			t.Errorf("namesASecret(%q) = false; the generated list hid it", name)
+		}
+	}
+}
+
+func TestRedactBodyHidesSecretsAndKeepsTheRest(t *testing.T) {
+	body := `{
+		"integration": "twilio",
+		"credentials": {"apiKeySid": "SK1", "apiKeySecret": "canary-twilio", "accountSid": "AC1"},
+		"headers": [
+			{"key": "Authorization", "value": "Bearer canary-header"},
+			{"key": "x-api-key", "value": "canary-api-key-header"},
+			{"key": "X-Region", "value": "eu-west"}
+		],
+		"settings": {"clientSecret": "canary-nested", "hasPassword": true, "maxTokens": 512}
+	}`
+
+	out := redactBody([]byte(body))
+
+	for _, secret := range []string{"canary-twilio", "canary-header", "canary-api-key-header", "canary-nested"} {
+		if strings.Contains(out, secret) {
+			t.Errorf("%q leaked:\n%s", secret, out)
+		}
+	}
+	for _, kept := range []string{`"integration": "twilio"`, `"X-Region"`, `"eu-west"`, `"hasPassword": true`, `"maxTokens": 512`} {
+		if !strings.Contains(out, kept) {
+			t.Errorf("%s was hidden:\n%s", kept, out)
+		}
+	}
+}
+
+// A credential pair hides only a value that could be a secret. A null, a
+// boolean or a number keeps its value and its JSON type.
+func TestRedactBodyKeepsACredentialPairsNonSecretValue(t *testing.T) {
+	body := `{"headers": [
+		{"key": "Authorization", "value": null},
+		{"key": "X-Api-Key", "value": false},
+		{"key": "X-Auth-Token", "value": 7}
+	]}`
+
+	out := redactBody([]byte(body))
+
+	if strings.Contains(out, redacted) {
+		t.Errorf("a null, boolean or number was redacted:\n%s", out)
+	}
+	for _, kept := range []string{`"value": null`, `"value": false`, `"value": 7`} {
+		if !strings.Contains(out, kept) {
+			t.Errorf("%s was changed:\n%s", kept, out)
+		}
+	}
+}
+
+func TestRedactRequestBodyHidesTheValueOnSecretEndpoints(t *testing.T) {
+	cases := []struct {
+		path, body, secret, kept string
+	}{
+		{"/v2/stable/secret", `{"name":"STRIPE_KEY","defaultValue":"canary-create","visibility":"masked"}`, "canary-create", "STRIPE_KEY"},
+		{"/v1/stable/secret/abc123/value", `{"value":"canary-set","environmentAlias":"main"}`, "canary-set", `"environmentAlias": "main"`},
+	}
+	for _, tc := range cases {
+		out := redactRequestBody(tc.path, []byte(tc.body))
+		if strings.Contains(out, tc.secret) || !strings.Contains(out, tc.kept) {
+			t.Errorf("%s: got\n%s\nwant %q hidden and %q kept", tc.path, out, tc.secret, tc.kept)
+		}
+	}
+
+	// Elsewhere these fields are ordinary: a variable's default stays visible.
+	out := redactRequestBody("/v1/stable/variable", []byte(`{"name":"plan","defaultValue":"free"}`))
+	if !strings.Contains(out, `"defaultValue": "free"`) {
+		t.Errorf("defaultValue was hidden outside the secret endpoints:\n%s", out)
+	}
+}