From c5360bad6acac3119359b1d0b1a82b3095c22076 Mon Sep 17 00:00:00 2001 From: defangdevs Date: Wed, 19 Aug 2026 15:42:55 -0700 Subject: [PATCH 1/2] fix: don't leak the debug compose-project YAML dump into --json stdout Loader.loadProject() unconditionally wrote the whole compose project as YAML to stdout via term.Println whenever debug logging was on, bypassing the stderr redirection that every other Info/Warn/Debug helper in pkg/term applies under --json. `defang services --json` calls this on the way to resolving the project name, so with DEFANG_DEBUG=1 set the YAML dump landed on stdout ahead of the JSON array, corrupting it for machine consumers (e.g. defang-github-action's deployment summary, which then failed on `jq: parse error: Invalid numeric literal`). Add term.DoJSON() and skip the dump when JSON mode is active. --- src/pkg/cli/compose/loader.go | 2 +- src/pkg/cli/compose/loader_test.go | 34 ++++++++++++++++++++++++++++++ src/pkg/term/colorizer.go | 8 +++++++ src/pkg/term/colorizer_test.go | 14 ++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/pkg/cli/compose/loader.go b/src/pkg/cli/compose/loader.go index 3102d3d94..f880e90cf 100644 --- a/src/pkg/cli/compose/loader.go +++ b/src/pkg/cli/compose/loader.go @@ -173,7 +173,7 @@ func (l *Loader) loadProject(ctx context.Context, suppressWarn bool) (*Project, return nil, err } - if term.DoDebug() { + if term.DoDebug() && !term.DoJSON() { b, _ := yaml.Marshal(project) term.Println(string(b)) } diff --git a/src/pkg/cli/compose/loader_test.go b/src/pkg/cli/compose/loader_test.go index 83b18342b..02d22e9f0 100644 --- a/src/pkg/cli/compose/loader_test.go +++ b/src/pkg/cli/compose/loader_test.go @@ -38,6 +38,40 @@ func TestResolveProjectWorkingDirDoesNotLoadOrCacheProject(t *testing.T) { assert.Equal(t, dir, workingDir) } +// TestLoadProjectSkipsDebugDumpInJSONMode is a regression test: `defang +// services --json` with DEFANG_DEBUG=1 used to write the project's YAML dump +// to stdout ahead of the JSON payload, corrupting it for callers like +// `defang-github-action`'s deployment summary (jq: invalid numeric literal). +func TestLoadProjectSkipsDebugDumpInJSONMode(t *testing.T) { + dir := t.TempDir() + composePath := filepath.Join(dir, "compose.yaml") + require.NoError(t, os.WriteFile(composePath, []byte("services:\n web:\n image: alpine\n"), 0o644)) + + oldTerm := term.DefaultTerm + t.Cleanup(func() { term.DefaultTerm = oldTerm }) + + t.Run("debug alone still dumps the project", func(t *testing.T) { + var output bytes.Buffer + term.DefaultTerm = term.NewTerm(os.Stdin, &output, &output) + term.DefaultTerm.SetDebug(true) + + _, err := NewLoader(WithPath(composePath)).LoadProject(t.Context()) + require.NoError(t, err) + assert.Contains(t, output.String(), "services:") + }) + + t.Run("debug plus json suppresses the dump", func(t *testing.T) { + var output bytes.Buffer + term.DefaultTerm = term.NewTerm(os.Stdin, &output, &output) + term.DefaultTerm.SetDebug(true) + term.DefaultTerm.SetJSON(true) + + _, err := NewLoader(WithPath(composePath)).LoadProject(t.Context()) + require.NoError(t, err) + assert.Empty(t, output.String()) + }) +} + func TestResolveProjectWorkingDirDoesNotSuppressProjectWarnings(t *testing.T) { dir := t.TempDir() composePath := filepath.Join(dir, "compose.yaml") diff --git a/src/pkg/term/colorizer.go b/src/pkg/term/colorizer.go index b3bea0775..cbb4f09de 100644 --- a/src/pkg/term/colorizer.go +++ b/src/pkg/term/colorizer.go @@ -97,6 +97,10 @@ func (t *Term) DoDebug() bool { return t.debug.Load() } +func (t *Term) DoJSON() bool { + return t.json +} + func (t *Term) HasDarkBackground() bool { return t.hasDarkBg } @@ -381,6 +385,10 @@ func DoDebug() bool { return DefaultTerm.DoDebug() } +func DoJSON() bool { + return DefaultTerm.DoJSON() +} + func HasDarkBackground() bool { return DefaultTerm.HasDarkBackground() } diff --git a/src/pkg/term/colorizer_test.go b/src/pkg/term/colorizer_test.go index 295adfca6..ad7e34262 100644 --- a/src/pkg/term/colorizer_test.go +++ b/src/pkg/term/colorizer_test.go @@ -162,6 +162,20 @@ func TestIsTerminal(t *testing.T) { t.Error("Expected IsTerminal() to return false") } } + +func TestDoJSON(t *testing.T) { + oldTerm := DefaultTerm + t.Cleanup(func() { DefaultTerm = oldTerm }) + DefaultTerm = NewTerm(os.Stdin, &bytes.Buffer{}, &bytes.Buffer{}) + + if DoJSON() { + t.Error("Expected DoJSON() to default to false") + } + SetJSON(true) + if !DoJSON() { + t.Error("Expected DoJSON() to return true after SetJSON(true)") + } +} func TestWarn(t *testing.T) { tests := []struct { msgs []string From 67b8c6662630d0464378e118fcafaee91dca15d8 Mon Sep 17 00:00:00 2001 From: defangdevs Date: Wed, 19 Aug 2026 16:03:04 -0700 Subject: [PATCH 2/2] fix: route term.Print*/Printc to stderr in JSON mode term.Print/Println/Printf/Printc always wrote to stdout, unlike Info/Warn which already route through outOrErr() to stderr under --json. Any of the ~40 call sites using Print* was one command-path change away from leaking human-readable text into a --json payload, the same bug class as the debug YAML dump fixed in the previous commit. Fix it at the source instead of gating each call site: Print*/Printc now go through outOrErr() too, since the only legitimate way JSON output reaches stdout is jsonTable()'s json.Encoder, which writes t.out directly. This makes the loader.go DoJSON() guard from the previous commit redundant (term.Println itself now handles it), so drop it and the now-unused term.DoJSON() getter, and strengthen the regression test to assert the debug dump lands on stderr rather than being silently dropped. --- src/pkg/cli/compose/loader.go | 4 ++-- src/pkg/cli/compose/loader_test.go | 30 +++++++++++++++------------ src/pkg/term/colorizer.go | 20 ++++++++---------- src/pkg/term/colorizer_test.go | 33 ++++++++++++++++++++++-------- 4 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/pkg/cli/compose/loader.go b/src/pkg/cli/compose/loader.go index f880e90cf..312e5f9c8 100644 --- a/src/pkg/cli/compose/loader.go +++ b/src/pkg/cli/compose/loader.go @@ -173,9 +173,9 @@ func (l *Loader) loadProject(ctx context.Context, suppressWarn bool) (*Project, return nil, err } - if term.DoDebug() && !term.DoJSON() { + if term.DoDebug() { b, _ := yaml.Marshal(project) - term.Println(string(b)) + term.Println(string(b)) // term.Println routes to stderr in JSON mode, so this never corrupts --json stdout } l.cached = project diff --git a/src/pkg/cli/compose/loader_test.go b/src/pkg/cli/compose/loader_test.go index 02d22e9f0..d682878c5 100644 --- a/src/pkg/cli/compose/loader_test.go +++ b/src/pkg/cli/compose/loader_test.go @@ -38,11 +38,13 @@ func TestResolveProjectWorkingDirDoesNotLoadOrCacheProject(t *testing.T) { assert.Equal(t, dir, workingDir) } -// TestLoadProjectSkipsDebugDumpInJSONMode is a regression test: `defang -// services --json` with DEFANG_DEBUG=1 used to write the project's YAML dump -// to stdout ahead of the JSON payload, corrupting it for callers like -// `defang-github-action`'s deployment summary (jq: invalid numeric literal). -func TestLoadProjectSkipsDebugDumpInJSONMode(t *testing.T) { +// TestLoadProjectDebugDumpNeverHitsStdoutInJSONMode is a regression test: +// `defang services --json` with DEFANG_DEBUG=1 used to write the project's +// YAML dump to stdout ahead of the JSON payload, corrupting it for callers +// like `defang-github-action`'s deployment summary (jq: invalid numeric +// literal). term.Println now routes to stderr in JSON mode, so the dump +// must never appear on stdout, whether or not JSON mode is on. +func TestLoadProjectDebugDumpNeverHitsStdoutInJSONMode(t *testing.T) { dir := t.TempDir() composePath := filepath.Join(dir, "compose.yaml") require.NoError(t, os.WriteFile(composePath, []byte("services:\n web:\n image: alpine\n"), 0o644)) @@ -50,25 +52,27 @@ func TestLoadProjectSkipsDebugDumpInJSONMode(t *testing.T) { oldTerm := term.DefaultTerm t.Cleanup(func() { term.DefaultTerm = oldTerm }) - t.Run("debug alone still dumps the project", func(t *testing.T) { - var output bytes.Buffer - term.DefaultTerm = term.NewTerm(os.Stdin, &output, &output) + t.Run("debug alone dumps the project to stdout", func(t *testing.T) { + var stdout, stderr bytes.Buffer + term.DefaultTerm = term.NewTerm(os.Stdin, &stdout, &stderr) term.DefaultTerm.SetDebug(true) _, err := NewLoader(WithPath(composePath)).LoadProject(t.Context()) require.NoError(t, err) - assert.Contains(t, output.String(), "services:") + assert.Contains(t, stdout.String(), "services:") + assert.Empty(t, stderr.String()) }) - t.Run("debug plus json suppresses the dump", func(t *testing.T) { - var output bytes.Buffer - term.DefaultTerm = term.NewTerm(os.Stdin, &output, &output) + t.Run("debug plus json moves the dump to stderr", func(t *testing.T) { + var stdout, stderr bytes.Buffer + term.DefaultTerm = term.NewTerm(os.Stdin, &stdout, &stderr) term.DefaultTerm.SetDebug(true) term.DefaultTerm.SetJSON(true) _, err := NewLoader(WithPath(composePath)).LoadProject(t.Context()) require.NoError(t, err) - assert.Empty(t, output.String()) + assert.Empty(t, stdout.String()) + assert.Contains(t, stderr.String(), "services:") }) } diff --git a/src/pkg/term/colorizer.go b/src/pkg/term/colorizer.go index cbb4f09de..90ee4c965 100644 --- a/src/pkg/term/colorizer.go +++ b/src/pkg/term/colorizer.go @@ -97,10 +97,6 @@ func (t *Term) DoDebug() bool { return t.debug.Load() } -func (t *Term) DoJSON() bool { - return t.json -} - func (t *Term) HasDarkBackground() bool { return t.hasDarkBg } @@ -199,20 +195,24 @@ func ensurePrefix(prefix prefixChars, s string) string { return string(prefix) + s } +// Printc, Print, Println, and Printf are for human-readable text; they write +// to stderr instead of stdout when JSON mode is on, so they never corrupt a +// command's --json payload. The only thing that belongs on stdout in JSON +// mode is the JSON payload itself (see jsonTable in table.go). func (t *Term) Printc(c Color, v ...any) (int, error) { - return output(t.out, c, fmt.Sprint(v...)) + return output(t.outOrErr(), c, fmt.Sprint(v...)) } func (t *Term) Print(v ...any) (int, error) { - return fmt.Fprint(t.out, v...) + return fmt.Fprint(t.outOrErr(), v...) } func (t *Term) Println(v ...any) (int, error) { - return fmt.Fprintln(t.out, v...) + return fmt.Fprintln(t.outOrErr(), v...) } func (t *Term) Printf(format string, v ...any) (int, error) { - return fmt.Fprintf(t.out, format, v...) + return fmt.Fprintf(t.outOrErr(), format, v...) } func (t *Term) Debug(v ...any) (int, error) { @@ -385,10 +385,6 @@ func DoDebug() bool { return DefaultTerm.DoDebug() } -func DoJSON() bool { - return DefaultTerm.DoJSON() -} - func HasDarkBackground() bool { return DefaultTerm.HasDarkBackground() } diff --git a/src/pkg/term/colorizer_test.go b/src/pkg/term/colorizer_test.go index ad7e34262..36d5caad0 100644 --- a/src/pkg/term/colorizer_test.go +++ b/src/pkg/term/colorizer_test.go @@ -163,17 +163,32 @@ func TestIsTerminal(t *testing.T) { } } -func TestDoJSON(t *testing.T) { - oldTerm := DefaultTerm - t.Cleanup(func() { DefaultTerm = oldTerm }) - DefaultTerm = NewTerm(os.Stdin, &bytes.Buffer{}, &bytes.Buffer{}) +// TestPrintRoutingInJSONMode is a regression test: Print/Println/Printf/Printc +// used to always write to stdout, so any of them reachable from a command +// that also emits --json output (e.g. via a debug dump) would corrupt that +// JSON payload. They must move to stderr in JSON mode, like Info/Warn do. +func TestPrintRoutingInJSONMode(t *testing.T) { + var stdout, stderr bytes.Buffer + defaultTerm := NewTerm(os.Stdin, &stdout, &stderr) - if DoJSON() { - t.Error("Expected DoJSON() to default to false") + defaultTerm.Print("a") + defaultTerm.Println("b") + defaultTerm.Printf("%s", "c") + defaultTerm.Printc(InfoColor, "d") + if stdout.String() == "" || stderr.String() != "" { + t.Errorf("expected Print* to write to stdout when JSON mode is off; stdout=%q stderr=%q", stdout.String(), stderr.String()) } - SetJSON(true) - if !DoJSON() { - t.Error("Expected DoJSON() to return true after SetJSON(true)") + + stdout.Reset() + stderr.Reset() + defaultTerm.SetJSON(true) + + defaultTerm.Print("a") + defaultTerm.Println("b") + defaultTerm.Printf("%s", "c") + defaultTerm.Printc(InfoColor, "d") + if stdout.String() != "" || stderr.String() == "" { + t.Errorf("expected Print* to write to stderr when JSON mode is on; stdout=%q stderr=%q", stdout.String(), stderr.String()) } } func TestWarn(t *testing.T) {