b4 prep-perpatch-check-cmd conf via .b4-config
Hi,
I'm yet another (very) happy user of b4.
Thank you for developping and maintaining it.
I've noticed that v0.14+ contains the new b4 prep --check command.
This can be customized via b4.prep-perpatch-check-cmd [1] However, we cannot customize this via a project specific .b4-config [2] file.
It would be useful to be able to customize this via .b4-config, for example to invoke checkpatch.pl with additional arguments (on a per-project basis).
For example, in U-Boot, I would have the following .b4-config:
[b4]
prep-perpatch-check-cmd = ./scripts/checkpatch.pl -q --terse --no-summary --mailback --u-boot --showfile
Looking at _setup_main_config(), I can see that wtglobs does not contain 'prep-*', but adding it seems not sufficient, probably due to multivals parsing.
I don't have enough knowledge/time to dive deeper on this, but could we consider adding support for configuring prep-perpatch-check-cmd in .b4-config?
Thank you!
Mattijs
[1] https://b4.docs.kernel.org/en/latest/config.html#contributor-oriented-settings
[2] https://b4.docs.kernel.org/en/latest/config.html#per-project-defaults
Hi Mattijs,
On 23/05/2024 10:50, Mattijs Korpershoek wrote:
(...)
Quoted 5 lines
I've noticed that v0.14+ contains the new b4 prep --check command.
This can be customized via b4.prep-perpatch-check-cmd [1] However, we cannot customize this via a project specific .b4-config [2] file.
Even if it is would be handy, this feature has been explicitly removed for security reasons:
https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=a1360385
There could be a solution similar to 'direnv' where a 'b4 allow' command would mark a version of '.b4-config' as trusted, e.g. by adding a checksum in the local git config and checking it before reading the config file?
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Hi Matthieu,
Thank you for your reply.
Quoted 8 lines · Matthieu Baerts
Hi Mattijs,
On 23/05/2024 10:50, Mattijs Korpershoek wrote:
(...)
Quoted 2 lines
I've noticed that v0.14+ contains the new b4 prep --check command.
Quoted 8 lines
Quoted 3 lines
This can be customized via b4.prep-perpatch-check-cmd [1] However, we cannot customize this via a project specific .b4-config [2] file.
Even if it is would be handy, this feature has been explicitly removed for security reasons:
https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=a1360385
Ah indeed, I missed that. Seems like a sane reason to not allow this.
Quoted 5 lines
There could be a solution similar to 'direnv' where a 'b4 allow' command would mark a version of '.b4-config' as trusted, e.g. by adding a checksum in the local git config and checking it before reading the config file?
Maybe that's a better idea.
Since you pointed using local git config, I actually gave this another thought and I can solve my problem by using a project specific git config:
$ cd ~/work/upstream/u-boot $ git config b4.prep-perpatch-check-cmd './scripts/checkpatch.pl -q --terse --no-summary --mailback --u-boot --showfile'
Thanks for pointing me in the right direction!
Mattijs
Quoted 5 lines
Cheers,
MattSignature · 1 line
Sponsored by the NGI0 Core fund.
Quoted 6 lines · Mattijs Korpershoek
Quoted 6 lines
Quoted 5 lines
I've noticed that v0.14+ contains the new b4 prep --check command.
This can be customized via b4.prep-perpatch-check-cmd [1] However, we cannot customize this via a project specific .b4-config [2] file.
Quoted 6 lines
Quoted 4 lines
Even if it is would be handy, this feature has been explicitly removed for security reasons:
https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=a1360385
Ah indeed, I missed that. Seems like a sane reason to not allow this.
I may consider allowing prep-perpatch-check-cmd, but not am-perpatch-check-cmd to be set in the .b4-config file. The scenario I'm worried about is where someone sends a patch series to review, and that patch series happens to contain malicious edits to the .b4-config. This scenario is unlikely to happen for a prep-managed branch where the author themselves works on patches.
It's still something I'd worry about, but it may be an acceptable trade-off if the goal is to make it easier for newcomers and one-off contributors to submit patches.
Let me make this a bug so I don't forget about it.
bugbot assign to me
Quoted 6 lines
Quoted 4 lines
There could be a solution similar to 'direnv' where a 'b4 allow' command would mark a version of '.b4-config' as trusted, e.g. by adding a checksum in the local git config and checking it before reading the config file?
Maybe that's a better idea.
We could use this approach for am-perpatch-check-cmd, but I'd much rather not allow setting this via .b4-config at all for the moment.
Regards,
-K
Hello:
This conversation is now tracked by Kernel.org Bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=218877
There is no need to do anything else, just keep talking.
Signature · 2 lines
Deet-doot-dot, I am a bot. Kernel.org Bugzilla (peebz 0.1)
Konstantin Ryabitsev writes in commit f00a9d14a57e953c6c7e9f0b9c41a0742cf44f7b:
Allow prep-perpatch-check-cmd to be set in .b4-config
Upon further reflection, allow to set prep-perpatch-check-cmd, but not am-perpatch-check-cmd. This allows projects to define what local checks they would like contributors to run, but makes it more difficult to abuse this in a way that would cause the maintainer to run a malicious command.
(via https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=f00a9d14a57e)
Signature · 2 lines
Deet-doot-dot, I am a bot. Kernel.org Bugzilla (peebz 0.1)