Disable importers for self-managed by default and require them to be enabled to be used
Where is the idea coming from?
This was proposed by @nmalcolm here:
This would be a separate issue, but do we have data on how often importers are used and whether we might disable one/some/all of them by default on self-managed?
- On one hand it could be a "bad look" to disable features, on the other it's pragmatic to ship a product with reduced attack surface if a feature is rarely used.
- This wouldn't prevent vulnerabilities from existing, but would protect customers that have them disabled
- I can imagine having Import from Template enabled makes sense, but how many import from Gitea or do Bulk Imports or import from GitHub / GitLab (except maybe for a month or two when first migrating to GitLab).
In general, I think it is a good idea (at least from a security point of view). Under the admin settings, there is already the possibility to enable/disable import features:
However, the list is not complete (for example
GitLab migration
is missing here). I propose the following (and I will create a separate issue in a bit for the items below):
- Let's make sure that the list of imported sources contains all available importers
- Let's ensure that disabling an importer will disable it entirely (both on the UI but also on the API level - if applicable)
- Let's also discuss the default value (enabled/disabled) for each of these importers for self-managed instances. I suppose, if we decide to disable some, we should disable all by default (as GitLab migration and GitHub importer have been the ones being affected the most recently) and not just importers that have low usage? Also, I'd like to hear what Product (cc @m_frankiewicz) thinks about this idea as I would imagine it will have an impact on the user experience.
Questions
-
If admins need to turn importers on, and this requires manual steps in the UX, will they then just leave them on to avoid the headache in the future? - that's no longer on us
-
It’s not currently clear to self-managed users why importers would be disabled. We would need to document the inherent risk of enabling them or else disabling them makes no sense. But we probably need to be careful here because documenting the risk is also broadcasting the risk of our SaaS offering which could lead to further abuse. - we won't be additionally documenting
-
If any/some/all importers should be disabled by default on SM, how should they be showed (or not) on SaaS? - it should get enabled by gitlab.com admin after change is in Production
-
Would disabling importers by default on SM require, apart from change itself and update to docs, any special communication to users on SM? Can this be done in any release? - standard release blog post, milestone 16.0, see comment
Documentation changes required
From comment:
The docs mostly treat those as if they are off by default (or could be interpreted either way):
- We have generic docs at: https://docs.gitlab.com/ee/user/admin_area/settings/visibility_and_access_controls.html#configure-allowed-import-sources.
- We have feature specific docs sometimes also: https://docs.gitlab.com/ee/user/project/settings/import_export.html#configure-file-exports-as-an-import-source.
When self-managed and GitLab.com settings diverge, we document that at: https://docs.gitlab.com/ee/user/gitlab_com/.
So I think the docs solution on top of the code solution is:
- Ensure there is one set of documentation for the feature, and cross link to it (don't duplicate docs on enabling an import source for each type of import.). Add that import sources are off by default (assuming they are all set that way). If some are and some aren't off by default, see next point
- Add a new section to https://docs.gitlab.com/ee/user/gitlab_com/ that lists which import sources are on by default on GitLab.com. Probably a table with checkmarks in it like: https://docs.gitlab.com/ee/user/gitlab_com/#gitlab-pages. We can actually list there what the self-managed default setting is as we have for other settings..
Proposal for disabling importers by default
Enabled importers - aside from Direct Transfer/Bulk Import are stored in the application_settings.import_sources
database table. It contains an array of import sources, e.g.:
["github", "bitbucket", "gitlab", "fogbugz", "git", "gitlab_project", "gitea", "manifest"]
Whenever an importer is enabled or disabled via the admin UI this column gets updated, the importer added or removed from the array.
Application settings are initialized via the /config/initializers/1_settings.rb. file:
Settings.gitlab['import_sources'] ||= Gitlab::ImportSources.values
This looks in the DB, and if there are no application.import_sources
it loads them from Gitlab::ImportSources.values.
We've decided that moving forward these importers should be disabled for new installations (and Admins of existing installations be informed that they should disable any importers they don't use).
To disable by default it will be as simple as removing the second half of this conditional., however we need to handle the case for For reference, .com
where by default all importers will be enabled. This will require a Production change request to gitlab.com admin, which @wortschi will take care of once the changes are in production..com
already has all the import_sources enabled:
[ gprd ] production> Settings.gitlab['import_sources']
=> ["github", "bitbucket", "bitbucket_server", "gitlab", "fogbugz", "git", "gitlab_project", "gitea", "manifest", "phabricator", "gitlab_custom_project_template"]
For Direct Transfer/Bulk Import, do we want to add this as an importer to the list of we'll discuss adding them to the other inport-sources once the feature is out of Beta.import_sources
?