From 7fe9fc8763125be73a56f7327b86dd54303719dd Mon Sep 17 00:00:00 2001 From: Sahil Bansal Date: Wed, 11 Feb 2026 09:22:18 +0530 Subject: [PATCH] provide package blocked feedback in proxy mode (#154) * provide package blocked feedback in proxy mode * update test case & clear status on block * refactor reporting * clearStatus on `continueExecution` * set spinnerChan to nil * add sync for spinner --- guard/guard.go | 27 +++++++++++---------------- guard/guard_test.go | 25 ++++++------------------- internal/flows/proxy_flow.go | 2 -- internal/ui/report.go | 9 ++++++++- internal/ui/spinner.go | 23 ++++++++++++++++++++++- 5 files changed, 47 insertions(+), 39 deletions(-) diff --git a/guard/guard.go b/guard/guard.go index 31072a2..3c1b817 100644 --- a/guard/guard.go +++ b/guard/guard.go @@ -202,7 +202,8 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm result.BlockedPackages = append(result.BlockedPackages, analysisResult) blockConfig.MalwarePackages = append(blockConfig.MalwarePackages, analysisResult) g.logMalwareDetection(analysisResult, true) - return result, g.blockInstallation(blockConfig) + + return result, nil } if analysisResult.Action == analyzer.ActionConfirm { @@ -227,7 +228,8 @@ func (g *packageManagerGuard) Run(ctx context.Context, args []string, parsedComm result.BlockedPackages = append(result.BlockedPackages, pkg) } result.WasUserCancelled = true - return result, g.blockInstallation(blockConfig) + + return result, nil } // User confirmed installation despite warning @@ -394,12 +396,12 @@ func (g *packageManagerGuard) setStatus(status string) { g.interaction.SetStatus(status) } -func (g *packageManagerGuard) blockInstallation(config *ui.BlockConfig) error { - if g.interaction.Block == nil { - return nil +func (g *packageManagerGuard) showWarning(message string) { + if g.interaction.ShowWarning == nil { + return } - return g.interaction.Block(config) + g.interaction.ShowWarning(message) } func (g *packageManagerGuard) clearStatus() { @@ -410,14 +412,6 @@ func (g *packageManagerGuard) clearStatus() { g.interaction.ClearStatus() } -func (g *packageManagerGuard) showWarning(message string) { - if g.interaction.ShowWarning == nil { - return - } - - g.interaction.ShowWarning(message) -} - func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, parsedCommand *packagemanager.ParsedCommand) (*GuardResult, error) { result := &GuardResult{} @@ -497,7 +491,7 @@ func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, pa g.logMalwareDetection(analysisResult, true) - return result, g.blockInstallation(blockConfig) + return result, nil } if analysisResult.Action == analyzer.ActionConfirm { @@ -525,7 +519,8 @@ func (g *packageManagerGuard) handleManifestInstallation(ctx context.Context, pa } result.WasUserCancelled = true - return result, g.blockInstallation(blockConfig) + + return result, nil } // User confirmed installation despite warning diff --git a/guard/guard_test.go b/guard/guard_test.go index b863b12..09aa467 100644 --- a/guard/guard_test.go +++ b/guard/guard_test.go @@ -124,16 +124,8 @@ func TestGuardInsecureInstallation(t *testing.T) { config.DryRun = true config.ResolveDependencies = false // Disable dependency resolution to avoid nil pointer issues - blockCalled := false - var blockedPackages []*analyzer.PackageVersionAnalysisResult - interaction := PackageManagerGuardInteraction{ ShowWarning: func(message string) {}, - Block: func(config *ui.BlockConfig) error { - blockCalled = true - blockedPackages = config.MalwarePackages - return nil - }, } pg, err := NewPackageManagerGuard(config, nil, nil, @@ -161,21 +153,17 @@ func TestGuardInsecureInstallation(t *testing.T) { }, } - _, err = pg.Run(context.Background(), []string{"npm", "install", "nyc-config@10.0.0"}, parsedCommand) + r, err := pg.Run(context.Background(), []string{"npm", "install", "nyc-config@10.0.0"}, parsedCommand) // We expect no error from the guard itself (blocking is handled via the Block callback) assert.NoError(t, err) - // Block should be called because InsecureInstallation is disabled - assert.True(t, blockCalled, "Block should be called when InsecureInstallation is disabled") - // Verify that the malicious package was detected and blocked - assert.NotEmpty(t, blockedPackages, "Blocked packages should not be empty") - if len(blockedPackages) > 0 { - assert.Equal(t, "nyc-config", blockedPackages[0].PackageVersion.GetPackage().GetName()) - assert.Equal(t, "10.0.0", blockedPackages[0].PackageVersion.GetVersion()) - assert.Equal(t, analyzer.ActionBlock, blockedPackages[0].Action) - } + assert.NotEmpty(t, r.BlockedPackages, "Blocked packages should not be empty") + assert.Greater(t, r.BlockedCount, 0) + assert.Equal(t, "nyc-config", r.BlockedPackages[0].PackageVersion.GetPackage().GetName()) + assert.Equal(t, "10.0.0", r.BlockedPackages[0].PackageVersion.GetVersion()) + assert.Equal(t, analyzer.ActionBlock, r.BlockedPackages[0].Action) }) t.Run("should continue execution for commands without install targets when InsecureInstallation is enabled", func(t *testing.T) { @@ -269,4 +257,3 @@ func TestGuardInsecureInstallation(t *testing.T) { assert.False(t, config.InsecureInstallation, "InsecureInstallation should default to false") }) } - diff --git a/internal/flows/proxy_flow.go b/internal/flows/proxy_flow.go index a79ac3c..9365e3e 100644 --- a/internal/flows/proxy_flow.go +++ b/internal/flows/proxy_flow.go @@ -85,8 +85,6 @@ func (f *proxyFlow) Run(ctx context.Context, args []string, parsedCmd *packagema return nil } - ui.SetStatus("Initializing proxy mode...") - // Setup CA certificate for MITM caCert, caCertPath, err := f.setupCACertificate() if err != nil { diff --git a/internal/ui/report.go b/internal/ui/report.go index ecf39ad..eb2b5df 100644 --- a/internal/ui/report.go +++ b/internal/ui/report.go @@ -119,6 +119,8 @@ func (r *ReportData) WasSuccessful() bool { func Report(data *ReportData) { data.Finalize() + StopSpinner() + switch verbosityLevel { case VerbosityLevelSilent: reportSilent(data) @@ -133,7 +135,6 @@ func Report(data *ReportData) { // Normal successful execution produces no output func reportSilent(data *ReportData) { // Silent mode: no report output - // Block messages and errors are already shown via ui.Block() and ui.ErrorExit() } // reportNormal shows minimal, assuring output @@ -171,6 +172,12 @@ func reportNormal(data *ReportData) { switch data.Outcome { case OutcomeBlocked: + fmt.Println() + fmt.Printf("%s %s\n", Colors.Red("✗"), Colors.Red("Malicious package blocked")) + + printMaliciousPackagesList(data.BlockedPackages) + fmt.Println() + icon = Colors.Red("✗") message = fmt.Sprintf("PMG: %d packages analyzed, %d blocked", data.TotalAnalyzed, data.BlockedCount) diff --git a/internal/ui/spinner.go b/internal/ui/spinner.go index 5f43b49..021bfb0 100644 --- a/internal/ui/spinner.go +++ b/internal/ui/spinner.go @@ -2,10 +2,14 @@ package ui import ( "fmt" + "sync" "time" ) -var spinnerChan chan bool +var ( + spinnerChan chan bool + spinnerMu sync.Mutex +) func StartSpinner(msg string) { StartSpinnerWithColor(msg, Colors.Normal) @@ -20,7 +24,15 @@ func StartSpinnerWithColor(msg string, c ColorFn) { frames := []rune(style) length := len(frames) + spinnerMu.Lock() + + // If a previous spinner exists, stop it cleanly before starting a new one + if spinnerChan != nil { + close(spinnerChan) + spinnerChan = nil + } spinnerChan = make(chan bool) + spinnerMu.Unlock() ticker := time.NewTicker(100 * time.Millisecond) go func() { @@ -40,6 +52,13 @@ func StartSpinnerWithColor(msg string, c ColorFn) { } func StopSpinner() { + spinnerMu.Lock() + defer spinnerMu.Unlock() + + if spinnerChan == nil { + return + } + // Gracefully handle the case where the spinner is already stopped // and the channel is closed, yet client code calls StopSpinner() again. defer func() { @@ -48,6 +67,8 @@ func StopSpinner() { close(spinnerChan) + spinnerChan = nil + fmt.Printf("\r") fmt.Println() }