Skip to content

[BUG] plugin-manager add installs the HTTP error body when a plugin download fails #968

Description

@psrvere

What version of crane are you running, and what are your clutsters+platform

crane built from v0.11.0-alpha.1 (fe0fa07), darwin/arm64. No cluster involved: this is entirely client-side, in plugin-manager add.

The same code is on main today.

What did you expect to happen?

crane plugin-manager add fetches a plugin binary over HTTP. When that request answers 404, I expected the command to fail and to install nothing.

What actually happened?

It reported success and installed the 404 page as the plugin.

downloadBinary in cmd/plugin-manager/add/add.go checks only the transport error from http.Get, never resp.StatusCode, then copies the body straight into the plugin directory:

resp, err := http.Get(url)
if err != nil {
    return err
}
defer resp.Body.Close()
binaryContents = resp.Body
...
pluginBinary, err := os.OpenFile(filepath+"/"+filename, syscall.O_RDWR|syscall.O_CREAT|syscall.O_TRUNC, 0777)
...
_, err = io.Copy(pluginBinary, binaryContents)

So any non-2xx body becomes an executable file named after the plugin, and the command exits 0.

The reason this is worth fixing rather than living with: the failure surfaces a long way from its cause. Nothing at install time hints that anything went wrong, and the error that eventually appears points at the plugin rather than at the download. Someone hitting this reasonably suspects a bad release build or an architecture mismatch.

It is easy to hit by accident. A plugin index entry whose release has not been published yet is enough, and a draft GitHub release serves 404 for its assets, so the window exists for any plugin between the index PR merging and the release going public.

Please include any relevant logs or errors

Reproduction, with an index entry pointing at a release that does not exist yet:

$ curl -sIL "https://github.com/migtools/crane-plugin-buildconfig-to-builds/releases/download/v0.1.0/arm64-darwin-buildconfigtobuildsplugin-v0.1.0" | tail -1
HTTP/2 404

$ DEFAULT_REPO_URL=/tmp/idx/index.yaml crane plugin-manager add BuildConfigToBuildsPlugin --plugin-dir /tmp/pd
time="2026-09-11T14:11:50+05:30" level=info msg="pluginBinary BuildConfigToBuildsPlugin added to the path - /tmp/pd"

$ echo $?
0

$ ls -l /tmp/pd/
-rwxr-xr-x  1 user  wheel  268458 Sep 11 14:11 BuildConfigToBuildsPlugin

$ file /tmp/pd/BuildConfigToBuildsPlugin
/tmp/pd/BuildConfigToBuildsPlugin: HTML document text, Unicode text, UTF-8 text, with very long lines (35890)

What the user sees later, which is where they start debugging:

$ crane plugin-manager list --installed --plugin-dir /tmp/pd
level=error msg="unable to run the plugin binary" pluginPath=/tmp/pd/BuildConfigToBuildsPlugin
level=error msg="error running the plugin metadata command" pluginPath=/tmp/pd/BuildConfigToBuildsPlugin
Error: error running the plugin metadata command: unable to run the plugin binary, err: fork/exec /tmp/pd/BuildConfigToBuildsPlugin: exec format error

A resp.StatusCode check before the copy would catch it, and returning an error naming the URL and the status would make the message match the cause. Whether a non-2xx should be a hard failure, and whether a Content-Type check is worth adding on top, is your call.

Happy to open a PR if you would like one.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind/bugCategorizes issue or PR as related to a bug.

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions