[Buildbot-devel] Branch name restrictions

[Brian Warner]

Jean-Paul Calderone <exarkun at divmod.com> writes:

>>I would say that we should definitely at the very least block quote 
>>characters (or escape them) to prevent web-build based expoits.
>>Those build-names are passed as args to forked commands.
>
> They shouldn't be passed through a shell.  I don't know that they /aren't/,
> but there's no need for them to be.

In general I agree, and the unix-based ShellCommands do the right thing. At
one point, I think the windows side had to do " ".join(), because that seemed
to make it work. But now that I look at the code, it appears to not do this
anymore (buildbot/slave/commands.py:ShellCommand._startCommand):

        if type(self.command) in types.StringTypes:
            if runtime.platformType  == 'win32':
                argv = [os.environ['COMSPEC'], '/c', self.command]
            else:
                # for posix, use /bin/sh. for other non-posix, well, doesn't
                # hurt to try
                argv = ['/bin/sh', '-c', self.command]
        else:
            if runtime.platformType  == 'win32':
                argv = [os.environ['COMSPEC'], '/c'] + list(self.command)
            else:
                argv = self.command

Will this handle spaces and backslashes and other nasties? I'd love for
someone who knows about Twisted and windows and spawning processes to look at
this and inspire some confidence that it works correctly..

thanks,
 -Brian