Skip to content

Mask JSON bodies served as application/octet-stream - #104

Open
jordanenglish wants to merge 1 commit into
hashicorp:mainfrom
jordanenglish:fix/mask-octet-stream-json-output
Open

Mask JSON bodies served as application/octet-stream#104
jordanenglish wants to merge 1 commit into
hashicorp:mainfrom
jordanenglish:fix/mask-octet-stream-json-output

Conversation

@jordanenglish

Copy link
Copy Markdown
Contributor

Description

Follow-up to #101. CopyRaw only attempted to parse and mask a raw response body when its Content-Type was application/json (or ended in +json). At least one Terraform Enterprise endpoint (the plan JSON export) serves valid JSON labeled application/octet-stream instead, so that response bypassed masking entirely.

application/octet-stream isn't treated as an unconditional mask candidate, since this API also uses that label for genuinely binary or large bodies elsewhere (state archives, plan/apply logs fetched via signed archivist URLs), and buffering one of those just because a sibling endpoint is mislabeled would trade a confirmed small leak for a real cost on unrelated responses. Instead, a body labeled application/octet-stream is peeked at, without consuming or buffering it, to check whether its first non-whitespace byte opens a JSON object or array. Only then does it proceed to the existing buffer-and-mask path; otherwise it streams through unread, exactly as before #101.

Tests

  • internal/pkg/format/redact_test.go, TestCopyRaw: extended with five new subtests covering the application/octet-stream branch specifically:
    • masks a JSON body labeled application/octet-stream, the reported bug
    • masks the same case with leading whitespace before the opening brace, exercising the peek's whitespace-skip
    • passes a genuinely binary application/octet-stream body through unread (leading bytes that aren't {/[)
    • passes through, unmasked, an application/octet-stream body shaped like a real plan/apply log: human-readable text followed by embedded JSON lines, mirroring the exact shape already covered by TestRunAPI_GetArbitraryURL in internal/commands/api. This is the regression case that motivated the peek approach over a blanket Content-Type change: that existing test's fixture is real evidence this API serves non-JSON content under application/octet-stream, and it must not start getting buffered as a side effect of this fix
    • confirms a body under an unrelated content type (text/plain) still isn't touched, out of scope for this fix
  • go test ./...: full suite passes, including internal/commands/api (home of the log-shaped octet-stream fixture above) and internal/commands/run
  • gofmt and go vet ./...: clean
  • Manually verified against a Terraform Enterprise instance where the plan JSON export endpoint was confirmed (via --debug) to serve Content-Type: application/octet-stream for a body containing several sensitive = true Terraform variables, including one PEM-format private key and one token matching the GitHub token shape rule. Before this fix, all of them rendered in cleartext through tfctl api .../json-output despite Redact sensitive values from command output #101. After, all mask to (redacted) with the usual WARNING: masked N sensitive fields report, and an ordinary structured tfctl get call against the same instance is unaffected

PR Checklist

  • Run npx changie new or install changie to prepare a new changelog entry for the next set of release notes.
  • Ensure any command changes are sensitive to these global flags:
    • No command-level changes, global-flags checklist doesn't apply
  • Get the logging interface from the context and add debug logging for interesting conditions and nonfatal situations.
  • Run make gen/screenshot if the root command output changes.
    • No root command output change, no screenshot regen needed
  • Add the Autocomplete field to positional arguments and flags to assist shell autocomplete.
    • No new flags/arguments, no Autocomplete field needed

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

    • Reverting fully removes the change; no migration.
  • If applicable, I've documented the impact of any changes to security controls.

CopyRaw, added in hashicorp#101, only attempted to parse and mask a raw
response body when its Content-Type was application/json (or ended
in +json). At least one Terraform Enterprise endpoint (the plan JSON
export) serves valid JSON labeled application/octet-stream instead,
so that response bypassed masking entirely and streamed straight
through.

application/octet-stream is not treated as an unconditional mask
candidate, since this API also uses it for genuinely binary or large
bodies elsewhere (state archives, plan/apply logs fetched via signed
archivist URLs), and buffering one of those into memory just because
a sibling endpoint is mislabeled would trade a confirmed small leak
for a real cost on unrelated responses. Instead, a body labeled
application/octet-stream is peeked at, without consuming or buffering
it, to check whether its first non-whitespace byte opens a JSON
object or array. Only then does it proceed to the existing
buffer-and-mask path; otherwise it streams through unread, exactly as
it did before hashicorp#101.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant