executable bit not checked when loading plugin manually

[ghost]

When I download a precompiled binary plugin from GitHub I can load it manually (snaptel plugin load plugin_name) even if it does not have executable bit set, but when I put the same plugin on auto_discover_path it is ignored and log message like "Auto-loading of plugin 'snap-plugin-name' skipped (plugin not executable)" is generated.
So currently there is inconsistency in the behavior when loading plugins manually or automatically - I think we should either enforce executable bit for loading plugins everywhere (preferred option) or nowhere.

[IzabellaRaulin]

cc @intelsdi-x/snap-maintainers

[jcooklin]

When a plugin is loaded from the CLI the plugin (file) is posted to the REST interface where the server (snapteld) then writes it to a temporary location where it actually runs.

If we were to check the permissions on loading it could happen in the client.

If we changed the behavior and checked for execute permissions in the client we could end up causing as many (or more) user errors than we avoid due to workflows that involve using wget to download and then load the plugin.

other thoughts, opinions?

cc: @jtlisi

[jtlisi]

The current implementation works well for me. I think as long as the release binaries for snap plugins have the executable bit set it should not be an issue for anyone starting out. However, if a change were to be made I think altering the auto-discover-path to use a temp file would be better than the reverse. A temp file would help make cicd a slightly easier process for deploying new versions plugins on the auto-discover-path while snap is running. I described the issue to @nanliu last week. It is not a major issue, and having an option that runs the plugin executable directly comes with some advantages.

[jcooklin]

altering the auto-discover-path to use a temp file ...

@jtlisi: Issue #1502 has been created to track resolving this.

[jcooklin]

Thanks for submitting this issue and highlighting our need for documentation. Improving the documentation related to the execute bit being required for autoloading plugins will be tracked by #1499.

Manual plugin loading should not require the execute bit.

• The user is already being explicit about wanting the plugin to be loaded
• There is no need to conform to some standard on permissions
  • The fact that plugins in the autoload path need to have proper permissions including the execute bit is serving a different set of uses cases

Closing this issue.