Skip to content

✨ feat(audit): add audit logging to plugin-manager subcommands + pre-existing security issues #913

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
Tamar-Dinavetsky wants to merge 2 commits into
base: main
Choose a base branch
from
Open

✨ feat(audit): add audit logging to plugin-manager subcommands + pre-existing security issues #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00223a9
:sparkles: feat(audit): add audit logging to plugin-manager subcomm…
Tamar-Dinavetsky Sep 2, 2026
945b554
validate plugin filename against path traversal and restrict binary …
Tamar-Dinavetsky Sep 3, 2026
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
38 changes: 31 additions & 7 deletions cmd/plugin-manager/add/add.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,34 +50,41 @@ func (o *Options) Complete(c *cobra.Command, args []string) error {

func (o *Options) Validate(args []string) error {
// TODO: @jgabani
log := o.log

if len(args) != 1 {
log.Warnf("Expected exactly one plugin name, got %d", len(args))
return errors.New("please input only one plugin name")
}

if o.Global {
if o.PluginDir == os.Getenv("HOME")+plugin.DefaultLocalPluginDir {
o.PluginDir = plugin.GlobalPluginDir
} else {
log.Warnf("--plugin-dir and --global cannot be used together")
return errors.New("--plugin-dir and --global should not be used together.")
}
}

pluginDir, err := filepath.Abs(o.PluginDir)
if err != nil {
log.Errorf("Failed to resolve plugin directory path %q: %v", o.PluginDir, err)
return err
}

files, err := ioutil.ReadDir(pluginDir)
if err != nil {
if os.IsNotExist(err) {
log.Debugf("Plugin directory %q does not exist, no installed plugins to check", pluginDir)
return nil
}
log.Errorf("Failed to read plugin directory %q: %v", pluginDir, err)
return err
}

paths, err := plugin.LocateBinaryInPluginDir(o.PluginDir, args[0], files)
if err != nil {
log.Errorf("Failed to locate plugin %s in %q: %v", args[0], o.PluginDir, err)
return err
}

Expand All @@ -86,6 +93,7 @@ func (o *Options) Validate(args []string) error {
for _, path := range paths {
fmt.Printf("%s \n", path)
}
log.Warnf("Plugin %s is already installed", args[0])
return errors.New("the binary is already installed in the above path, either delete the binary or mention a repo from which the binary is needed")
}
return nil
Expand Down Expand Up @@ -134,8 +142,11 @@ func addFlagsForOptions(o *Flags, cmd *cobra.Command) {
func (o *Options) run(args []string) error {
log := o.log

log.Infof("Starting plugin-manager add %s...", args[0])

manifestMap, err := plugin.BuildManifestMap(log, args[0], o.Repo)
if err != nil {
log.Errorf("Failed to build manifest for plugin %s: %v", args[0], err)
return nil
}

Expand All @@ -160,6 +171,7 @@ func (o *Options) run(args []string) error {
if value.Name != "" && (o.Version == "" || string(value.Version) == o.Version) {
uri, err := binaryURIForPlatform(value)
if err != nil {
log.Errorf("No binary available for plugin %s on this platform: %v", value.Name, err)
return err
}
return downloadBinary(o.PluginDir, value.Name, uri, log)
Expand All @@ -182,6 +194,7 @@ func (o *Options) run(args []string) error {
if string(value.Version) == installVersion {
uri, err := binaryURIForPlatform(value)
if err != nil {
log.Errorf("No binary available for plugin %s %s on this platform: %v", value.Name, installVersion, err)
return err
}
return downloadBinary(o.PluginDir, value.Name, uri, log)
Expand All @@ -192,23 +205,29 @@ func (o *Options) run(args []string) error {
default:
// throw error saying that the plugin doest exists
log.Errorf("The plugin %s is not found", args[0])
fmt.Println(fmt.Sprintf("Run \"crane plugin-manager list\" to list all the available plugins \n"))
fmt.Println("Run \"crane plugin-manager list\" to list all the available plugins")
}
}
default:
// throw error saying that the plugin doest exists
fmt.Println(fmt.Sprintf("Run \"crane plugin-manager list\" to list all the available plugins \n"))
log.Warnf("The plugin %s is not found", args[0])
fmt.Println("Run \"crane plugin-manager list\" to list all the available plugins")
return errors.New(fmt.Sprintf("The plugin %s is not found", args[0]))
}
return nil
}

func downloadBinary(filepath string, filename string, url string, log *logrus.Logger) error {
func downloadBinary(pluginDir string, filename string, url string, log *logrus.Logger) error {
if filepath.Base(filename) != filename || filename == "." || filename == ".." {
return fmt.Errorf("invalid plugin filename %q", filename)
}

var binaryContents io.Reader
isUrl, url := plugin.IsUrl(url)
if !isUrl {
srcPlugin, err := os.Open(url)
if err != nil {
log.Errorf("Failed to open local plugin binary %s: %v", url, err)
return err
}
defer srcPlugin.Close()
Expand All @@ -217,36 +236,41 @@ func downloadBinary(filepath string, filename string, url string, log *logrus.Lo
// Get the data
resp, err := http.Get(url)
if err != nil {
log.Errorf("Failed to download plugin binary from %s: %v", url, err)
return err
}
defer resp.Body.Close()
binaryContents = resp.Body
}
// Create dir if not exists
if _, err := os.Stat(filepath); os.IsNotExist(err) {
err = os.MkdirAll(filepath, os.ModePerm)
if _, err := os.Stat(pluginDir); os.IsNotExist(err) {
err = os.MkdirAll(pluginDir, os.ModePerm)
if err != nil {
log.Errorf("Failed to create plugin directory %s: %v", pluginDir, err)
return err
}
}

// Create the file
pluginBinary, err := os.OpenFile(filepath+"/"+filename, syscall.O_RDWR|syscall.O_CREAT|syscall.O_TRUNC, 0777)
pluginBinary, err := os.OpenFile(filepath.Join(pluginDir, filename), syscall.O_RDWR|syscall.O_CREAT|syscall.O_TRUNC, 0755)
if err != nil {
log.Errorf("Failed to create plugin file %s/%s: %v", pluginDir, filename, err)
return err
}
defer pluginBinary.Close()

// Write the body to filePluginDir
_, err = io.Copy(pluginBinary, binaryContents)
if err != nil {
log.Errorf("Failed to write plugin binary %s: %v", filename, err)
return err
}
err = pluginBinary.Sync()
if err != nil {
log.Errorf("Failed to sync plugin binary %s: %v", filename, err)
return err
}
log.Infof("pluginBinary %s added to the path - %s", filename, filepath)
log.Infof("pluginBinary %s added to the path - %s", filename, pluginDir)
return err
}

Expand Down
10 changes: 8 additions & 2 deletions cmd/plugin-manager/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,18 @@ func addFlagsForOptions(o *Flags, cmd *cobra.Command) {

func (o *Options) run() error {
log := o.log

log.Infof("Starting plugin-manager list...")

if o.Installed {
// retrieve list of all the plugins that are installed within plugin dir
// TODO: differentiate between multiple repos
plugins, err := plugin.GetFilteredPlugins(o.PluginDir, []string{}, log)
if err != nil {
log.Errorf("Failed to get installed plugins from %s: %v", o.PluginDir, err)
return err
}
fmt.Println(fmt.Sprintf("Listing plugins from path - %s, along with default plugin", o.PluginDir))
fmt.Printf("Listing plugins from path - %s, along with default plugin\n", o.PluginDir)
printInstalledInformation(plugins)
return nil
}
Expand All @@ -117,6 +121,7 @@ func (o *Options) run() error {
if o.Name != "" && (o.Params || o.Versions) {
manifestMap, err := plugin.BuildManifestMap(log, o.Name, o.Repo)
if err != nil {
log.Errorf("Failed to build manifest for plugin %s: %v", o.Name, err)
return nil
}
if len(manifestMap) == 0 {
Expand Down Expand Up @@ -145,11 +150,12 @@ func (o *Options) run() error {
} else {
manifestMap, err := plugin.BuildManifestMap(log, "", o.Repo)
if err != nil {
log.Errorf("Failed to build manifest: %v", err)
return nil
}

if o.Name != "" {
log.Info(fmt.Sprintf("\"--name\" flag should be used with either \"--versions\" or \"--params\" flag to get more information about the plugin, example: \"crane plugin-manager --name %s --versions\" or \"crane plugin-manager --name %s --params\"\n", o.Name, o.Name))
log.Infof("\"--name\" flag should be used with either \"--versions\" or \"--params\" flag to get more information about the plugin, example: \"crane plugin-manager --name %s --versions\" or \"crane plugin-manager --name %s --params\"", o.Name, o.Name)
} else if o.Params {
// retrieve all the information for all the versions available for a specific plugin
for repo, pluginsMap := range manifestMap {
Expand Down
11 changes: 9 additions & 2 deletions cmd/plugin-manager/remove/remove.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,25 +80,31 @@ func NewRemoveCommand(f *flags.GlobalFlags) *cobra.Command {

func (o *Options) run(args []string) error {
log := o.log

log.Infof("Starting plugin-manager remove %s...", args[0])

pluginDir, err := filepath.Abs(fmt.Sprintf("%v/%v", o.PluginDir, o.Repo))
if err != nil {
log.Errorf("Failed to resolve plugin directory path: %v", err)
return err
}

files, err := ioutil.ReadDir(pluginDir)
if err != nil {
log.Errorf("Failed to read plugin directory %s: %v", pluginDir, err)
return err
}

paths, err := plugin.LocateBinaryInPluginDir(pluginDir, args[0], files)
if err != nil {
log.Errorf("Failed to locate plugin %s in %s: %v", args[0], pluginDir, err)
return err
}

if len(paths) > 1 {
// fail and ask for a specific repo
log.Errorf("The binary is installed from multiple source, please specify repository from which you want to remove the plugin using --repo \n")
fmt.Printf("The binary is present in the following path")
log.Errorf("The binary is installed from multiple sources, please specify repository from which you want to remove the plugin using --repo")
fmt.Println("The binary is present in the following path")
for _, path := range paths {
fmt.Printf("%s \n", path)
}
Expand All @@ -108,6 +114,7 @@ func (o *Options) run(args []string) error {
} else {
err = os.Remove(paths[0])
if err != nil {
log.Errorf("Failed to remove plugin %s from %s: %v", args[0], paths[0], err)
return err
}
log.Infof("The plugin %s removed from path - %s", args[0], paths[0])
Expand Down
Loading