From b0823c9d95034bcfc437876801d389d911955c44 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Wed, 11 Mar 2026 11:15:05 +0100 Subject: [PATCH 1/3] Write diagnostic warnings to stderr instead of stdout Expiration warnings, profile-not-found warnings, and config permission warnings were written to data.Output (stdout), breaking JSON parsing when --json was used. --- pkg/app/expiry_warning_test.go | 49 ++++++++++++++++++++-------------- pkg/app/run.go | 9 ++++--- pkg/global/global.go | 3 ++- pkg/testutil/args.go | 1 + 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/pkg/app/expiry_warning_test.go b/pkg/app/expiry_warning_test.go index 4b2240e0d..9c3962c11 100644 --- a/pkg/app/expiry_warning_test.go +++ b/pkg/app/expiry_warning_test.go @@ -18,9 +18,10 @@ import ( func expiringTokenData(out *bytes.Buffer) *global.Data { soon := time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) return &global.Data{ - Output: out, - ErrLog: fsterr.Log, - Manifest: &manifest.Data{}, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, + Manifest: &manifest.Data{}, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -52,8 +53,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -76,8 +78,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -99,9 +102,10 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, - Env: config.Environment{APIToken: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.fake"}, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, + Env: config.Environment{APIToken: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.fake"}, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -123,9 +127,10 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, - Flags: global.Flags{Token: "some-raw-token"}, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, + Flags: global.Flags{Token: "some-raw-token"}, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -147,8 +152,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "deleted-token", @@ -164,8 +170,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -187,7 +194,8 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, + Output: out, + ErrOutput: out, Config: config.File{ Auth: config.Auth{ Default: "mytoken", @@ -209,8 +217,9 @@ func TestCheckTokenExpirationWarning(t *testing.T) { commandName: "service list", data: func(out *bytes.Buffer) *global.Data { return &global.Data{ - Output: out, - ErrLog: fsterr.Log, + Output: out, + ErrOutput: out, + ErrLog: fsterr.Log, Config: config.File{ Auth: config.Auth{ Default: "sso-tok", diff --git a/pkg/app/run.go b/pkg/app/run.go index 2375c5500..1f5c3c254 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -172,6 +172,7 @@ var Init = func(args []string, stdin io.Reader) (*global.Data, error) { ConfigPath: config.FilePath, Env: e, ErrLog: fsterr.Log, + ErrOutput: os.Stderr, ExecuteWasmTools: compute.ExecuteWasmTools, HTTPClient: httpClient, Manifest: &md, @@ -287,9 +288,9 @@ func Exec(data *global.Data) error { if !data.Flags.Quiet && data.Flags.Token == "" && data.Env.APIToken == "" && data.Manifest != nil && data.Manifest.File.Profile != "" { if data.Config.GetAuthToken(data.Manifest.File.Profile) == nil { if defaultName, _ := data.Config.GetDefaultAuthToken(); defaultName != "" { - text.Warning(data.Output, "fastly.toml profile %q not found in auth config, using default token %q.\n", data.Manifest.File.Profile, defaultName) + text.Warning(data.ErrOutput, "fastly.toml profile %q not found in auth config, using default token %q.\n", data.Manifest.File.Profile, defaultName) } else { - text.Warning(data.Output, "fastly.toml profile %q not found in auth config and no default token is configured.\n", data.Manifest.File.Profile) + text.Warning(data.ErrOutput, "fastly.toml profile %q not found in auth config and no default token is configured.\n", data.Manifest.File.Profile) } } } @@ -316,7 +317,7 @@ func Exec(data *global.Data) error { displayToken(tokenSource, data) } if !data.Flags.Quiet { - checkConfigPermissions(tokenSource, data.Output) + checkConfigPermissions(tokenSource, data.ErrOutput) } data.APIClient, data.RTSClient, err = configureClients(token, apiEndpoint, data.APIClientFactory, data.Flags.Debug) @@ -541,7 +542,7 @@ func checkTokenExpirationWarning(data *global.Data, commandName string) { summary := authcmd.ExpirationSummary(status, expires, time.Now()) remediation := authcmd.ExpirationRemediation(at.Type) - text.Warning(data.Output, "Your active token %s. %s\n", summary, remediation) + text.Warning(data.ErrOutput, "Your active token %s. %s\n", summary, remediation) } // isAuthRelatedCommand reports whether commandName belongs to an auth-related diff --git a/pkg/global/global.go b/pkg/global/global.go index 99f2d8cec..00a94e71a 100644 --- a/pkg/global/global.go +++ b/pkg/global/global.go @@ -62,7 +62,8 @@ type Data struct { // Env is all the data that is provided by the environment. Env config.Environment // ErrLog provides an interface for recording errors to disk. - ErrLog fsterr.LogInterface + ErrLog fsterr.LogInterface + ErrOutput io.Writer // ExecuteWasmTools is a function that executes the wasm-tools binary. ExecuteWasmTools func(bin string, args []string, global *Data) error // Flags are all the global CLI flags. diff --git a/pkg/testutil/args.go b/pkg/testutil/args.go index 21cd56723..a5e409135 100644 --- a/pkg/testutil/args.go +++ b/pkg/testutil/args.go @@ -124,6 +124,7 @@ func MockGlobalData(args []string, stdout io.Writer) *global.Data { ConfigPath: configPath, Env: config.Environment{}, ErrLog: errors.Log, + ErrOutput: stdout, ExecuteWasmTools: func(bin string, args []string, d *global.Data) error { fmt.Printf("bin: %s\n", bin) fmt.Printf("args: %#v\n", args) From b027b006bd30539bfc192449e2537ea7d36a742e Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Thu, 12 Mar 2026 15:21:07 +0100 Subject: [PATCH 2/3] Add a test showing that that warnings don't corrupt JSON output --- pkg/app/run_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/pkg/app/run_test.go b/pkg/app/run_test.go index e3e53cea9..f989d0848 100644 --- a/pkg/app/run_test.go +++ b/pkg/app/run_test.go @@ -3,6 +3,7 @@ package app_test import ( "bufio" "bytes" + "encoding/json" "io" "os" "strings" @@ -178,6 +179,38 @@ func TestExecConfigShowsExpiryWarning(t *testing.T) { } } +func TestExecJSONLeavesStdoutCleanAndWritesWarningToStderr(t *testing.T) { + var ( + stdout bytes.Buffer + stderr bytes.Buffer + ) + + args := testutil.SplitArgs("config -l --json") + app.Init = func(_ []string, _ io.Reader) (*global.Data, error) { + data := testutil.MockGlobalData(args, &stdout) + data.ErrOutput = &stderr + data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) + return data, nil + } + err := app.Run(args, nil) + if err != nil { + t.Fatalf("app.Run returned unexpected error: %v", err) + } + + if strings.Contains(stdout.String(), "expires in") { + t.Fatalf("expected stdout to contain only JSON, got: %s", stdout.String()) + } + + var decoded any + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("expected stdout to remain valid JSON, got %q: %v", stdout.String(), err) + } + + if !strings.Contains(stderr.String(), "expires in") { + t.Fatalf("expected stderr warning, got: %s", stderr.String()) + } +} + // stripTrailingSpace removes any trailing spaces from the multiline str. func stripTrailingSpace(str string) string { buf := bytes.NewBuffer(nil) From bbc98f995c720fc81d0b7452afcc6fde4ffbecd4 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Thu, 12 Mar 2026 16:13:21 +0100 Subject: [PATCH 3/3] Stop --json from suppressing stderr warnings --- pkg/app/expiry_warning_test.go | 20 +++++++++++++++----- pkg/app/run.go | 19 +++++++++++-------- pkg/app/run_test.go | 23 ++++++++++------------- pkg/global/global.go | 3 +++ 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/pkg/app/expiry_warning_test.go b/pkg/app/expiry_warning_test.go index 9c3962c11..3dc80f3a7 100644 --- a/pkg/app/expiry_warning_test.go +++ b/pkg/app/expiry_warning_test.go @@ -347,11 +347,6 @@ func TestCheckTokenExpirationWarningSuppression(t *testing.T) { commandName: "service list", flags: global.Flags{Quiet: true}, }, - { - name: "suppressed with --json flag (sets Quiet)", - commandName: "service list", - flags: global.Flags{Quiet: true}, // --json sets Quiet=true in Exec - }, } originalEnv := os.Getenv("FASTLY_DISABLE_AUTH_COMMAND") @@ -374,6 +369,21 @@ func TestCheckTokenExpirationWarningSuppression(t *testing.T) { } } +// TestCheckTokenExpirationWarningShownForJSON verifies that --json mode still +// emits the warning (to stderr) rather than suppressing it entirely. +func TestCheckTokenExpirationWarningShownForJSON(t *testing.T) { + var buf bytes.Buffer + data := expiringTokenData(&buf) + data.Flags = global.Flags{JSON: true} + + checkTokenExpirationWarning(data, "service list") + + output := buf.String() + if !strings.Contains(output, "expires in") { + t.Errorf("expected expiry warning in --json mode (written to stderr), got: %s", output) + } +} + // TestCheckTokenExpirationWarningNotSuppressedForNonAuth ensures that commands // starting with "auth" as a prefix of another word (e.g. "authtoken") are not // incorrectly suppressed. diff --git a/pkg/app/run.go b/pkg/app/run.go index 1f5c3c254..58778dbdc 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -201,9 +201,11 @@ func Exec(data *global.Data) error { return err } - // Check for --json flag early and set quiet mode if found. + // Check for --json flag early. JSON mode suppresses stdout-bound noise + // (metadata notices, update checks) but still allows stderr warnings + // (token expiry, profile mismatch) so they don't corrupt JSON output. if slices.Contains(data.Args, "--json") { - data.Flags.Quiet = true + data.Flags.JSON = true } // We short-circuit the execution for specific cases: @@ -220,7 +222,7 @@ func Exec(data *global.Data) error { } metadataDisable, _ := strconv.ParseBool(data.Env.WasmMetadataDisable) - if !slices.Contains(data.Args, "--metadata-disable") && !metadataDisable && !data.Config.CLI.MetadataNoticeDisplayed && commandCollectsData(commandName) && !data.Flags.Quiet { + if !slices.Contains(data.Args, "--metadata-disable") && !metadataDisable && !data.Config.CLI.MetadataNoticeDisplayed && commandCollectsData(commandName) && !data.Flags.Quiet && !data.Flags.JSON { text.Important(data.Output, "The Fastly CLI is configured to collect data related to Wasm builds (e.g. compilation times, resource usage, and other non-identifying data). To learn more about what data is being collected, why, and how to disable it: https://www.fastly.com/documentation/reference/cli") text.Break(data.Output) data.Config.CLI.MetadataNoticeDisplayed = true @@ -231,7 +233,7 @@ func Exec(data *global.Data) error { time.Sleep(5 * time.Second) // this message is only displayed once so give the user a chance to see it before it possibly scrolls off screen } - if data.Flags.Quiet { + if data.Flags.Quiet || data.Flags.JSON { data.Manifest.File.SetQuiet(true) } @@ -329,7 +331,7 @@ func Exec(data *global.Data) error { checkTokenExpirationWarning(data, commandName) - f := checkForUpdates(data.Versioners.CLI, commandName, data.Flags.Quiet) + f := checkForUpdates(data.Versioners.CLI, commandName, data.Flags.Quiet || data.Flags.JSON) defer f(data.Output) return command.Exec(data.Input, data.Output) @@ -507,9 +509,10 @@ func checkAndRefreshAuthSSOToken(name string, at *config.AuthToken, data *global // This matches the set hidden by FASTLY_DISABLE_AUTH_COMMAND (pkg/env/env.go). var authRelatedCommands = []string{"auth", "auth-token", "sso", "profile", "whoami"} -// checkTokenExpirationWarning prints a warning if the active stored token is -// about to expire. Only fires for SourceAuth tokens; env/flag tokens are opaque. -// Suppressed for auth-related commands and when --quiet or --json is active. +// checkTokenExpirationWarning prints a warning to stderr if the active stored +// token is about to expire. Only fires for SourceAuth tokens; env/flag tokens +// are opaque. Suppressed for auth-related commands and when --quiet is active. +// In --json mode the warning still fires (written to stderr via data.ErrOutput). func checkTokenExpirationWarning(data *global.Data, commandName string) { if data.Flags.Quiet { return diff --git a/pkg/app/run_test.go b/pkg/app/run_test.go index f989d0848..cb9aadfd6 100644 --- a/pkg/app/run_test.go +++ b/pkg/app/run_test.go @@ -3,7 +3,6 @@ package app_test import ( "bufio" "bytes" - "encoding/json" "io" "os" "strings" @@ -133,9 +132,7 @@ whoami } // TestExecQuietSuppressesExpiryWarning exercises the full Exec path to verify -// that --quiet suppresses the expiration warning end-to-end. (--json also sets -// Quiet=true at run.go:204, but config doesn't accept --json; the unit test -// TestCheckTokenExpirationWarningSuppression covers the Quiet flag directly.) +// that --quiet suppresses the expiration warning end-to-end. func TestExecQuietSuppressesExpiryWarning(t *testing.T) { var stdout bytes.Buffer @@ -179,16 +176,22 @@ func TestExecConfigShowsExpiryWarning(t *testing.T) { } } +// TestExecJSONLeavesStdoutCleanAndWritesWarningToStderr verifies that in +// --json mode, the expiry warning is written to stderr (not stdout) so it +// does not corrupt JSON output. Because the config command does not register +// --json as a flag, we simulate the effect by pre-setting Flags.JSON (which +// is what Exec does when it sees --json in the args). func TestExecJSONLeavesStdoutCleanAndWritesWarningToStderr(t *testing.T) { var ( stdout bytes.Buffer stderr bytes.Buffer ) - args := testutil.SplitArgs("config -l --json") + args := testutil.SplitArgs("config -l") app.Init = func(_ []string, _ io.Reader) (*global.Data, error) { data := testutil.MockGlobalData(args, &stdout) data.ErrOutput = &stderr + data.Flags.JSON = true data.Config.Auth.Tokens["user"].APITokenExpiresAt = time.Now().Add(3 * 24 * time.Hour).Format(time.RFC3339) return data, nil } @@ -198,16 +201,10 @@ func TestExecJSONLeavesStdoutCleanAndWritesWarningToStderr(t *testing.T) { } if strings.Contains(stdout.String(), "expires in") { - t.Fatalf("expected stdout to contain only JSON, got: %s", stdout.String()) - } - - var decoded any - if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { - t.Fatalf("expected stdout to remain valid JSON, got %q: %v", stdout.String(), err) + t.Errorf("expected stdout free of expiry warning, got: %s", stdout.String()) } - if !strings.Contains(stderr.String(), "expires in") { - t.Fatalf("expected stderr warning, got: %s", stderr.String()) + t.Errorf("expected expiry warning on stderr, got: %s", stderr.String()) } } diff --git a/pkg/global/global.go b/pkg/global/global.go index 00a94e71a..f8fd67759 100644 --- a/pkg/global/global.go +++ b/pkg/global/global.go @@ -205,6 +205,9 @@ type Flags struct { AutoYes bool // Debug enables the CLI's debug mode. Debug bool + // JSON indicates --json output was requested. Detected automatically by + // Exec. Unlike Quiet, JSON mode does not suppress stderr warnings. + JSON bool // NonInteractive auto-resolves all prompts. NonInteractive bool // Profile indicates the profile to use (consequently the 'token' used).