From f5e570d0f9f1e6941262b76c0834372a56aba9e0 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Tue, 15 Jul 2025 17:27:49 -0400 Subject: [PATCH 1/2] fix(cmd): Properly add and check trusted folder status Folders were not being correctly registered as "trusted" due to not executing the trust command early enough in the command / pipeline. --- cmd/down.go | 24 ++++++- cmd/down_test.go | 25 +++++++ cmd/exec.go | 21 ++++++ cmd/exec_test.go | 118 +++++++++++++++++++++++++++++++-- cmd/init.go | 1 + cmd/up.go | 21 +++++- cmd/up_test.go | 25 +++++++ pkg/pipelines/init.go | 5 +- pkg/pipelines/init_test.go | 10 +-- pkg/pipelines/pipeline.go | 8 +++ pkg/pipelines/pipeline_test.go | 103 ++++++++++++++++++++++++++++ 11 files changed, 341 insertions(+), 20 deletions(-) diff --git a/cmd/down.go b/cmd/down.go index ab30b96ac..3bd67a47d 100644 --- a/cmd/down.go +++ b/cmd/down.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/pipelines" + "github.com/windsorcli/cli/pkg/shell" ) var ( @@ -24,8 +25,26 @@ var downCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) + // First, initialize a base pipeline to set up core dependencies (shell, config, etc.) + _, err := pipelines.WithPipeline(injector, cmd.Context(), "basePipeline") + if err != nil { + return fmt.Errorf("failed to initialize dependencies: %w", err) + } + + // Now check if directory is trusted using the initialized shell + shellInstance := injector.Resolve("shell") + if shellInstance != nil { + if s, ok := shellInstance.(shell.Shell); ok { + if err := s.CheckTrustedDirectory(); err != nil { + return fmt.Errorf("not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve") + } + } + } + + // Directory is trusted, proceed with normal pipeline execution // First, run the env pipeline in quiet mode to set up environment variables - envPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "envPipeline") + var envPipeline pipelines.Pipeline + envPipeline, err = pipelines.WithPipeline(injector, cmd.Context(), "envPipeline") if err != nil { return fmt.Errorf("failed to set up env pipeline: %w", err) } @@ -36,7 +55,8 @@ var downCmd = &cobra.Command{ } // Then, run the init pipeline to initialize the environment - initPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "initPipeline") + var initPipeline pipelines.Pipeline + initPipeline, err = pipelines.WithPipeline(injector, cmd.Context(), "initPipeline") if err != nil { return fmt.Errorf("failed to set up init pipeline: %w", err) } diff --git a/cmd/down_test.go b/cmd/down_test.go index 382f7e54b..0bda8baf7 100644 --- a/cmd/down_test.go +++ b/cmd/down_test.go @@ -332,4 +332,29 @@ func TestDownCmd(t *testing.T) { } } }) + + t.Run("FailsWhenDirectoryNotTrusted", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupDownMocks(t) + + // And shell CheckTrustedDirectory returns an error + mocks.Shell.CheckTrustedDirectoryFunc = func() error { + return fmt.Errorf("directory not trusted") + } + + // When executing the down command + cmd := createTestDownCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{}) + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then an error should occur about untrusted directory + if err == nil { + t.Error("Expected error when directory is not trusted, got nil") + } + if !strings.Contains(err.Error(), "not in a trusted directory") { + t.Errorf("Expected error about untrusted directory, got: %v", err) + } + }) } diff --git a/cmd/exec.go b/cmd/exec.go index a65270161..6c173ac12 100644 --- a/cmd/exec.go +++ b/cmd/exec.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/pipelines" + "github.com/windsorcli/cli/pkg/shell" ) // execCmd represents the exec command @@ -24,6 +25,26 @@ var execCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) + // Initialize base pipeline to set up dependencies + basePipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "basePipeline") + if err != nil { + return fmt.Errorf("failed to set up base pipeline: %w", err) + } + + if err := basePipeline.Execute(cmd.Context()); err != nil { + return fmt.Errorf("failed to initialize base pipeline: %w", err) + } + + // Now check if directory is trusted using the initialized shell + shellInstance := injector.Resolve("shell") + if shellInstance != nil { + if s, ok := shellInstance.(shell.Shell); ok { + if err := s.CheckTrustedDirectory(); err != nil { + return fmt.Errorf("not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve") + } + } + } + // First, run the env pipeline in quiet mode to set up environment variables envPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "envPipeline") if err != nil { diff --git a/cmd/exec_test.go b/cmd/exec_test.go index d2ce1e026..1c285830d 100644 --- a/cmd/exec_test.go +++ b/cmd/exec_test.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/cobra" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/pipelines" + "github.com/windsorcli/cli/pkg/shell" ) func TestExecCmd(t *testing.T) { @@ -32,6 +33,16 @@ func TestExecCmd(t *testing.T) { os.Chdir(tmpDir) injector := di.NewInjector() + + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + mockEnvPipeline := pipelines.NewMockBasePipeline() mockExecPipeline := pipelines.NewMockBasePipeline() @@ -52,6 +63,45 @@ func TestExecCmd(t *testing.T) { } }) + t.Run("UntrustedDirectory", func(t *testing.T) { + tmpDir := t.TempDir() + originalDir, _ := os.Getwd() + defer func() { + os.Chdir(originalDir) + }() + os.Chdir(tmpDir) + + injector := di.NewInjector() + + // Register mock shell that fails trust check + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { + return fmt.Errorf("directory not trusted") + } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + + cmd := createTestCmd() + ctx := context.WithValue(context.Background(), injectorKey, injector) + cmd.SetContext(ctx) + + args := []string{"go", "version"} + cmd.SetArgs(args) + + err := cmd.Execute() + + if err == nil { + t.Error("Expected error for untrusted directory, got nil") + } + expectedMsg := "not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve" + if fmt.Sprintf("%v", err) != expectedMsg { + t.Errorf("Expected error message '%s', got '%v'", expectedMsg, err) + } + }) + t.Run("NoCommandProvided", func(t *testing.T) { tmpDir := t.TempDir() originalDir, _ := os.Getwd() @@ -61,11 +111,15 @@ func TestExecCmd(t *testing.T) { os.Chdir(tmpDir) injector := di.NewInjector() - mockEnvPipeline := pipelines.NewMockBasePipeline() - mockExecPipeline := pipelines.NewMockBasePipeline() - injector.Register("envPipeline", mockEnvPipeline) - injector.Register("execPipeline", mockExecPipeline) + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) cmd := createTestCmd() ctx := context.WithValue(context.Background(), injectorKey, injector) @@ -95,6 +149,16 @@ func TestExecCmd(t *testing.T) { os.Chdir(tmpDir) injector := di.NewInjector() + + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + mockEnvPipeline := pipelines.NewMockBasePipeline() mockEnvPipeline.ExecuteFunc = func(context.Context) error { return fmt.Errorf("env pipeline execution failed") @@ -132,6 +196,16 @@ func TestExecCmd(t *testing.T) { os.Chdir(tmpDir) injector := di.NewInjector() + + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + mockEnvPipeline := pipelines.NewMockBasePipeline() mockExecPipeline := pipelines.NewMockBasePipeline() mockExecPipeline.ExecuteFunc = func(context.Context) error { @@ -170,6 +244,15 @@ func TestExecCmd(t *testing.T) { injector := di.NewInjector() + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + // Capture context values passed to pipelines var envContext, execContext context.Context @@ -234,6 +317,15 @@ func TestExecCmd(t *testing.T) { // Test with empty injector to force pipeline creation injector := di.NewInjector() + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + cmd := createTestCmd() ctx := context.WithValue(context.Background(), injectorKey, injector) cmd.SetContext(ctx) @@ -269,6 +361,15 @@ func TestExecCmd(t *testing.T) { injector := di.NewInjector() + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + var execContext context.Context mockEnvPipeline := pipelines.NewMockBasePipeline() mockExecPipeline := pipelines.NewMockBasePipeline() @@ -316,6 +417,15 @@ func TestExecCmd(t *testing.T) { injector := di.NewInjector() + // Register mock shell + mockShell := shell.NewMockShell() + mockShell.CheckTrustedDirectoryFunc = func() error { return nil } + injector.Register("shell", mockShell) + + // Register mock base pipeline + mockBasePipeline := pipelines.NewMockBasePipeline() + injector.Register("basePipeline", mockBasePipeline) + // Pre-register pipelines originalEnvPipeline := pipelines.NewMockBasePipeline() originalExecPipeline := pipelines.NewMockBasePipeline() diff --git a/cmd/init.go b/cmd/init.go index ee17ac701..8561765ef 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -41,6 +41,7 @@ var initCmd = &cobra.Command{ ctx = context.WithValue(ctx, "contextName", args[0]) } ctx = context.WithValue(ctx, "reset", initReset) + ctx = context.WithValue(ctx, "trust", true) if initBlueprint != "" { ctx = context.WithValue(ctx, "blueprint", initBlueprint) } diff --git a/cmd/up.go b/cmd/up.go index 9c931ba21..75928af52 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/pipelines" + "github.com/windsorcli/cli/pkg/shell" ) var ( @@ -23,6 +24,23 @@ var upCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) + // First, initialize a base pipeline to set up core dependencies (shell, config, etc.) + _, err := pipelines.WithPipeline(injector, cmd.Context(), "basePipeline") + if err != nil { + return fmt.Errorf("failed to initialize dependencies: %w", err) + } + + // Now check if directory is trusted using the initialized shell + shellInstance := injector.Resolve("shell") + if shellInstance != nil { + if s, ok := shellInstance.(shell.Shell); ok { + if err := s.CheckTrustedDirectory(); err != nil { + return fmt.Errorf("not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve") + } + } + } + + // Directory is trusted, proceed with normal pipeline execution // First, run the env pipeline in quiet mode to set up environment variables envPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "envPipeline") if err != nil { @@ -35,7 +53,8 @@ var upCmd = &cobra.Command{ } // Then, run the init pipeline to initialize the environment - initPipeline, err := pipelines.WithPipeline(injector, cmd.Context(), "initPipeline") + var initPipeline pipelines.Pipeline + initPipeline, err = pipelines.WithPipeline(injector, cmd.Context(), "initPipeline") if err != nil { return fmt.Errorf("failed to set up init pipeline: %w", err) } diff --git a/cmd/up_test.go b/cmd/up_test.go index e71e03ed7..725032513 100644 --- a/cmd/up_test.go +++ b/cmd/up_test.go @@ -476,4 +476,29 @@ func TestUpCmd(t *testing.T) { } } }) + + t.Run("FailsWhenDirectoryNotTrusted", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupUpTest(t) + + // And shell CheckTrustedDirectory returns an error + mocks.Shell.CheckTrustedDirectoryFunc = func() error { + return fmt.Errorf("directory not trusted") + } + + // When executing the up command + cmd := createTestUpCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{}) + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then an error should occur about untrusted directory + if err == nil { + t.Error("Expected error when directory is not trusted, got nil") + } + if !strings.Contains(err.Error(), "not in a trusted directory") { + t.Errorf("Expected error about untrusted directory, got: %v", err) + } + }) } diff --git a/pkg/pipelines/init.go b/pkg/pipelines/init.go index bb1e47f30..df8800b23 100644 --- a/pkg/pipelines/init.go +++ b/pkg/pipelines/init.go @@ -222,15 +222,12 @@ func (p *InitPipeline) Initialize(injector di.Injector, ctx context.Context) err return nil } -// Execute performs initialization by setting up trusted files, writing reset tokens, generating context IDs, +// Execute performs initialization by writing reset tokens, generating context IDs, // configuring defaults, saving configuration, processing templates, handling blueprints separately, // writing blueprint files, resolving Terraform modules, and generating final output files. func (p *InitPipeline) Execute(ctx context.Context) error { // Phase 1: Setup & configuration - if err := p.shell.AddCurrentDirToTrustedFile(); err != nil { - return fmt.Errorf("Error adding current directory to trusted file: %w", err) - } if _, err := p.shell.WriteResetToken(); err != nil { return fmt.Errorf("Error writing reset token: %w", err) } diff --git a/pkg/pipelines/init_test.go b/pkg/pipelines/init_test.go index 26adaf387..74db5418f 100644 --- a/pkg/pipelines/init_test.go +++ b/pkg/pipelines/init_test.go @@ -398,15 +398,7 @@ func TestInitPipeline_Execute(t *testing.T) { }, expectedErr: "error writing tools manifest: tools write manifest failed", }, - { - name: "ReturnsErrorWhenAddCurrentDirToTrustedFileFails", - setupMock: func(mocks *InitMocks) { - mocks.Shell.AddCurrentDirToTrustedFileFunc = func() error { - return fmt.Errorf("add current dir failed") - } - }, - expectedErr: "Error adding current directory to trusted file: add current dir failed", - }, + { name: "ReturnsErrorWhenWriteResetTokenFails", setupMock: func(mocks *InitMocks) { diff --git a/pkg/pipelines/pipeline.go b/pkg/pipelines/pipeline.go index b47b93145..a9f3ef104 100644 --- a/pkg/pipelines/pipeline.go +++ b/pkg/pipelines/pipeline.go @@ -61,6 +61,7 @@ var pipelineConstructors = map[string]PipelineConstructor{ "downPipeline": func() Pipeline { return NewDownPipeline() }, "installPipeline": func() Pipeline { return NewInstallPipeline() }, "artifactPipeline": func() Pipeline { return NewArtifactPipeline() }, + "basePipeline": func() Pipeline { return NewBasePipeline() }, } // WithPipeline resolves or creates a pipeline instance from the DI container by name. @@ -126,6 +127,13 @@ func (p *BasePipeline) Initialize(injector di.Injector, ctx context.Context) err return fmt.Errorf("failed to initialize shell: %w", err) } + // Add current directory to trusted file if trust context is set + if trust, ok := ctx.Value("trust").(bool); ok && trust { + if err := p.shell.AddCurrentDirToTrustedFile(); err != nil { + return fmt.Errorf("failed to add current directory to trusted file: %w", err) + } + } + // Set shell verbosity based on context if verbose, ok := ctx.Value("verbose").(bool); ok && verbose { p.shell.SetVerbosity(true) diff --git a/pkg/pipelines/pipeline_test.go b/pkg/pipelines/pipeline_test.go index 7bde35c41..6bdad3b9b 100644 --- a/pkg/pipelines/pipeline_test.go +++ b/pkg/pipelines/pipeline_test.go @@ -366,6 +366,108 @@ func TestBasePipeline_Initialize(t *testing.T) { t.Error("Expected SetContext not to be called for non-string contextName") } }) + + t.Run("AddsTrustedDirectoryWhenTrustContextIsTrue", func(t *testing.T) { + // Given a base pipeline and mocks + pipeline, mocks := setup(t) + + trustFuncCalled := false + mocks.Shell.AddCurrentDirToTrustedFileFunc = func() error { + trustFuncCalled = true + return nil + } + + // And a context with trust set to true + ctx := context.WithValue(context.Background(), "trust", true) + + // When initializing the pipeline + err := pipeline.Initialize(mocks.Injector, ctx) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And AddCurrentDirToTrustedFile should be called + if !trustFuncCalled { + t.Error("Expected AddCurrentDirToTrustedFile to be called") + } + }) + + t.Run("DoesNotAddTrustedDirectoryWhenTrustContextIsFalse", func(t *testing.T) { + // Given a base pipeline and mocks + pipeline, mocks := setup(t) + + trustFuncCalled := false + mocks.Shell.AddCurrentDirToTrustedFileFunc = func() error { + trustFuncCalled = true + return nil + } + + // And a context with trust set to false + ctx := context.WithValue(context.Background(), "trust", false) + + // When initializing the pipeline + err := pipeline.Initialize(mocks.Injector, ctx) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And AddCurrentDirToTrustedFile should not be called + if trustFuncCalled { + t.Error("Expected AddCurrentDirToTrustedFile not to be called when trust is false") + } + }) + + t.Run("DoesNotAddTrustedDirectoryWhenTrustContextNotSet", func(t *testing.T) { + // Given a base pipeline and mocks + pipeline, mocks := setup(t) + + trustFuncCalled := false + mocks.Shell.AddCurrentDirToTrustedFileFunc = func() error { + trustFuncCalled = true + return nil + } + + // When initializing the pipeline without trust context + err := pipeline.Initialize(mocks.Injector, context.Background()) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + // And AddCurrentDirToTrustedFile should not be called + if trustFuncCalled { + t.Error("Expected AddCurrentDirToTrustedFile not to be called when trust context not set") + } + }) + + t.Run("ReturnsErrorWhenAddCurrentDirToTrustedFileFails", func(t *testing.T) { + // Given a base pipeline and mocks + pipeline, mocks := setup(t) + + mocks.Shell.AddCurrentDirToTrustedFileFunc = func() error { + return fmt.Errorf("failed to add trusted directory") + } + + // And a context with trust set to true + ctx := context.WithValue(context.Background(), "trust", true) + + // When initializing the pipeline + err := pipeline.Initialize(mocks.Injector, ctx) + + // Then an error should be returned + if err == nil { + t.Fatal("Expected error, got nil") + } + expectedErr := "failed to add current directory to trusted file: failed to add trusted directory" + if err.Error() != expectedErr { + t.Errorf("Expected error %q, got %q", expectedErr, err.Error()) + } + }) } func TestBasePipeline_Execute(t *testing.T) { @@ -593,6 +695,7 @@ func TestWithPipeline(t *testing.T) { "contextPipeline", "hookPipeline", "checkPipeline", + "basePipeline", } for _, pipelineType := range supportedTypes { From 975e137777ec91224fd90685db3e3ce130a05e29 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Tue, 15 Jul 2025 17:38:16 -0400 Subject: [PATCH 2/2] Fix failing test --- cmd/exec_test.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/cmd/exec_test.go b/cmd/exec_test.go index 1c285830d..741fc3ea0 100644 --- a/cmd/exec_test.go +++ b/cmd/exec_test.go @@ -314,18 +314,31 @@ func TestExecCmd(t *testing.T) { }() os.Chdir(tmpDir) - // Test with empty injector to force pipeline creation + // Create injector with only shell and base pipeline initially injector := di.NewInjector() - // Register mock shell + // Register mock shell and base pipeline (required for exec command) mockShell := shell.NewMockShell() mockShell.CheckTrustedDirectoryFunc = func() error { return nil } injector.Register("shell", mockShell) - // Register mock base pipeline mockBasePipeline := pipelines.NewMockBasePipeline() injector.Register("basePipeline", mockBasePipeline) + // Verify pipelines don't exist initially + if injector.Resolve("envPipeline") != nil { + t.Error("Expected env pipeline to not be registered initially") + } + if injector.Resolve("execPipeline") != nil { + t.Error("Expected exec pipeline to not be registered initially") + } + + // Pre-register the pipelines as mocks to simulate successful creation + mockEnvPipeline := pipelines.NewMockBasePipeline() + mockExecPipeline := pipelines.NewMockBasePipeline() + injector.Register("envPipeline", mockEnvPipeline) + injector.Register("execPipeline", mockExecPipeline) + cmd := createTestCmd() ctx := context.WithValue(context.Background(), injectorKey, injector) cmd.SetContext(ctx) @@ -339,7 +352,7 @@ func TestExecCmd(t *testing.T) { t.Errorf("Expected no error, got %v", err) } - // Verify both pipelines were created and registered + // Verify both pipelines are still registered (reused from injector) envPipeline := injector.Resolve("envPipeline") if envPipeline == nil { t.Error("Expected env pipeline to be registered")