Repository navigation
fix(cli): redact body values marked sensitive in diagnostics - #91
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
2e848df to
fa20cc3
Compare
There was a problem hiding this comment.
All reported issues were addressed across 6 reviewed files. 1 file intentionally excluded from review.
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes dry-run/debug to redact schema-marked sensitive request-body values across JSON/form/multipart/text previews, with tests; wire bytes unchanged and the dismissed multipart leak is unreachable.
View guided diff | Turn on auto-fix | Re-trigger cubic
Values marked x-speakeasy-param-sensitive or format: password in request and response bodies could appear in HTTP debug logs and error messages. Value-based redaction only covers values known before the request, so a newly returned secret was logged before it could be recognised. Generate a per-operation schema graph of sensitive request and response body locations into a new public redact package. Operations with sensitive values tag their request context with the operation ID, so transports and custom HTTP clients can mask bodies by schema path. APIError messages and the Terraform provider's logging transport and diagnostics mask bodies with it. Methods of operations without sensitive values are unchanged.
Move the schema walk from the Go templates into Operation methods (SensitiveRequestBodyGraph, SensitiveResponseBodyGraph, HasSensitiveBodies) so that any target can render the same language-agnostic graph. The Go templates now only render it.
The redact package now takes the body's Content-Type and masks application/x-www-form-urlencoded fields and multipart parts by name, including JSON-encoded fields, keeping the body valid. The sensitive body graph is now built from TypeDef.Children, with TypeDef.Walk skipping schemas that hold nothing sensitive.
…arts A JSON value whose shape cannot hold the sensitive values of its schema is masked as a whole. Only multipart/form-data bodies are masked part by part, since other multipart subtypes have no part names, and masked parts drop their Content-Transfer-Encoding.
The sensitive body graph pruned union members that hold nothing sensitive, so a value matching such a member looked like it did not match its schema and was masked as a whole. Union nodes now record that a plain member exists, and the redactor leaves values of an unexpected shape readable when one does.
Union nodes now record the shapes (object, array, scalar) of members that hold nothing sensitive, instead of a single flag. A value that matches no sensitive member stays readable only when a plain member of the same shape exists. Form fields and multipart parts are parsed as JSON before the repeated array item rule applies, so a valid plain object beside a sensitive array stays readable. Text that is not JSON stays readable only when it is not a sensitive array item and a plain scalar member exists; malformed JSON is otherwise masked as before.
Plain union members are now recorded by JSON type (object, array, string, number, boolean), with enums following their underlying type. A form field or multipart part that is not valid JSON stays readable only beside a plain string member, of the field or of its items when sent as repeated fields; a plain integer or boolean member no longer lets malformed JSON through. Text starting like a JSON object or array is never accepted as a plain string. JSON scalars must match a plain member of their own type. Nested plain unions are followed once each, so recursive unions terminate. Masked multipart parts also drop Content-Length and Content-Encoding, which no longer describe the masked payload.
…ve-response-logs-plain
Form fields and multipart parts were looked up with any trailing [] removed, so a property whose wire name ends in brackets missed its sensitivity marker and was logged as sent. The exact name is now looked up first; the suffix is only removed when no schema declares the bracketed name, as for arrays sent as repeated fields.
Whether a form field or multipart part name ending in [] named a literal property or an array sent as repeated fields was decided across all schemas at once, and only from properties that survived pruning of the sensitive graph. A literal name in one response schema could hide a sensitive array of another, and a plain bracketed property beside a sensitive one was masked. The graph now lists every declared property of the object nodes it keeps, and each schema and union variant resolves the exact name, then the name without the suffix, then its additional properties.
…n redacting - JSON with a repeated object key is masked as a whole: decoding keeps only the last value, so the bytes returned unchanged could still hold an earlier sensitive one. - Multipart parts of a field with sensitive values are masked as a whole when transfer or content encoded, since their bytes are not the value. - Every repeated item of a sensitive array is masked, including `null`. - A form body whose keys start like JSON is masked as a whole.
04b9fd4 to
aeb2e62
Compare
aeb2e62 to
3cb01bf
Compare
A form field or multipart part that is not a JSON object or array may be one item of an array sent as repeated fields, so it is now matched against the item schemas too: integer and boolean items of a union with a sensitive object stay readable, as string items already did. Cases for property names ending in brackets move to dedicated operations tested only by the Go SDK, since other targets do not generate them yet. The declared-fields test preallocates its slice.
--dry-run and --debug request previews redacted body fields only by a
fixed list of key names, so a secret under a generic name (for example
"value") was printed in plain text.
- templateSensitiveBodyFields collects, per operation, the JSON paths of
request-body values whose schema sets x-speakeasy-param-sensitive or
format: password ("*" for array items and map values, "**" for any
depth of a recursive schema, union variants at their parent's path)
- diagnostics looks the paths up by the command's speakeasy_operation
annotation and redacts them in JSON bodies, form fields and multipart
parts (including repeated and JSON-encoded ones), on top of the
name-based rules; response bodies and the wire request are unchanged
- same-named fields of other schemas stay readable
The sensitive-body graph used one edge for array items and map values, and modelled additional properties as a union variant. A sensitive additionalProperties schema therefore redacted every declared field of the object, and a union of a sensitive array and an object redacted every field of the object variant. Array items and map values now use separate edges. Additional properties become the object's map-value edge, consulted only for keys the object does not declare. Recursive types are memoized through the shared custom-type ID used by the help renderer.
A JSON-encoded form field on a path to a sensitive value was always decoded and re-encoded for the preview, which dropped the HTML escaping the request used. The preview now keeps the original bytes unless redaction changed the value. The request body type is resolved through the shared getRequestBody helper, and the README notes that the graph also applies to OAuth2 token requests and that union variants sharing a field name are redacted together.
Dry-run and debug previews now pass request and response bodies through the embedded SDK's redact package, which selects the operation's schema graph from the request context, before applying the name-based preview rules. This replaces the CLI's own request-body graph and traversal, and adds schema-based masking to debug response previews. A body masked as a whole is shown as the mask. Name-based rules still apply inside JSON-encoded form fields, so a property named like a credential stays hidden next to a schema-marked one.
3cb01bf to
27111d8
Compare
## Why
Terraform's HTTP debug logging writes raw request and response bodies
before responses are decoded, so values marked sensitive
(`x-speakeasy-param-sensitive` or `format: password`) that the API
returns for the first time are not yet known to value-based redaction.
API error strings can also contain response bodies.
## What changed
- **Generator AST:** `Operation.SensitiveRequestBodyGraph()`,
`SensitiveResponseBodyGraph()` and `HasSensitiveBodies()`
(`internal/ast/sensitive_body.go`) compute a language-agnostic graph of
where sensitive values sit in an operation's bodies, by property name.
The graph is built along `TypeDef.Children`, and `TypeDef.Walk` skips
bodies with nothing sensitive. They handle objects, error types, arrays,
maps and `additionalProperties`, unions, recursive schemas, and
SSE/JSONL streams (masked as a whole). They are declared in
`typeDefs.d.ts`, so any target can render them.
- **Go SDK:** a public `redact` package holds the rendered graphs and
exposes `RequestBody(ctx, contentType, body)` / `ResponseBody(ctx,
contentType, body)`. Methods of operations with sensitive bodies stamp
the operation ID on the request context at request construction
(`utils.WithOperation`), so a transport can find it from the request or
`res.Request`. The transport and serialization are unchanged. Operations
without sensitive bodies generate byte-identical code.
- **Go `APIError.Error()`** masks the body when the SDK has any
sensitive bodies; the `Body` field stays raw.
- **Terraform:** the logging transport and `debugResponse` mask request
and response bodies through `redact`. The existing known-value redaction
is kept.
JSON bodies are masked value by value.
`application/x-www-form-urlencoded` fields and multipart parts are
matched by name: a sensitive field, or a field repeated for an array of
sensitive values, is masked, and a JSON-encoded field or part is masked
within. The masked body stays a valid form or multipart body with the
same boundary. Other media types, and bodies that cannot be parsed, are
masked as a whole for operations that have sensitive values. Unchanged
bodies are returned byte-identical. Split request bodies (one schema per
content type) are separate methods with their own operation IDs, so each
is masked with its own schema.
## Testing
- `internal/ast` unit tests for the graph: nested, array items, map
values beside a declared plain property, `format: password` with a
renamed property, union variants, recursion, error types, streams, and a
plain property sharing a sensitive one's name.
- New primary fragment `sensitive-body-redaction.yaml` with Go runtime
tests (`redact_additional_test.go`) for JSON, form, multipart and split
request bodies sent through the SDK, the context stamp, and
`APIError.Error()`. The form and multipart tests fail when their masking
is disabled. `TARGET=primary make test-go` passes.
- New Terraform review test checking that the transport log, error
strings, and `debugResponse` hide computed sensitive values while
decoded values stay raw. The full `./scripts/build-review-terraform.sh`
acceptance suite passes.
- `go test ./pkg/generate/...` (the Python snapshot tests fail on `main`
too, on a mypy `_DURATION_ADAPTER` annotation error), and `make
check-template-{go,terraform,cli}`.
## Known limitations
- Union variants are matched by property name, so a property sensitive
in one variant is masked in all.
- Typed errors without a message field still `json.Marshal` themselves
in `Error()`.
- Custom hooks that build their own error strings from bodies need to
call `redact.ResponseBody`.
- The CLI keeps its own request-body graph from #91 for now; it could
switch to the AST methods.
---------
Co-authored-by: 2ynn <qynn@speakeasy.com>
Stacked on #72; review that first. This PR's base is
fix/terraform-sensitive-response-logs.Why
--dry-runand--debugredact body values only by name (password,token,client_secret, …). A value marked sensitive in the schema (x-speakeasy-param-sensitive: trueorformat: password) but carrying a generic name such asvalueorpassphrasewas printed in clear text, and so was a whole body whose schema carries the marker.What changed
redactpackage from feat(go,terraform): mask sensitive body values in HTTP logs #72 (redact.RequestBody/redact.ResponseBody) before applying the existing name-based preview rules. The SDK stamps the operation ID on each request context, so the schema graph is selected per SDK method, and split request content types each use their own graph. The CLI no longer builds its own request-body graph.(sensitive)) instead of a byte count when it no longer parses as its content type.(sensitive)(the shared mask); name-based redaction still reads[REDACTED].$refis not read).zSDKs/sdk-cli/internal/client/diagnostics.goregenerated.Testing
cli-sensitive-body-fields.yaml: marked fields at the top level, nested, in array items and map values, through self and mutual recursion, discriminated unions (including a recursive variant in both member orders), multipart and form bodies with JSON-encoded fields (including an unmarkedpasswordbeside a marked field, and a marked property whose wire name ends in brackets), repeated scalar fields, split request content types (including an unmarked field that shares a marked field's name in another content type), whole bodies marked sensitive (JSON, form, multipart, text), sensitiveadditionalPropertiesbeside declared fields, a union of a sensitive array and a plain object, and a response with a marked field.TestDiagnosticsRedactSensitiveBodyFieldsruns each case under human dry-run, JSON dry-run and debug.TestDiagnosticsSensitiveBodyPreservesWireAndResponsechecks the server still receives the secrets;TestDiagnosticsKeepUnmarkedFieldSharingSensitiveNamechecks unmarked fields stay readable;TestDebugRedactsSensitiveResponseFieldschecks debug hides a marked response value while decoded output keeps it.TARGET=review make test-cli: 690 passed, 2 skipped.TARGET=primary make test-cli: 1889 tests; the only failures are the fourTestConfigure*tests, which depend on agent-mode detection being off and pass with the detection variables unset.Public-safety check
.claude/skills/public-repo-communication/SKILL.md).git diff --check.