Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions pkg/cli/shell_completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
// For now, we only use the CommandProvider interface methods
cmd, ok := rootCmd.(*cobra.Command)
if !ok {
return errors.New("rootCmd must be a *cobra.Command")

Check failure on line 99 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

error message uses negative language without constructive guidance; include expected/requires/should/example details
}

shellType := DetectShell()
Expand All @@ -122,6 +122,17 @@
}
}

// validateRcPath cleans a shell rc file path (for example ~/.bashrc) and ensures it is
// absolute before it is read. It returns the cleaned path or an actionable error.
Comment thread
Copilot marked this conversation as resolved.
Outdated
func validateRcPath(rcName string, rcPath string) (string, error) {
cleanPath := filepath.Clean(rcPath)
if !filepath.IsAbs(cleanPath) {
shellCompletionLog.Printf("Invalid %s path (not absolute): %s", rcName, rcPath)
return "", fmt.Errorf("%s path %q is not an absolute path — expected an absolute path such as '/home/user/.%s'; set $HOME to an absolute home directory and retry", rcName, rcPath, rcName)
}
return cleanPath, nil
}

// installBashCompletion installs bash completion
func installBashCompletion(verbose bool, cmd *cobra.Command) error {
shellCompletionLog.Print("Installing bash completion")
Expand All @@ -129,7 +140,7 @@
// Generate completion script using Cobra
var buf bytes.Buffer
if err := cmd.GenBashCompletion(&buf); err != nil {
return fmt.Errorf("failed to generate bash completion: %w", err)

Check failure on line 143 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

avoid generic 'failed to ...: %w' wrapping; add specific recovery guidance

Check failure on line 143 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

error message uses negative language without constructive guidance; include expected/requires/should/example details
}

completionScript := buf.String()
Expand Down Expand Up @@ -173,7 +184,7 @@
if strings.HasPrefix(completionDir, homeDir) {
// Use restrictive permissions (0750) following principle of least privilege
if err := os.MkdirAll(completionDir, constants.DirPermSensitive); err != nil {
return fmt.Errorf("failed to create completion directory: %w", err)

Check failure on line 187 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

avoid generic 'failed to ...: %w' wrapping; add specific recovery guidance

Check failure on line 187 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

error message uses negative language without constructive guidance; include expected/requires/should/example details
}
}

Expand All @@ -186,14 +197,14 @@
completionPath = filepath.Join(homeDir, ".bash_completion.d", "gh-aw")
// Use restrictive permissions (0750) following principle of least privilege
if err := os.MkdirAll(filepath.Dir(completionPath), constants.DirPermSensitive); err != nil {
return fmt.Errorf("failed to create user completion directory: %w", err)

Check failure on line 200 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

avoid generic 'failed to ...: %w' wrapping; add specific recovery guidance

Check failure on line 200 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

error message uses negative language without constructive guidance; include expected/requires/should/example details
}
// Use restrictive permissions (0600) following principle of least privilege
if err := os.WriteFile(completionPath, []byte(completionScript), constants.FilePermSensitive); err != nil {
return fmt.Errorf("failed to write completion file: %w", err)

Check failure on line 204 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

avoid generic 'failed to ...: %w' wrapping; add specific recovery guidance

Check failure on line 204 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

error message uses negative language without constructive guidance; include expected/requires/should/example details
}
} else if err != nil {
return fmt.Errorf("failed to write completion file: %w", err)

Check failure on line 207 in pkg/cli/shell_completion.go

View workflow job for this annotation

GitHub Actions / lint-go-custom

error message uses negative language without constructive guidance; include expected/requires/should/example details
}

fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Installed bash completion to: "+completionPath))
Expand All @@ -203,10 +214,9 @@
if strings.HasPrefix(completionPath, homeDir) {
// For user-level installations, check if .bashrc sources the completion directory
// Clean and validate the path to prevent path traversal
cleanBashrcPath := filepath.Clean(bashrcPath)
if !filepath.IsAbs(cleanBashrcPath) {
shellCompletionLog.Printf("Invalid bashrc path (not absolute): %s", bashrcPath)
return fmt.Errorf("invalid bashrc path: %s", bashrcPath)
cleanBashrcPath, err := validateRcPath("bashrc", bashrcPath)
if err != nil {
return err
}
// #nosec G304 -- bashrcPath is constructed from trusted os.UserHomeDir() and a constant filename
bashrcContent, err := os.ReadFile(cleanBashrcPath)
Expand Down Expand Up @@ -275,10 +285,9 @@
// Check if .zshrc configures fpath
zshrcPath := filepath.Join(homeDir, ".zshrc")
// Clean and validate the path to prevent path traversal
cleanZshrcPath := filepath.Clean(zshrcPath)
if !filepath.IsAbs(cleanZshrcPath) {
shellCompletionLog.Printf("Invalid zshrc path (not absolute): %s", zshrcPath)
return fmt.Errorf("invalid zshrc path: %s", zshrcPath)
cleanZshrcPath, err := validateRcPath("zshrc", zshrcPath)
if err != nil {
return err
}
// #nosec G304 -- zshrcPath is constructed from trusted os.UserHomeDir() and a constant filename
zshrcContent, err := os.ReadFile(cleanZshrcPath)
Expand Down
18 changes: 18 additions & 0 deletions pkg/cli/shell_completion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package cli

import (
"os"
"path/filepath"
"runtime"
"testing"

Expand Down Expand Up @@ -413,3 +414,20 @@ func TestUninstallShellCompletion(t *testing.T) {
})
}
}

func TestValidateRcPath(t *testing.T) {
t.Run("returns cleaned path for absolute path", func(t *testing.T) {
homeDir := t.TempDir()
cleanPath, err := validateRcPath("bashrc", filepath.Join(homeDir, ".", ".bashrc"))
require.NoError(t, err)
require.Equal(t, filepath.Join(homeDir, ".bashrc"), cleanPath)
})

t.Run("returns actionable error for relative path", func(t *testing.T) {
_, err := validateRcPath("zshrc", ".zshrc")
require.Error(t, err)
require.ErrorContains(t, err, `zshrc path ".zshrc" is not an absolute path`)
require.ErrorContains(t, err, "expected an absolute path such as '/home/user/.zshrc'")
require.ErrorContains(t, err, "set $HOME to an absolute home directory and retry")
})
}
Loading