PSA: Did you know that it’s **unsafe** to put code diffs into your commit messages?

[zekjur@mas.to]

PSA: Did you know that it’s **unsafe** to put code diffs into your commit messages?

Like https://github.com/i3/i3/pull/6564 for example

Such diffs will be applied by patch(1) (also git-am(1)) as part of the code change!

This is how a sleep(1) made it into i3 4.25-2 in Debian unstable.

[bmarinov@hachyderm.io]

@zekjur this cannot be intentional. No way.

[kaleissin@wandering.shop]

@zekjur Malicious prompts in commit messages when?

[zekjur@mas.to]

FWIW, git cherry-pick works as expected (at least)

[yvandasilva@hachyderm.io]

@zekjur til

[max@toet.dnzm.nl]

@bmarinov @zekjur One of the posts in the screenshots explicitly mentions that it is, in fact, intentional. Somewhat naive, perhaps, but intentional.

[heiglandreas@phpc.social]

@zekjur The commit-message should explain the "why", not the "how".

So when one needs a full diff (that can be retrieved by git itself) to explain the "why" ... I'd consider that an ... interesting edge-case...

[zekjur@mas.to]

@heiglandreas Have you actually read the commit message in question? It’s perfectly reasonable.

[musicmatze@social.linux.pizza]

@zekjur cherry-pick does only work if it has been committed already.

If you receive a patch by email, you have to apply it first.
IMO this is clearly an attack vector for projects.

[musicmatze@social.linux.pizza]

@zekjur I wonder whether it is reproducible with gix... but I am not sure whether they already have "git-am" reimplementd.

[heiglandreas@phpc.social]

@zekjur I have read the commit message.

Whether that is reasonable is ... debatable.

Adding an actual commit with the change and explaining why it was done that way and then adding a second commit removing it with the respective why would have been a better option separating code and explanation.

Alternatively explaining that a test with `sleep(1)` after line xyz was done.

But perhaps that is the `edge-case` I was talking about? 🤷

[cyrevolt@mastodon.social]

@zekjur gotcha - or rather: gitcha!

[musicmatze@social.linux.pizza]

@zekjur FWIW I reported this to the git ML... lets see

[ratsnakegames@mastodon.social]

@bmarinov @zekjur @max the behaviour of `patch` is intentional but the behaviour of the system as a whole is not.