Mask JSON bodies served as application/octet-stream - #104
Open
jordanenglish wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #101.
CopyRawonly attempted to parse and mask a raw response body when itsContent-Typewasapplication/json(or ended in+json). At least one Terraform Enterprise endpoint (the plan JSON export) serves valid JSON labeledapplication/octet-streaminstead, so that response bypassed masking entirely.application/octet-streamisn'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 labeledapplication/octet-streamis 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 theapplication/octet-streambranch specifically:application/octet-stream, the reported bugapplication/octet-streambody through unread (leading bytes that aren't{/[)application/octet-streambody shaped like a real plan/apply log: human-readable text followed by embedded JSON lines, mirroring the exact shape already covered byTestRunAPI_GetArbitraryURLininternal/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 underapplication/octet-stream, and it must not start getting buffered as a side effect of this fixtext/plain) still isn't touched, out of scope for this fixgo test ./...: full suite passes, includinginternal/commands/api(home of the log-shapedoctet-streamfixture above) andinternal/commands/rungofmtandgo vet ./...: clean--debug) to serveContent-Type: application/octet-streamfor a body containing severalsensitive = trueTerraform 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 throughtfctl api .../json-outputdespite Redact sensitive values from command output #101. After, all mask to(redacted)with the usualWARNING: masked N sensitive fieldsreport, and an ordinary structuredtfctl getcall against the same instance is unaffectedPR Checklist
npx changie newor install changie to prepare a new changelog entry for the next set of release notes.make gen/screenshotif the root command output changes.Autocompletefield to positional arguments and flags to assist shell autocomplete.Autocompletefield neededPCI 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.
If applicable, I've documented the impact of any changes to security controls.