-
Notifications
You must be signed in to change notification settings - Fork 789
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.
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
feat: Disable short-name repository with config value #1227
feat: Disable short-name repository with config value #1227
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jrbeverly you are right, as discussed in #1194 #952, we do not wish to expose the plugin repo as a public API by committing to a specification. This solution would commit us to that API.
With regards to your "Other Information" section. I would be in favour of a boolean
flag to disable the shortname repo for those who are security conscious. Would you consider repurposing this PR for that solution? I imagine behaviour would log that the shortname repo is disabled and that you can add plugins via asdf plugin add <name> <git_url>
.
I will perform final review of this soon. |
@jrbeverly Thanks for reworking this PR 🙏 I made some additional changes:
Can you give this a look and comment with your approval before I merge? |
LGTM, the doc improvements talking about sync events make a lot of sense 👍 |
Awesome, thanks for your contribution @jrbeverly [Edit] This always happens right after I click merge, but we probably should have noted how this is different from the setting |
This looks great. Thanks @jthegedus and @jrbeverly ! |
Summary
Support disabling the installation & updating of the short-name repository using the configuration value
disable_short_name_repository
. This does not remove any existing cache/copy of the short-name repository.This change allows the short-name plugins repository to be disabled in distributed zip files (or existing installations) by setting the the configuration field
disable_short_name_repository
asyes
. This has the effect of disablingasdf plugin add <name>
, as no short-name repository will be configured to allow installation without a URL.Fixes: #1194
Motivation
One of the issues that I've encountered with
asdf
is in the compliance/security realm. Since the short-name repository is a map of git repositories for lookup, it hides the true source of how a plugin is installed & works. This can result in confusion scenarios where someone wasn't aware they were running bash scripts from an unknown github repository, of which they were working off latest.Documentation can always recommend running
asdf plugin add <name> <git-url>
(wheregit-url
is from a known 👍 set), but it is a bit of a concern point about "what ifs" and all that.For this change I wanted to allow the plugin repository to be disabled, requiring
asdf plugin add <name> <git-url>
in a way that was a minimally disruptive to the existing workflows that might rely onasdf plugin add <name>
.Other Information
For this change I was looking for a way to disable the default short-name plugins repository, so that
asdf plugin add <name> <git-url>
was the only way to install a plugin. I think completely removing the short-name plugins repository would be a major breaking change that would negatively impact a lot of workflows. So this change focuses on allowing it to be disabled.An opt-out system (e.g.disable_asdf_plugins_repository
) would accomplish this too, but a bug or misconfiguration could result in theasdf plugin add <name>
functionality becoming enabled. By using theasdf_repository_url
, as long as the distributedasdf.tar.gz
has thedefaults
file patched to remove this configuration, then the short-name repository should never be configured.Distributing theasdf.tar.gz
with the git URL patched (or the/repository
directory pre-configured) is a way of disabling the short-name repository at the moment.A concern with this change is that the introduction of a configuration field likeasdf_repository_url
could/may/will give the false(?) impression that providing a custom short-name plugins repository is a supported feature (with associated overhead of defining API/etc/etc), which isn't the intention of this change (e.g.asdf plugin add <name> <git-url>
). I'm assuming based on that previous conversation that it isn't your intention to support that, so I have added to the docs accordingly. If that isn't the case, then this bit can be ignored (& relatedwarning
in docs removed)Previous discussions: #952