[RFC] Is it time to switch 'el-get-allow-insecure to nil#2439
[RFC] Is it time to switch 'el-get-allow-insecure to nil#2439manandbytes wants to merge 1 commit intodimitri:masterfrom
Conversation
manandbytes
force-pushed
the
insecure-check
branch
from
4698630 to
4396e20
Compare
| (when (and (not el-get-allow-insecure) | ||
| (not (stringp (car (member protocol el-get-secure-protocols)))) | ||
| (not file-local) | ||
| (not (string-match "^[-_\.A-Za-z0-9]+@" url))) |
There was a problem hiding this comment.
I can't remember a signle case of such URL and going to remove support for URLs starting with 'USERNAME@'.
There was a problem hiding this comment.
ssh+git URLs can start with USERNAME@, see https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#The-SSH-Protocol. Apparently ssh://USERNAME@... also works, but github always offers git@github.com... for ssh URLs.
We're currently in a "rolling release" model, which simplifies the questions of how to upgrade from an older release to a new one :) So before cutting a release we'd have to think about how to answer those. |
| (defcustom el-get-secure-protocols '("https" "ssh" "git+ssh" "bzr+ssh" "sftp") | ||
| "List of secure protocols" | ||
| :group 'el-get | ||
| :package-version '(el-get-custom . "6") |
There was a problem hiding this comment.
No sure if el-get-custom is the right package as el-get-methods.el actually depends on el-get-secure-protocols. Or even (el-get . "6")?
There was a problem hiding this comment.
The package should be just el-get, I think. el-get-custom is not versioned independently.
manandbytes
force-pushed
the
insecure-check
branch
from
4396e20 to
8cdba8c
Compare
|
Hmm, perhaps we should tag some kind of release, since it seems that el-get is in melpa-stable right now (see #2438). |
manandbytes
force-pushed
the
insecure-check
branch
3 times, most recently
from
28bae7e to
a001240
Compare
| (defun el-get-insecure-check (package url) | ||
| (let* ((checksum (plist-get (el-get-package-def package) :checksum)) | ||
| (defun el-get-insecure-check (PACKAGE URL) | ||
| "Check if it's safe to install PACKAGE from url. |
There was a problem hiding this comment.
Still not sure about the docstring...
There was a problem hiding this comment.
Should be from URL, and I think the docstring should mention that an error is thrown if it's insecure/not safe. Otherwise looks good to me.
|
As it turned out this PR's main feature is defcustom for the list of secure URL protocols, I'm not sure what to do next. I'm going to create a separate PR for this, is it 🆗? These commits are not ready anyway and should not block the rest:
|
Sure thing. |
Done, #2441. |
manandbytes
force-pushed
the
insecure-check
branch
from
a001240 to
4cc3591
Compare
manandbytes
force-pushed
the
insecure-check
branch
from
4cc3591 to
48cca90
Compare
2 participants
...and cut a new release? Please, do not merge yet as, in the first place, I'm looking for feedback if I'm doing something completely stupid ;-)
I'm going update some commit messages and documentation later.
Unresolved issues
username@URL protocol.git clone user@server:project.gitis a shortcut forgit clone ssh://user@server/project.gitrequireingurl-parseintroduces a circular dependency on loading some recipes, CEDET recipe broken after change tomethods/el-get-git.el(16d6594) #1939 😟