From 689e032a5b49f99c0d03fb6eeef11c0a35088525 Mon Sep 17 00:00:00 2001 From: Jordan English <6087717+jordanenglish@users.noreply.github.com> Date: Thu, 20 Aug 2026 08:39:16 -0400 Subject: [PATCH] Mask JSON bodies served as application/octet-stream CopyRaw, added in #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 #101. --- .../unreleased/BUG FIXES-20260820-083800.yaml | 3 + internal/pkg/format/output.go | 60 ++++++++++++++- internal/pkg/format/redact_test.go | 73 +++++++++++++++++++ 3 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 .changes/unreleased/BUG FIXES-20260820-083800.yaml diff --git a/.changes/unreleased/BUG FIXES-20260820-083800.yaml b/.changes/unreleased/BUG FIXES-20260820-083800.yaml new file mode 100644 index 0000000..f7c2ae0 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20260820-083800.yaml @@ -0,0 +1,3 @@ +kind: BUG FIXES +body: "Fixed a gap in the output masking added in #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 (plan JSON export) serves valid JSON labeled application/octet-stream, so its response bypassed masking entirely. A body labeled application/octet-stream is now peeked at, without buffering it, to check whether it opens with a JSON object or array before deciding whether to mask it, so this content type is no longer treated as a blanket mask candidate: genuinely binary or large bodies served under the same label, such as state archives and plan or apply logs, are streamed through exactly as before" +time: 2026-08-20T08:38:00.000000-04:00 diff --git a/internal/pkg/format/output.go b/internal/pkg/format/output.go index 6b42cbf..35a02cb 100644 --- a/internal/pkg/format/output.go +++ b/internal/pkg/format/output.go @@ -4,6 +4,7 @@ package format import ( + "bufio" "bytes" "encoding/json" "fmt" @@ -443,7 +444,13 @@ func pluralize(word string, count int) string { // exempt. The original bytes are written unchanged when nothing was masked, so // output stays byte-for-byte identical in the common case. func (o *Outputter) CopyRaw(body io.Reader, contentType string) error { - if !o.redactor.Enabled() || !isJSONContentType(contentType) { + if !o.redactor.Enabled() { + _, err := io.Copy(o.io.Out(), body) + return err + } + + body, attempt := prepareForMasking(body, contentType) + if !attempt { _, err := io.Copy(o.io.Out(), body) return err } @@ -478,9 +485,56 @@ func (o *Outputter) CopyRaw(body io.Reader, contentType string) error { return nil } -func isJSONContentType(contentType string) bool { +// prepareForMasking decides whether body is worth buffering and parsing as +// JSON, and returns the reader CopyRaw should use afterward either way. +// +// application/json (and any +json subtype) is always attempted, unchanged +// from the original behavior. application/octet-stream is not trusted +// outright: at least one Terraform Enterprise endpoint (the plan JSON export) +// serves valid JSON mislabeled this way, but octet-stream is also the label +// this API uses for genuinely binary or large bodies elsewhere, such as state +// archives and plan/apply logs fetched via signed archivist URLs. 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, possibly +// large, responses. So octet-stream gets a cheap peek instead: the reader is +// wrapped in a bufio.Reader and Peek is used to look at, at most, the first +// 64 bytes without consuming them, meaning the same reader can still be read +// from the beginning afterward regardless of which branch this takes. Any +// other content type is left exactly as before, out of scope for this fix. +func prepareForMasking(body io.Reader, contentType string) (io.Reader, bool) { mediaType := strings.TrimSpace(strings.Split(contentType, ";")[0]) - return mediaType == "application/json" || strings.HasSuffix(mediaType, "+json") + + if mediaType == "application/json" || strings.HasSuffix(mediaType, "+json") { + return body, true + } + + if mediaType != "application/octet-stream" { + return body, false + } + + buffered := bufio.NewReader(body) + return buffered, peekLooksLikeJSON(buffered) +} + +// peekLooksLikeJSON reports whether the next non-whitespace byte available +// from r opens a JSON object or array. It never reads more than a small, +// fixed window, and Peek does not advance r, so the body is still intact and +// unread from the start no matter what this returns. +func peekLooksLikeJSON(r *bufio.Reader) bool { + const window = 64 + + peeked, _ := r.Peek(window) + for _, b := range peeked { + switch b { + case ' ', '\t', '\n', '\r': + continue + case '{', '[': + return true + default: + return false + } + } + return false } // Show outputs the given val using the DisplayFields function. diff --git a/internal/pkg/format/redact_test.go b/internal/pkg/format/redact_test.go index 418bf5b..cd1ae36 100644 --- a/internal/pkg/format/redact_test.go +++ b/internal/pkg/format/redact_test.go @@ -237,6 +237,79 @@ func TestCopyRaw(t *testing.T) { r.Equal(planJSON, io.Output.String()) }) + + t.Run("masks a JSON body labeled application/octet-stream", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + // HCP Terraform's plan JSON export serves this exact content type for a + // JSON body, so it must be treated as JSON rather than passed through. + out, io := newRedactingOutputter(t, redact.ModeStrict) + r.NoError(out.CopyRaw(strings.NewReader(planJSON), "application/octet-stream")) + + r.NotContains(io.Output.String(), "hunter2") + r.Contains(io.Output.String(), redact.Placeholder) + r.Contains(io.Error.String(), "masked 1 sensitive field") + }) + + t.Run("masks an octet-stream JSON body with leading whitespace", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + padded := "\n \t" + planJSON + + out, io := newRedactingOutputter(t, redact.ModeStrict) + r.NoError(out.CopyRaw(strings.NewReader(padded), "application/octet-stream")) + + r.NotContains(io.Output.String(), "hunter2") + r.Contains(io.Output.String(), redact.Placeholder) + }) + + t.Run("passes a genuinely binary octet-stream body through unread", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + binary := string([]byte{0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00}) + + out, io := newRedactingOutputter(t, redact.ModeStrict) + r.NoError(out.CopyRaw(strings.NewReader(binary), "application/octet-stream")) + + r.Equal(binary, io.Output.String()) + }) + + t.Run("passes a plan-log-shaped octet-stream body through unmasked", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + // tfctl api against an arbitrary URL (TestRunAPI_GetArbitraryURL) already + // exercises this exact shape: human-readable log text, labeled + // application/octet-stream, with JSON *lines* embedded further down + // rather than being one JSON document itself. The leading bytes are not + // '{' or '[', so this must never reach the buffer-and-mask path, whether + // or not any embedded line happens to look sensitive. + const logOutput = "Terraform v1.2.8\non linux_amd64\n" + + `{"@level":"info","@message":"Terraform 1.2.8"}` + + out, io := newRedactingOutputter(t, redact.ModeStrict) + r.NoError(out.CopyRaw(strings.NewReader(logOutput), "application/octet-stream")) + + r.Equal(logOutput, io.Output.String()) + r.Empty(io.Error.String()) + }) + + t.Run("does not attempt to mask other content types", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + out, io := newRedactingOutputter(t, redact.ModeStrict) + r.NoError(out.CopyRaw(strings.NewReader(planJSON), "text/plain")) + + // Out of scope for this fix: only application/json (and +json) and + // application/octet-stream are mask candidates. A body labeled + // something else streams exactly as it did before. + r.Equal(planJSON, io.Output.String()) + r.Empty(io.Error.String()) + }) } func TestReportRedactions_QuietSuppressesTheReport(t *testing.T) {