Skip to content

🛡️ Sentinel: [CRITICAL] Fix path traversal in plugin manager #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
himattm wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: [CRITICAL] Fix path traversal in plugin manager #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-05-23 - Prevent Path Traversal in Plugin Paths
**Vulnerability:** Plugin names parsed from untrusted metadata (`# @name`) or extracted from direct URLs were passed unsanitized into `filepath.Join`, allowing path traversal (e.g., writing or deleting arbitrary files outside the plugin directory).
**Learning:** `filepath.Join` cleans paths but evaluates `../` sequences internally. Thus, combining a base directory with untrusted input containing `../` still allows escaping the base directory.
**Prevention:** Sanitize untrusted input using `filepath.Base(filepath.Clean("/" + name))` *before* joining it with base directories to explicitly restrict it to a single filename.
18 changes: 12 additions & 6 deletions internal/plugin/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ func NewManager() *Manager {
}
}

// sanitizePluginName ensures the given name cannot escape the target directory
// by stripping path components.
func sanitizePluginName(name string) string {
return filepath.Base(filepath.Clean("/" + name))
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation returns a path separator (e.g., / or \) if the input name is empty or resolves to the root (e.g., ..). This results in an invalid filename like prism-plugin-/, which will cause subsequent file operations to fail with an "invalid argument" or "is a directory" error. Consider returning a safe default name or an empty string in these cases to ensure a valid filename is always generated.

}

// Discover finds all installed plugins (both scripts and binaries)
func (m *Manager) Discover() ([]Plugin, error) {
if err := os.MkdirAll(m.pluginDir, 0755); err != nil {
Expand Down Expand Up @@ -365,7 +371,7 @@ func (m *Manager) addBinaryPlugin(owner, repo, pluginName string) error {
return err
}

destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", pluginName))
destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", sanitizePluginName(pluginName)))
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Sanitize the pluginName variable itself. This ensures that the sanitized name is used consistently throughout the function, including in the metadata saved at line 388 and for the display name in the UI, rather than just for the destination path.

Suggested change
destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", sanitizePluginName(pluginName)))
pluginName = sanitizePluginName(pluginName)
destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", pluginName))


// Check if already installed
if err := m.checkExistingPlugin(destPath, pluginName); err != nil {
Expand Down Expand Up @@ -441,7 +447,7 @@ func (m *Manager) addScriptPlugin(owner, repo, pluginName string) error {
return err
}

destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", meta.Name))
destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", sanitizePluginName(meta.Name)))
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Sanitize the meta.Name field directly. This ensures the sanitized name is used for the destination path, the existence check, and the final confirmation message, providing a consistent user experience and preventing UI spoofing with path-like names.

Suggested change
destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", sanitizePluginName(meta.Name)))
meta.Name = sanitizePluginName(meta.Name)
destPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", meta.Name))


if err := m.checkExistingPlugin(destPath, meta.Name); err != nil {
return err
Expand Down Expand Up @@ -492,9 +498,9 @@ func (m *Manager) addFromDirectURL(url string) error {

var destPath string
if isScript {
destPath = filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", pluginName))
destPath = filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", sanitizePluginName(pluginName)))
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Sanitize pluginName here so that the sanitized version is used for both the path construction and the metadata/display logic later in the function (lines 518 and 522).

Suggested change
destPath = filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", sanitizePluginName(pluginName)))
pluginName = sanitizePluginName(pluginName)
destPath = filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", pluginName))

} else {
destPath = filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", pluginName))
destPath = filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", sanitizePluginName(pluginName)))
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Sanitize pluginName here to ensure consistency between the filename and the metadata/display name used later in the function.

pluginName = sanitizePluginName(pluginName)
		destPath = filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", pluginName))

}

if err := m.checkExistingPlugin(destPath, pluginName); err != nil {
Expand Down Expand Up @@ -784,8 +790,8 @@ func (m *Manager) updateScriptPlugin(p Plugin, client *http.Client) error {
// Remove uninstalls a plugin (handles both binaries and scripts)
func (m *Manager) Remove(name string) error {
// Try binary first, then script
binaryPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", name))
scriptPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", name))
binaryPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", sanitizePluginName(name)))
scriptPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", sanitizePluginName(name)))
Comment on lines +793 to +794
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Sanitize the name variable once at the start of the function. This simplifies the path construction and ensures that the sanitized name is used in the final "Removed" message at line 811, avoiding potentially confusing output if a path-like name was provided.

Suggested change
binaryPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", sanitizePluginName(name)))
scriptPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", sanitizePluginName(name)))
name = sanitizePluginName(name)
binaryPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s", name))
scriptPath := filepath.Join(m.pluginDir, fmt.Sprintf("prism-plugin-%s.sh", name))


var path string
if _, err := os.Stat(binaryPath); err == nil {
Expand Down
Loading