diff --git a/.changeset/fix-offline-command-telemetry.md b/.changeset/fix-offline-command-telemetry.md new file mode 100644 index 00000000..76f67b56 --- /dev/null +++ b/.changeset/fix-offline-command-telemetry.md @@ -0,0 +1,5 @@ +--- +"cre-cli": patch +--- + +Fix telemetry for commands that don't require login (`cre generate-bindings`, `cre workflow build`, `cre workflow hash`, etc.). Credentials are now attached silently when they exist on disk, so usage events from logged-in users are sent instead of being dropped. Commands remain fully usable while logged out. diff --git a/cmd/root.go b/cmd/root.go index 421e7ea1..fff6c442 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -83,6 +83,10 @@ func Execute() { } func newRootCommand() *cobra.Command { + // Without this, a subcommand-level PersistentPreRunE would silently shadow + // root's, skipping credential loading and telemetry for that whole subtree. + cobra.EnableTraverseRunHooks = true + rootLogger := createLogger() rootViper := createViper() runtimeContext := runtime.NewContext(rootLogger, rootViper) @@ -259,6 +263,12 @@ func newRootCommand() *cobra.Command { return err } } + } else { + // Commands that don't require login (e.g. generate-bindings, workflow + // build) still emit telemetry, which the server only accepts when + // authenticated. Attach credentials silently when they exist so those + // events can be sent; never validate, prompt, or fail. + runtimeContext.TryAttachCredentials() } // load settings from yaml files @@ -586,37 +596,39 @@ func isLoadSettings(cmd *cobra.Command) bool { return !exists } -func isLoadCredentials(cmd *cobra.Command) bool { - // It is not expected to have the credentials loaded when running the following commands - var excludedCommands = map[string]struct{}{ - "cre version": {}, - "cre login": {}, - "cre logout": {}, - "cre completion bash": {}, - "cre completion fish": {}, - "cre completion powershell": {}, - "cre completion zsh": {}, - "cre help": {}, - "cre generate-bindings": {}, - "cre generate-bindings evm": {}, - "cre generate-bindings solana": {}, - "cre update": {}, - "cre workflow": {}, - "cre execution": {}, - "cre workflow limits": {}, - "cre workflow limits export": {}, - "cre account": {}, - "cre secrets": {}, - "cre workflow build": {}, - "cre workflow hash": {}, - "cre templates": {}, - "cre templates list": {}, - "cre templates add": {}, - "cre templates remove": {}, - "cre": {}, - } +// LoginExemptCommands are excluded from credential loading/validation in +// isLoadCredentials. They still get best-effort credentials via +// TryAttachCredentials so telemetry keeps working. +var LoginExemptCommands = map[string]struct{}{ + "cre version": {}, + "cre login": {}, + "cre logout": {}, + "cre completion bash": {}, + "cre completion fish": {}, + "cre completion powershell": {}, + "cre completion zsh": {}, + "cre help": {}, + "cre generate-bindings": {}, + "cre generate-bindings evm": {}, + "cre generate-bindings solana": {}, + "cre update": {}, + "cre workflow": {}, + "cre execution": {}, + "cre workflow limits": {}, + "cre workflow limits export": {}, + "cre account": {}, + "cre secrets": {}, + "cre workflow build": {}, + "cre workflow hash": {}, + "cre templates": {}, + "cre templates list": {}, + "cre templates add": {}, + "cre templates remove": {}, + "cre": {}, +} - _, exists := excludedCommands[cmd.CommandPath()] +func isLoadCredentials(cmd *cobra.Command) bool { + _, exists := LoginExemptCommands[cmd.CommandPath()] return !exists } diff --git a/internal/runtime/runtime_context.go b/internal/runtime/runtime_context.go index 21a936eb..7d2466ca 100644 --- a/internal/runtime/runtime_context.go +++ b/internal/runtime/runtime_context.go @@ -120,6 +120,21 @@ func (ctx *Context) AttachCredentials(validationCtx context.Context, skipValidat return nil } +// TryAttachCredentials silently loads credentials from the environment or disk +// without validation, prompting, or failing. It is used for commands that do +// not require authentication so their telemetry events can still be attributed +// to the logged-in user; telemetry is skipped when no usable credentials exist. +func (ctx *Context) TryAttachCredentials() { + creds, err := credentials.New(ctx.Logger) + if err != nil || creds == nil { + return + } + if creds.APIKey == "" && (creds.Tokens == nil || creds.Tokens.AccessToken == "") { + return + } + ctx.Credentials = creds +} + // AttachTenantContext loads the user context for the current environment. // If the manifest is missing, it is fetched from the service first. func (ctx *Context) AttachTenantContext(validationCtx context.Context) error { diff --git a/internal/runtime/runtime_context_test.go b/internal/runtime/runtime_context_test.go index ef4227e9..d04aa1a7 100644 --- a/internal/runtime/runtime_context_test.go +++ b/internal/runtime/runtime_context_test.go @@ -1,10 +1,16 @@ package runtime import ( + "io" + "os" + "path/filepath" "testing" + "github.com/rs/zerolog" "github.com/stretchr/testify/require" + "github.com/smartcontractkit/cre-cli/internal/creconfig" + "github.com/smartcontractkit/cre-cli/internal/credentials" "github.com/smartcontractkit/cre-cli/internal/environments" "github.com/smartcontractkit/cre-cli/internal/settings" ) @@ -82,3 +88,47 @@ func TestValidateOnchainRegistryRPC(t *testing.T) { require.Contains(t, err.Error(), "missing RPC URL") }) } + +func TestTryAttachCredentials(t *testing.T) { + discardLogger := zerolog.New(io.Discard) + + t.Run("attaches API key credentials from environment", func(t *testing.T) { + t.Setenv(credentials.CreApiKeyVar, "test-api-key") + + ctx := &Context{Logger: &discardLogger} + ctx.TryAttachCredentials() + + require.NotNil(t, ctx.Credentials) + require.Equal(t, "test-api-key", ctx.Credentials.APIKey) + require.Equal(t, credentials.AuthTypeApiKey, ctx.Credentials.AuthType) + }) + + t.Run("attaches bearer credentials from config file", func(t *testing.T) { + t.Setenv(credentials.CreApiKeyVar, "") + home := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(home, creconfig.Dir), 0o700)) + require.NoError(t, os.WriteFile( + filepath.Join(home, creconfig.Dir, credentials.ConfigFile), + []byte("AccessToken: test-access-token\n"), + 0o600, + )) + t.Setenv("HOME", home) + + ctx := &Context{Logger: &discardLogger} + ctx.TryAttachCredentials() + + require.NotNil(t, ctx.Credentials) + require.NotNil(t, ctx.Credentials.Tokens) + require.Equal(t, "test-access-token", ctx.Credentials.Tokens.AccessToken) + }) + + t.Run("leaves credentials nil when none exist", func(t *testing.T) { + t.Setenv(credentials.CreApiKeyVar, "") + t.Setenv("HOME", t.TempDir()) + + ctx := &Context{Logger: &discardLogger} + ctx.TryAttachCredentials() + + require.Nil(t, ctx.Credentials) + }) +} diff --git a/internal/telemetry/emitter.go b/internal/telemetry/emitter.go index ebe52dab..1baab8a3 100644 --- a/internal/telemetry/emitter.go +++ b/internal/telemetry/emitter.go @@ -47,7 +47,7 @@ func EmitCommandEvent(cmd *cobra.Command, args []string, exitCode int, runtimeCt } // Check if this command should be excluded - if shouldExcludeCommand(cmd) { + if ShouldExcludeCommand(cmd) { debugLog("command %s excluded from telemetry", cmd.Name()) return } @@ -81,8 +81,8 @@ func debugLog(format string, args ...interface{}) { } } -// shouldExcludeCommand determines if a command should not emit telemetry events -func shouldExcludeCommand(cmd *cobra.Command) bool { +// ShouldExcludeCommand determines if a command should not emit telemetry events. +func ShouldExcludeCommand(cmd *cobra.Command) bool { if cmd == nil { return true } diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go index c5a758cb..00071963 100644 --- a/internal/telemetry/telemetry_test.go +++ b/internal/telemetry/telemetry_test.go @@ -110,7 +110,7 @@ func TestShouldExcludeCommand(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cmd := &cobra.Command{Use: tt.cmdName} - assert.Equal(t, tt.shouldExclude, shouldExcludeCommand(cmd)) + assert.Equal(t, tt.shouldExclude, ShouldExcludeCommand(cmd)) }) } } diff --git a/test/graphql_mock.go b/test/graphql_mock.go index f9ee608d..4354b2cc 100644 --- a/test/graphql_mock.go +++ b/test/graphql_mock.go @@ -1,9 +1,15 @@ package test import ( + "encoding/json" + "net/http" "net/http/httptest" + "strings" + "sync" "testing" + "github.com/smartcontractkit/cre-cli/internal/environments" + "github.com/smartcontractkit/cre-cli/internal/telemetry" "github.com/smartcontractkit/cre-cli/internal/testutil" ) @@ -12,3 +18,74 @@ import ( func NewGraphQLMockServerGetOrganization(t *testing.T) *httptest.Server { return testutil.NewGraphQLMockServerGetOrganization(t) } + +// RecordedTelemetry gives thread-safe access to the reportUserEvent mutations +// received by NewGraphQLMockServerRecordingTelemetry. The CLI sends telemetry +// from a goroutine killed by os.Exit, so callers must poll Events() briefly +// after the CLI process exits rather than reading it immediately. +type RecordedTelemetry struct { + mu sync.Mutex + events []telemetry.UserEventInput +} + +// Events returns a snapshot of the reportUserEvent mutations received so far. +func (r *RecordedTelemetry) Events() []telemetry.UserEventInput { + r.mu.Lock() + defer r.mu.Unlock() + return append([]telemetry.UserEventInput(nil), r.events...) +} + +func (r *RecordedTelemetry) record(e telemetry.UserEventInput) { + r.mu.Lock() + defer r.mu.Unlock() + r.events = append(r.events, e) +} + +// NewGraphQLMockServerRecordingTelemetry starts an httptest.Server that answers +// getCreOrganizationInfo/getTenantConfig like NewGraphQLMockServerGetOrganization, +// and additionally records every reportUserEvent mutation it receives so tests +// can assert telemetry was actually sent over the wire. It sets EnvVarGraphQLURL. +// Caller must defer srv.Close(). +func NewGraphQLMockServerRecordingTelemetry(t *testing.T) (*httptest.Server, *RecordedTelemetry) { + t.Helper() + testutil.IsolateCLIHome(t) + + rec := &RecordedTelemetry{} + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/graphql") && r.Method == http.MethodPost { + var req struct { + Query string `json:"query"` + Variables struct { + Event telemetry.UserEventInput `json:"event"` + } `json:"variables"` + } + _ = json.NewDecoder(r.Body).Decode(&req) + w.Header().Set("Content-Type", "application/json") + + if strings.Contains(req.Query, "reportUserEvent") { + rec.record(req.Variables.Event) + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "reportUserEvent": map[string]any{"success": true, "message": ""}, + }, + }) + return + } + if strings.Contains(req.Query, "getCreOrganizationInfo") { + _ = json.NewEncoder(w).Encode(testutil.MockGetCreOrganizationInfoGraphQLPayload()) + return + } + if testutil.QueryIsGetTenantConfig(req.Query) { + _ = json.NewEncoder(w).Encode(testutil.MockGetTenantConfigGraphQLPayload()) + return + } + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]any{ + "errors": []map[string]string{{"message": "Unsupported GraphQL query"}}, + }) + } + })) + t.Setenv(environments.EnvVarGraphQLURL, srv.URL+"/graphql") + return srv, rec +} diff --git a/test/telemetry_test.go b/test/telemetry_test.go new file mode 100644 index 00000000..aeb3fdea --- /dev/null +++ b/test/telemetry_test.go @@ -0,0 +1,131 @@ +package test + +import ( + "os/exec" + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/cre-cli/cmd" + "github.com/smartcontractkit/cre-cli/internal/credentials" + "github.com/smartcontractkit/cre-cli/internal/telemetry" +) + +// commandsSkippedFromTelemetryE2E excepts login-exempt commands that reach a +// real external service or start an interactive flow. Everything else returned +// by cmd.LoginExemptCommandPaths() is exercised by default. +var commandsSkippedFromTelemetryE2E = map[string]string{ + "cre login": "starts an interactive OAuth flow (opens a browser, waits for a callback)", + "cre update": "hits api.github.com for the latest release", + "cre templates list": "fetches the template list from GitHub", + "cre templates add": "fetches template contents from GitHub", + "cre templates remove": "shares the same GitHub-backed template repo client as add/list", +} + +// TestTelemetryForLoginExemptCommands verifies that commands excluded from +// credential loading (isLoadCredentials) still attach best-effort credentials +// so their telemetry reaches the server, instead of being silently dropped. +// It walks cmd.LoginExemptCommands directly, so a command added to that +// list later is covered automatically, with no test change needed. +func TestTelemetryForLoginExemptCommands(t *testing.T) { + // help and completion are cobra built-ins that cobra only wires onto the + // tree lazily inside Execute(); force that now so Find below can see them. + cmd.RootCmd.InitDefaultHelpCmd() + cmd.RootCmd.InitDefaultCompletionCmd() + + for path := range cmd.LoginExemptCommands { + t.Run(path, func(t *testing.T) { + if reason, skip := commandsSkippedFromTelemetryE2E[path]; skip { + t.Skipf("skipped: %s", reason) + } + + segments := strings.Fields(path)[1:] // drop the leading "cre" + found, _, err := cmd.RootCmd.Find(segments) + require.NoError(t, err, "could not resolve %q in the real command tree", path) + + if !found.Runnable() { + t.Skipf("%q has no Run/RunE (a pure help group); cobra shows help before "+ + "PersistentPreRunE ever runs, so it can never emit telemetry regardless of credentials", path) + } + + srv, rec := NewGraphQLMockServerRecordingTelemetry(t) + defer srv.Close() + + t.Setenv(credentials.CreApiKeyVar, "test-api-key") + + args := append(append([]string{}, segments...), probeArgs(t, found)...) + cliCmd := exec.Command(CLIPath, args...) // #nosec G204 -- args are derived from the real command tree above, not external input + _ = cliCmd.Run() // exit code is irrelevant: most of these fail fast without a real project, but must still emit telemetry + + wantCount := 1 + if telemetry.ShouldExcludeCommand(found) { + wantCount = 0 // e.g. version/help/completion: excluded by emitter.go independent of credentials + } + + events := waitForTelemetryEventCount(t, rec, wantCount) + require.Len(t, events, wantCount, "telemetry event count for %v", args) + if wantCount == 0 { + return + } + + want := telemetry.CollectCommandInfo(found, nil) + require.Equal(t, want.Action, events[0].Command.Action, "action for %v", args) + require.Equal(t, want.Subcommand, events[0].Command.Subcommand, "subcommand for %v", args) + }) + } +} + +// TestTelemetrySkippedWithoutCredentials is the negative control: without any +// credentials on disk or in the environment, telemetry must stay silent rather +// than send an unauthenticated event, confirming credentials are what gate delivery. +func TestTelemetrySkippedWithoutCredentials(t *testing.T) { + srv, rec := NewGraphQLMockServerRecordingTelemetry(t) + defer srv.Close() + + t.Setenv(credentials.CreApiKeyVar, "") + + cliCmd := exec.Command(CLIPath, "generate-bindings", "evm") + _ = cliCmd.Run() + + time.Sleep(300 * time.Millisecond) // grace period; nothing should arrive, late or not + require.Empty(t, rec.Events(), "expected no telemetry events without credentials") +} + +// probeArgs returns the smallest slice of placeholder positional args that +// satisfies found's Args validator, so commands requiring e.g. exactly one +// argument (like "workflow build ") can still be invoked generically. +func probeArgs(t *testing.T, found *cobra.Command) []string { + t.Helper() + if found.Args == nil { + return nil + } + for n := range 4 { + candidate := make([]string, n) + for i := range candidate { + candidate[i] = "nonexistent-arg" + } + if found.Args(found, candidate) == nil { + return candidate + } + } + t.Fatalf("could not find a valid positional arg count (0-3) for %q", found.CommandPath()) + return nil +} + +// waitForTelemetryEventCount polls rec because the CLI sends telemetry from a +// background goroutine and os.Exit can race the HTTP request reaching the +// local mock server. +func waitForTelemetryEventCount(t *testing.T, rec *RecordedTelemetry, want int) []telemetry.UserEventInput { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for { + events := rec.Events() + if len(events) >= want || time.Now().After(deadline) { + return events + } + time.Sleep(20 * time.Millisecond) + } +}