From c51f0851ca41816afd1df67c0eb4aa10b4b3f43e Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 12:39:17 -0700 Subject: [PATCH 1/4] ESD-1650: Polish WASM output composition and HTTP transport fidelity Repeated --help calls were re-coloring and re-suffixing the same help text because the command tree persists across WASM invocations. Cache each command's pristine Long text so help always renders from a clean base. Structured output (JSON/CSV/XML/table) was overwritten on every PrintOutput call, so a command emitting more than one document only kept the last. Buffers now accumulate across calls within an invocation and are cleared only between invocations. The WASM HTTP transport dropped every header value but the first and sent request bodies as Go strings, which would mangle binary bodies. Multi-value headers are now joined per HTTP semantics and bodies are passed as raw bytes via a Uint8Array. --- cmd/megaport/help_wasm_test.go | 64 +++++++++++++ cmd/megaport/megaport_wasm.go | 12 ++- internal/base/output/common.go | 6 ++ internal/base/output/output_wasm.go | 53 ++++++++--- internal/base/output/output_wasm_test.go | 115 +++++++++++++++++++++++ internal/base/output/table_wasm.go | 23 +++-- internal/base/output/table_wasm_test.go | 24 +++++ internal/wasm/wasmhttp/transport.go | 10 +- internal/wasm/wasmhttp/transport_test.go | 37 ++++++++ 9 files changed, 318 insertions(+), 26 deletions(-) create mode 100644 cmd/megaport/help_wasm_test.go diff --git a/cmd/megaport/help_wasm_test.go b/cmd/megaport/help_wasm_test.go new file mode 100644 index 00000000..dccd3b81 --- /dev/null +++ b/cmd/megaport/help_wasm_test.go @@ -0,0 +1,64 @@ +//go:build js && wasm + +package megaport + +import ( + "testing" + + "github.com/megaport/megaport-cli/internal/wasm" + "github.com/stretchr/testify/assert" +) + +// ESD-1650: the command tree persists across WASM invocations in a browser +// session, so a naive SetHelpFunc that rebuilds a command's help text from its +// own (already mutated) cmd.Long re-colors and re-suffixes it on every call. +// These tests drive the real --help path (ExecuteWithArgs) so they fail if +// that regresses. + +// TestWasmHelp_RepeatedInvocationsProduceIdenticalOutput covers the +// accumulation bug directly: calling --help on the same subcommand twice must +// yield byte-identical output, not growing/duplicated help text. +func TestWasmHelp_RepeatedInvocationsProduceIdenticalOutput(t *testing.T) { + wasm.ResetOutputBuffers() + ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + first := wasm.WasmOutputBuffer.String() + assert.NotEmpty(t, first) + + wasm.ResetOutputBuffers() + ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + second := wasm.WasmOutputBuffer.String() + + assert.Equal(t, first, second, "repeated --help calls must produce identical output, not accumulate coloring/suffixes") +} + +// TestWasmHelp_RepeatedInvocationsAcrossManyCalls guards against slow growth +// that a two-call comparison might miss. +func TestWasmHelp_RepeatedInvocationsAcrossManyCalls(t *testing.T) { + wasm.ResetOutputBuffers() + ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + baseline := wasm.WasmOutputBuffer.String() + assert.NotEmpty(t, baseline) + + for i := 0; i < 5; i++ { + wasm.ResetOutputBuffers() + ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + out := wasm.WasmOutputBuffer.String() + assert.Equal(t, baseline, out, "help output must not drift across repeated invocations") + } +} + +// TestWasmHelp_RootCommandRepeatedInvocationsProduceIdenticalOutput covers the +// same guarantee for the root command's help, which is rebuilt from a static +// LongDesc string each time rather than a cached original. +func TestWasmHelp_RootCommandRepeatedInvocationsProduceIdenticalOutput(t *testing.T) { + wasm.ResetOutputBuffers() + ExecuteWithArgs([]string{"megaport-cli", "--help"}) + first := wasm.WasmOutputBuffer.String() + assert.NotEmpty(t, first) + + wasm.ResetOutputBuffers() + ExecuteWithArgs([]string{"megaport-cli", "--help"}) + second := wasm.WasmOutputBuffer.String() + + assert.Equal(t, first, second, "repeated root --help calls must produce identical output") +} diff --git a/cmd/megaport/megaport_wasm.go b/cmd/megaport/megaport_wasm.go index 4e324a16..3e7250cf 100644 --- a/cmd/megaport/megaport_wasm.go +++ b/cmd/megaport/megaport_wasm.go @@ -180,6 +180,11 @@ func init() { } } + // The command tree persists across WASM invocations in a browser session, so cache each + // command's pristine Long text the first time it's seen. Without this, repeated --help + // calls would feed the already-colored, already-suffixed cmd.Long back into the builder. + originalLongs := make(map[*cobra.Command]string) + // Create a help function that runs the help.CommandHelpBuilder with the current noColor setting rootCmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { // For the root command, regenerate the help text completely @@ -187,11 +192,16 @@ func init() { rootHelp := getRootHelpBuilder(noColor) cmd.Long = rootHelp.Build(rootCmd) } else if cmd.Long != "" { + original, cached := originalLongs[cmd] + if !cached { + original = cmd.Long + originalLongs[cmd] = original + } // For non-root commands, modify the existing help text only if there is a Long description helpBuilder := &help.CommandHelpBuilder{ CommandName: cmd.UseLine(), ShortDesc: cmd.Short, - LongDesc: cmd.Long, + LongDesc: original, DisableColor: noColor, } cmd.Long = helpBuilder.Build(rootCmd) diff --git a/internal/base/output/common.go b/internal/base/output/common.go index 8e97c56c..2850ee93 100644 --- a/internal/base/output/common.go +++ b/internal/base/output/common.go @@ -154,11 +154,17 @@ func ResetErrorEmitted() { errorEmittedMu.Unlock() } +// resetWasmStructuredBuffers clears the WASM structured-output buffers +// (JSON/CSV/XML/table) between command invocations. It is a no-op on native +// builds; output_wasm.go's init overrides it. +var resetWasmStructuredBuffers = func() {} + // ResetState clears all output configuration back to defaults. // Intended for the WASM entry point to prevent state bleed between invocations. func ResetState() { ApplyOutputConfig(defaultOutputConfig()) ResetErrorEmitted() + resetWasmStructuredBuffers() } // printOptions is the per-call snapshot of the field/query/header/template diff --git a/internal/base/output/output_wasm.go b/internal/base/output/output_wasm.go index 9c2035a7..e17cb3f0 100644 --- a/internal/base/output/output_wasm.go +++ b/internal/base/output/output_wasm.go @@ -30,11 +30,25 @@ var WasmCSVWriter = &bytes.Buffer{} // WasmXMLWriter is a global buffer for capturing XML output in WASM var WasmXMLWriter = &bytes.Buffer{} -// printJSON is the WASM-specific implementation that properly captures JSON output +func init() { + resetWasmStructuredBuffers = func() { + wasmBufMu.Lock() + defer wasmBufMu.Unlock() + WasmJSONWriter.Reset() + WasmCSVWriter.Reset() + WasmXMLWriter.Reset() + WasmTableWriter.Reset() + } +} + +// printJSON is the WASM-specific implementation that properly captures JSON output. +// A command may call this more than once (multiple PrintOutput calls emitting +// several structured documents); each call appends to WasmJSONWriter rather than +// overwriting it, so the global reflects every document emitted so far. The +// buffer is cleared between WASM invocations by resetWasmStructuredBuffers. func printJSON[T OutputFields](data []T, opts printOptions) error { wasmBufMu.Lock() defer wasmBufMu.Unlock() - WasmJSONWriter.Reset() if data == nil { data = []T{} @@ -45,6 +59,7 @@ func printJSON[T OutputFields](data []T, opts printOptions) error { return err } + before := WasmJSONWriter.Len() encoder := json.NewEncoder(WasmJSONWriter) encoder.SetIndent("", " ") if err := encoder.Encode(toEncode); err != nil { @@ -56,19 +71,24 @@ func printJSON[T OutputFields](data []T, opts printOptions) error { // GetCapturedOutput/GetCompletionOutput and written to xterm without // escaping, and a field value can carry an API-controlled control byte // that Go's JSON encoder does not escape (e.g. the C1 range). See - // wasm.SanitizeTerminalOutput. - jsonOutput := wasm.SanitizeTerminalOutput(WasmJSONWriter.String()) - fmt.Print(jsonOutput) + // wasm.SanitizeTerminalOutput. Sanitize the new slice on its own rather + // than slicing the sanitized whole buffer by a raw-byte offset: sanitizing + // drops bytes, so a raw offset no longer lines up with the sanitized string. + raw := WasmJSONWriter.String() + fmt.Print(wasm.SanitizeTerminalOutput(raw[before:])) + jsonOutput := wasm.SanitizeTerminalOutput(raw) js.Global().Set("wasmJSONOutput", jsonOutput) return nil } -// printCSV is the WASM-specific implementation that properly captures CSV output +// printCSV is the WASM-specific implementation that properly captures CSV +// output. Like printJSON, it appends to WasmCSVWriter so a command emitting +// multiple CSV documents has all of them captured, not just the last. func printCSV[T OutputFields](data []T, opts printOptions) error { wasmBufMu.Lock() defer wasmBufMu.Unlock() - WasmCSVWriter.Reset() + before := WasmCSVWriter.Len() w := csv.NewWriter(WasmCSVWriter) defer w.Flush() @@ -185,17 +205,21 @@ func printCSV[T OutputFields](data []T, opts printOptions) error { w.Flush() // See the sanitize comment in printJSON above; the same applies to CSV. - csvOutput := wasm.SanitizeTerminalOutput(WasmCSVWriter.String()) - fmt.Print(csvOutput) + raw := WasmCSVWriter.String() + fmt.Print(wasm.SanitizeTerminalOutput(raw[before:])) + csvOutput := wasm.SanitizeTerminalOutput(raw) js.Global().Set("wasmCSVOutput", csvOutput) return nil } -// printXML is the WASM-specific implementation that properly captures XML output +// printXML is the WASM-specific implementation that properly captures XML +// output. Like printJSON, it appends to WasmXMLWriter so a command emitting +// multiple XML documents has all of them captured, not just the last. func printXML[T OutputFields](data []T, opts printOptions) error { wasmBufMu.Lock() defer wasmBufMu.Unlock() - WasmXMLWriter.Reset() + + before := WasmXMLWriter.Len() if data == nil { data = []T{} @@ -204,7 +228,7 @@ func printXML[T OutputFields](data []T, opts printOptions) error { writeEmpty := func() { WasmXMLWriter.WriteString(xml.Header + "\n") xmlOutput := WasmXMLWriter.String() - fmt.Print(xmlOutput) + fmt.Print(xmlOutput[before:]) js.Global().Set("wasmXMLOutput", xmlOutput) } @@ -356,8 +380,9 @@ func printXML[T OutputFields](data []T, opts printOptions) error { WasmXMLWriter.WriteString("\n") // See the sanitize comment in printJSON above; the same applies to XML. - xmlOutput := wasm.SanitizeTerminalOutput(WasmXMLWriter.String()) - fmt.Print(xmlOutput) + raw := WasmXMLWriter.String() + fmt.Print(wasm.SanitizeTerminalOutput(raw[before:])) + xmlOutput := wasm.SanitizeTerminalOutput(raw) js.Global().Set("wasmXMLOutput", xmlOutput) return nil } diff --git a/internal/base/output/output_wasm_test.go b/internal/base/output/output_wasm_test.go index bd2bcc98..c0627cfd 100644 --- a/internal/base/output/output_wasm_test.go +++ b/internal/base/output/output_wasm_test.go @@ -165,6 +165,121 @@ func TestPrintCSV_WASM_SpecialCharacters(t *testing.T) { assert.Contains(t, output, "Item with") } +// TestPrintJSON_WASM_AccumulatesMultipleCalls verifies that a command emitting +// more than one JSON document in a single invocation has all of them captured, +// not just the last (ESD-1650). +func TestPrintJSON_WASM_AccumulatesMultipleCalls(t *testing.T) { + first := []SimpleStruct{{ID: 1, Name: "First", Active: true}} + second := []SimpleStruct{{ID: 2, Name: "Second", Active: false}} + + WasmJSONWriter.Reset() + js.Global().Delete("wasmJSONOutput") + + assert.NoError(t, printJSON(first, currentPrintOptions())) + assert.NoError(t, printJSON(second, currentPrintOptions())) + + output := WasmJSONWriter.String() + assert.Contains(t, output, "First", "first document must survive a second call") + assert.Contains(t, output, "Second", "second document must also be present") + + global := js.Global().Get("wasmJSONOutput") + assert.Equal(t, output, global.String(), "global must reflect the full accumulated buffer") + + WasmJSONWriter.Reset() + js.Global().Delete("wasmJSONOutput") +} + +// TestPrintCSV_WASM_AccumulatesMultipleCalls mirrors the JSON accumulation +// test for CSV output. +func TestPrintCSV_WASM_AccumulatesMultipleCalls(t *testing.T) { + first := []SimpleStruct{{ID: 1, Name: "First", Active: true}} + second := []SimpleStruct{{ID: 2, Name: "Second", Active: false}} + + WasmCSVWriter.Reset() + js.Global().Delete("wasmCSVOutput") + + assert.NoError(t, printCSV(first, currentPrintOptions())) + assert.NoError(t, printCSV(second, currentPrintOptions())) + + output := WasmCSVWriter.String() + assert.Contains(t, output, "First", "first document must survive a second call") + assert.Contains(t, output, "Second", "second document must also be present") + + global := js.Global().Get("wasmCSVOutput") + assert.Equal(t, output, global.String(), "global must reflect the full accumulated buffer") + + WasmCSVWriter.Reset() + js.Global().Delete("wasmCSVOutput") +} + +// TestPrintXML_WASM verifies basic XML output capture in WASM, a case not +// previously covered by this suite. +func TestPrintXML_WASM(t *testing.T) { + data := []SimpleStruct{ + {ID: 1, Name: "Item 1", Active: true}, + } + + WasmXMLWriter.Reset() + js.Global().Delete("wasmXMLOutput") + + err := printXML(data, currentPrintOptions()) + assert.NoError(t, err) + + output := WasmXMLWriter.String() + assert.NotEmpty(t, output) + assert.Contains(t, output, "") + assert.Contains(t, output, "") + assert.Contains(t, output, "Item 1") + + global := js.Global().Get("wasmXMLOutput") + assert.False(t, global.IsUndefined()) + assert.Equal(t, output, global.String()) + + WasmXMLWriter.Reset() + js.Global().Delete("wasmXMLOutput") +} + +// TestPrintXML_WASM_AccumulatesMultipleCalls mirrors the JSON/CSV +// accumulation tests for XML output. +func TestPrintXML_WASM_AccumulatesMultipleCalls(t *testing.T) { + first := []SimpleStruct{{ID: 1, Name: "First", Active: true}} + second := []SimpleStruct{{ID: 2, Name: "Second", Active: false}} + + WasmXMLWriter.Reset() + js.Global().Delete("wasmXMLOutput") + + assert.NoError(t, printXML(first, currentPrintOptions())) + assert.NoError(t, printXML(second, currentPrintOptions())) + + output := WasmXMLWriter.String() + assert.Contains(t, output, "First", "first document must survive a second call") + assert.Contains(t, output, "Second", "second document must also be present") + + global := js.Global().Get("wasmXMLOutput") + assert.Equal(t, output, global.String(), "global must reflect the full accumulated buffer") + + WasmXMLWriter.Reset() + js.Global().Delete("wasmXMLOutput") +} + +// TestResetWasmStructuredBuffers_ClearsAllBuffers verifies the between- +// invocation reset hook (wired through output.ResetState) clears every +// structured-output buffer, so state never bleeds from one WASM command +// invocation into the next. +func TestResetWasmStructuredBuffers_ClearsAllBuffers(t *testing.T) { + WasmJSONWriter.WriteString("stale json") + WasmCSVWriter.WriteString("stale csv") + WasmXMLWriter.WriteString("stale xml") + WasmTableWriter.WriteString("stale table") + + ResetState() + + assert.Empty(t, WasmJSONWriter.String()) + assert.Empty(t, WasmCSVWriter.String()) + assert.Empty(t, WasmXMLWriter.String()) + assert.Empty(t, WasmTableWriter.String()) +} + // TestPrintOutput_WASM_Formats verifies all output formats func TestPrintOutput_WASM_Formats(t *testing.T) { data := []SimpleStruct{ diff --git a/internal/base/output/table_wasm.go b/internal/base/output/table_wasm.go index 21afe5ba..0b07b2a3 100644 --- a/internal/base/output/table_wasm.go +++ b/internal/base/output/table_wasm.go @@ -50,17 +50,22 @@ func calculateDynamicWidth(termWidth int, minWidth, maxPercentage int) int { return maxWidth } -// printTable is the WASM-specific implementation that properly captures table output +// printTable is the WASM-specific implementation that properly captures table +// output. In WASM, render into the global WasmTableWriter so the rendered +// table is available to the JS-accessible wasmTableOutput global that the web +// UI reads. An empty slice still renders a header-only table (native parity); +// any "No X found" warning is surfaced above it by GetCapturedOutput, which +// prepends the direct status buffer to the table output. +// +// A command may call this more than once; each call appends to +// WasmTableWriter rather than overwriting it, so the global reflects every +// table emitted so far. The buffer is cleared between WASM invocations by +// resetWasmStructuredBuffers. func printTable[T OutputFields](data []T, noColor bool, opts printOptions) error { wasmBufMu.Lock() defer wasmBufMu.Unlock() - // In WASM, render into the global WasmTableWriter so the rendered table is - // available to the JS-accessible wasmTableOutput global that the web UI reads. - // An empty slice still renders a header-only table (native parity); any - // "No X found" warning is surfaced above it by GetCapturedOutput, which - // prepends the direct status buffer to the table output. - WasmTableWriter.Reset() + before := WasmTableWriter.Len() if err := printTableToWriter(WasmTableWriter, data, noColor, opts); err != nil { return err } @@ -72,8 +77,8 @@ func printTable[T OutputFields](data []T, noColor bool, opts printOptions) error // are preserved; see wasm.SanitizeTerminalOutput. tableOutput := wasm.SanitizeTerminalOutput(WasmTableWriter.String()) - // Write the table output to stdout so it can be captured by wasm buffers. - fmt.Print(tableOutput) + // Write the newly-rendered table to stdout so it can be captured by wasm buffers. + fmt.Print(tableOutput[before:]) // Also write to a JavaScript-accessible global variable. js.Global().Set("wasmTableOutput", tableOutput) diff --git a/internal/base/output/table_wasm_test.go b/internal/base/output/table_wasm_test.go index 711b23c3..5bfa03da 100644 --- a/internal/base/output/table_wasm_test.go +++ b/internal/base/output/table_wasm_test.go @@ -143,6 +143,30 @@ func TestPrintTable_WASM_ComplexData(t *testing.T) { assert.Contains(t, output, "Complex Item") } +// TestPrintTable_WASM_AccumulatesMultipleCalls verifies that a command +// rendering more than one table in a single invocation has all of them +// captured, not just the last (ESD-1650). +func TestPrintTable_WASM_AccumulatesMultipleCalls(t *testing.T) { + first := []SimpleStruct{{ID: 1, Name: "First", Active: true}} + second := []SimpleStruct{{ID: 2, Name: "Second", Active: false}} + + WasmTableWriter.Reset() + js.Global().Delete("wasmTableOutput") + + assert.NoError(t, printTable(first, false, currentPrintOptions())) + assert.NoError(t, printTable(second, false, currentPrintOptions())) + + output := WasmTableWriter.String() + assert.Contains(t, output, "First", "first table must survive a second call") + assert.Contains(t, output, "Second", "second table must also be present") + + global := js.Global().Get("wasmTableOutput") + assert.Equal(t, output, global.String(), "global must reflect the full accumulated buffer") + + WasmTableWriter.Reset() + js.Global().Delete("wasmTableOutput") +} + // TestCalculateDynamicWidth verifies column width calculation func TestCalculateDynamicWidth(t *testing.T) { tests := []struct { diff --git a/internal/wasm/wasmhttp/transport.go b/internal/wasm/wasmhttp/transport.go index ba59c5c6..faae04db 100644 --- a/internal/wasm/wasmhttp/transport.go +++ b/internal/wasm/wasmhttp/transport.go @@ -79,7 +79,9 @@ func (t *WasmHTTPTransport) buildFetchOptions(req *http.Request) (map[string]int continue } if len(values) > 0 { - headers[key] = values[0] // fetch expects single string values + // fetch's Headers init dict accepts one string per key, so combine + // multi-value headers the same way HTTP field values are combined. + headers[key] = strings.Join(values, ", ") } } @@ -99,7 +101,11 @@ func (t *WasmHTTPTransport) buildFetchOptions(req *http.Request) (map[string]int req.Body = io.NopCloser(bytes.NewReader(bodyBytes)) if len(bodyBytes) > 0 { - fetchOpts["body"] = string(bodyBytes) + // Pass raw bytes as a Uint8Array rather than a Go string, which fetch + // would otherwise treat as UTF-8 text and mangle a binary body. + jsBody := js.Global().Get("Uint8Array").New(len(bodyBytes)) + js.CopyBytesToJS(jsBody, bodyBytes) + fetchOpts["body"] = jsBody } } diff --git a/internal/wasm/wasmhttp/transport_test.go b/internal/wasm/wasmhttp/transport_test.go index e6c62288..5efe652d 100644 --- a/internal/wasm/wasmhttp/transport_test.go +++ b/internal/wasm/wasmhttp/transport_test.go @@ -3,6 +3,7 @@ package wasmhttp import ( + "bytes" "context" "errors" "net/http" @@ -292,3 +293,39 @@ func TestBuildFetchOptions_NoForbiddenAcceptEncodingHeader(t *testing.T) { _, exists := headers["Accept-Encoding"] assert.False(t, exists, "Accept-Encoding is a forbidden fetch header and must not be set") } + +func TestBuildFetchOptions_MultiValueHeaderJoined(t *testing.T) { + transport := &WasmHTTPTransport{} + req, err := http.NewRequest(http.MethodGet, "https://example.invalid/test", nil) + require.NoError(t, err) + req.Header.Add("X-Custom", "first") + req.Header.Add("X-Custom", "second") + + opts, err := transport.buildFetchOptions(req) + require.NoError(t, err) + + headers, ok := opts["headers"].(map[string]interface{}) + require.True(t, ok) + + assert.Equal(t, "first, second", headers["X-Custom"], + "every value of a multi-value header should be forwarded, not just the first") +} + +func TestBuildFetchOptions_BinaryBodyPreservedAsBytes(t *testing.T) { + transport := &WasmHTTPTransport{} + binary := []byte{0x00, 0xff, 0xfe, 'h', 'i', 0x80, 0x81} + req, err := http.NewRequest(http.MethodPost, "https://example.invalid/test", bytes.NewReader(binary)) + require.NoError(t, err) + + opts, err := transport.buildFetchOptions(req) + require.NoError(t, err) + + jsBody, ok := opts["body"].(js.Value) + require.True(t, ok, "body should be passed as a js.Value (Uint8Array), not a Go string") + assert.Equal(t, "Uint8Array", jsBody.Get("constructor").Get("name").String()) + + length := jsBody.Get("length").Int() + got := make([]byte, length) + js.CopyBytesToGo(got, jsBody) + assert.Equal(t, binary, got, "binary body bytes must round-trip without UTF-8 mangling") +} From b66b4de8a1e339d18dd40f1cc46fca23240db432 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 12:50:11 -0700 Subject: [PATCH 2/4] Truncate WASM structured-output buffers on encode error printJSON/printCSV/printXML/printTable no longer reset per call, so a partial write from a failed encode could persist and get prepended to a later successful call within the same invocation. Roll the buffer back to its pre-call length whenever the call returns an error. Also note the Cookie-header exception to the multi-value header join. --- internal/base/output/output_wasm.go | 32 ++++++++++++++++++++++------- internal/base/output/table_wasm.go | 11 ++++++++-- internal/wasm/wasmhttp/transport.go | 4 +++- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/internal/base/output/output_wasm.go b/internal/base/output/output_wasm.go index e17cb3f0..20f0e543 100644 --- a/internal/base/output/output_wasm.go +++ b/internal/base/output/output_wasm.go @@ -45,8 +45,10 @@ func init() { // A command may call this more than once (multiple PrintOutput calls emitting // several structured documents); each call appends to WasmJSONWriter rather than // overwriting it, so the global reflects every document emitted so far. The -// buffer is cleared between WASM invocations by resetWasmStructuredBuffers. -func printJSON[T OutputFields](data []T, opts printOptions) error { +// buffer is cleared between WASM invocations by resetWasmStructuredBuffers, so +// callers must not invoke this per data item in a loop; it accumulates without +// bound within an invocation. +func printJSON[T OutputFields](data []T, opts printOptions) (err error) { wasmBufMu.Lock() defer wasmBufMu.Unlock() @@ -60,6 +62,12 @@ func printJSON[T OutputFields](data []T, opts printOptions) error { } before := WasmJSONWriter.Len() + defer func() { + if err != nil { + WasmJSONWriter.Truncate(before) + } + }() + encoder := json.NewEncoder(WasmJSONWriter) encoder.SetIndent("", " ") if err := encoder.Encode(toEncode); err != nil { @@ -84,12 +92,17 @@ func printJSON[T OutputFields](data []T, opts printOptions) error { // printCSV is the WASM-specific implementation that properly captures CSV // output. Like printJSON, it appends to WasmCSVWriter so a command emitting // multiple CSV documents has all of them captured, not just the last. -func printCSV[T OutputFields](data []T, opts printOptions) error { +func printCSV[T OutputFields](data []T, opts printOptions) (err error) { wasmBufMu.Lock() defer wasmBufMu.Unlock() before := WasmCSVWriter.Len() w := csv.NewWriter(WasmCSVWriter) + defer func() { + if err != nil { + WasmCSVWriter.Truncate(before) + } + }() defer w.Flush() var sample T @@ -215,11 +228,19 @@ func printCSV[T OutputFields](data []T, opts printOptions) error { // printXML is the WASM-specific implementation that properly captures XML // output. Like printJSON, it appends to WasmXMLWriter so a command emitting // multiple XML documents has all of them captured, not just the last. -func printXML[T OutputFields](data []T, opts printOptions) error { +func printXML[T OutputFields](data []T, opts printOptions) (err error) { wasmBufMu.Lock() defer wasmBufMu.Unlock() before := WasmXMLWriter.Len() + encoder := xml.NewEncoder(WasmXMLWriter) + encoder.Indent("", " ") + defer func() { + if err != nil { + _ = encoder.Flush() + WasmXMLWriter.Truncate(before) + } + }() if data == nil { data = []T{} @@ -321,9 +342,6 @@ func printXML[T OutputFields](data []T, opts printOptions) error { fields = filtered } - encoder := xml.NewEncoder(WasmXMLWriter) - encoder.Indent("", " ") - WasmXMLWriter.WriteString(xml.Header) start := xml.StartElement{Name: xml.Name{Local: "items"}} if err := encoder.EncodeToken(start); err != nil { diff --git a/internal/base/output/table_wasm.go b/internal/base/output/table_wasm.go index 0b07b2a3..a0efc689 100644 --- a/internal/base/output/table_wasm.go +++ b/internal/base/output/table_wasm.go @@ -60,12 +60,19 @@ func calculateDynamicWidth(termWidth int, minWidth, maxPercentage int) int { // A command may call this more than once; each call appends to // WasmTableWriter rather than overwriting it, so the global reflects every // table emitted so far. The buffer is cleared between WASM invocations by -// resetWasmStructuredBuffers. -func printTable[T OutputFields](data []T, noColor bool, opts printOptions) error { +// resetWasmStructuredBuffers, so callers must not invoke this per data item +// in a loop; it accumulates without bound within an invocation. +func printTable[T OutputFields](data []T, noColor bool, opts printOptions) (err error) { wasmBufMu.Lock() defer wasmBufMu.Unlock() before := WasmTableWriter.Len() + defer func() { + if err != nil { + WasmTableWriter.Truncate(before) + } + }() + if err := printTableToWriter(WasmTableWriter, data, noColor, opts); err != nil { return err } diff --git a/internal/wasm/wasmhttp/transport.go b/internal/wasm/wasmhttp/transport.go index faae04db..1649fd58 100644 --- a/internal/wasm/wasmhttp/transport.go +++ b/internal/wasm/wasmhttp/transport.go @@ -80,7 +80,9 @@ func (t *WasmHTTPTransport) buildFetchOptions(req *http.Request) (map[string]int } if len(values) > 0 { // fetch's Headers init dict accepts one string per key, so combine - // multi-value headers the same way HTTP field values are combined. + // multi-value headers the same way HTTP field values are combined + // (RFC 7230 3.2.2). Cookie combines with "; " instead, but the SDK + // never sets a multi-value Cookie header through this transport. headers[key] = strings.Join(values, ", ") } } From 7c96d32418bdf87b2b86822199fe0c3a430578a1 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 22 Jul 2026 05:36:31 -0700 Subject: [PATCH 3/4] Fix raw-offset slicing of sanitized WASM table output printTable sliced the already-sanitized buffer by a raw-byte offset, but sanitization drops bytes, so the offset stops lining up once enough control sequences are stripped. Slice the raw buffer instead and sanitize each piece separately, matching the sibling printJSON/ printCSV/printXML implementations. --- internal/base/output/table_wasm.go | 11 +++++--- internal/base/output/table_wasm_test.go | 34 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/internal/base/output/table_wasm.go b/internal/base/output/table_wasm.go index a0efc689..7142e532 100644 --- a/internal/base/output/table_wasm.go +++ b/internal/base/output/table_wasm.go @@ -81,14 +81,17 @@ func printTable[T OutputFields](data []T, noColor bool, opts printOptions) (err // GetCapturedOutput/GetCompletionOutput and written to xterm without // escaping, and a colorized cell value (colorizeValue) can carry a // resource name or other API field an attacker controls. SGR color codes - // are preserved; see wasm.SanitizeTerminalOutput. - tableOutput := wasm.SanitizeTerminalOutput(WasmTableWriter.String()) + // are preserved; see wasm.SanitizeTerminalOutput. Sanitize the new slice + // on its own rather than slicing the sanitized whole buffer by a raw-byte + // offset: sanitizing drops bytes, so a raw offset no longer lines up with + // the sanitized string. + raw := WasmTableWriter.String() // Write the newly-rendered table to stdout so it can be captured by wasm buffers. - fmt.Print(tableOutput[before:]) + fmt.Print(wasm.SanitizeTerminalOutput(raw[before:])) // Also write to a JavaScript-accessible global variable. - js.Global().Set("wasmTableOutput", tableOutput) + js.Global().Set("wasmTableOutput", wasm.SanitizeTerminalOutput(raw)) return nil } diff --git a/internal/base/output/table_wasm_test.go b/internal/base/output/table_wasm_test.go index 5bfa03da..95a9797c 100644 --- a/internal/base/output/table_wasm_test.go +++ b/internal/base/output/table_wasm_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/megaport/megaport-cli/internal/wasm" ) // TestWasmTableWriter verifies the WASM table writer buffer @@ -167,6 +169,38 @@ func TestPrintTable_WASM_AccumulatesMultipleCalls(t *testing.T) { js.Global().Delete("wasmTableOutput") } +// TestPrintTable_WASM_AccumulatesWhenSanitizerDropsBytes guards the raw-offset +// alignment fixed in ESD-1650. The first table's cell carries injected control +// bytes the sanitizer strips, so the sanitized whole buffer is shorter than the +// raw byte offset recorded before the second call; slicing the sanitized string +// by that raw offset (the pre-fix behavior) reads past its end and panics. The +// global must still equal the sanitized full buffer and both rows must survive. +func TestPrintTable_WASM_AccumulatesWhenSanitizerDropsBytes(t *testing.T) { + noisy := "acme" + strings.Repeat("\x1b[2K\x1b[H\x7f", 400) + first := []SimpleStruct{{ID: 1, Name: noisy, Active: true}} + second := []SimpleStruct{{ID: 2, Name: "Second", Active: false}} + + SetTerminalWidthForTesting(0) + defer SetTerminalWidthForTesting(0) + WasmTableWriter.Reset() + js.Global().Delete("wasmTableOutput") + + assert.NotPanics(t, func() { + assert.NoError(t, printTable(first, true, currentPrintOptions())) + assert.NoError(t, printTable(second, true, currentPrintOptions())) + }) + + global := js.Global().Get("wasmTableOutput").String() + assert.Equal(t, wasm.SanitizeTerminalOutput(WasmTableWriter.String()), global, + "global must equal the sanitized full accumulated buffer") + assert.NotContains(t, global, "\x1b[2K", "injected control bytes must be stripped") + assert.Contains(t, global, "acme") + assert.Contains(t, global, "Second") + + WasmTableWriter.Reset() + js.Global().Delete("wasmTableOutput") +} + // TestCalculateDynamicWidth verifies column width calculation func TestCalculateDynamicWidth(t *testing.T) { tests := []struct { From a390cecdb507054d59b33aebfac7e5c80f1d80f5 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Mon, 27 Jul 2026 16:19:28 -0700 Subject: [PATCH 4/4] Check ExecuteWithArgs return in WASM help tests to satisfy errcheck --- cmd/megaport/help_wasm_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/megaport/help_wasm_test.go b/cmd/megaport/help_wasm_test.go index dccd3b81..d9a93282 100644 --- a/cmd/megaport/help_wasm_test.go +++ b/cmd/megaport/help_wasm_test.go @@ -20,12 +20,12 @@ import ( // yield byte-identical output, not growing/duplicated help text. func TestWasmHelp_RepeatedInvocationsProduceIdenticalOutput(t *testing.T) { wasm.ResetOutputBuffers() - ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + _ = ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) first := wasm.WasmOutputBuffer.String() assert.NotEmpty(t, first) wasm.ResetOutputBuffers() - ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + _ = ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) second := wasm.WasmOutputBuffer.String() assert.Equal(t, first, second, "repeated --help calls must produce identical output, not accumulate coloring/suffixes") @@ -35,13 +35,13 @@ func TestWasmHelp_RepeatedInvocationsProduceIdenticalOutput(t *testing.T) { // that a two-call comparison might miss. func TestWasmHelp_RepeatedInvocationsAcrossManyCalls(t *testing.T) { wasm.ResetOutputBuffers() - ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + _ = ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) baseline := wasm.WasmOutputBuffer.String() assert.NotEmpty(t, baseline) for i := 0; i < 5; i++ { wasm.ResetOutputBuffers() - ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) + _ = ExecuteWithArgs([]string{"megaport-cli", "ports", "list", "--help"}) out := wasm.WasmOutputBuffer.String() assert.Equal(t, baseline, out, "help output must not drift across repeated invocations") } @@ -52,12 +52,12 @@ func TestWasmHelp_RepeatedInvocationsAcrossManyCalls(t *testing.T) { // LongDesc string each time rather than a cached original. func TestWasmHelp_RootCommandRepeatedInvocationsProduceIdenticalOutput(t *testing.T) { wasm.ResetOutputBuffers() - ExecuteWithArgs([]string{"megaport-cli", "--help"}) + _ = ExecuteWithArgs([]string{"megaport-cli", "--help"}) first := wasm.WasmOutputBuffer.String() assert.NotEmpty(t, first) wasm.ResetOutputBuffers() - ExecuteWithArgs([]string{"megaport-cli", "--help"}) + _ = ExecuteWithArgs([]string{"megaport-cli", "--help"}) second := wasm.WasmOutputBuffer.String() assert.Equal(t, first, second, "repeated root --help calls must produce identical output")