From 581ee1480e0db4773a7a90a52a06e764e93eb653 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 15 Jul 2026 11:21:09 -0700 Subject: [PATCH 1/2] fix: treat empty on-disk JSON state files as empty state, not a parse error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Truncated writes to the CLI's JSON state files (credentials.json, ~/.slack/config.json, project .slack/config.json, .slack/apps.json, .slack/apps.dev.json) can leave a file at zero bytes on disk when a prior process is interrupted between afero.WriteFile's O_TRUNC and the actual write. Once that happens, every subsequent CLI invocation reads the same 0-byte file, hits "unexpected end of JSON input" in the read helper, and raises ErrUnableToParseJSON. The June 2026 monthly report showed 1,317,610 unable_to_parse_json events from just 38 users (a hot loop concentrated in a small cohort running `platform run` and `api` in tight automation loops), which matches this failure mode: a file that never repairs itself keeps re-triggering on every invocation. Add a shared goutils.IsEmptyJSON helper and use it in the five hot readers (auth credentials, system config, project config, deployed apps, local dev apps). When the file exists but is empty or whitespace-only, return the same zero-value state the "file does not exist" branch already returns — the app_client readers also re-serialise a valid `{}` so the file self-heals on the next successful write. Genuinely malformed JSON still returns ErrUnableToParseJSON as before. Test coverage: added unit tests covering zero-byte and whitespace-only inputs for each of the five readers, plus a unit test for the IsEmptyJSON helper itself. Existing "broken JSON" tests are unchanged. --- internal/app/app_client.go | 30 +++++++++++++++++++++ internal/app/app_client_test.go | 47 +++++++++++++++++++++++++++++++++ internal/auth/auth.go | 10 +++++++ internal/auth/auth_test.go | 24 +++++++++++++++++ internal/config/project.go | 12 +++++++++ internal/config/project_test.go | 32 ++++++++++++++++++++++ internal/config/system.go | 12 +++++++++ internal/config/system_test.go | 31 ++++++++++++++++++++++ internal/goutils/json.go | 11 ++++++++ internal/goutils/json_test.go | 20 ++++++++++++++ 10 files changed, 229 insertions(+) diff --git a/internal/app/app_client.go b/internal/app/app_client.go index aecaf513..17b6697a 100644 --- a/internal/app/app_client.go +++ b/internal/app/app_client.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/slackapi/slack-cli/internal/config" + "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" @@ -297,6 +298,24 @@ func (ac *AppClient) readDeployedApps() error { return err } + // Treat an empty (or whitespace-only) file as "no apps saved yet" rather + // than a parse error. This state occurs when a prior write was truncated + // but not completed (e.g. a process was interrupted between afero.WriteFile's + // O_TRUNC and the actual write). Without this guard, every subsequent CLI + // invocation logs ErrUnableToParseJSON on the same file — see the June 2026 + // monthly report showing a hot loop of unable_to_parse_json events from a + // small user cohort. + if goutils.IsEmptyJSON(f) { + ac.apps = types.Apps{ + DeployedApps: map[string]types.App{}, + LocalApps: map[string]types.App{}, + } + if err = ac.saveDeployedApps(); err != nil { + return err + } + return nil + } + if err = json.Unmarshal(f, &ac.apps); err != nil { return slackerror.New(slackerror.ErrUnableToParseJSON). WithMessage("Failed to parse contents of deployed apps file"). @@ -371,6 +390,17 @@ func (ac *AppClient) readLocalApps() error { return err } + // Treat an empty (or whitespace-only) file as "no local apps saved yet" + // rather than a parse error. See readDeployedApps for the corresponding + // note on why this state occurs. + if goutils.IsEmptyJSON(f) { + ac.apps.LocalApps = map[string]types.App{} + if err = ac.saveLocalApps(); err != nil { + return err + } + return nil + } + err = json.Unmarshal(f, &ac.apps.LocalApps) if err != nil { return slackerror.New(slackerror.ErrUnableToParseJSON). diff --git a/internal/app/app_client_test.go b/internal/app/app_client_test.go index 56778e70..f3151732 100644 --- a/internal/app/app_client_test.go +++ b/internal/app/app_client_test.go @@ -165,6 +165,30 @@ func Test_AppClient_ReadDeployedApps_BrokenAppsJSON(t *testing.T) { assert.Equal(t, err.(*slackerror.Error).Code, slackerror.ErrUnableToParseJSON) } +// Test that a zero-byte or whitespace-only apps.json (e.g. from a truncated +// write after a prior process interrupt) is treated as empty state, not a +// parse error. This prevents a hot loop of unable_to_parse_json events on +// every CLI invocation while the file remains empty on disk. +func Test_AppClient_ReadDeployedApps_EmptyAppsJSON(t *testing.T) { + tests := map[string]string{ + "zero-byte file": "", + "whitespace-only file": " \n\t\n", + } + for name, contents := range tests { + t.Run(name, func(t *testing.T) { + ac, _, _, pathToAppsJSON, _, teardown := setup(t) + defer teardown(t) + err := afero.WriteFile(ac.fs, pathToAppsJSON, []byte(contents), 0600) + require.NoError(t, err) + err = ac.readDeployedApps() + require.NoError(t, err) + // The empty file should be rewritten as a valid empty apps object. + f, _ := afero.ReadFile(ac.fs, pathToAppsJSON) + assert.Equal(t, "{}", string(f)) + }) + } +} + // Test that pre-existing dev app details get read from apps.dev.json func Test_AppClient_ReadDevApps_ExistingAppsJSON(t *testing.T) { ac, _, _, _, pathToDevAppsJSON, teardown := setup(t) @@ -207,6 +231,29 @@ func Test_AppClient_ReadDevApps_BrokenAppsJSON(t *testing.T) { assert.Equal(t, err.(*slackerror.Error).Code, slackerror.ErrUnableToParseJSON) } +// Test that a zero-byte or whitespace-only apps.dev.json (e.g. from a +// truncated write after a prior process interrupt) is treated as empty +// state, not a parse error. See Test_AppClient_ReadDeployedApps_EmptyAppsJSON. +func Test_AppClient_ReadDevApps_EmptyAppsJSON(t *testing.T) { + tests := map[string]string{ + "zero-byte file": "", + "whitespace-only file": " \n\t\n", + } + for name, contents := range tests { + t.Run(name, func(t *testing.T) { + ac, _, _, _, pathToDevAppsJSON, teardown := setup(t) + defer teardown(t) + err := afero.WriteFile(ac.fs, pathToDevAppsJSON, []byte(contents), 0600) + require.NoError(t, err) + err = ac.readLocalApps() + require.NoError(t, err) + // The empty file should be rewritten as a valid empty apps object. + f, _ := afero.ReadFile(ac.fs, pathToDevAppsJSON) + assert.Equal(t, "{}", string(f)) + }) + } +} + // Test that a team flag config defines the default app name in an empty AppClient func Test_AppClient_getDeployedAppTeamDomain_ViaCLIFlag(t *testing.T) { ac, _, _, _, _, teardown := setup(t) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index b3dcd8c6..624b6247 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -216,6 +216,16 @@ func (c *Client) auths(ctx context.Context) (map[string]types.SlackAuth, error) return auths, err } + // Treat an empty (or whitespace-only) credentials file as "no credentials + // stored" rather than a parse error. This state occurs when a prior write + // was truncated but not completed (e.g. a process was interrupted between + // afero.WriteFile's O_TRUNC and the actual write). Without this guard, + // every subsequent CLI invocation reads the same 0-byte file and logs + // ErrUnableToParseJSON in a hot loop. + if goutils.IsEmptyJSON(raw) { + return types.AuthByTeamID{}, nil + } + err = json.Unmarshal(raw, &auths) if err != nil { return auths, slackerror.New(slackerror.ErrUnableToParseJSON). diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 3d7cc5b7..e0a4c70c 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -222,6 +222,30 @@ func Test_AuthGettersAndSetters(t *testing.T) { require.Error(t, err) assert.Equal(t, err.(*slackerror.Error).Code, slackerror.ErrUnableToParseJSON) }) + + // A zero-byte or whitespace-only credentials.json (e.g. from a truncated + // write after a prior process interrupt) should be treated as empty state + // rather than a parse error, otherwise every CLI invocation would log + // ErrUnableToParseJSON in a hot loop. + t.Run("empty credentials file returns empty auth state, not a parse error", func(t *testing.T) { + cases := map[string]string{ + "zero-byte file": "", + "whitespace-only file": " \n\t\n", + } + for name, contents := range cases { + t.Run(name, func(t *testing.T) { + ctx, authClient := setup(t) + dir, err := authClient.config.SystemConfig.SlackConfigDir(ctx) + require.NoError(t, err) + path := filepath.Join(dir, credentialsFileName) + err = afero.WriteFile(authClient.fs, path, []byte(contents), 0o600) + require.NoError(t, err) + got, err := authClient.auths(ctx) + require.NoError(t, err) + require.Empty(t, got) + }) + } + }) } func Test_AuthsRotation(t *testing.T) { diff --git a/internal/config/project.go b/internal/config/project.go index c2c03be1..26381e33 100644 --- a/internal/config/project.go +++ b/internal/config/project.go @@ -25,6 +25,7 @@ import ( "github.com/google/uuid" "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/cache" + "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" @@ -254,6 +255,17 @@ func ReadProjectConfigFile(ctx context.Context, fs afero.Fs, os types.Os) (Proje return projectConfig, err } + // Treat an empty (or whitespace-only) project config as "no project config + // saved yet" rather than a parse error. This state occurs when a prior + // write was truncated but not completed (e.g. a process was interrupted + // between afero.WriteFile's O_TRUNC and the actual write). Without this + // guard, every subsequent CLI invocation reads the same 0-byte file and + // logs ErrUnableToParseJSON in a hot loop. + if goutils.IsEmptyJSON(projectConfigFileBytes) { + projectConfig.Surveys = map[string]SurveyConfig{} + return projectConfig, nil + } + err = json.Unmarshal(projectConfigFileBytes, &projectConfig) if err != nil { return projectConfig, slackerror.New(slackerror.ErrUnableToParseJSON). diff --git a/internal/config/project_test.go b/internal/config/project_test.go index 7c3f1573..1ffa36d3 100644 --- a/internal/config/project_test.go +++ b/internal/config/project_test.go @@ -352,6 +352,38 @@ func Test_ProjectConfig_ReadProjectConfigFile(t *testing.T) { assert.Equal(t, slackerror.ToSlackError(err).Code, slackerror.ErrUnableToParseJSON) assert.Equal(t, slackerror.ToSlackError(err).Message, "Failed to parse contents of project-level config file") }) + + // A zero-byte or whitespace-only .slack/config.json (e.g. from a truncated + // write after a prior process interrupt) should be treated as empty state + // rather than a parse error, otherwise every CLI invocation would log + // ErrUnableToParseJSON in a hot loop. + t.Run("empty project config file returns default config, not a parse error", func(t *testing.T) { + cases := map[string]string{ + "zero-byte file": "", + "whitespace-only file": " \n\t\n", + } + for name, contents := range cases { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + + os.AddDefaultMocks() + addProjectMocks(t, fs) + + projectDirPath, err := GetProjectDirPath(fs, os) + require.NoError(t, err) + projectConfigFilePath := GetProjectConfigJSONFilePath(projectDirPath) + + err = afero.WriteFile(fs, projectConfigFilePath, []byte(contents), 0600) + require.NoError(t, err) + + got, err := ReadProjectConfigFile(ctx, fs, os) + require.NoError(t, err) + require.NotNil(t, got.Surveys) + }) + } + }) } func Test_ProjectConfig_WriteProjectConfigFile(t *testing.T) { diff --git a/internal/config/system.go b/internal/config/system.go index b1ae969c..28945b53 100644 --- a/internal/config/system.go +++ b/internal/config/system.go @@ -26,6 +26,7 @@ import ( "github.com/google/uuid" "github.com/opentracing/opentracing-go" + "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" @@ -127,6 +128,17 @@ func (c *SystemConfig) UserConfig(ctx context.Context) (*SystemConfig, error) { return &config, err } + // Treat an empty (or whitespace-only) config file as "no user config saved + // yet" rather than a parse error. This state occurs when a prior write was + // truncated but not completed (e.g. a process was interrupted between + // afero.WriteFile's O_TRUNC and the actual write). Without this guard, + // every subsequent CLI invocation reads the same 0-byte file and logs + // ErrUnableToParseJSON in a hot loop. + if goutils.IsEmptyJSON(configFileBytes) { + config.Surveys = map[string]SurveyConfig{} + return &config, nil + } + err = json.Unmarshal(configFileBytes, &config) if err != nil { return &config, slackerror.New(slackerror.ErrUnableToParseJSON). diff --git a/internal/config/system_test.go b/internal/config/system_test.go index 34f85b7a..835953a2 100644 --- a/internal/config/system_test.go +++ b/internal/config/system_test.go @@ -107,6 +107,37 @@ func Test_SystemConfig_UserConfig(t *testing.T) { assert.Equal(t, slackerror.ToSlackError(err).Code, slackerror.ErrUnableToParseJSON) assert.Equal(t, slackerror.ToSlackError(err).Message, "Failed to parse contents of system-level config file") }) + + // A zero-byte or whitespace-only ~/.slack/config.json (e.g. from a truncated + // write after a prior process interrupt) should be treated as empty state + // rather than a parse error, otherwise every CLI invocation would log + // ErrUnableToParseJSON in a hot loop. + t.Run("empty configuration file returns default config, not a parse error", func(t *testing.T) { + cases := map[string]string{ + "zero-byte file": "", + "whitespace-only file": " \n\t\n", + } + for name, contents := range cases { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + + os.AddDefaultMocks() + + configFilePath := filepath.Join(slackdeps.MockHomeDirectory, configFolderName, configFileName) + err := afero.WriteFile(fs, configFilePath, []byte(contents), 0600) + require.NoError(t, err) + + systemConfig := NewSystemConfig(fs, os) + got, err := systemConfig.UserConfig(ctx) + + require.NoError(t, err) + require.NotNil(t, got) + require.NotNil(t, got.Surveys) + }) + } + }) } func Test_Config_SlackConfigDir(t *testing.T) { diff --git a/internal/goutils/json.go b/internal/goutils/json.go index 59b5a95a..94aeb2af 100644 --- a/internal/goutils/json.go +++ b/internal/goutils/json.go @@ -84,3 +84,14 @@ func JSONUnmarshal(data []byte, v interface{}) error { } return nil } + +// IsEmptyJSON returns true when the provided bytes are empty or contain only +// whitespace. This matches the state left behind when a JSON state file was +// truncated but not yet rewritten, e.g. after a process was interrupted between +// afero.WriteFile's O_TRUNC and the actual write. Callers that read on-disk +// config/auth/app state should treat this case as "empty state" rather than a +// parse error, otherwise every subsequent CLI invocation logs +// ErrUnableToParseJSON until the user manually clears the file. +func IsEmptyJSON(data []byte) bool { + return len(bytes.TrimSpace(data)) == 0 +} diff --git a/internal/goutils/json_test.go b/internal/goutils/json_test.go index 62640fd3..b8f91622 100644 --- a/internal/goutils/json_test.go +++ b/internal/goutils/json_test.go @@ -159,6 +159,26 @@ func Test_JSONMarshalUnescapedIndent(t *testing.T) { } } +func Test_IsEmptyJSON(t *testing.T) { + for name, tc := range map[string]struct { + data string + expected bool + }{ + "nil bytes": {data: "", expected: true}, + "empty string": {data: "", expected: true}, + "single space": {data: " ", expected: true}, + "whitespace mix": {data: " \n\t\n", expected: true}, + "empty JSON object": {data: "{}", expected: false}, + "whitespace around JSON": {data: " {} ", expected: false}, + "JSON payload": {data: `{"one":"1"}`, expected: false}, + "invalid JSON is nonzero": {data: "{", expected: false}, + } { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, IsEmptyJSON([]byte(tc.data))) + }) + } +} + func Test_UnmarshalJSON(t *testing.T) { type testConfig struct { One string `json:"one,omitempty"` From 77804be6eb561383deb13ae52d243b42cf7ef48b Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 15 Jul 2026 14:26:34 -0700 Subject: [PATCH 2/2] chore: drop internal telemetry reference from comment --- internal/app/app_client.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/app/app_client.go b/internal/app/app_client.go index 17b6697a..cc44830f 100644 --- a/internal/app/app_client.go +++ b/internal/app/app_client.go @@ -302,9 +302,7 @@ func (ac *AppClient) readDeployedApps() error { // than a parse error. This state occurs when a prior write was truncated // but not completed (e.g. a process was interrupted between afero.WriteFile's // O_TRUNC and the actual write). Without this guard, every subsequent CLI - // invocation logs ErrUnableToParseJSON on the same file — see the June 2026 - // monthly report showing a hot loop of unable_to_parse_json events from a - // small user cohort. + // invocation logs ErrUnableToParseJSON on the same file. if goutils.IsEmptyJSON(f) { ac.apps = types.Apps{ DeployedApps: map[string]types.App{},