fix(plugin): harden plugin system security boundaries
- Validate git URL protocol: reject file:// and local paths, only allow https/ssh - Reject symlink entries in ZIP extraction to prevent path traversal - Validate build.output must be relative path within plugin directory - Reject absolute paths in stdio server command declarations - Block dangerous env var names (PATH, LD_PRELOAD, etc.) from plugin config injection - Remove conference from default managed plugins (source not yet available)
This commit is contained in:
1 parent
9f78e51ae7
commit
d25b106e4f
4 files changed
+52
-3
No files matched your search
@@ -427,9 +427,16 @@ func (l *Loader) InstallFromGit(gitURL string) (*Plugin, error) {
|
||||
|
||||
// parseGitURL extracts workspace and repo name from a git URL.
|
||||
// Supports: https://github.com/org/repo.git, git@github.com:org/repo.git
|
||||
// Rejects file:// and other local protocols to prevent reading local files.
|
||||
func parseGitURL(gitURL string) (workspace, repoName string, err error) {
|
||||
gitURL = strings.TrimSpace(gitURL)
|
||||
|
||||
// Reject dangerous protocols that could read local files.
|
||||
lower := strings.ToLower(gitURL)
|
||||
if strings.HasPrefix(lower, "file://") || strings.HasPrefix(lower, "/") || strings.HasPrefix(lower, ".") {
|
||||
return "", "", fmt.Errorf("local paths and file:// URLs are not allowed: %q", gitURL)
|
||||
}
|
||||
|
||||
// Handle SSH format: git@github.com:org/repo.git
|
||||
if strings.HasPrefix(gitURL, "git@") {
|
||||
parts := strings.SplitN(gitURL, ":", 2)
|
||||
@@ -450,6 +457,11 @@ func parseGitURL(gitURL string) (workspace, repoName string, err error) {
|
||||
return "", "", fmt.Errorf("cannot parse URL %q: %w", gitURL, err)
|
||||
}
|
||||
|
||||
// Only allow https:// and http:// schemes.
|
||||
if u.Scheme != "https" && u.Scheme != "http" {
|
||||
return "", "", fmt.Errorf("unsupported URL scheme %q: only https and ssh are allowed", u.Scheme)
|
||||
}
|
||||
|
||||
path := strings.TrimSuffix(strings.Trim(u.Path, "/"), ".git")
|
||||
segments := strings.Split(path, "/")
|
||||
if len(segments) < 2 {
|
||||
@@ -612,6 +624,19 @@ func (l *Loader) ListPluginConfig(pluginName string) map[string]string {
|
||||
//
|
||||
// Environment variables already set by the user take precedence — only
|
||||
// keys not already present in the environment are injected.
|
||||
// dangerousEnvVars contains environment variable names that must never be
|
||||
// set from plugin config because they can alter process behavior in
|
||||
// security-critical ways (library injection, executable search path, etc.).
|
||||
var dangerousEnvVars = map[string]bool{
|
||||
"PATH": true, "HOME": true, "USER": true, "SHELL": true,
|
||||
"LD_PRELOAD": true, "LD_LIBRARY_PATH": true,
|
||||
"DYLD_INSERT_LIBRARIES": true, "DYLD_LIBRARY_PATH": true, "DYLD_FRAMEWORK_PATH": true,
|
||||
"NODE_OPTIONS": true, "PYTHONPATH": true, "RUBYLIB": true,
|
||||
"GOPATH": true, "GOROOT": true,
|
||||
"HTTP_PROXY": true, "HTTPS_PROXY": true, "ALL_PROXY": true, "NO_PROXY": true,
|
||||
"http_proxy": true, "https_proxy": true, "all_proxy": true, "no_proxy": true,
|
||||
}
|
||||
|
||||
func (l *Loader) InjectPluginConfigEnv() {
|
||||
settings := l.loadSettings()
|
||||
if len(settings.PluginConfigs) == 0 {
|
||||
@@ -623,6 +648,12 @@ func (l *Loader) InjectPluginConfigEnv() {
|
||||
if !ok || strVal == "" {
|
||||
continue
|
||||
}
|
||||
// Block dangerous environment variable names.
|
||||
if dangerousEnvVars[key] {
|
||||
slog.Warn("plugin: blocked dangerous env var from config",
|
||||
"key", key)
|
||||
continue
|
||||
}
|
||||
// Do not override existing environment variables.
|
||||
if _, exists := os.LookupEnv(key); exists {
|
||||
continue
|
||||
@@ -761,6 +792,17 @@ func runBuild(pluginDir string, build *BuildConfig) error {
|
||||
return fmt.Errorf("build.command is empty")
|
||||
}
|
||||
|
||||
// Validate build.output is a relative path within the plugin directory.
|
||||
if build.Output != "" {
|
||||
if filepath.IsAbs(build.Output) {
|
||||
return fmt.Errorf("build.output must be a relative path, got %q", build.Output)
|
||||
}
|
||||
cleanOut := filepath.Clean(build.Output)
|
||||
if strings.HasPrefix(cleanOut, "..") {
|
||||
return fmt.Errorf("build.output must not escape plugin directory: %q", build.Output)
|
||||
}
|
||||
}
|
||||
|
||||
slog.Info("plugin: building", "dir", pluginDir, "command", build.Command)
|
||||
|
||||
var cmd *exec.Cmd
|
||||
|
||||
@@ -157,6 +157,10 @@ func validateMCPServer(key string, srv *MCPServer) error {
|
||||
if strings.TrimSpace(srv.Command) == "" {
|
||||
return fmt.Errorf("mcpServers[%q]: stdio requires command", key)
|
||||
}
|
||||
// Reject absolute paths in command to encourage relative paths within plugin root.
|
||||
if filepath.IsAbs(srv.Command) {
|
||||
return fmt.Errorf("mcpServers[%q]: command must be a relative path, got %q", key, srv.Command)
|
||||
}
|
||||
default:
|
||||
return fmt.Errorf("mcpServers[%q]: unsupported type %q (must be streamable-http or stdio)", key, srv.Type)
|
||||
}
|
||||
|
||||
@@ -361,6 +361,11 @@ func extractPluginZip(zipPath, destDir string) error {
|
||||
return fmt.Errorf("invalid file path in zip: %s", file.Name)
|
||||
}
|
||||
|
||||
// Reject symlinks in ZIP to prevent path traversal attacks.
|
||||
if file.FileInfo().Mode()&os.ModeSymlink != 0 {
|
||||
return fmt.Errorf("symlinks are not allowed in plugin zip: %s", file.Name)
|
||||
}
|
||||
|
||||
if file.FileInfo().IsDir() {
|
||||
if err := os.MkdirAll(filePath, 0o755); err != nil {
|
||||
return fmt.Errorf("create directory: %w", err)
|
||||
|
||||
@@ -138,6 +138,4 @@ const (
|
||||
// automatically pulled on first run if not already present locally.
|
||||
// Each entry is the short plugin name (without the workspace prefix);
|
||||
// the full qualified name is OfficialPluginWorkspace + "/" + name.
|
||||
var DefaultManagedPlugins = []string{
|
||||
"conference",
|
||||
}
|
||||
var DefaultManagedPlugins = []string{}
|
||||
Reference in new issue
Block a user