Re: [RFC PATCH 0/2] MVP implementation of remote-suggested hooks

From
brian m. carlson <hidden>
Date
2021-06-24 23:11:39
Reply to
Jonathan Tan, 2021-06-23
On 2021-06-23 at 22:58:09, Jonathan Tan wrote:
quoted
If we do add this feature (which, as I said, I'm opposed to) and we
decide to store it in a ref, that ref should not be a normal branch by
default (it should be a special one-level ref, like refs/stash or such),
Any particular reason not to expose it as a branch (besides following
from your general idea that a user should seek out such a feature and
not have it presented to them up-front)?
Branches are for the main code of the project.  While it's possible to
have orphan branches that do other things, I think that's in general an
anti-pattern, and using a special ref for things which are separate and
independent from the main code of the repository would be a more elegant
solution.
quoted
In addition, there should be an advice.* option that allows people to
turn this off once and for all, and it should be clearly documented.
Ideally it should be off by default.
I don't think this would be considered "advice" like the other options,
but having an option to turn this off once and for all makes sense.
Making it off by default would probably mean that projects that use such
hooks would recommend cloning with "git -c my-config=1 clone $URL", but
perhaps that's OK.
Sure, I'm not picky about what it looks like in "advice" vs something
else.  I think forcing projects to explicitly opt-in to this behavior
means that the social engineering and security problems are much
reduced, and while I'm still not wild about the idea, I would feel much
better about it.
quoted
This also makes me deeply nervous for much of the same reasons.  There
are situations where e.g. ignoring whitespace can lead to security
problems in code review (think Python), and in general it's hard to
reason about all the ways people can do malicious things.  Typically
adding untrusted config ends poorly (think of all the modeline
vulnerabilities in Vim).

I'd definitely want support for this to be off with no prompting by
default.
To use your example, the model we're proposing is more of only using the
modelines from sources we trust - as opposed to ensuring that all
possible options set by modelines are benign. Admittedly, the
administrator of the source may have difficulty ensuring that bad code
doesn't slip through code review, for example, but that is a problem
they already deal with (at least for projects with any form of
executable code in them, e.g. production code or a build script).
As I think I've previously mentioned, I don't want to receive
configuration of my development environment from sources I trust.  Even
at work, I don't want the repositories I work with to modify my
development environment in this way.  I tend to have a highly customized
configuration that breaks many people's expectations about tooling, so
the cases that this isn't a security problem (in repositories I trust)
can still result in a functionality problem.

Also, since we don't know what repositories the user trusts, the only
safe assumption is that the user trusts nothing unless they explicitly
tell us.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

Attachments

Thread (25 messages) 25 messages, 7 authors, last reply 2021-07-27
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help