Michael Haggerty [off-list ref] writes:
While we're at it, I think it would be prudent to ban '-' at the
beginning of reference name segments. For example, reference names like
refs/heads/--cmd=/sbin/halt
refs/tags/--exec=forkbomb(){forkbomb|forkbomb&};forkbomb
are currently both legal, but I think they shouldn't be.
I think we forbid these at the Porcelain level ("git branch", "git
checkout -b" and "git tag" should not let you create "-aBranch"),
while leaving the plumbing lax to allow people experimenting with
their repositories.
It may be sensible to discuss and agree on what exactly should be
forbidden (we saw "leading dash", "semicolon and dollar anywhere"
so far in the discussion) and plan for transition to forbid them
everywhere in a next big version bump (it is too late for 2.0).
On 2014-04-22 13:38, Junio C Hamano wrote:
Michael Haggerty [off-list ref] writes:
quoted
While we're at it, I think it would be prudent to ban '-' at the
beginning of reference name segments. For example, reference names like
refs/heads/--cmd=/sbin/halt
refs/tags/--exec=forkbomb(){forkbomb|forkbomb&};forkbomb
are currently both legal, but I think they shouldn't be.
I think we forbid these at the Porcelain level ("git branch", "git
checkout -b" and "git tag" should not let you create "-aBranch"),
while leaving the plumbing lax to allow people experimenting with
their repositories.
It may be sensible to discuss and agree on what exactly should be
forbidden (we saw "leading dash", "semicolon and dollar anywhere"
so far in the discussion)
Also backquote anywhere.
and plan for transition to forbid them
everywhere in a next big version bump (it is too late for 2.0).
Would it be acceptable to have a config option to forbid these in a
non-major version bump? Does parsing config files add too much overhead
for this to be feasible?
If it's OK to have a config option, then here's one possible transition
path (probably flawed, but my intent is to bootstrap discussion):
1. Add an option to forbid dangerous characters. The option defaults
to disabled for compatibility. If the option is unset, print a
warning upon encountering a ref name that would be forbidden.
2. Later, flip the default to enabled.
3. Later, in the weeks/months leading up to the next major version
release, print the warning even if the config option is set to
disabled.
Thanks,
Richard