diff --git a/install.sh b/install.sh index 9cb2e83..4612891 100755 --- a/install.sh +++ b/install.sh @@ -130,27 +130,27 @@ fi # --------------------------------------------------------------------------- # Verify checksum (sha256) # --------------------------------------------------------------------------- -if curl -fsSL "$checksum_url" -o "$tmp/checksums.txt" 2>/dev/null; then - expected=$(grep " ${asset}\$" "$tmp/checksums.txt" | awk '{print $1}') - if [ -n "$expected" ]; then - if has sha256sum; then - actual=$(sha256sum "$tmp/$asset" | awk '{print $1}') - elif has shasum; then - actual=$(shasum -a 256 "$tmp/$asset" | awk '{print $1}') - else - warn "no sha256 tool found — skipping checksum verification" - actual="" - fi - if [ -n "$actual" ] && [ "$expected" != "$actual" ]; then - err "checksum mismatch (expected=${expected} got=${actual})" - fi - [ -n "$actual" ] && step "Verified checksum" - else - warn "${asset} not listed in checksums.txt — skipping verification" - fi +if ! curl -fsSL "$checksum_url" -o "$tmp/checksums.txt" 2>/dev/null; then + err "failed to download checksums.txt; refusing to install without checksum verification" +fi + +expected=$(grep " ${asset}\$" "$tmp/checksums.txt" | awk '{print $1}') +if [ -z "$expected" ]; then + err "${asset} not listed in checksums.txt; refusing to install without checksum verification" +fi + +if has sha256sum; then + actual=$(sha256sum "$tmp/$asset" | awk '{print $1}') +elif has shasum; then + actual=$(shasum -a 256 "$tmp/$asset" | awk '{print $1}') else - warn "could not fetch checksums.txt — skipping verification" + err "no sha256 tool found; install sha256sum or shasum and retry" +fi + +if [ "$expected" != "$actual" ]; then + err "checksum mismatch (expected=${expected} got=${actual})" fi +step "Verified checksum" # --------------------------------------------------------------------------- # Extract and install diff --git a/internal/cmd/api.go b/internal/cmd/api.go index 0936d54..04a2313 100644 --- a/internal/cmd/api.go +++ b/internal/cmd/api.go @@ -10,6 +10,8 @@ import ( "net/url" "os" "strings" + + "github.com/chatwoot/cli/internal/output" ) type ApiCmd struct { @@ -131,8 +133,9 @@ func printAPIResponse(w io.Writer, body []byte) { return } - _, _ = w.Write(body) - if body[len(body)-1] != '\n' { + safeBody := output.SanitizeText(string(body)) + _, _ = io.WriteString(w, safeBody) + if !strings.HasSuffix(safeBody, "\n") { _, _ = fmt.Fprintln(w) } } diff --git a/internal/cmd/api_test.go b/internal/cmd/api_test.go index 1028c42..9208a9e 100644 --- a/internal/cmd/api_test.go +++ b/internal/cmd/api_test.go @@ -123,6 +123,22 @@ func TestApiCmdSendsMethodBodyAndHeaders(t *testing.T) { } } +func TestPrintAPIResponseSanitizesNonJSONBody(t *testing.T) { + var out bytes.Buffer + + printAPIResponse(&out, []byte("hello\x1b]52;c;Zm9v\aworld\x1b[31m")) + + got := out.String() + for _, disallowed := range []string{"\x1b", "\a", "]52", "[31m"} { + if strings.Contains(got, disallowed) { + t.Fatalf("non-JSON API response contained terminal control %q: %q", disallowed, got) + } + } + if !strings.Contains(got, "helloworld") { + t.Fatalf("non-JSON API response stripped printable content: %q", got) + } +} + func TestNormalizeAPIPathRejectsAbsoluteURLs(t *testing.T) { if _, _, err := normalizeAPIPath("https://example.com/api/v1/profile", false); err == nil { t.Fatal("expected absolute URL error") diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 84834fa..5c10a6b 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -74,10 +74,14 @@ func (c *AuthLoginCmd) Run(app *App) error { return fmt.Errorf("failed to save config: %w", err) } - fmt.Printf("Logged in as %s (%s)\n", profile.Name, profile.Email) + fmt.Print(loginSuccessMessage(profile.Name, profile.Email)) return nil } +func loginSuccessMessage(name, email string) string { + return fmt.Sprintf("Logged in as %s (%s)\n", output.SanitizeText(name), output.SanitizeText(email)) +} + func readAPIKey(reader *bufio.Reader) (string, error) { fmt.Print("API Key: ") @@ -111,10 +115,8 @@ func (c *AuthLogoutCmd) Run(app *App) error { return err } - if cfg != nil { - if err := config.DeleteAPIKey(cfg); err != nil { - return err - } + if err := config.DeleteAPIKey(cfg); err != nil { + return err } if err := os.Remove(path); err != nil { diff --git a/internal/cmd/auth_test.go b/internal/cmd/auth_test.go index 3806950..572ec6d 100644 --- a/internal/cmd/auth_test.go +++ b/internal/cmd/auth_test.go @@ -2,6 +2,7 @@ package cmd import ( "bytes" + "errors" "net/http" "net/http/httptest" "strings" @@ -112,6 +113,18 @@ func TestAuthStatusReportsIdentityAndCredentialSource(t *testing.T) { } } +func TestLoginSuccessMessageStripsTerminalControls(t *testing.T) { + got := loginSuccessMessage("Eve\x1b]52;c;Zm9v\a", "eve@example.com\x1b[31m") + for _, disallowed := range []string{"\x1b", "\a", "]52", "[31m"} { + if strings.Contains(got, disallowed) { + t.Fatalf("login success message contained terminal control %q: %q", disallowed, got) + } + } + if !strings.Contains(got, "Logged in as Eve (eve@example.com)") { + t.Fatalf("login success message stripped printable content: %q", got) + } +} + func TestMeAndWhoamiAliasAuthStatus(t *testing.T) { profile := `{ "id": 7, @@ -152,6 +165,27 @@ func TestAuthStatusNotLoggedIn(t *testing.T) { } } +func TestAuthLogoutRemovesKeyringTokenWithoutConfig(t *testing.T) { + keyring.MockInit() + if err := keyring.DeleteAll("chatwoot-cli"); err != nil { + t.Fatalf("keyring.DeleteAll: %v", err) + } + t.Setenv("HOME", t.TempDir()) + t.Setenv(config.APIKeyEnv, "") + + if err := keyring.Set("chatwoot-cli", "api-key", "stale-token"); err != nil { + t.Fatalf("keyring.Set: %v", err) + } + + if err := (&AuthLogoutCmd{}).Run(&App{}); err != nil { + t.Fatalf("Run: %v", err) + } + + if _, err := keyring.Get("chatwoot-cli", "api-key"); !errors.Is(err, keyring.ErrNotFound) { + t.Fatalf("expected logout to delete stale keyring token, err = %v", err) + } +} + func TestAuthStatusSelfHealsCachedUserID(t *testing.T) { profile := `{ "id": 99, diff --git a/internal/config/credentials.go b/internal/config/credentials.go index 714ff65..eb9cd0c 100644 --- a/internal/config/credentials.go +++ b/internal/config/credentials.go @@ -1,6 +1,7 @@ package config import ( + "encoding/json" "errors" "fmt" "os" @@ -10,8 +11,9 @@ import ( ) const ( - APIKeyEnv = "CHATWOOT_API_KEY" - keyringService = "chatwoot-cli" + APIKeyEnv = "CHATWOOT_API_KEY" + keyringService = "chatwoot-cli" + apiKeyKeyringEntry = "api-key" ) type CredentialSource string @@ -24,6 +26,12 @@ const ( var ErrAPIKeyNotFound = errors.New("api key not found") +type savedCredential struct { + BaseURL string `json:"base_url"` + AccountID int `json:"account_id"` + APIKey string `json:"api_key"` +} + // ResolveAPIKey implements the auth flow for the CLI. YAML config intentionally // stores only non-secrets, and plaintext api_key values from older configs are // ignored. CHATWOOT_API_KEY wins for CI, coding agents, and temporary overrides; @@ -37,14 +45,33 @@ func ResolveAPIKey(cfg *Config) (string, CredentialSource, error) { return "", CredentialSourceMissing, missingAPIKeyError() } - apiKey, err := keyring.Get(keyringService, credentialKey(cfg)) + stored, err := keyring.Get(keyringService, apiKeyKeyringEntry) if err == nil { + apiKey, err := parseSavedCredential(stored, cfg) + if err != nil { + return "", CredentialSourceMissing, err + } return apiKey, CredentialSourceKeyring, nil } - if errors.Is(err, keyring.ErrNotFound) { - return "", CredentialSourceMissing, missingAPIKeyError() + if !errors.Is(err, keyring.ErrNotFound) { + return "", CredentialSourceMissing, fmt.Errorf("failed to read API key from keyring: %w", err) } - return "", CredentialSourceMissing, fmt.Errorf("failed to read API key from keyring: %w", err) + + // TODO(v1): remove this legacy key migration after users have had a release + // cycle to move from URL/account-scoped keyring entries to api-key. + apiKey, err := keyring.Get(keyringService, legacyCredentialKey(cfg)) + if err == nil { + if err := saveAPIKeyToKeyring(cfg, apiKey); err != nil { + return "", CredentialSourceMissing, fmt.Errorf("failed to migrate API key in keyring: %w", err) + } + _ = keyring.Delete(keyringService, legacyCredentialKey(cfg)) + return apiKey, CredentialSourceKeyring, nil + } + if !errors.Is(err, keyring.ErrNotFound) { + return "", CredentialSourceMissing, fmt.Errorf("failed to read legacy API key from keyring: %w", err) + } + + return "", CredentialSourceMissing, missingAPIKeyError() } func SaveAPIKey(cfg *Config, apiKey string) error { @@ -55,30 +82,58 @@ func SaveAPIKey(cfg *Config, apiKey string) error { if cfg == nil || !cfg.IsValid() { return fmt.Errorf("valid config is required to save API key") } - if err := keyring.Set(keyringService, credentialKey(cfg), apiKey); err != nil { + if err := saveAPIKeyToKeyring(cfg, apiKey); err != nil { return fmt.Errorf("failed to save API key to keyring: %w", err) } return nil } -func DeleteAPIKey(cfg *Config) error { - if cfg == nil || !cfg.IsValid() { - return nil +func saveAPIKeyToKeyring(cfg *Config, apiKey string) error { + credential := savedCredential{ + BaseURL: normalizeBaseURL(cfg.BaseURL), + AccountID: cfg.AccountID, + APIKey: apiKey, } - if err := keyring.Delete(keyringService, credentialKey(cfg)); err != nil { - if errors.Is(err, keyring.ErrNotFound) { - return nil - } - return fmt.Errorf("failed to delete API key from keyring: %w", err) + + data, err := json.Marshal(credential) + if err != nil { + return err } - return nil + return keyring.Set(keyringService, apiKeyKeyringEntry, string(data)) +} + +func parseSavedCredential(stored string, cfg *Config) (string, error) { + var credential savedCredential + if err := json.Unmarshal([]byte(stored), &credential); err != nil { + return "", credentialScopeMismatchError() + } + if credential.APIKey == "" || + credential.BaseURL != normalizeBaseURL(cfg.BaseURL) || + credential.AccountID != cfg.AccountID { + return "", credentialScopeMismatchError() + } + return credential.APIKey, nil +} + +// DeleteAPIKey removes every credential saved by this CLI service. This avoids +// leaving stale keyring entries behind when config.yaml was edited or removed. +func DeleteAPIKey(_ *Config) error { + err := keyring.DeleteAll(keyringService) + if err == nil || errors.Is(err, keyring.ErrNotFound) { + return nil + } + return fmt.Errorf("failed to delete API keys from keyring: %w", err) } func missingAPIKeyError() error { return fmt.Errorf("%w; run 'chatwoot auth login' or set %s", ErrAPIKeyNotFound, APIKeyEnv) } -func credentialKey(cfg *Config) string { +func credentialScopeMismatchError() error { + return fmt.Errorf("%w; saved keyring credential does not match configured instance; run 'chatwoot auth login' for this base URL and account", ErrAPIKeyNotFound) +} + +func legacyCredentialKey(cfg *Config) string { return fmt.Sprintf("%s/accounts/%d", normalizeBaseURL(cfg.BaseURL), cfg.AccountID) } diff --git a/internal/config/credentials_test.go b/internal/config/credentials_test.go index 0294d97..31bb463 100644 --- a/internal/config/credentials_test.go +++ b/internal/config/credentials_test.go @@ -1,6 +1,7 @@ package config import ( + "encoding/json" "errors" "testing" @@ -55,6 +56,87 @@ func TestResolveAPIKeyFromKeyring(t *testing.T) { if source != CredentialSourceKeyring { t.Fatalf("source = %q, want %q", source, CredentialSourceKeyring) } + + stored, err := keyring.Get(keyringService, apiKeyKeyringEntry) + if err != nil { + t.Fatalf("keyring.Get() error = %v", err) + } + var credential savedCredential + if err := json.Unmarshal([]byte(stored), &credential); err != nil { + t.Fatalf("stored credential is not JSON: %v", err) + } + if credential.BaseURL != "https://app.chatwoot.com" || credential.AccountID != 124 || credential.APIKey != "keyring-token" { + t.Fatalf("stored credential = %#v, want scoped api key", credential) + } +} + +func TestResolveAPIKeyRejectsMismatchedStableCredential(t *testing.T) { + initMockKeyring(t) + cfg := &Config{BaseURL: "https://app.chatwoot.com", AccountID: 124} + + if err := SaveAPIKey(cfg, "keyring-token"); err != nil { + t.Fatalf("SaveAPIKey() error = %v", err) + } + + _, source, err := ResolveAPIKey(&Config{BaseURL: "https://evil.example", AccountID: 124}) + if !errors.Is(err, ErrAPIKeyNotFound) { + t.Fatalf("ResolveAPIKey() error = %v, want ErrAPIKeyNotFound", err) + } + if source != CredentialSourceMissing { + t.Fatalf("source = %q, want %q", source, CredentialSourceMissing) + } +} + +func TestResolveAPIKeyRejectsRawStableCredential(t *testing.T) { + initMockKeyring(t) + cfg := &Config{BaseURL: "https://app.chatwoot.com", AccountID: 124} + + if err := keyring.Set(keyringService, apiKeyKeyringEntry, "raw-token"); err != nil { + t.Fatalf("keyring.Set() error = %v", err) + } + + _, source, err := ResolveAPIKey(cfg) + if !errors.Is(err, ErrAPIKeyNotFound) { + t.Fatalf("ResolveAPIKey() error = %v, want ErrAPIKeyNotFound", err) + } + if source != CredentialSourceMissing { + t.Fatalf("source = %q, want %q", source, CredentialSourceMissing) + } +} + +func TestResolveAPIKeyMigratesLegacyKeyringToken(t *testing.T) { + initMockKeyring(t) + cfg := &Config{BaseURL: "https://app.chatwoot.com/", AccountID: 127} + + if err := keyring.Set(keyringService, legacyCredentialKey(cfg), "legacy-token"); err != nil { + t.Fatalf("keyring.Set() error = %v", err) + } + + apiKey, source, err := ResolveAPIKey(&Config{BaseURL: "https://app.chatwoot.com", AccountID: 127}) + if err != nil { + t.Fatalf("ResolveAPIKey() error = %v", err) + } + if apiKey != "legacy-token" { + t.Fatalf("apiKey = %q, want legacy-token", apiKey) + } + if source != CredentialSourceKeyring { + t.Fatalf("source = %q, want %q", source, CredentialSourceKeyring) + } + + stable, err := keyring.Get(keyringService, apiKeyKeyringEntry) + if err != nil { + t.Fatalf("stable keyring token missing after migration: %v", err) + } + var credential savedCredential + if err := json.Unmarshal([]byte(stable), &credential); err != nil { + t.Fatalf("stable keyring token is not JSON after migration: %v", err) + } + if credential.BaseURL != "https://app.chatwoot.com" || credential.AccountID != 127 || credential.APIKey != "legacy-token" { + t.Fatalf("stable credential = %#v, want migrated scoped credential", credential) + } + if _, err := keyring.Get(keyringService, legacyCredentialKey(cfg)); !errors.Is(err, keyring.ErrNotFound) { + t.Fatalf("legacy token still present after migration, err = %v", err) + } } func TestResolveAPIKeyMissing(t *testing.T) { @@ -86,3 +168,28 @@ func TestDeleteAPIKeyRemovesKeyringToken(t *testing.T) { t.Fatalf("ResolveAPIKey() error = %v, want ErrAPIKeyNotFound", err) } } + +func TestDeleteAPIKeyRemovesAllServiceEntries(t *testing.T) { + initMockKeyring(t) + cfg := &Config{BaseURL: "https://app.chatwoot.com", AccountID: 128} + + if err := keyring.Set(keyringService, apiKeyKeyringEntry, "stable-token"); err != nil { + t.Fatalf("keyring.Set(stable) error = %v", err) + } + if err := keyring.Set(keyringService, "stale-entry", "stale-token"); err != nil { + t.Fatalf("keyring.Set(stale) error = %v", err) + } + + if err := DeleteAPIKey(cfg); err != nil { + t.Fatalf("DeleteAPIKey() error = %v", err) + } + + for name, key := range map[string]string{ + "stable": apiKeyKeyringEntry, + "stale": "stale-entry", + } { + if _, err := keyring.Get(keyringService, key); !errors.Is(err, keyring.ErrNotFound) { + t.Fatalf("%s token still present after DeleteAPIKey, err = %v", name, err) + } + } +} diff --git a/internal/output/output.go b/internal/output/output.go index cf40752..cb709d7 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -39,7 +39,7 @@ func (p *Printer) PrintTable(headers []string, rows [][]string) { if p.Quiet { for _, row := range rows { if len(row) > 0 { - _, _ = fmt.Fprintln(p.Writer, sanitizeText(row[0])) + _, _ = fmt.Fprintln(p.Writer, SanitizeText(row[0])) } } return @@ -83,7 +83,7 @@ func (p *Printer) PrintDetail(pairs []KeyValue) { } for _, kv := range pairs { - _, _ = fmt.Fprintf(p.Writer, "%-*s %s\n", maxKey, kv.Key+":", sanitizeText(kv.Value)) + _, _ = fmt.Fprintf(p.Writer, "%-*s %s\n", maxKey, kv.Key+":", SanitizeText(kv.Value)) } } @@ -127,12 +127,14 @@ func (p *Printer) tableAsCSV(headers []string, rows [][]string) { func sanitizeTextRow(row []string) []string { out := make([]string, len(row)) for i, cell := range row { - out[i] = sanitizeText(cell) + out[i] = SanitizeText(cell) } return out } -func sanitizeText(s string) string { +// SanitizeText removes terminal control sequences and other non-printable +// control characters from text before it is written to a terminal. +func SanitizeText(s string) string { var b strings.Builder b.Grow(len(s)) diff --git a/internal/sdk/client.go b/internal/sdk/client.go index 97de9a1..d3b312b 100644 --- a/internal/sdk/client.go +++ b/internal/sdk/client.go @@ -9,6 +9,8 @@ import ( "os" "strings" "time" + + "github.com/chatwoot/cli/internal/output" ) type Client struct { @@ -102,7 +104,7 @@ func (c *Client) do(req *http.Request, v interface{}) error { if resp.StatusCode >= 400 { body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("API error %d: %s", resp.StatusCode, string(body)) + return fmt.Errorf("API error %d: %s", resp.StatusCode, output.SanitizeText(string(body))) } if v == nil { @@ -149,7 +151,7 @@ func (c *Client) doRaw(req *http.Request) (*RawResponse, error) { } if resp.StatusCode >= 400 { - return nil, fmt.Errorf("API error %d: %s", resp.StatusCode, string(body)) + return nil, fmt.Errorf("API error %d: %s", resp.StatusCode, output.SanitizeText(string(body))) } return &RawResponse{ @@ -163,14 +165,14 @@ func (c *Client) doRaw(req *http.Request) (*RawResponse, error) { func redactSensitiveJSON(body []byte) string { var value interface{} if err := json.Unmarshal(body, &value); err != nil { - return string(body) + return output.SanitizeText(string(body)) } redactSensitiveFields(value) redacted, err := json.Marshal(value) if err != nil { - return string(body) + return output.SanitizeText(string(body)) } return string(redacted) } @@ -193,12 +195,16 @@ func redactSensitiveFields(value interface{}) { } func isSensitiveField(key string) bool { - switch strings.ToLower(key) { - case "access_token", "api_access_token", "pubsub_token": + key = strings.ToLower(key) + switch key { + case "access_token", "api_access_token", "pubsub_token", "hmac_identifier": return true - default: - return false } + return strings.HasSuffix(key, "_token") || + strings.HasSuffix(key, "_secret") || + strings.HasSuffix(key, "_key") || + strings.HasPrefix(key, "hmac_") || + strings.Contains(key, "_hmac_") } func (c *Client) Get(path string, params url.Values, v interface{}) error { diff --git a/internal/sdk/client_test.go b/internal/sdk/client_test.go index cbd449e..b78a675 100644 --- a/internal/sdk/client_test.go +++ b/internal/sdk/client_test.go @@ -45,6 +45,7 @@ func TestVerboseResponseLoggingDoesNotLeakNestedSensitiveTokens(t *testing.T) { const ( accessSecret = "nested-access-secret" pubsubSecret = "nested-pubsub-secret" + hmacSecret = "nested-hmac-secret" ) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -54,7 +55,8 @@ func TestVerboseResponseLoggingDoesNotLeakNestedSensitiveTokens(t *testing.T) { "user": { "id": 7, "access_token": "`+accessSecret+`", - "pubsub_token": "`+pubsubSecret+`" + "pubsub_token": "`+pubsubSecret+`", + "hmac_identifier": "`+hmacSecret+`" } } }`) @@ -70,7 +72,86 @@ func TestVerboseResponseLoggingDoesNotLeakNestedSensitiveTokens(t *testing.T) { } }) - assertDoesNotContainSensitiveValues(t, stderr, accessSecret, pubsubSecret) + assertDoesNotContainSensitiveValues(t, stderr, accessSecret, pubsubSecret, hmacSecret) +} + +func TestVerboseResponseLoggingRedactsSecretLikeFields(t *testing.T) { + const ( + tokenSecret = "generic-token-secret" + apiSecret = "generic-api-secret" + signingKey = "generic-signing-key" + hmacValue = "generic-hmac-value" + ) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{ + "payload": { + "session_token": "`+tokenSecret+`", + "webhook_secret": "`+apiSecret+`", + "signing_key": "`+signingKey+`", + "hmac_digest": "`+hmacValue+`" + } + }`) + })) + t.Cleanup(server.Close) + + client := NewClient(server.URL, "api-key", 1, WithHTTPClient(server.Client()), WithVerbose(true)) + + var decoded map[string]any + stderr := captureStderr(t, func() { + if err := client.Get("/secrets", nil, &decoded); err != nil { + t.Fatalf("Get returned error: %v", err) + } + }) + + assertDoesNotContainSensitiveValues(t, stderr, tokenSecret, apiSecret, signingKey, hmacValue) +} + +func TestVerboseNonJSONResponseLoggingStripsTerminalControls(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain") + _, _ = io.WriteString(w, "hello\x1b]52;c;Zm9v\aworld\x1b[31m") + })) + t.Cleanup(server.Close) + + client := NewClient(server.URL, "api-key", 1, WithHTTPClient(server.Client()), WithVerbose(true)) + + var decoded map[string]any + stderr := captureStderr(t, func() { + if err := client.Get("/plain", nil, &decoded); err == nil { + t.Fatal("expected JSON decode error") + } + }) + + assertDoesNotContainSensitiveValues(t, stderr, "\x1b", "\a", "]52", "[31m") + if !strings.Contains(stderr, "helloworld") { + t.Fatalf("verbose output stripped printable content:\n%s", stderr) + } +} + +func TestAPIErrorBodyStripsTerminalControls(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "bad\x1b]52;c;Zm9v\aerror", http.StatusInternalServerError) + })) + t.Cleanup(server.Close) + + client := NewClient(server.URL, "api-key", 1, WithHTTPClient(server.Client())) + + var decoded map[string]any + err := client.Get("/boom", nil, &decoded) + if err == nil { + t.Fatal("expected API error") + } + got := err.Error() + for _, disallowed := range []string{"\x1b", "\a", "]52"} { + if strings.Contains(got, disallowed) { + t.Fatalf("API error contained terminal control %q: %q", disallowed, got) + } + } + if !strings.Contains(got, "baderror") { + t.Fatalf("API error stripped printable content: %q", got) + } } func captureStderr(t *testing.T, fn func()) string { diff --git a/scripts/smoke.sh b/scripts/smoke.sh deleted file mode 100755 index 6f9ec21..0000000 --- a/scripts/smoke.sh +++ /dev/null @@ -1,339 +0,0 @@ -#!/usr/bin/env bash -# smoke.sh — exercise the chatwoot CLI surface against a local Chatwoot. -# -# Refuses to run unless ~/.chatwoot/config.yaml's base_url is localhost. -# This is destructive: it changes status/assignee/labels/priority on a real -# conversation, and posts a public reply and a private note. Only run -# against a dev/local instance. -# -# Beyond exit-code checks, the writes are verified via state read-back so -# the script catches "API accepted but didn't apply" bug classes (e.g. the -# priority-null/none discrepancy we shipped against). - -set -uo pipefail - -# ---------------------------------------------------------------------------- -# Guards -# ---------------------------------------------------------------------------- - -config_file="${HOME}/.chatwoot/config.yaml" - -if [[ ! -f "$config_file" ]]; then - echo "smoke: no config at $config_file (run 'chatwoot auth login')" >&2 - exit 1 -fi - -base_url=$(sed -nE 's/^base_url:[[:space:]]*//p' "$config_file" | head -1 | tr -d '"' | tr -d "'") - -# Parse the URL safely: scheme must be http, no userinfo allowed, host must be -# a literal loopback address. Bash glob matches like `http://localhost:*` are -# unsafe because `http://localhost:3000@attacker.example` matches the pattern -# but Go's URL parser treats `attacker.example` as the host. -case "$base_url" in - http://*) url_rest="${base_url#http://}" ;; - *) - echo "smoke: refusing to run against non-http base_url: ${base_url}" >&2 - exit 1 - ;; -esac -case "$url_rest" in - *@*) - echo "smoke: refusing — base_url contains userinfo: ${base_url}" >&2 - exit 1 - ;; -esac -host_port="${url_rest%%/*}" -host="${host_port%%:*}" -case "$host" in - localhost|127.0.0.1|0.0.0.0) - ;; - *) - echo "smoke: refusing to run against non-localhost base_url: ${base_url}" >&2 - echo " update ~/.chatwoot/config.yaml or run 'chatwoot auth login' against localhost." >&2 - exit 1 - ;; -esac - -if ! command -v jq >/dev/null 2>&1; then - echo "smoke: jq is required" >&2 - exit 1 -fi - -# ---------------------------------------------------------------------------- -# Build the binary fresh -# ---------------------------------------------------------------------------- - -cd "$(dirname "$0")/.." || exit 1 -go build -o chatwoot ./cmd/chatwoot/ -BIN="$(pwd)/chatwoot" - -echo "smoke: localhost ok ($base_url) — running CLI surface" -echo - -# ---------------------------------------------------------------------------- -# Test harness -# ---------------------------------------------------------------------------- - -PASS=0 -FAIL=0 -FAILURES=() - -pass() { printf ' PASS %s\n' "$1"; PASS=$((PASS+1)); } -fail() { printf ' FAIL %s\n' "$1"; FAIL=$((FAIL+1)); FAILURES+=("$1"); } - -# expect -expect() { - local name="$1"; local pattern="$2"; shift 2 - local out rc=0 - out=$("$@" 2>&1) || rc=$? - if [[ $rc -ne 0 ]]; then - fail "$name (exit $rc)" - printf '%s\n' "$out" | sed 's/^/ /' - return - fi - if [[ -z "$pattern" ]] || printf '%s' "$out" | grep -Eq -- "$pattern"; then - pass "$name" - else - fail "$name (no match for /$pattern/)" - printf '%s\n' "$out" | sed 's/^/ /' - fi -} - -# expect_fail -expect_fail() { - local name="$1"; local pattern="$2"; shift 2 - local out rc=0 - out=$("$@" 2>&1) || rc=$? - if [[ $rc -eq 0 ]]; then - fail "$name (expected nonzero exit)" - return - fi - if printf '%s' "$out" | grep -Eq -- "$pattern"; then - pass "$name" - else - fail "$name (failed but message missed /$pattern/)" - printf '%s\n' "$out" | sed 's/^/ /' - fi -} - -# state_eq -# Reads `conv $conv_id -o json` and asserts a jq path == expected (string compare). -state_eq() { - local name="$1"; local expr="$2"; local expected="$3" - local got - got=$("$BIN" conv "$conv_id" -o json 2>/dev/null | jq -rc "$expr" 2>/dev/null) || got="" - if [[ "$got" == "$expected" ]]; then - pass "$name" - else - fail "$name (want=$expected got=$got)" - fi -} - -# ---------------------------------------------------------------------------- -# Plurals (list) -# ---------------------------------------------------------------------------- - -echo "## plurals" -expect "convs" "^ID|No conversations" "$BIN" convs --assignee all -expect "contacts" "^ID|No contacts" "$BIN" contacts -expect "inboxes" "^ID|No inboxes" "$BIN" inboxes -expect "agents" "^ID|No agents" "$BIN" agents -expect "labels" "^ID|No labels" "$BIN" labels -expect "teams" "^ID|No teams" "$BIN" teams -expect "me" "^ID:" "$BIN" me - -# JSON sanity: must parse. -expect "convs -o json (parse)" "" bash -c "'$BIN' convs --assignee all -o json | jq -e . >/dev/null" -expect "contacts -o json (parse)" "" bash -c "'$BIN' contacts -o json | jq -e . >/dev/null" -expect "agents -o json (parse)" "" bash -c "'$BIN' agents -o json | jq -e . >/dev/null" - -# Quiet mode: convs -q is the README's headline scripting affordance — IDs -# only, one per line, all numeric. -quiet_out=$("$BIN" convs --assignee all -q 2>&1) -if [[ -z "$quiet_out" ]]; then - pass "convs -q (no convs found)" -elif printf '%s' "$quiet_out" | awk 'NF && $0 !~ /^[0-9]+$/ { exit 1 }'; then - pass "convs -q (numeric ids only)" -else - fail "convs -q (non-numeric output: $(printf '%s' "$quiet_out" | head -1))" -fi - -# Aliases. -expect "conversations (plural alias)" "^ID|No conversations" "$BIN" conversations --assignee all - -# ---------------------------------------------------------------------------- -# Discover IDs we can poke at for singular tests -# ---------------------------------------------------------------------------- - -echo -echo "## discovering test IDs" -conv_id=$("$BIN" convs --assignee all -o json 2>/dev/null | jq -r '.data.payload[0].id // empty') -contact_id=$("$BIN" contacts -o json 2>/dev/null | jq -r '.payload[0].id // empty') -inbox_id=$("$BIN" inboxes -o json 2>/dev/null | jq -r '.payload[0].id // empty') -agent_id=$("$BIN" agents -o json 2>/dev/null | jq -r '.[0].id // empty') -team_id=$("$BIN" teams -o json 2>/dev/null | jq -r '.[0].id // empty') -echo " conv=$conv_id contact=$contact_id inbox=$inbox_id agent=$agent_id team=$team_id" - -# ---------------------------------------------------------------------------- -# Singular reads (id-first, verb-first, alias) -# ---------------------------------------------------------------------------- - -echo -echo "## singular reads" -if [[ -n "$conv_id" ]]; then - expect "conv $conv_id (default view)" "^ID:" "$BIN" conv "$conv_id" - expect "conv $conv_id view (id-first)" "^ID:" "$BIN" conv "$conv_id" view - expect "conv view $conv_id (verb-first)" "^ID:" "$BIN" conv view "$conv_id" - expect "conversation $conv_id view (alias)" "^ID:" "$BIN" conversation "$conv_id" view - expect "conv $conv_id messages" "^ID|No messages" "$BIN" conv "$conv_id" messages -else - echo " SKIP no conversations available — singular conv tests skipped" -fi - -if [[ -n "$contact_id" ]]; then - expect "contact $contact_id" "^ID:" "$BIN" contact "$contact_id" - expect "contact $contact_id conversations" "^ID|No conversations" "$BIN" contact "$contact_id" conversations -fi - -if [[ -n "$inbox_id" ]]; then - expect "inbox $inbox_id" "^ID:" "$BIN" inbox "$inbox_id" -fi - -# ---------------------------------------------------------------------------- -# Writes — every write is followed by a state read-back so we catch the -# "API accepted but didn't apply" bug class. -# ---------------------------------------------------------------------------- - -if [[ -z "$conv_id" ]]; then - echo - echo "## writes — SKIPPED (no conversation to operate on)" -else - echo - echo "## writes (will mutate conv $conv_id; verifying state after each)" - - # ---- reply: capture message id via -q, verify content round-trip ---- - unique="smoke reply $(date +%s)-$RANDOM" - reply_id=$("$BIN" -q conv "$conv_id" reply "$unique" 2>/dev/null || true) - if [[ "$reply_id" =~ ^[0-9]+$ ]]; then - pass "reply (msg id captured: $reply_id)" - got=$("$BIN" conv "$conv_id" messages -o json 2>/dev/null \ - | jq -r --arg id "$reply_id" '.payload[] | select(.id == ($id|tonumber)) | .content') - if [[ "$got" == "$unique" ]]; then - pass "reply (content round-trip)" - else - fail "reply content mismatch (got=$got)" - fi - else - fail "reply (-q did not return numeric id: '$reply_id')" - fi - - # ---- private note: same approach, also verify .private == true ---- - unique_note="smoke note $(date +%s)-$RANDOM" - note_id=$("$BIN" -q conv "$conv_id" reply "$unique_note" --private 2>/dev/null || true) - if [[ "$note_id" =~ ^[0-9]+$ ]]; then - pass "reply --private (msg id captured: $note_id)" - got=$("$BIN" conv "$conv_id" messages -o json 2>/dev/null \ - | jq -rc --arg id "$note_id" '.payload[] | select(.id == ($id|tonumber)) | [.content, .private]') - if [[ "$got" == "[\"$unique_note\",true]" ]]; then - pass "reply --private (content+private flag round-trip)" - else - fail "reply --private mismatch (got=$got)" - fi - else - fail "reply --private (-q did not return numeric id: '$note_id')" - fi - - # ---- status verbs: each followed by state check ---- - expect "resolve" "-> resolved" "$BIN" conv "$conv_id" resolve - state_eq "resolve (state)" '.status' "resolved" - - expect "open" "-> open" "$BIN" conv "$conv_id" open - state_eq "open (state)" '.status' "open" - - expect "pending" "-> pending" "$BIN" conv "$conv_id" pending - state_eq "pending (state)" '.status' "pending" - - expect "snooze" "-> snoozed" "$BIN" conv "$conv_id" snooze - state_eq "snooze (state)" '.status' "snoozed" - - expect "snooze --until 24h" "until" "$BIN" conv "$conv_id" snooze --until 24h - state_eq "snooze --until 24h (state)" '.status' "snoozed" - - expect "snooze --until 7d" "until" "$BIN" conv "$conv_id" snooze --until 7d - state_eq "snooze --until 7d (state)" '.status' "snoozed" - - expect "snooze --until date" "until" "$BIN" conv "$conv_id" snooze --until 2030-01-01 - state_eq "snooze --until date (state)" '.status' "snoozed" - - expect "open (cleanup)" "-> open" "$BIN" conv "$conv_id" open - state_eq "open cleanup (state)" '.status' "open" - - # ---- verb-first regression for a write verb ---- - expect "conv resolve N (verb-first)" "-> resolved" "$BIN" conv resolve "$conv_id" - state_eq "verb-first resolve (state)" '.status' "resolved" - "$BIN" conv "$conv_id" open >/dev/null # restore - - # ---- assign --agent N: verify meta.assignee.id ---- - if [[ -n "$agent_id" ]]; then - expect "assign --agent N" "assigned" "$BIN" conv "$conv_id" assign --agent "$agent_id" - state_eq "assign --agent N (state)" '.meta.assignee.id' "$agent_id" - - # 'me' resolves via cached or lazy-fetched user_id (commit c786562 + later fix). - expect "assign --agent me" "assigned" "$BIN" conv "$conv_id" assign --agent me - me_id=$("$BIN" me -o json 2>/dev/null | jq -r '.id') - state_eq "assign --agent me (state)" '.meta.assignee.id' "$me_id" - fi - - # ---- assign --team N (the team-only review-fix path): omit assignee_id ---- - if [[ -n "$team_id" ]]; then - # First unassign agent so we can verify team-only assign without prior agent override. - "$BIN" conv "$conv_id" unassign >/dev/null - expect "assign --team N (team-only)" "team" "$BIN" conv "$conv_id" assign --team "$team_id" - state_eq "assign --team N (state)" '.meta.team.id' "$team_id" - fi - - # ---- unassign: verify meta.assignee == null ---- - expect "unassign" "unassigned" "$BIN" conv "$conv_id" unassign - state_eq "unassign (state)" '.meta.assignee' "null" - - # ---- label: verify both labels appear in .labels ---- - l1="smoke-$RANDOM"; l2="test-$RANDOM" - expect "label $l1,$l2" "labels:" "$BIN" conv "$conv_id" label "$l1,$l2" - state_eq "label set (state)" \ - "(.labels | (index(\"$l1\") != null and index(\"$l2\") != null))" "true" - - # ---- priority urgent ---- - expect "priority urgent" "-> urgent" "$BIN" conv "$conv_id" priority urgent - state_eq "priority urgent (state)" '.priority' "urgent" - - # ---- priority none (review-fix: server returned 500 on literal "none"; - # we now send null which is the supported clear). State must be null/empty. - expect "priority none" "-> none" "$BIN" conv "$conv_id" priority none - state_eq "priority none (state)" '.priority' "null" -fi - -# ---------------------------------------------------------------------------- -# Failure paths — assert the *error message*, not just the exit code. -# ---------------------------------------------------------------------------- - -echo -echo "## expected failures" -if [[ -n "$conv_id" ]]; then - expect_fail "assign with no flags" "--agent or --team required" "$BIN" conv "$conv_id" assign - expect_fail "priority bogus" "must be one of|priority" "$BIN" conv "$conv_id" priority bogus -fi -expect_fail "snooze --until garbage" "invalid --until" "$BIN" conv "${conv_id:-1}" snooze --until "not-a-time" -expect_fail "conv 999999999 (404)" "404|not found|Resource" "$BIN" conv 999999999 - -# ---------------------------------------------------------------------------- -# Summary -# ---------------------------------------------------------------------------- - -echo -echo "## summary" -printf ' %d passed, %d failed\n' "$PASS" "$FAIL" -if [[ $FAIL -gt 0 ]]; then - echo " failures:" - printf ' - %s\n' "${FAILURES[@]}" - exit 1 -fi