diff --git a/internal/cache/cache.go b/internal/cache/cache.go index c2d002c..9de89e9 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -11,7 +11,7 @@ import ( "github.com/hashicorp/go-hclog" - "github.com/mozilla-ai/mcpd/v2/internal/context" + "github.com/mozilla-ai/mcpd/v2/internal/files" ) // Cache manages cached registry manifests. @@ -42,7 +42,7 @@ func NewCache(logger hclog.Logger, opts ...Option) (*Cache, error) { // Only create cache directory if caching is enabled. if options.enabled { - if err := context.EnsureAtLeastRegularDir(options.dir); err != nil { + if err := files.EnsureAtLeastRegularDir(options.dir); err != nil { return nil, fmt.Errorf("failed to create cache directory: %w", err) } } diff --git a/internal/config/plugin_config.go b/internal/config/plugin_config.go index 35891fa..0165ed1 100644 --- a/internal/config/plugin_config.go +++ b/internal/config/plugin_config.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/mozilla-ai/mcpd/v2/internal/context" + "github.com/mozilla-ai/mcpd/v2/internal/files" ) const ( @@ -247,9 +248,45 @@ func (p *PluginConfig) Validate() error { } } + // Validate directory and plugins if Dir is configured. + if err := p.validatePluginDirectory(); err != nil { + validationErrors = append(validationErrors, err) + } + return errors.Join(validationErrors...) } +// validatePluginDirectory validates that the plugin directory exists and contains all configured plugins. +// Returns nil if Dir is empty (plugins disabled). +func (p *PluginConfig) validatePluginDirectory() error { + if strings.TrimSpace(p.Dir) == "" { + return nil + } + + available, err := files.DiscoverExecutables(p.Dir) + if err != nil { + return fmt.Errorf("plugin directory %s: %w", p.Dir, err) + } + + return p.validateConfiguredPluginsExist(available) +} + +// validateConfiguredPluginsExist checks that all configured plugins exist in the available set. +func (p *PluginConfig) validateConfiguredPluginsExist(available map[string]struct{}) error { + var missingPlugins []error + + for name := range p.PluginNamesDistinct() { + if _, exists := available[name]; !exists { + missingPlugins = append( + missingPlugins, + fmt.Errorf("plugin %s not found in directory %s", name, p.Dir), + ) + } + } + + return errors.Join(missingPlugins...) +} + // categorySlice returns a pointer to the category slice for the given category name. func (p *PluginConfig) categorySlice(category Category) (*[]PluginEntry, error) { switch category { diff --git a/internal/context/context.go b/internal/context/context.go index 6c8ce22..fcb9bf3 100644 --- a/internal/context/context.go +++ b/internal/context/context.go @@ -12,17 +12,10 @@ import ( "github.com/BurntSushi/toml" + "github.com/mozilla-ai/mcpd/v2/internal/files" "github.com/mozilla-ai/mcpd/v2/internal/perms" ) -const ( - // EnvVarXDGConfigHome is the XDG Base Directory env var name for config files. - EnvVarXDGConfigHome = "XDG_CONFIG_HOME" - - // EnvVarXDGCacheHome is the XDG Base Directory env var name for cache file. - EnvVarXDGCacheHome = "XDG_CACHE_HOME" -) - // DefaultLoader loads execution context configurations. type DefaultLoader struct{} @@ -126,13 +119,13 @@ func (c *ExecutionContextConfig) List() []ServerExecutionContext { // SaveConfig saves the execution context configuration to a file with secure permissions. // Used for runtime execution contexts that may contain sensitive data. func (c *ExecutionContextConfig) SaveConfig() error { - return c.saveConfig(EnsureAtLeastSecureDir, perms.SecureFile) + return c.saveConfig(files.EnsureAtLeastSecureDir, perms.SecureFile) } // SaveExportedConfig saves the execution context configuration to a file with regular permissions. // Used for exported configurations that are sanitized and suitable for sharing. func (c *ExecutionContextConfig) SaveExportedConfig() error { - return c.saveConfig(EnsureAtLeastRegularDir, perms.RegularFile) + return c.saveConfig(files.EnsureAtLeastRegularDir, perms.RegularFile) } // Upsert updates the execution context for the given server name. @@ -216,27 +209,6 @@ func (s *ServerExecutionContext) IsEmpty() bool { return len(s.Args) == 0 && len(s.Env) == 0 && len(s.Volumes) == 0 } -// AppDirName returns the name of the application directory for use in user-specific operations where data is being written. -func AppDirName() string { - return "mcpd" -} - -// EnsureAtLeastRegularDir creates a directory with standard permissions if it doesn't exist, -// and verifies that it has at least the required regular permissions if it already exists. -// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error. -// Used for cache directories, data directories, and documentation. -func EnsureAtLeastRegularDir(path string) error { - return ensureAtLeastDir(path, perms.RegularDir) -} - -// EnsureAtLeastSecureDir creates a directory with secure permissions if it doesn't exist, -// and verifies that it has at least the required secure permissions if it already exists. -// It does not attempt to repair ownership or permissions: if they are wrong, -// it returns an error. -func EnsureAtLeastSecureDir(path string) error { - return ensureAtLeastDir(path, perms.SecureDir) -} - // NewExecutionContextConfig returns a newly initialized ExecutionContextConfig. func NewExecutionContextConfig(path string) *ExecutionContextConfig { return &ExecutionContextConfig{ @@ -245,46 +217,6 @@ func NewExecutionContextConfig(path string) *ExecutionContextConfig { } } -// UserSpecificCacheDir returns the directory that should be used to store any user-specific cache files. -// It adheres to the XDG Base Directory Specification, respecting the XDG_CACHE_HOME environment variable. -// When XDG_CACHE_HOME is not set, it defaults to ~/.cache/mcpd/ -// See: https://specifications.freedesktop.org/basedir-spec/latest/ -func UserSpecificCacheDir() (string, error) { - return userSpecificDir(EnvVarXDGCacheHome, ".cache") -} - -// UserSpecificConfigDir returns the directory that should be used to store any user-specific configuration. -// It adheres to the XDG Base Directory Specification, respecting the XDG_CONFIG_HOME environment variable. -// When XDG_CONFIG_HOME is not set, it defaults to ~/.config/mcpd/ -// See: https://specifications.freedesktop.org/basedir-spec/latest/ -func UserSpecificConfigDir() (string, error) { - return userSpecificDir(EnvVarXDGConfigHome, ".config") -} - -// ensureAtLeastDir creates a directory with the specified permissions if it doesn't exist, -// and verifies that it has at least the required permissions if it already exists. -// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error. -func ensureAtLeastDir(path string, perm os.FileMode) error { - if err := os.MkdirAll(path, perm); err != nil { - return fmt.Errorf("could not ensure directory exists for '%s': %w", path, err) - } - - info, err := os.Stat(path) - if err != nil { - return fmt.Errorf("could not stat directory '%s': %w", path, err) - } - - if !isPermissionAcceptable(info.Mode().Perm(), perm) { - return fmt.Errorf( - "incorrect permissions for directory '%s' (%#o, want %#o or more restrictive)", - path, info.Mode().Perm(), - perm, - ) - } - - return nil -} - // equalSlices compares two string slices for equality, ignoring order. func equalSlices(a []string, b []string) bool { if len(a) != len(b) { @@ -300,14 +232,6 @@ func equalSlices(a []string, b []string) bool { return slices.Equal(sortedA, sortedB) } -// isPermissionAcceptable checks if the actual permissions are acceptable for the required permissions. -// It returns true if the actual permissions are equal to or more restrictive than required. -// "More restrictive" means: no permission bit set in actual that isn't also set in required. -func isPermissionAcceptable(actual, required os.FileMode) bool { - // Check that actual doesn't grant any permissions that required doesn't grant - return (actual & ^required) == 0 -} - // loadExecutionContextConfig loads a runtime execution context file from disk and expands environment variables. // // The function parses the TOML file at the specified path and automatically expands all ${VAR} references @@ -396,31 +320,3 @@ func (c *ExecutionContextConfig) saveConfig(ensureDirFunc func(string) error, fi return nil } - -// userSpecificDir returns a user-specific directory following XDG Base Directory Specification. -// It respects the given environment variable, falling back to homeDir/dir/AppDirName() if not set. -// The envVar must have XDG_ prefix to follow the specification. -func userSpecificDir(envVar string, dir string) (string, error) { - envVar = strings.TrimSpace(envVar) - // Validate that the environment variable follows XDG naming convention. - if !strings.HasPrefix(envVar, "XDG_") { - return "", fmt.Errorf( - "environment variable '%s' does not follow XDG Base Directory Specification", - envVar, - ) - } - - // If the relevant environment variable is present and configured, then use it. - if ch, ok := os.LookupEnv(envVar); ok && strings.TrimSpace(ch) != "" { - home := strings.TrimSpace(ch) - return filepath.Join(home, AppDirName()), nil - } - - // Attempt to locate the home directory for the current user and return the path that follows the spec. - homeDir, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("failed to get user home directory: %w", err) - } - - return filepath.Join(homeDir, dir, AppDirName()), nil -} diff --git a/internal/context/context_test.go b/internal/context/context_test.go index 15f341c..4a63697 100644 --- a/internal/context/context_test.go +++ b/internal/context/context_test.go @@ -7,8 +7,6 @@ import ( "testing" "github.com/stretchr/testify/require" - - "github.com/mozilla-ai/mcpd/v2/internal/perms" ) func TestLoadOrInitExecutionContext(t *testing.T) { @@ -119,150 +117,6 @@ func TestSaveAndLoadExecutionContextConfig(t *testing.T) { require.Equal(t, original, loaded) } -func TestAppDirName(t *testing.T) { - t.Parallel() - - require.Equal(t, "mcpd", AppDirName()) -} - -func TestContext_UserSpecificConfigDir(t *testing.T) { - tests := []struct { - name string - xdgValue string - expectedDir func(t *testing.T) string - }{ - { - name: "XDG_CONFIG_HOME is set and used", - xdgValue: "/custom/xdg/path", - expectedDir: func(t *testing.T) string { - return filepath.Join("/custom/xdg/path", AppDirName()) - }, - }, - { - name: "XDG_CONFIG_HOME is set with whitespace and trimmed", - xdgValue: " /trimmed/xdg/path ", - expectedDir: func(t *testing.T) string { - return filepath.Join("/trimmed/xdg/path", AppDirName()) - }, - }, - { - name: "XDG_CONFIG_HOME is empty, fall back to default", - xdgValue: "", - expectedDir: func(t *testing.T) string { - home, err := os.UserHomeDir() - require.NoError(t, err) - return filepath.Join(home, ".config", AppDirName()) - }, - }, - { - name: "XDG_CONFIG_HOME is only whitespace, fall back to default", - xdgValue: " ", - expectedDir: func(t *testing.T) string { - home, err := os.UserHomeDir() - require.NoError(t, err) - return filepath.Join(home, ".config", AppDirName()) - }, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Setenv(EnvVarXDGConfigHome, tc.xdgValue) - - result, err := UserSpecificConfigDir() - require.NoError(t, err) - require.Equal(t, tc.expectedDir(t), result) - }) - } -} - -func TestUserSpecificDir_InvalidEnvVar(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - envVar string - dir string - }{ - { - name: "environment variable without XDG_ prefix", - envVar: "CONFIG_HOME", - dir: ".config", - }, - { - name: "empty environment variable name", - envVar: "", - dir: ".cache", - }, - { - name: "environment variable with wrong prefix", - envVar: "CACHE_HOME", - dir: ".cache", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - _, err := userSpecificDir(tc.envVar, tc.dir) - require.Error(t, err) - require.Contains(t, err.Error(), "does not follow XDG Base Directory Specification") - }) - } -} - -func TestContext_UserSpecificCacheDir(t *testing.T) { - tests := []struct { - name string - xdgValue string - expectedDir func(t *testing.T) string - }{ - { - name: "XDG_CACHE_HOME is set and used", - xdgValue: "/custom/cache/path", - expectedDir: func(t *testing.T) string { - return filepath.Join("/custom/cache/path", AppDirName()) - }, - }, - { - name: "XDG_CACHE_HOME is set with whitespace and trimmed", - xdgValue: " /trimmed/cache/path ", - expectedDir: func(t *testing.T) string { - return filepath.Join("/trimmed/cache/path", AppDirName()) - }, - }, - { - name: "XDG_CACHE_HOME is empty, fall back to default", - xdgValue: "", - expectedDir: func(t *testing.T) string { - home, err := os.UserHomeDir() - require.NoError(t, err) - return filepath.Join(home, ".cache", AppDirName()) - }, - }, - { - name: "XDG_CACHE_HOME is only whitespace, fall back to default", - xdgValue: " ", - expectedDir: func(t *testing.T) string { - home, err := os.UserHomeDir() - require.NoError(t, err) - return filepath.Join(home, ".cache", AppDirName()) - }, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Setenv(EnvVarXDGCacheHome, tc.xdgValue) - - result, err := UserSpecificCacheDir() - require.NoError(t, err) - require.Equal(t, tc.expectedDir(t), result) - }) - } -} - func TestUpsert(t *testing.T) { t.Parallel() @@ -657,498 +511,6 @@ DEBUG = "true"` require.Equal(t, expected, cfg.Servers) } -func TestIsPermissionAcceptable(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - actual os.FileMode - required os.FileMode - want bool - }{ - // Exact matches should always be acceptable - { - name: "exact match 0755", - actual: 0o755, - required: 0o755, - want: true, - }, - { - name: "exact match 0700", - actual: 0o700, - required: 0o700, - want: true, - }, - { - name: "exact match 0644", - actual: 0o644, - required: 0o644, - want: true, - }, - // More restrictive should be acceptable - { - name: "0700 is acceptable when 0755 is required", - actual: 0o700, - required: 0o755, - want: true, - }, - { - name: "0600 is acceptable when 0644 is required", - actual: 0o600, - required: 0o644, - want: true, - }, - { - name: "0000 is acceptable for any requirement (most restrictive)", - actual: 0o000, - required: 0o755, - want: true, - }, - // Less restrictive should NOT be acceptable - { - name: "0755 is not acceptable when 0700 is required", - actual: 0o755, - required: 0o700, - want: false, - }, - { - name: "0777 is not acceptable when 0755 is required", - actual: 0o777, - required: 0o755, - want: false, - }, - { - name: "0666 is not acceptable when 0644 is required", - actual: 0o666, - required: 0o644, - want: false, - }, - // Different permission patterns - { - name: "0711 is acceptable when 0755 is required (more restrictive for group/others)", - actual: 0o711, - required: 0o755, - want: true, - }, - { - name: "0750 is acceptable when 0755 is required (more restrictive for others)", - actual: 0o750, - required: 0o755, - want: true, - }, - { - name: "0705 is acceptable when 0755 is required (more restrictive for group)", - actual: 0o705, - required: 0o755, - want: true, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - got := isPermissionAcceptable(tc.actual, tc.required) - require.Equal( - t, - tc.want, - got, - "isPermissionAcceptable(%#o, %#o) should return %v", - tc.actual, - tc.required, - tc.want, - ) - }) - } -} - -func TestEnsureAtLeastSecureDir(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - setup func(t *testing.T) string - wantErr bool - errMsg string - }{ - { - name: "creates directory when it doesn't exist", - setup: func(t *testing.T) string { - return filepath.Join(t.TempDir(), "new-secure-dir") - }, - wantErr: false, - }, - { - name: "succeeds when directory exists with correct permissions", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "existing-secure-dir") - err := os.MkdirAll(dir, perms.SecureDir) - require.NoError(t, err) - return dir - }, - wantErr: false, - }, - { - name: "fails when directory exists with wrong permissions (0755)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "wrong-perms-755") - err := os.MkdirAll(dir, 0o755) - require.NoError(t, err) - return dir - }, - wantErr: true, - errMsg: "incorrect permissions", - }, - { - name: "fails when directory exists with wrong permissions (0644)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "wrong-perms-644") - err := os.MkdirAll(dir, 0o644) - require.NoError(t, err) - return dir - }, - wantErr: true, - errMsg: "incorrect permissions", - }, - { - name: "fails when directory exists with overly permissive settings (0777)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "wrong-perms-777") - err := os.MkdirAll(dir, 0o777) - require.NoError(t, err) - return dir - }, - wantErr: true, - errMsg: "incorrect permissions", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - path := tc.setup(t) - err := EnsureAtLeastSecureDir(path) - - if tc.wantErr { - require.Error(t, err) - if tc.errMsg != "" { - require.Contains(t, err.Error(), tc.errMsg) - } - } else { - require.NoError(t, err) - - // Verify the directory exists and has acceptable permissions. - info, statErr := os.Stat(path) - require.NoError(t, statErr) - require.True(t, info.IsDir()) - // For secure directories, we typically get exactly what we asked for - require.True(t, isPermissionAcceptable(info.Mode().Perm(), perms.SecureDir), - "Directory permissions %#o should be acceptable for secure requirement %#o", - info.Mode().Perm(), perms.SecureDir) - } - }) - } -} - -func TestEnsureAtLeastSecureDirWithNestedPaths(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - relativePath string - setup func(t *testing.T, basePath string) string - wantErr bool - errMsg string - }{ - { - name: "creates nested path when none exists", - relativePath: "a/b/c/secure", - setup: func(t *testing.T, basePath string) string { - return filepath.Join(basePath, "a/b/c/secure") - }, - wantErr: false, - }, - { - name: "succeeds when parent directories have correct permissions", - relativePath: "parent/child", - setup: func(t *testing.T, basePath string) string { - parentPath := filepath.Join(basePath, "parent") - err := os.MkdirAll(parentPath, perms.SecureDir) - require.NoError(t, err) - return filepath.Join(parentPath, "child") - }, - wantErr: false, - }, - { - name: "creates child even when parent has different permissions", - relativePath: "regular-parent/secure-child", - setup: func(t *testing.T, basePath string) string { - parentPath := filepath.Join(basePath, "regular-parent") - err := os.MkdirAll(parentPath, perms.RegularDir) - require.NoError(t, err) - return filepath.Join(parentPath, "secure-child") - }, - wantErr: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - baseDir := t.TempDir() - path := tc.setup(t, baseDir) - err := EnsureAtLeastSecureDir(path) - - if tc.wantErr { - require.Error(t, err) - if tc.errMsg != "" { - require.Contains(t, err.Error(), tc.errMsg) - } - } else { - require.NoError(t, err) - - // Verify the target directory has acceptable secure permissions. - info, statErr := os.Stat(path) - require.NoError(t, statErr) - require.True(t, info.IsDir()) - require.True(t, isPermissionAcceptable(info.Mode().Perm(), perms.SecureDir), - "Directory permissions %#o should be acceptable for secure requirement %#o", - info.Mode().Perm(), perms.SecureDir) - } - }) - } -} - -func TestEnsureAtLeastSecureDirErrorMessages(t *testing.T) { - t.Parallel() - - t.Run("permission error shows expected vs actual permissions", func(t *testing.T) { - t.Parallel() - - dir := filepath.Join(t.TempDir(), "test-dir") - err := os.MkdirAll(dir, 0o755) - require.NoError(t, err) - - err = EnsureAtLeastSecureDir(dir) - require.Error(t, err) - - // Check that error message contains both actual and expected permissions in octal format. - require.Contains(t, err.Error(), "0755", "Error should show actual permissions") - require.Contains(t, err.Error(), "0700", "Error should show expected permissions") - require.Contains(t, err.Error(), dir, "Error should include the path") - }) -} - -func TestEnsureAtLeastRegularDir(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - setup func(t *testing.T) string - wantErr bool - errMsg string - }{ - { - name: "creates directory when it doesn't exist", - setup: func(t *testing.T) string { - return filepath.Join(t.TempDir(), "new-regular-dir") - }, - wantErr: false, - }, - { - name: "succeeds when directory exists with correct permissions", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "existing-regular-dir") - err := os.MkdirAll(dir, perms.RegularDir) - require.NoError(t, err) - return dir - }, - wantErr: false, - }, - { - name: "succeeds when directory exists with more restrictive permissions (0700)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "more-restrictive-700") - err := os.MkdirAll(dir, 0o700) - require.NoError(t, err) - return dir - }, - wantErr: false, - }, - { - name: "fails when directory exists with less restrictive permissions (0777)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "wrong-perms-777") - err := os.MkdirAll(dir, 0o777) - require.NoError(t, err) - // Create directory and explicitly set permissions to override umask. - // Without chmod, umask (typically 022) would convert 0777 -> 0755. - // For tests that need exact permissions, we must use os.Chmod() after creation. - err = os.Chmod(dir, 0o777) - require.NoError(t, err) - return dir - }, - wantErr: true, - errMsg: "incorrect permissions", - }, - { - name: "succeeds when directory exists with more restrictive permissions (0711)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "more-restrictive-711") - err := os.MkdirAll(dir, 0o711) - require.NoError(t, err) - return dir - }, - wantErr: false, - }, - { - name: "succeeds when directory exists with more restrictive permissions (0750)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "more-restrictive-750") - err := os.MkdirAll(dir, 0o750) - require.NoError(t, err) - return dir - }, - wantErr: false, - }, - { - name: "succeeds when directory exists with more restrictive permissions (0644)", - setup: func(t *testing.T) string { - dir := filepath.Join(t.TempDir(), "more-restrictive-644") - err := os.MkdirAll(dir, 0o644) - require.NoError(t, err) - return dir - }, - wantErr: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - path := tc.setup(t) - err := EnsureAtLeastRegularDir(path) - - if tc.wantErr { - require.Error(t, err) - if tc.errMsg != "" { - require.Contains(t, err.Error(), tc.errMsg) - } - } else { - require.NoError(t, err) - - // Verify the directory exists and has acceptable permissions. - info, statErr := os.Stat(path) - require.NoError(t, statErr) - require.True(t, info.IsDir()) - // Permissions should be at least as restrictive as required - require.True(t, isPermissionAcceptable(info.Mode().Perm(), perms.RegularDir), - "Directory permissions %#o should be acceptable for requirement %#o", - info.Mode().Perm(), perms.RegularDir) - } - }) - } -} - -func TestEnsureAtLeastRegularDirWithNestedPaths(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - relativePath string - setup func(t *testing.T, basePath string) string - wantErr bool - errMsg string - }{ - { - name: "creates nested path when none exists", - relativePath: "cache/registry/manifests/nested", - setup: func(t *testing.T, basePath string) string { - return filepath.Join(basePath, "cache/registry/manifests/nested") - }, - wantErr: false, - }, - { - name: "succeeds when parent directories have correct permissions", - relativePath: "parent/child", - setup: func(t *testing.T, basePath string) string { - parentPath := filepath.Join(basePath, "parent") - err := os.MkdirAll(parentPath, perms.RegularDir) - require.NoError(t, err) - return filepath.Join(parentPath, "child") - }, - wantErr: false, - }, - { - name: "creates child even when parent has different permissions", - relativePath: "secure-parent/regular-child", - setup: func(t *testing.T, basePath string) string { - parentPath := filepath.Join(basePath, "secure-parent") - err := os.MkdirAll(parentPath, perms.SecureDir) - require.NoError(t, err) - return filepath.Join(parentPath, "regular-child") - }, - wantErr: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - baseDir := t.TempDir() - path := tc.setup(t, baseDir) - err := EnsureAtLeastRegularDir(path) - - if tc.wantErr { - require.Error(t, err) - if tc.errMsg != "" { - require.Contains(t, err.Error(), tc.errMsg) - } - } else { - require.NoError(t, err) - - // Verify the target directory exists and has acceptable permissions. - info, statErr := os.Stat(path) - require.NoError(t, statErr) - require.True(t, info.IsDir()) - // Permissions should be at least as restrictive as required - require.True(t, isPermissionAcceptable(info.Mode().Perm(), perms.RegularDir), - "Directory permissions %#o should be acceptable for requirement %#o", - info.Mode().Perm(), perms.RegularDir) - } - }) - } -} - -func TestEnsureAtLeastRegularDirErrorMessages(t *testing.T) { - t.Parallel() - - t.Run("permission error shows expected vs actual permissions", func(t *testing.T) { - t.Parallel() - - dir := filepath.Join(t.TempDir(), "test-dir") - err := os.MkdirAll(dir, 0o777) - require.NoError(t, err) - // Create directory and explicitly set permissions to override umask. - // Without chmod, umask (typically 022) would convert 0777 -> 0755. - // For tests that need exact permissions, we must use os.Chmod() after creation. - err = os.Chmod(dir, 0o777) - require.NoError(t, err) - - err = EnsureAtLeastRegularDir(dir) - require.Error(t, err) - - // Check that error message contains both actual and expected permissions in octal format. - require.Contains(t, err.Error(), "0777", "Error should show actual permissions") - require.Contains(t, err.Error(), "0755", "Error should show expected permissions") - require.Contains(t, err.Error(), dir, "Error should include the path") - }) -} - -// TestLoadExecutionContextConfig_UndefinedVariables tests that undefined environment variables expand to empty strings. func TestLoadExecutionContextConfig_UndefinedVariables(t *testing.T) { t.Parallel() diff --git a/internal/files/files.go b/internal/files/files.go new file mode 100644 index 0000000..381c094 --- /dev/null +++ b/internal/files/files.go @@ -0,0 +1,193 @@ +package files + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/mozilla-ai/mcpd/v2/internal/perms" +) + +const ( + // EnvVarXDGConfigHome is the XDG Base Directory env var name for config files. + EnvVarXDGConfigHome = "XDG_CONFIG_HOME" + + // EnvVarXDGCacheHome is the XDG Base Directory env var name for cache files. + EnvVarXDGCacheHome = "XDG_CACHE_HOME" +) + +// AppDirName returns the name of the application directory for use in user-specific operations where data is being written. +func AppDirName() string { + return "mcpd" +} + +// DiscoverExecutables scans a directory and returns a set of executable file names. +// Skips directories and hidden files (starting with "."). +func DiscoverExecutables(dir string) (map[string]struct{}, error) { + executables, err := DiscoverExecutablesWithPaths(dir, nil) + if err != nil { + return nil, err + } + + result := make(map[string]struct{}, len(executables)) + for name := range executables { + result[name] = struct{}{} + } + + return result, nil +} + +// DiscoverExecutablesWithPaths scans a directory and returns a map of executable names to their full paths. +// Skips directories and hidden files (starting with "."). +// Only includes files present in the allowed set if provided (nil allowed means include all). +func DiscoverExecutablesWithPaths(dir string, allowed map[string]struct{}) (map[string]string, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + + executables := make(map[string]string) + for _, entry := range entries { + if entry.IsDir() { + continue + } + + if strings.HasPrefix(entry.Name(), ".") { + continue + } + + // Skip if not in allowed list (when allowed list is provided). + if allowed != nil { + if _, ok := allowed[entry.Name()]; !ok { + continue + } + } + + fullPath := filepath.Join(dir, entry.Name()) + + // Use os.Stat to follow symlinks and get target info. + info, err := os.Stat(fullPath) + if err != nil { + // Skip unreadable or broken symlinks instead of aborting. + continue + } + + // Only include regular files with execute permission. + if info.Mode().IsRegular() && info.Mode().Perm()&0o111 != 0 { + executables[entry.Name()] = fullPath + } + } + + return executables, nil +} + +// EnsureAtLeastRegularDir creates a directory with standard permissions if it doesn't exist, +// and verifies that it has at least the required regular permissions if it already exists. +// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error. +// Used for cache directories, data directories, and documentation. +func EnsureAtLeastRegularDir(path string) error { + return ensureAtLeastDir(path, perms.RegularDir) +} + +// EnsureAtLeastSecureDir creates a directory with secure permissions if it doesn't exist, +// and verifies that it has at least the required secure permissions if it already exists. +// It does not attempt to repair ownership or permissions: if they are wrong, +// it returns an error. +func EnsureAtLeastSecureDir(path string) error { + return ensureAtLeastDir(path, perms.SecureDir) +} + +// UserSpecificCacheDir returns the directory that should be used to store any user-specific cache files. +// It adheres to the XDG Base Directory Specification, respecting the XDG_CACHE_HOME environment variable. +// When XDG_CACHE_HOME is not set, it defaults to ~/.cache/mcpd +// See: https://specifications.freedesktop.org/basedir-spec/latest/ +func UserSpecificCacheDir() (string, error) { + return userSpecificDir(EnvVarXDGCacheHome, ".cache") +} + +// UserSpecificConfigDir returns the directory that should be used to store any user-specific configuration. +// It adheres to the XDG Base Directory Specification, respecting the XDG_CONFIG_HOME environment variable. +// When XDG_CONFIG_HOME is not set, it defaults to ~/.config/mcpd +// See: https://specifications.freedesktop.org/basedir-spec/latest/ +func UserSpecificConfigDir() (string, error) { + return userSpecificDir(EnvVarXDGConfigHome, ".config") +} + +// ensureAtLeastDir creates a directory with the specified permissions if it doesn't exist, +// and verifies that it has at least the required permissions if it already exists. +// It does not attempt to repair ownership or permissions: if they are wrong, it returns an error. +// Rejects symlinked directories for security. +// +// NOTE: This function only secures the final directory and its contents with the specified +// permissions. Antecedent directories may have default permissions (typically 0755), which +// is intentional and sufficient for the intended use case. The goal is to protect the +// contents of the final directory, not to secure the entire path hierarchy. +func ensureAtLeastDir(path string, perm os.FileMode) error { + if err := os.MkdirAll(path, perm); err != nil { + return fmt.Errorf("could not ensure directory exists for '%s': %w", path, err) + } + + info, err := os.Lstat(path) + if err != nil { + return fmt.Errorf("could not stat directory '%s': %w", path, err) + } + + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("path '%s' is a symlink, not a directory", path) + } + + if !info.IsDir() { + return fmt.Errorf("path '%s' is not a directory", path) + } + + if !isPermissionAcceptable(info.Mode().Perm(), perm) { + return fmt.Errorf( + "incorrect permissions for directory '%s' (%#o, want %#o or more restrictive)", + path, info.Mode().Perm(), + perm, + ) + } + + return nil +} + +// isPermissionAcceptable checks if the actual permissions are acceptable for the required permissions. +// It returns true if the actual permissions are equal to or more restrictive than required. +// "More restrictive" means: no permission bit set in actual that isn't also set in required. +func isPermissionAcceptable(actual, required os.FileMode) bool { + // Check that actual doesn't grant any permissions that required doesn't grant. + return (actual & ^required) == 0 +} + +// userSpecificDir returns a user-specific directory following XDG Base Directory Specification. +// It respects the given environment variable, falling back to homeDir/dir/AppDirName() if not set. +// The envVar must have XDG_ prefix to follow the specification. +func userSpecificDir(envVar string, dir string) (string, error) { + envVar = strings.TrimSpace(envVar) + // Validate that the environment variable follows XDG naming convention. + if !strings.HasPrefix(envVar, "XDG_") { + return "", fmt.Errorf( + "environment variable '%s' does not follow XDG Base Directory Specification", + envVar, + ) + } + + // If the relevant environment variable is present and configured, then use it. + if ch, ok := os.LookupEnv(envVar); ok && strings.TrimSpace(ch) != "" { + home := strings.TrimSpace(ch) + if filepath.IsAbs(home) { + return filepath.Join(home, AppDirName()), nil + } + + return "", fmt.Errorf("environment variable '%s' must be an absolute path, got: %s", envVar, home) + } + + // Attempt to locate the home directory for the current user and return the path that follows the spec. + homeDir, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("failed to get user home directory: %w", err) + } + + return filepath.Join(homeDir, dir, AppDirName()), nil +} diff --git a/internal/files/files_test.go b/internal/files/files_test.go new file mode 100644 index 0000000..bf9de11 --- /dev/null +++ b/internal/files/files_test.go @@ -0,0 +1,608 @@ +package files + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/mozilla-ai/mcpd/v2/internal/perms" +) + +func TestAppDirName(t *testing.T) { + t.Parallel() + + require.Equal(t, "mcpd", AppDirName()) +} + +func TestUserSpecificConfigDir(t *testing.T) { + tests := []struct { + name string + xdgValue string + expectedDir func(t *testing.T) string + }{ + { + name: "XDG_CONFIG_HOME is set and used", + xdgValue: "/custom/xdg/path", + expectedDir: func(t *testing.T) string { + return filepath.Join("/custom/xdg/path", AppDirName()) + }, + }, + { + name: "XDG_CONFIG_HOME is set with whitespace and trimmed", + xdgValue: " /trimmed/xdg/path ", + expectedDir: func(t *testing.T) string { + return filepath.Join("/trimmed/xdg/path", AppDirName()) + }, + }, + { + name: "XDG_CONFIG_HOME is empty, fall back to default", + xdgValue: "", + expectedDir: func(t *testing.T) string { + home, err := os.UserHomeDir() + require.NoError(t, err) + return filepath.Join(home, ".config", AppDirName()) + }, + }, + { + name: "XDG_CONFIG_HOME is only whitespace, fall back to default", + xdgValue: " ", + expectedDir: func(t *testing.T) string { + home, err := os.UserHomeDir() + require.NoError(t, err) + return filepath.Join(home, ".config", AppDirName()) + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(EnvVarXDGConfigHome, tc.xdgValue) + + result, err := UserSpecificConfigDir() + require.NoError(t, err) + require.Equal(t, tc.expectedDir(t), result) + }) + } +} + +func TestUserSpecificCacheDir(t *testing.T) { + tests := []struct { + name string + xdgValue string + expectedDir func(t *testing.T) string + }{ + { + name: "XDG_CACHE_HOME is set and used", + xdgValue: "/custom/cache/path", + expectedDir: func(t *testing.T) string { + return filepath.Join("/custom/cache/path", AppDirName()) + }, + }, + { + name: "XDG_CACHE_HOME is set with whitespace and trimmed", + xdgValue: " /trimmed/cache/path ", + expectedDir: func(t *testing.T) string { + return filepath.Join("/trimmed/cache/path", AppDirName()) + }, + }, + { + name: "XDG_CACHE_HOME is empty, fall back to default", + xdgValue: "", + expectedDir: func(t *testing.T) string { + home, err := os.UserHomeDir() + require.NoError(t, err) + return filepath.Join(home, ".cache", AppDirName()) + }, + }, + { + name: "XDG_CACHE_HOME is only whitespace, fall back to default", + xdgValue: " ", + expectedDir: func(t *testing.T) string { + home, err := os.UserHomeDir() + require.NoError(t, err) + return filepath.Join(home, ".cache", AppDirName()) + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(EnvVarXDGCacheHome, tc.xdgValue) + + result, err := UserSpecificCacheDir() + require.NoError(t, err) + require.Equal(t, tc.expectedDir(t), result) + }) + } +} + +func TestUserSpecificDir_InvalidEnvVar(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + envVar string + dir string + }{ + { + name: "environment variable without XDG_ prefix", + envVar: "CONFIG_HOME", + dir: ".config", + }, + { + name: "empty environment variable name", + envVar: "", + dir: ".cache", + }, + { + name: "environment variable with wrong prefix", + envVar: "CACHE_HOME", + dir: ".cache", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + _, err := userSpecificDir(tc.envVar, tc.dir) + require.Error(t, err) + require.ErrorContains(t, err, "does not follow XDG Base Directory Specification") + }) + } +} + +func TestIsPermissionAcceptable(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + actual os.FileMode + required os.FileMode + want bool + }{ + // Exact matches should always be acceptable. + { + name: "exact match 0755", + actual: 0o755, + required: 0o755, + want: true, + }, + { + name: "exact match 0700", + actual: 0o700, + required: 0o700, + want: true, + }, + { + name: "exact match 0644", + actual: 0o644, + required: 0o644, + want: true, + }, + // More restrictive should be acceptable. + { + name: "0700 is acceptable when 0755 is required", + actual: 0o700, + required: 0o755, + want: true, + }, + { + name: "0600 is acceptable when 0644 is required", + actual: 0o600, + required: 0o644, + want: true, + }, + { + name: "0000 is acceptable for any requirement (most restrictive)", + actual: 0o000, + required: 0o755, + want: true, + }, + // Less restrictive should NOT be acceptable. + { + name: "0755 is not acceptable when 0700 is required", + actual: 0o755, + required: 0o700, + want: false, + }, + { + name: "0777 is not acceptable when 0755 is required", + actual: 0o777, + required: 0o755, + want: false, + }, + { + name: "0666 is not acceptable when 0644 is required", + actual: 0o666, + required: 0o644, + want: false, + }, + // Different permission patterns. + { + name: "0711 is acceptable when 0755 is required (more restrictive for group/others)", + actual: 0o711, + required: 0o755, + want: true, + }, + { + name: "0750 is acceptable when 0755 is required (more restrictive for others)", + actual: 0o750, + required: 0o755, + want: true, + }, + { + name: "0705 is acceptable when 0755 is required (more restrictive for group)", + actual: 0o705, + required: 0o755, + want: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := isPermissionAcceptable(tc.actual, tc.required) + require.Equal( + t, + tc.want, + got, + "isPermissionAcceptable(%#o, %#o) should return %v", + tc.actual, + tc.required, + tc.want, + ) + }) + } +} + +func TestEnsureAtLeastSecureDir(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(t *testing.T) string + wantErr bool + errMsg string + }{ + { + name: "creates directory when it doesn't exist", + setup: func(t *testing.T) string { + return filepath.Join(t.TempDir(), "new-secure-dir") + }, + wantErr: false, + }, + { + name: "accepts directory with exact required permissions", + setup: func(t *testing.T) string { + dir := filepath.Join(t.TempDir(), "exact-perms") + require.NoError(t, os.Mkdir(dir, perms.SecureDir)) + return dir + }, + wantErr: false, + }, + { + name: "accepts directory with 0o600 (more restrictive)", + setup: func(t *testing.T) string { + dir := filepath.Join(t.TempDir(), "more-restrictive") + require.NoError(t, os.Mkdir(dir, 0o600)) + return dir + }, + wantErr: false, + }, + { + name: "rejects directory with less restrictive permissions", + setup: func(t *testing.T) string { + dir := filepath.Join(t.TempDir(), "less-restrictive") + require.NoError(t, os.Mkdir(dir, 0o755)) + return dir + }, + wantErr: true, + errMsg: "incorrect permissions", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dir := tc.setup(t) + err := EnsureAtLeastSecureDir(dir) + + if tc.wantErr { + require.Error(t, err) + if tc.errMsg != "" { + require.ErrorContains(t, err, tc.errMsg) + } + } else { + require.NoError(t, err) + } + }) + } +} + +func TestEnsureAtLeastSecureDirWithNestedPaths(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + nestedPath := filepath.Join(tempDir, "level1", "level2", "level3") + + err := EnsureAtLeastSecureDir(nestedPath) + require.NoError(t, err) + + info, err := os.Stat(nestedPath) + require.NoError(t, err) + require.True(t, info.IsDir()) + require.True(t, isPermissionAcceptable(info.Mode().Perm(), perms.SecureDir)) +} + +func TestEnsureAtLeastSecureDirErrorMessages(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + tooOpen := filepath.Join(tempDir, "too-open") + require.NoError(t, os.Mkdir(tooOpen, 0o755)) + + err := EnsureAtLeastSecureDir(tooOpen) + require.Error(t, err) + expectedErr := fmt.Sprintf( + "incorrect permissions for directory '%s' (0755, want 0700 or more restrictive)", + tooOpen, + ) + require.EqualError(t, err, expectedErr) +} + +func TestEnsureAtLeastRegularDir(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(t *testing.T) string + wantErr bool + errMsg string + }{ + { + name: "creates directory when it doesn't exist", + setup: func(t *testing.T) string { + return filepath.Join(t.TempDir(), "new-regular-dir") + }, + wantErr: false, + }, + { + name: "accepts directory with exact required permissions", + setup: func(t *testing.T) string { + dir := filepath.Join(t.TempDir(), "exact-perms") + require.NoError(t, os.Mkdir(dir, perms.RegularDir)) + return dir + }, + wantErr: false, + }, + { + name: "accepts directory with more restrictive permissions", + setup: func(t *testing.T) string { + dir := filepath.Join(t.TempDir(), "more-restrictive") + require.NoError(t, os.Mkdir(dir, 0o700)) + return dir + }, + wantErr: false, + }, + { + name: "rejects directory with less restrictive permissions", + setup: func(t *testing.T) string { + dir := filepath.Join(t.TempDir(), "less-restrictive") + // NOTE: os.Mkdir applies the process umask, so passing 0o777 does not guarantee + // world-writable permissions (e.g. 0o777 &^ 0o022 = 0o755 on most systems). + // We explicitly chmod here to ensure the directory actually has 0o777 permissions + // for the test to validate incorrect-permission handling correctly. + require.NoError(t, os.Mkdir(dir, 0o755)) + require.NoError(t, os.Chmod(dir, 0o777)) + return dir + }, + wantErr: true, + errMsg: "incorrect permissions", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dir := tc.setup(t) + err := EnsureAtLeastRegularDir(dir) + + if tc.wantErr { + require.Error(t, err) + if tc.errMsg != "" { + require.ErrorContains(t, err, tc.errMsg) + } + } else { + require.NoError(t, err) + } + }) + } +} + +func TestEnsureAtLeastRegularDirWithNestedPaths(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + nestedPath := filepath.Join(tempDir, "level1", "level2", "level3") + + err := EnsureAtLeastRegularDir(nestedPath) + require.NoError(t, err) + + info, err := os.Stat(nestedPath) + require.NoError(t, err) + require.True(t, info.IsDir()) + require.True(t, isPermissionAcceptable(info.Mode().Perm(), perms.RegularDir)) +} + +func TestEnsureAtLeastRegularDirErrorMessages(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + tooOpen := filepath.Join(tempDir, "too-open") + // NOTE: os.Mkdir applies the process umask, so passing 0o777 does not guarantee + // world-writable permissions (e.g. 0o777 &^ 0o022 = 0o755 on most systems). + // We explicitly chmod here to ensure the directory actually has 0o777 permissions + // for the test to validate incorrect-permission handling correctly. + require.NoError(t, os.Mkdir(tooOpen, 0o755)) + require.NoError(t, os.Chmod(tooOpen, 0o777)) + + err := EnsureAtLeastRegularDir(tooOpen) + require.Error(t, err) + require.ErrorContains(t, err, "incorrect permissions") + require.ErrorContains(t, err, tooOpen) +} + +func TestDiscoverExecutables(t *testing.T) { + t.Parallel() + + t.Run("non-existent directory", func(t *testing.T) { + t.Parallel() + + _, err := DiscoverExecutables("/nonexistent/path") + require.Error(t, err) + }) + + t.Run("empty directory", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + executables, err := DiscoverExecutables(tempDir) + require.NoError(t, err) + require.Empty(t, executables) + }) + + t.Run("directory with executable files", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Create executable plugin. + execPath := filepath.Join(tempDir, "test-plugin") + err := os.WriteFile(execPath, []byte("#!/bin/sh\necho test"), 0o755) + require.NoError(t, err) + + executables, err := DiscoverExecutables(tempDir) + require.NoError(t, err) + require.Len(t, executables, 1) + require.Contains(t, executables, "test-plugin") + }) + + t.Run("skips non-executable files", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Create executable. + execPath := filepath.Join(tempDir, "plugin") + err := os.WriteFile(execPath, []byte("#!/bin/sh"), 0o755) + require.NoError(t, err) + + // Create non-executable. + nonExecPath := filepath.Join(tempDir, "readme.txt") + err = os.WriteFile(nonExecPath, []byte("readme"), 0o644) + require.NoError(t, err) + + executables, err := DiscoverExecutables(tempDir) + require.NoError(t, err) + require.Len(t, executables, 1) + require.Contains(t, executables, "plugin") + require.NotContains(t, executables, "readme.txt") + }) + + t.Run("skips hidden files", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Create visible executable. + visiblePath := filepath.Join(tempDir, "visible-plugin") + err := os.WriteFile(visiblePath, []byte("#!/bin/sh"), 0o755) + require.NoError(t, err) + + // Create hidden executable. + hiddenPath := filepath.Join(tempDir, ".hidden-plugin") + err = os.WriteFile(hiddenPath, []byte("#!/bin/sh"), 0o755) + require.NoError(t, err) + + executables, err := DiscoverExecutables(tempDir) + require.NoError(t, err) + require.Len(t, executables, 1) + require.Contains(t, executables, "visible-plugin") + require.NotContains(t, executables, ".hidden-plugin") + }) + + t.Run("skips subdirectories", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Create executable file. + execPath := filepath.Join(tempDir, "plugin") + err := os.WriteFile(execPath, []byte("#!/bin/sh"), 0o755) + require.NoError(t, err) + + // Create subdirectory. + subDir := filepath.Join(tempDir, "subdir") + err = os.Mkdir(subDir, 0o755) + require.NoError(t, err) + + executables, err := DiscoverExecutables(tempDir) + require.NoError(t, err) + require.Len(t, executables, 1) + require.Contains(t, executables, "plugin") + }) +} + +func TestDiscoverExecutablesWithPaths(t *testing.T) { + t.Parallel() + + t.Run("filters by allowed list", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Create multiple executables. + for _, name := range []string{"plugin1", "plugin2", "plugin3"} { + path := filepath.Join(tempDir, name) + err := os.WriteFile(path, []byte("#!/bin/sh"), 0o755) + require.NoError(t, err) + } + + allowed := map[string]struct{}{ + "plugin1": {}, + "plugin3": {}, + } + + executables, err := DiscoverExecutablesWithPaths(tempDir, allowed) + require.NoError(t, err) + require.Len(t, executables, 2) + require.Contains(t, executables, "plugin1") + require.Contains(t, executables, "plugin3") + require.NotContains(t, executables, "plugin2") + require.Equal(t, filepath.Join(tempDir, "plugin1"), executables["plugin1"]) + require.Equal(t, filepath.Join(tempDir, "plugin3"), executables["plugin3"]) + }) + + t.Run("nil allowed list includes all executables", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Create executables. + execPath := filepath.Join(tempDir, "plugin") + err := os.WriteFile(execPath, []byte("#!/bin/sh"), 0o755) + require.NoError(t, err) + + executables, err := DiscoverExecutablesWithPaths(tempDir, nil) + require.NoError(t, err) + require.Len(t, executables, 1) + require.Contains(t, executables, "plugin") + require.Equal(t, filepath.Join(tempDir, "plugin"), executables["plugin"]) + }) +} diff --git a/internal/flags/config.go b/internal/flags/config.go index 1fed1db..1d0cfcb 100644 --- a/internal/flags/config.go +++ b/internal/flags/config.go @@ -8,7 +8,7 @@ import ( "github.com/spf13/pflag" - "github.com/mozilla-ai/mcpd/v2/internal/context" + "github.com/mozilla-ai/mcpd/v2/internal/files" ) const ( @@ -80,7 +80,7 @@ func initRuntimeVarsFile(fs *pflag.FlagSet) error { defaultRuntimeVarsFile := strings.TrimSpace(os.Getenv(EnvRuntimeFile)) // When empty or matching the default value, resolve the correct folder. if defaultRuntimeVarsFile == "" || defaultRuntimeVarsFile == DefaultRuntimeVarsFile { - dir, err := context.UserSpecificConfigDir() + dir, err := files.UserSpecificConfigDir() if err != nil { return fmt.Errorf("error configuring default value for runtime vars file: %w", err) } diff --git a/internal/flags/config_test.go b/internal/flags/config_test.go index 7735a72..acf6477 100644 --- a/internal/flags/config_test.go +++ b/internal/flags/config_test.go @@ -8,7 +8,7 @@ import ( "github.com/spf13/pflag" "github.com/stretchr/testify/require" - "github.com/mozilla-ai/mcpd/v2/internal/context" + "github.com/mozilla-ai/mcpd/v2/internal/files" ) func TestConfig_InitConfigFile_EnvVars(t *testing.T) { @@ -250,7 +250,7 @@ func TestConfig_RuntimeVarsFile_Precedence(t *testing.T) { envValue: "", cmdLineArgs: nil, expected: func() string { - dir, err := context.UserSpecificConfigDir() + dir, err := files.UserSpecificConfigDir() require.NoError(t, err) return filepath.Join(dir, DefaultRuntimeVarsFile) }(), @@ -260,7 +260,7 @@ func TestConfig_RuntimeVarsFile_Precedence(t *testing.T) { envValue: DefaultRuntimeVarsFile, cmdLineArgs: nil, expected: func() string { - dir, err := context.UserSpecificConfigDir() + dir, err := files.UserSpecificConfigDir() require.NoError(t, err) return filepath.Join(dir, DefaultRuntimeVarsFile) }(), @@ -269,16 +269,16 @@ func TestConfig_RuntimeVarsFile_Precedence(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - originalXDGConfigHome := os.Getenv(context.EnvVarXDGConfigHome) + originalXDGConfigHome := os.Getenv(files.EnvVarXDGConfigHome) t.Cleanup(func() { // Reset flag vars. RuntimeFile = "" // Reset env state - require.NoError(t, os.Setenv(context.EnvVarXDGConfigHome, originalXDGConfigHome)) + require.NoError(t, os.Setenv(files.EnvVarXDGConfigHome, originalXDGConfigHome)) }) // Clear XDG_CONFIG_HOME to ensure it cannot cause side effects in the test results. - t.Setenv(context.EnvVarXDGConfigHome, "") + t.Setenv(files.EnvVarXDGConfigHome, "") t.Setenv(EnvRuntimeFile, tc.envValue) fs := pflag.NewFlagSet("test", pflag.ContinueOnError) @@ -342,7 +342,7 @@ func TestConfig_LoggerFlags_Precedence(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - originalXDGConfigHome := os.Getenv(context.EnvVarXDGConfigHome) + originalXDGConfigHome := os.Getenv(files.EnvVarXDGConfigHome) t.Cleanup(func() { // Reset flag vars. ConfigFile = "" @@ -351,10 +351,10 @@ func TestConfig_LoggerFlags_Precedence(t *testing.T) { LogLevel = "" // Reset env state. - require.NoError(t, os.Setenv(context.EnvVarXDGConfigHome, originalXDGConfigHome)) + require.NoError(t, os.Setenv(files.EnvVarXDGConfigHome, originalXDGConfigHome)) }) // Clear XDG_CONFIG_HOME to ensure it cannot cause side effects in the test results. - t.Setenv(context.EnvVarXDGConfigHome, "") + t.Setenv(files.EnvVarXDGConfigHome, "") t.Setenv(EnvVarLogPath, tc.envLogPath) t.Setenv(EnvVarLogLevel, tc.envLogLevel) diff --git a/internal/plugin/manager.go b/internal/plugin/manager.go index b6abc51..5aee551 100644 --- a/internal/plugin/manager.go +++ b/internal/plugin/manager.go @@ -25,6 +25,7 @@ import ( pluginv1 "github.com/mozilla-ai/mcpd-plugins-sdk-go/pkg/plugins/v1/plugins" "github.com/mozilla-ai/mcpd/v2/internal/config" + "github.com/mozilla-ai/mcpd/v2/internal/files" ) const ( @@ -331,42 +332,11 @@ func (m *Manager) discoverPlugins(allowed map[string]struct{}) (map[string]strin return nil, nil } - entries, err := os.ReadDir(m.config.Dir) + plugins, err := files.DiscoverExecutablesWithPaths(m.config.Dir, allowed) if err != nil { return nil, fmt.Errorf("reading plugin directory %s: %w", m.config.Dir, err) } - plugins := make(map[string]string, len(allowed)) - - for _, entry := range entries { - // Skip directories. - if entry.IsDir() { - continue - } - - // Skip hidden files. - if strings.HasPrefix(entry.Name(), ".") { - continue - } - - // Skip any file that isn't in the configured allow list. - if _, ok := allowed[entry.Name()]; !ok { - continue - } - - // Check if file is executable. - info, err := entry.Info() - if err != nil { - return nil, fmt.Errorf("reading file info for %s: %w", entry.Name(), err) - } - - // Check for execute permission (0o111 = user/group/other execute bits). - if info.Mode()&0o111 != 0 { - fullPath := filepath.Join(m.config.Dir, entry.Name()) - plugins[entry.Name()] = fullPath - } - } - return plugins, nil } @@ -411,8 +381,7 @@ func (m *Manager) startPlugin(ctx context.Context, name string, binaryPath strin // Use plugin specific logger to configure stdio and stderr for the plugin to emit logs. stdWriter := func() io.Writer { return l.StandardWriter(&hclog.StandardLoggerOptions{ - InferLevels: true, - InferLevelsWithTimestamp: true, + InferLevels: true, }) } cmd.Stdout = stdWriter() diff --git a/internal/registry/options/build_options.go b/internal/registry/options/build_options.go index c7a66a5..3e8901d 100644 --- a/internal/registry/options/build_options.go +++ b/internal/registry/options/build_options.go @@ -5,7 +5,7 @@ import ( "time" "github.com/mozilla-ai/mcpd/v2/internal/config" - "github.com/mozilla-ai/mcpd/v2/internal/context" + "github.com/mozilla-ai/mcpd/v2/internal/files" ) // BuildOption defines a functional option for configuring registry builds. @@ -83,7 +83,7 @@ func WithCacheTTL(ttl time.Duration) BuildOption { // DefaultCacheDir returns the default cache directory for registry manifests. func DefaultCacheDir() (string, error) { - cacheDir, err := context.UserSpecificCacheDir() + cacheDir, err := files.UserSpecificCacheDir() if err != nil { return "", err }