Skip to content

fix(cli): redact body values marked sensitive in diagnostics - #91

Merged
2ynn merged 25 commits into
mainfrom
fix/cli/redact-sensitive-body-fields
Oct 8, 2026
Merged

2ynn merged 25 commits into
mainfrom
fix/cli/redact-sensitive-body-fields

Conversation

@2ynn

@2ynn 2ynn commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #72; review that first. This PR's base is fix/terraform-sensitive-response-logs.

Why

--dry-run and --debug redact body values only by name (password, token, client_secret, …). A value marked sensitive in the schema (x-speakeasy-param-sensitive: true or format: password) but carrying a generic name such as value or passphrase was printed in clear text, and so was a whole body whose schema carries the marker.

What changed

  • Shared redaction: dry-run and debug previews pass request and response bodies through the embedded SDK's redact package 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.
  • Responses: debug response previews now mask schema-marked values too; previously only name-based rules applied.
  • Whole bodies: a body masked as a whole is shown as the mask ((sensitive)) instead of a byte count when it no longer parses as its content type.
  • JSON-encoded form fields: name-based rules apply inside them, so a property named like a credential stays hidden next to a schema-marked one; a field with nothing to redact is shown exactly as sent.
  • Schema-marked values read (sensitive) (the shared mask); name-based redaction still reads [REDACTED].
  • Docs: the CLI template README describes the redaction order and its limits (union variants are not told apart; a marker beside a $ref is not read).
  • Review SDK: zSDKs/sdk-cli/internal/client/diagnostics.go regenerated.

Testing

  • Primary fixture 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 unmarked password beside 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), sensitive additionalProperties beside declared fields, a union of a sensitive array and a plain object, and a response with a marked field.
  • TestDiagnosticsRedactSensitiveBodyFields runs each case under human dry-run, JSON dry-run and debug. TestDiagnosticsSensitiveBodyPreservesWireAndResponse checks the server still receives the secrets; TestDiagnosticsKeepUnmarkedFieldSharingSensitiveName checks unmarked fields stay readable; TestDebugRedactsSensitiveResponseFields checks 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 four TestConfigure* tests, which depend on agent-mode detection being off and pass with the detection variables unset.

Public-safety check

  • This change contains no credentials, customer documents, private repository URLs, private filesystem paths, or unredacted private logs.
  • Title, body, comments, and commit messages name no customers or customer-derived identifiers, private paths or trackers, or workflow provenance, and are understandable without private context (.claude/skills/public-repo-communication/SKILL.md).
  • Generated fixtures and review SDK changes are public-safe.
  • I reviewed git diff --check.

@2ynn
2ynn requested a review from a team as a code owner October 7, 2026 19:07
@2ynn
2ynn requested a review from AshGodfrey October 7, 2026 19:07

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread zSDKs/sdk-cli/internal/client/diagnostics.go Outdated
@2ynn
2ynn force-pushed the fix/cli/redact-sensitive-body-fields branch from 2e848df to fa20cc3 Compare October 7, 2026 20:39

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread zSDKs/sdk-cli/internal/client/diagnostics.go Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
ThomasRooney and others added 11 commits October 8, 2026 18:21
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.
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.
@2ynn
2ynn force-pushed the fix/cli/redact-sensitive-body-fields branch from 04b9fd4 to aeb2e62 Compare October 8, 2026 19:37
@2ynn 2ynn changed the title fix(cli): redact request-body values marked sensitive in diagnostics fix(cli): redact body values marked sensitive in diagnostics Oct 8, 2026
@2ynn
2ynn changed the base branch from main to fix/terraform-sensitive-response-logs October 8, 2026 19:37
@2ynn
2ynn force-pushed the fix/cli/redact-sensitive-body-fields branch from aeb2e62 to 3cb01bf Compare October 8, 2026 19:41
2ynn added 5 commits October 8, 2026 15:59
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
2ynn added 6 commits October 8, 2026 16:00
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.
@2ynn
2ynn force-pushed the fix/cli/redact-sensitive-body-fields branch from 3cb01bf to 27111d8 Compare October 8, 2026 20:18
@2ynn
2ynn requested a review from ThomasRooney October 8, 2026 20:20
Base automatically changed from fix/terraform-sensitive-response-logs to main October 8, 2026 20:34
github-merge-queue Bot pushed a commit that referenced this pull request Oct 8, 2026
## 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>
@2ynn
2ynn added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 45bcbaf Oct 8, 2026
22 checks passed
@2ynn
2ynn deleted the fix/cli/redact-sensitive-body-fields branch October 8, 2026 21:14
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants