3 ms·

I peeked at the sandbox escape bug and its associated fix: https://github.com/Homebrew/brew/pull/17700/commits/f4e5e0c716585b072b5a401602f7847272ae5d4a https://

I peeked at the sandbox escape bug and its associated fix: https://github.com/Homebrew/brew/pull/17700/commits/f4e5e0c716585b072b5a401602f7847272ae5d4a https://github.com/Homebrew/brew/pull/17700/commits/f4e5e0c7.... Is this…correct? Like, this is to prevent it from being interpolated into a sandbox profile and messing things up. How confident are we about this list of characters?

js2 2y ago
The commit message could have answered that question, but instead, it only repeats what the diff already tells us: "Don't allow special characters in sandbox rule paths". It doesn't explain why, or what's special about the particular characters that it disallows. A better message would have said something like "Prevent certain characters in the path because otherwise ... We need to worry about these specific characters but not others because..." Even if I wrote this code and knew what it did today, if I come across this it a year from now, I'm left scratching my head: why did I make this change? A real world example of a good commit message: https://github.com/git/git/commit/92fe7c7d42cc941ed70d6fce988d6b7936a9765a https://github.com/git/git/commit/92fe7c7d42cc941ed70d6fce98...
frenchman99 2y ago
Surprised too, if that is the fix. Wouldnt a whitelist be better than a blacklist?