Skip to content

Commit 1bc31af

Browse files
erikburterikburt
committed
fix: refactor for readability
1 parent 9720931 commit 1bc31af

File tree

4 files changed

+148
-115
lines changed

4 files changed

+148
-115
lines changed

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ require (
3030
github.com/kylelemons/godebug v1.1.0
3131
github.com/lib/pq v1.10.9
3232
github.com/marcboeker/go-duckdb v1.8.5
33+
github.com/mattn/go-shellwords v1.0.12
3334
github.com/mr-tron/base58 v1.2.0
3435
github.com/pelletier/go-toml v1.9.5
3536
github.com/pelletier/go-toml/v2 v2.2.4

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,8 @@ github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Ky
256256
github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
257257
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
258258
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
259+
github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk=
260+
github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
259261
github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
260262
github.com/mattn/go-sqlite3 v2.0.3+incompatible h1:gXHsfypPkaMZrKbD5209QV9jbUTJKjyR5WD3HYQSd+U=
261263
github.com/mattn/go-sqlite3 v2.0.3+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=

‎pkg/loop/cmd/loopinstall/install.go‎

Lines changed: 128 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"strings"
1313
"sync"
1414
"time"
15+
16+
shellwords "github.com/mattn/go-shellwords"
1517
)
1618

1719
// execCommand is a function variable that can be replaced in tests
@@ -51,108 +53,149 @@ func mergeOrReplaceEnvVars(existing []string, newVars []string) []string {
5153
return result
5254
}
5355

54-
// downloadAndInstallPlugin downloads and installs a single plugin
55-
func downloadAndInstallPlugin(pluginType string, pluginIdx int, plugin PluginDef, defaults DefaultsConfig) error {
56-
if !isPluginEnabled(plugin) {
57-
log.Printf("Skipping disabled plugin %s[%d]", pluginType, pluginIdx)
58-
return nil
56+
func determineModuleDirectory(goPrivate, fullModulePath string) (string, error) {
57+
cmd := exec.Command("go", "mod", "download", "-json", fullModulePath)
58+
var out bytes.Buffer
59+
cmd.Stdout = &out
60+
cmd.Stderr = os.Stderr
61+
62+
if goPrivate != "" {
63+
// Inherit the current environment and override GOPRIVATE.
64+
// Note: Not really sure why this is needed - tried to simplify existing logic
65+
cmd.Env = append(os.Environ(), "GOPRIVATE="+goPrivate)
5966
}
6067

61-
moduleURI := plugin.ModuleURI
62-
gitRef := plugin.GitRef
63-
installPath := plugin.InstallPath
68+
if err := execCommand(cmd); err != nil {
69+
return "", fmt.Errorf("failed to download module %s: %w", fullModulePath, err)
70+
}
6471

65-
// Validate inputs
66-
if err := validateModuleURI(moduleURI); err != nil {
67-
return fmt.Errorf("validation failed: %w", err)
72+
var result ModDownloadResult
73+
if err := json.Unmarshal(out.Bytes(), &result); err != nil {
74+
return "", fmt.Errorf("failed to parse go mod download output: %w", err)
6875
}
6976

70-
if gitRef != "" {
71-
if err := validateGitRef(gitRef); err != nil {
72-
return fmt.Errorf("validation failed: %w", err)
77+
if result.Dir == "" {
78+
return "", fmt.Errorf("empty module directory returned for %s", fullModulePath)
79+
}
80+
81+
return result.Dir, nil
82+
}
83+
84+
func determineGoFlags(defaultGoFlags, pluginGoFlags string) ([]string, error) {
85+
var flags []string
86+
parser := shellwords.NewParser()
87+
88+
// Determine base flags
89+
if envGoFlags := os.Getenv("CL_PLUGIN_GOFLAGS"); envGoFlags != "" {
90+
log.Printf("Overriding config's default goflags with CL_PLUGIN_GOFLAGS env var: %s", envGoFlags)
91+
f, err := parser.Parse(envGoFlags)
92+
if err != nil {
93+
return nil, err
7394
}
95+
flags = f
96+
} else if defaultGoFlags != "" {
97+
f, err := parser.Parse(defaultGoFlags)
98+
if err != nil {
99+
return nil, err
100+
}
101+
flags = f
74102
}
75103

76-
if err := validateInstallPath(installPath); err != nil {
77-
return fmt.Errorf("validation failed: %w", err)
104+
// Append plugin-specific flags
105+
if pluginGoFlags != "" {
106+
f, err := parser.Parse(pluginGoFlags)
107+
if err != nil {
108+
return nil, err
109+
}
110+
flags = append(flags, f...)
78111
}
79112

80-
// Full module path with git reference
81-
fullModulePath := moduleURI
82-
if gitRef != "" {
83-
fullModulePath = fmt.Sprintf("%s@%s", moduleURI, gitRef)
113+
// Validate
114+
if len(flags) > 0 {
115+
if err := validateGoFlags(strings.Join(flags, " ")); err != nil {
116+
return nil, err
117+
}
84118
}
85119

86-
log.Printf("Installing plugin %s[%d] from %s", pluginType, pluginIdx, fullModulePath)
120+
return flags, nil
121+
}
87122

88-
// Get GOPRIVATE environment variable
89-
goPrivate := os.Getenv("GOPRIVATE")
123+
func determineInstallArg(installPath, moduleURI string) string {
124+
// Determine the actual argument for 'go install' based on installPath and moduleURI.
125+
// - installPath is the user-provided path from YAML (no environment variable expansion).
126+
// - moduleURI is the URI of the module being downloaded and installed (no environment variable expansion).
127+
// The 'go install' command will be run with cmd.Dir set to the root of the downloaded moduleURI.
128+
// Therefore, installArg must be "." or a path starting with "./" relative to the module root.
90129

91-
// Download the module and get its directory
92-
var moduleDir string
93-
{
94-
cmd := exec.Command("go", "mod", "download", "-json", fullModulePath)
95-
var out bytes.Buffer
96-
cmd.Stdout = &out
97-
cmd.Stderr = os.Stderr
130+
// Case 1: installPath is the moduleURI itself. Install the module root.
131+
if installPath == moduleURI {
132+
return "."
133+
}
98134

99-
// Set GOPRIVATE environment variable while preserving other environment variables
100-
if goPrivate != "" {
101-
// Start with all current environment variables
102-
env := os.Environ()
103-
104-
// Find and replace GOPRIVATE if it exists, or add it if it doesn't
105-
goprivateFound := false
106-
for i, e := range env {
107-
if strings.HasPrefix(e, "GOPRIVATE=") {
108-
env[i] = "GOPRIVATE=" + goPrivate
109-
goprivateFound = true
110-
break
111-
}
112-
}
135+
// Case 2: installPath is a sub-package of moduleURI (e.g., "moduleURI/cmd/plugin").
136+
if after, ok := strings.CutPrefix(installPath, moduleURI+"/"); ok {
137+
// Extract the relative path and prefix with "./".
138+
relativePath := after
139+
cleanedRelativePath := strings.TrimLeft(relativePath, "/") // Handles "moduleURI///subpath"
140+
if cleanedRelativePath == "" || cleanedRelativePath == "." { // Handles "moduleURI/" or "moduleURI/."
141+
return "."
142+
}
113143

114-
// Add GOPRIVATE if it wasn't already in the environment
115-
if !goprivateFound {
116-
env = append(env, "GOPRIVATE="+goPrivate)
117-
}
144+
// cleanedRelativePath is like "cmd/plugin" or "sub/../pkg". Prepend "./".
145+
return "./" + cleanedRelativePath
146+
}
118147

119-
cmd.Env = env
120-
}
148+
// Case 3: installPath is not moduleURI and not a sub-package of moduleURI.
149+
// Assumed to be:
150+
// a) A path already relative to the module root (e.g., "cmd/plugin", "./cmd/plugin", ".").
151+
// b) A full path to a different module (e.g., "github.com/other/mod").
152+
// For (b), prefixing with "./" when cmd.Dir is set is problematic but replicates prior behavior if any.
121153

122-
if err := execCommand(cmd); err != nil {
123-
return fmt.Errorf("failed to download module %s: %w", fullModulePath, err)
124-
}
154+
// Simple case
155+
if installPath == "." {
156+
return "."
157+
}
125158

126-
var result ModDownloadResult
127-
if err := json.Unmarshal(out.Bytes(), &result); err != nil {
128-
return fmt.Errorf("failed to parse go mod download output: %w", err)
129-
}
159+
// Already correctly formatted (e.g., "./cmd/plugin", "./sub/../pkg")
160+
if strings.HasPrefix(installPath, "./") {
161+
return installPath
162+
}
130163

131-
moduleDir = result.Dir
132-
if moduleDir == "" {
133-
return fmt.Errorf("empty module directory returned for %s", fullModulePath)
134-
}
164+
// Needs "./" prefix. Handles "cmd/plugin", "/cmd/plugin", "github.com/other/mod".
165+
return "./" + strings.TrimLeft(installPath, "/")
166+
}
167+
168+
// downloadAndInstallPlugin downloads and installs a single plugin
169+
func downloadAndInstallPlugin(pluginType string, pluginIdx int, plugin PluginDef, defaults DefaultsConfig) error {
170+
if !isPluginEnabled(plugin) {
171+
log.Printf("Skipping disabled plugin %s[%d]", pluginType, pluginIdx)
172+
return nil
135173
}
136174

137-
// Build goflags
138-
goflags := defaults.GoFlags
139-
if envGoFlags := os.Getenv("CL_PLUGIN_GOFLAGS"); envGoFlags != "" {
140-
goflags = envGoFlags
175+
// Validate inputs
176+
if err := plugin.Validate(); err != nil {
177+
return fmt.Errorf("plugin input validation failed: %w", err)
141178
}
142179

143-
// If goflags from plugindef is set, append it
144-
if plugin.Flags != "" {
145-
if goflags != "" {
146-
goflags += " "
147-
}
148-
goflags += plugin.Flags
180+
moduleURI := plugin.ModuleURI
181+
gitRef := plugin.GitRef
182+
installPath := plugin.InstallPath
183+
184+
// Full module path with git reference
185+
fullModulePath := moduleURI
186+
if gitRef != "" {
187+
fullModulePath = fmt.Sprintf("%s@%s", moduleURI, gitRef)
149188
}
150189

151-
// Validate goflags
152-
if goflags != "" {
153-
if err := validateGoFlags(goflags); err != nil {
154-
return fmt.Errorf("validation failed: %w", err)
155-
}
190+
log.Printf("Installing plugin %s[%d] from %s", pluginType, pluginIdx, fullModulePath)
191+
192+
// Get GOPRIVATE environment variable
193+
goPrivate := os.Getenv("GOPRIVATE")
194+
195+
// Download the module and get its directory
196+
moduleDir, err := determineModuleDirectory(goPrivate, fullModulePath)
197+
if err != nil {
198+
return fmt.Errorf("failed to determine module directory: %w", err)
156199
}
157200

158201
// Build env vars from defaults, environment variable, and plugin-specific settings
@@ -168,42 +211,7 @@ func downloadAndInstallPlugin(pluginType string, pluginIdx int, plugin PluginDef
168211

169212
// Install the plugin
170213
{
171-
// Determine the actual argument for 'go install' based on installPath and moduleURI.
172-
// installPath is the user-provided path from YAML (no environment variable expansion).
173-
// moduleURI is the URI of the module being downloaded and installed (no environment variable expansion).
174-
// The 'go install' command will be run with cmd.Dir set to the root of the downloaded moduleURI.
175-
// Therefore, installArg must be "." or a path starting with "./" relative to the module root.
176-
var installArg string
177-
if installPath == moduleURI {
178-
// Case 1: installPath is the moduleURI itself. Install the module root.
179-
installArg = "."
180-
} else if after, ok := strings.CutPrefix(installPath, moduleURI+"/"); ok {
181-
// Case 2: installPath is a sub-package of moduleURI (e.g., "moduleURI/cmd/plugin").
182-
// Extract the relative path and prefix with "./".
183-
relativePath := after
184-
cleanedRelativePath := strings.TrimLeft(relativePath, "/") // Handles "moduleURI///subpath"
185-
if cleanedRelativePath == "" || cleanedRelativePath == "." { // Handles "moduleURI/" or "moduleURI/."
186-
installArg = "."
187-
} else {
188-
// cleanedRelativePath is like "cmd/plugin" or "sub/../pkg". Prepend "./".
189-
installArg = "./" + cleanedRelativePath
190-
}
191-
} else {
192-
// Case 3: installPath is not moduleURI and not a sub-package of moduleURI.
193-
// Assumed to be:
194-
// a) A path already relative to the module root (e.g., "cmd/plugin", "./cmd/plugin", ".").
195-
// b) A full path to a different module (e.g., "github.com/other/mod").
196-
// For (b), prefixing with "./" when cmd.Dir is set is problematic but replicates prior behavior if any.
197-
if installPath == "." {
198-
installArg = "."
199-
} else if strings.HasPrefix(installPath, "./") {
200-
// Already correctly formatted (e.g., "./cmd/plugin", "./sub/../pkg")
201-
installArg = installPath
202-
} else {
203-
// Needs "./" prefix. Handles "cmd/plugin", "/cmd/plugin", "github.com/other/mod".
204-
installArg = "./" + strings.TrimLeft(installPath, "/")
205-
}
206-
}
214+
installArg := determineInstallArg(installPath, moduleURI)
207215

208216
binaryName := filepath.Base(installArg)
209217
if binaryName == "." {
@@ -222,9 +230,15 @@ func downloadAndInstallPlugin(pluginType string, pluginIdx int, plugin PluginDef
222230

223231
outputPath := filepath.Join(outputDir, binaryName)
224232

233+
// Build goflags
234+
goflags, err := determineGoFlags(defaults.GoFlags, plugin.Flags)
235+
if err != nil {
236+
return fmt.Errorf("validation failed: %w", err)
237+
}
238+
225239
args := []string{"build", "-o", outputPath}
226-
if goflags != "" {
227-
args = append(args, strings.Fields(goflags)...)
240+
if len(goflags) != 0 {
241+
args = append(args, goflags...)
228242
}
229243
args = append(args, installArg)
230244

‎pkg/loop/cmd/loopinstall/validation.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@ import (
66
"strings"
77
)
88

9+
func (plugin PluginDef) Validate() error {
10+
if err := validateModuleURI(plugin.ModuleURI); err != nil {
11+
return err
12+
}
13+
if plugin.GitRef != "" {
14+
if err := validateGitRef(plugin.GitRef); err != nil {
15+
return err
16+
}
17+
}
18+
if err := validateInstallPath(plugin.InstallPath); err != nil {
19+
return err
20+
}
21+
22+
return nil
23+
}
24+
925
// validateModuleURI ensures the module URI follows Go module conventions
1026
func validateModuleURI(uri string) error {
1127
// Check for valid Go module path format
@@ -35,7 +51,7 @@ func validateInstallPath(path string) error {
3551

3652
// validateGoFlags ensures flags are safe and prevents command injection
3753
func validateGoFlags(flags string) error {
38-
// Check for potentially dangerous characters that could enable command injection
54+
// Check for potentially dangerous characters or substrings that could enable command injection
3955
dangerousPatterns := []string{
4056
";", "&&", "||", "`", "$", "|", "<", ">", "#", "//",
4157
"shutdown", "reboot", "rm -", "format", "mkfs", "dd",

0 commit comments

Comments
 (0)