Merge pull request #86 from shubham-stepsecurity/sm/update

chore(info): Adding invocation_method and in-flight status_info

Author: ashishkurmi

internal/detector/nodescan.go

@@ -34,12 +34,23 @@ type NodeScanner struct {
 	exec         executor.Executor
 	log          *progress.Logger
 	loggedInUser string // when non-empty and running as root, commands run as this user
+	// ProgressHook, when non-nil, is invoked from inside ScanProjects /
+	// ScanGlobalPackages with a short human-readable detail string ("project
+	// 12 of 47", "scanning yarn", ...). Telemetry plumbs this into
+	// PhaseTracker.UpdateDetail so heartbeats surface mid-phase progress.
+	ProgressHook func(detail string)
 }
 
 func NewNodeScanner(exec executor.Executor, log *progress.Logger, loggedInUser string) *NodeScanner {
 	return &NodeScanner{exec: exec, log: log, loggedInUser: loggedInUser}
 }
 
+func (s *NodeScanner) emitProgress(detail string) {
+	if s.ProgressHook != nil {
+		s.ProgressHook(detail)
+	}
+}
+
 // shouldRunAsUser returns true when commands should be delegated to the logged-in user.
 // Only applies on Unix — RunAsUser uses sudo which is not available on Windows.
 func (s *NodeScanner) shouldRunAsUser() bool {
@@ -107,16 +118,19 @@ func (s *NodeScanner) checkPath(ctx context.Context, name string) error {
 func (s *NodeScanner) ScanGlobalPackages(ctx context.Context) []model.NodeScanResult {
 	var results []model.NodeScanResult
 
+	s.emitProgress("global: npm")
 	s.log.Progress("  Checking npm global packages...")
 	if r, ok := s.scanNPMGlobal(ctx); ok {
 		results = append(results, r)
 	}
 
+	s.emitProgress("global: yarn")
 	s.log.Progress("  Checking yarn global packages...")
 	if r, ok := s.scanYarnGlobal(ctx); ok {
 		results = append(results, r)
 	}
 
+	s.emitProgress("global: pnpm")
 	s.log.Progress("  Checking pnpm global packages...")
 	if r, ok := s.scanPnpmGlobal(ctx); ok {
 		results = append(results, r)
@@ -354,6 +368,10 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string) []m
 	var results []model.NodeScanResult
 	totalSize := int64(0)
 
+	totalProjects := len(projects)
+	if totalProjects > maxNodeProjects {
+		totalProjects = maxNodeProjects
+	}
 	for i, p := range projects {
 		if i >= maxNodeProjects {
 			s.log.Progress("  Reached maximum of %d projects, stopping search", maxNodeProjects)
@@ -366,6 +384,11 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string) []m
 			break
 		}
 
+		// Per-project sub-progress for the heartbeat goroutine. Surfaces
+		// to console as "current_phase_detail: project 12 of 47" so a
+		// stuck scan is visibly so, not just opaque "node_scan in progress".
+		s.emitProgress(fmt.Sprintf("project %d of %d", i+1, totalProjects))
+
 		s.log.Progress("  Found project: %s", p.dir)
 		pm := DetectProjectPM(s.exec, p.dir)
 		s.log.Progress("    Package manager: %s", pm)

internal/detector/pythonscan.go

@@ -15,12 +15,23 @@ import (
 type PythonScanner struct {
 	exec executor.Executor
 	log  *progress.Logger
+	// ProgressHook, when non-nil, is invoked from inside ScanGlobalPackages
+	// with a short human-readable detail string ("scanning pip3", ...).
+	// Telemetry plumbs this into PhaseTracker.UpdateDetail so heartbeats
+	// surface mid-phase progress.
+	ProgressHook func(detail string)
 }
 
 func NewPythonScanner(exec executor.Executor, log *progress.Logger) *PythonScanner {
 	return &PythonScanner{exec: exec, log: log}
 }
 
+func (s *PythonScanner) emitProgress(detail string) {
+	if s.ProgressHook != nil {
+		s.ProgressHook(detail)
+	}
+}
+
 type pythonScanSpec struct {
 	binary     string
 	name       string
@@ -49,6 +60,7 @@ func (s *PythonScanner) ScanGlobalPackages(ctx context.Context) []model.PythonSc
 			continue
 		}
 
+		s.emitProgress("scanning " + spec.name)
 		s.log.Progress("  Checking %s global packages...", spec.name)
 		version := s.getVersion(ctx, spec.binary, spec.versionCmd)
 

internal/launchd/launchd.go

@@ -20,8 +20,22 @@ const (
 	systemLogDir    = "/var/log/stepsecurity"
 )
 
+// DaemonPlistPath is the system-wide launchd plist installed when the agent
+// runs as root. Exported so other packages (notably telemetry's invocation
+// detector) can check for an installed footprint without re-deriving the path.
+const DaemonPlistPath = daemonPlistPath
+
+// UserPlistPath returns the per-user launchd plist path installed when the
+// agent runs without root. Empty when the home directory cannot be resolved.
+func UserPlistPath() string {
+	return agentPlistPath()
+}
+
 func agentPlistPath() string {
 	homeDir, _ := os.UserHomeDir()
+	if homeDir == "" {
+		return ""
+	}
 	return homeDir + "/Library/LaunchAgents/com.stepsecurity.agent.plist"
 }
 

internal/schtasks/schtasks.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	osexec "os/exec"
 	"strconv"
 
 	"github.com/step-security/dev-machine-guard/internal/config"
@@ -14,6 +15,17 @@ import (
 
 const taskName = "StepSecurity Dev Machine Guard"
 
+// IsTaskRegistered reports whether the Windows scheduled task created by
+// `dev-machine-guard install` is currently registered. Used by the
+// telemetry package's invocation detector to distinguish a manual CLI run
+// from a scheduler-triggered one. Any error or non-zero schtasks exit is
+// treated as "not registered" so a transient Schedule-service hiccup
+// degrades to "one_time" rather than erroring the run.
+func IsTaskRegistered() bool {
+	cmd := osexec.Command("schtasks", "/query", "/tn", taskName)
+	return cmd.Run() == nil
+}
+
 // Install configures Windows Task Scheduler for periodic scanning.
 // If already installed, upgrades by removing and re-creating the task.
 func Install(exec executor.Executor, log *progress.Logger) error {

internal/systemd/systemd.go

@@ -17,6 +17,18 @@ import (
 
 const unitName = "stepsecurity-dev-machine-guard"
 
+// TimerUnitPath returns the per-user systemd timer unit path installed for
+// periodic scanning. Exported so the telemetry package's invocation detector
+// can stat for an installed footprint without re-deriving the path. Returns
+// empty when the home directory cannot be resolved.
+func TimerUnitPath() string {
+	homeDir, _ := os.UserHomeDir()
+	if homeDir == "" {
+		return ""
+	}
+	return filepath.Join(homeDir, ".config", "systemd", "user", unitName+".timer")
+}
+
 // Install configures a systemd user timer for periodic scanning.
 // If already installed, upgrades by removing and re-creating the units.
 func Install(exec executor.Executor, log *progress.Logger) error {

internal/telemetry/heartbeat_shutdown_test.go

@@ -0,0 +1,49 @@
+package telemetry
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+// TestHeartbeatShutdown_NoDeadlock mirrors the cancel-then-wait pattern
+// Run() uses to shut down its heartbeat goroutine. Two `defer` statements
+// (cancel + wait) would deadlock under LIFO ordering — wait runs first,
+// blocks on the goroutine, and cancel never fires. Combining them into a
+// single deferred function is the fix; this test pins it down.
+func TestHeartbeatShutdown_NoDeadlock(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+		ticker := time.NewTicker(50 * time.Millisecond)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case <-ticker.C:
+				// no-op, mimics postPhase
+			}
+		}
+	}()
+
+	// This is the load-bearing pattern from Run(): cancel first, THEN wait.
+	// If a future refactor splits these into separate `defer` statements at
+	// the top level of Run(), the LIFO ordering will deadlock.
+	shutdownStart := time.Now()
+	func() {
+		defer func() {
+			cancel()
+			<-done
+		}()
+	}()
+	elapsed := time.Since(shutdownStart)
+
+	// 50ms ticker + small scheduler overhead; 1s is generous and signals a
+	// hang clearly if the pattern ever regresses.
+	if elapsed > time.Second {
+		t.Fatalf("heartbeat shutdown took %s — likely deadlocked on defer ordering", elapsed)
+	}
+}

internal/telemetry/invocation.go

@@ -0,0 +1,54 @@
+package telemetry
+
+import (
+	"os"
+	"runtime"
+
+	"github.com/step-security/dev-machine-guard/internal/launchd"
+	"github.com/step-security/dev-machine-guard/internal/schtasks"
+	"github.com/step-security/dev-machine-guard/internal/systemd"
+)
+
+// Wire-format values for the invocation_method field. Kept stable —
+// console and backend match on these literal strings.
+const (
+	InvocationInstall = "install"
+	InvocationOneTime = "one_time"
+)
+
+// DetectInvocationMethod returns "install" when the dev-machine-guard
+// scheduler footprint is present on this machine, else "one_time".
+//
+// The check is best-effort and never returns an error: a stat failure or a
+// flaky schtasks call degrades to "one_time" so an unknown environment is
+// never misreported as an installed agent. Detection is filesystem-based on
+// darwin/linux and a single schtasks query on windows, so an agent rolled
+// out before this code shipped starts reporting "install" on its next
+// scheduled fire without any installer changes.
+func DetectInvocationMethod() string {
+	if isSchedulerInstalled() {
+		return InvocationInstall
+	}
+	return InvocationOneTime
+}
+
+func isSchedulerInstalled() bool {
+	switch runtime.GOOS {
+	case "darwin":
+		return fileExists(launchd.DaemonPlistPath) || fileExists(launchd.UserPlistPath())
+	case "linux":
+		return fileExists(systemd.TimerUnitPath())
+	case "windows":
+		return schtasks.IsTaskRegistered()
+	default:
+		return false
+	}
+}
+
+func fileExists(path string) bool {
+	if path == "" {
+		return false
+	}
+	info, err := os.Stat(path)
+	return err == nil && !info.IsDir()
+}

internal/telemetry/invocation_test.go

@@ -0,0 +1,120 @@
+package telemetry
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/step-security/dev-machine-guard/internal/launchd"
+	"github.com/step-security/dev-machine-guard/internal/systemd"
+)
+
+func TestFileExists(t *testing.T) {
+	dir := t.TempDir()
+	present := filepath.Join(dir, "marker")
+	if err := os.WriteFile(present, []byte("x"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	cases := []struct {
+		name string
+		path string
+		want bool
+	}{
+		{"existing file", present, true},
+		{"missing file", filepath.Join(dir, "nope"), false},
+		{"empty path", "", false},
+		{"directory", dir, false}, // dirs intentionally don't count as installs
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := fileExists(tc.path); got != tc.want {
+				t.Fatalf("fileExists(%q) = %v, want %v", tc.path, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestDetectInvocationMethod_HostMachine exercises the detector against the
+// real machine. The result is whatever the current dev box reports; we can
+// only assert the value is one of the two valid wire-format strings.
+func TestDetectInvocationMethod_HostMachine(t *testing.T) {
+	got := DetectInvocationMethod()
+	if got != InvocationInstall && got != InvocationOneTime {
+		t.Fatalf("DetectInvocationMethod returned %q, want %q or %q",
+			got, InvocationInstall, InvocationOneTime)
+	}
+}
+
+// TestDetectInvocationMethod_RespondsToFilesystem covers the darwin/linux
+// path that stats a scheduler artifact. On Windows the check shells out to
+// schtasks, which we can't safely stub without an executor seam — skip there.
+//
+// Sandboxes HOME (Unix) and USERPROFILE (Windows-safe no-op on Unix) under
+// t.TempDir() so launchd.UserPlistPath / systemd.TimerUnitPath compute paths
+// that live entirely inside the temp tree. Without this the test would write
+// markers (and MkdirAll-created parent dirs) into the developer's real
+// ~/Library/LaunchAgents or ~/.config/systemd/user — leaving stray files
+// behind on CI and risking a tiny TOCTOU window against a real install.
+func TestDetectInvocationMethod_RespondsToFilesystem(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("windows uses schtasks /query, not filesystem")
+	}
+
+	tempHome := t.TempDir()
+	t.Setenv("HOME", tempHome)
+	t.Setenv("USERPROFILE", tempHome) // no-op on Unix but cheap and keeps the seam consistent
+
+	// Resolve the platform's expected artifact path AFTER the env override
+	// so os.UserHomeDir() returns tempHome.
+	var path string
+	switch runtime.GOOS {
+	case "darwin":
+		path = launchd.UserPlistPath()
+	case "linux":
+		path = systemd.TimerUnitPath()
+	default:
+		t.Skipf("no scheduler artifact path on %s", runtime.GOOS)
+	}
+	if path == "" {
+		t.Skip("could not resolve scheduler artifact path on this host")
+	}
+	if !strings.HasPrefix(path, tempHome) {
+		t.Fatalf("resolved path %q escaped tempHome %q — env sandbox is not effective", path, tempHome)
+	}
+
+	// Fresh temp home — detector starts at one_time, flips to install when
+	// the marker appears, flips back when it's removed.
+	if got := DetectInvocationMethod(); got != InvocationOneTime {
+		t.Fatalf("on clean temp home, detector returned %q, want %q",
+			got, InvocationOneTime)
+	}
+
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		t.Fatalf("prepare scheduler artifact dir: %v", err)
+	}
+	if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
+		t.Fatalf("write fake scheduler artifact: %v", err)
+	}
+	// No explicit cleanup: everything lives under t.TempDir() and is
+	// removed by the testing framework when the test ends.
+
+	if got := DetectInvocationMethod(); got != InvocationInstall {
+		t.Fatalf("after creating %q, detector returned %q, want %q",
+			path, got, InvocationInstall)
+	}
+
+	// Remove the marker mid-test and re-check — confirms detection is not
+	// cached and reflects current filesystem state.
+	if err := os.Remove(path); err != nil {
+		t.Fatalf("remove fake artifact: %v", err)
+	}
+
+	if got := DetectInvocationMethod(); got != InvocationOneTime {
+		t.Fatalf("after removing %q, detector returned %q, want %q",
+			path, got, InvocationOneTime)
+	}
+}