chore: re-vendor lib/

[cbrnrd]

This PR re-vendors lib/ (commit de6bd07264d). Among other changes, this includes a security fix to prevent attacker controlled filenames in batch changes from executing commands.

[keegancsmith]

lib is vendored in from our monorepo, so you need to make changes rather in the monorepo and then vendor it back in here.

I'm also not sure we should be escaping these values. Our templates are not just used for constructing commands in batch changes. I'll let @burmudar comment further, I think he had looked into this stuff not so long ago?? (Or did we both look into it? :) )

Generally batch changes are run on repos you have write access to though... which makes this not really an issue in practice. However, I do see us needing to provide some way to do shell escaping since that means previous uses of this would naively break i fyou r filename had a space in it

[keegancsmith]

no need to sort

[cbrnrd]

Why not? the previous version of the function sorted the paths.

[burmudar]

lib is vendored in from our monorepo, so you need to make changes rather in the monorepo and then vendor it back in here.

I'm also not sure we should be escaping these values. Our templates are not just used for constructing commands in batch changes. I'll let @burmudar comment further, I think he had looked into this stuff not so long ago?? (Or did we both look into it? :) )

Generally batch changes are run on repos you have write access to though... which makes this not really an issue in practice. However, I do see us needing to provide some way to do shell escaping since that means previous uses of this would naively break i fyou r filename had a space in it

Yeah we did at some point, it's all a bit muddled now but I took a jog down memory lane. This templating is used to render into step-condition or the run step so on some level it is good to escape this.

• I think we need to add shellquote_split too if we have join
• Document both so that customers can use them more deliberately.

[cbrnrd]

Generally batch changes are run on repos you have write access to though... which makes this not really an issue in practice.

I disagree. Even if you have access to the repository, that doesn't necessarily make it a trusted source you want to be running code from. Since the attack vector here is a filename, just by including a malicious repo in your spec can trigger command execution locally.

[cbrnrd]

@keegancsmith @burmudar I opened a PR in the monorepo for this: https://github.com/sourcegraph/sourcegraph/pull/12869
Once that's merged to main I'll update lib/ here.