b4 prep-perpatch-check-cmd conf via .b4-config

b4 prep-perpatch-check-cmd conf via .b4-config

[Mattijs Korpershoek]

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

Re: b4 prep-perpatch-check-cmd conf via .b4-config

[Matthieu Baerts]

Hi Mattijs,

On 23/05/2024 10:50, Mattijs Korpershoek wrote:

(...)

> 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
-- 
Sponsored by the NGI0 Core fund.

Re: b4 prep-perpatch-check-cmd conf via .b4-config

[Mattijs Korpershoek]

Hi Matthieu,

Thank you for your reply.

On jeu., mai 23, 2024 at 11:45, Matthieu Baerts <matttbe@kernel.org> wrote:

> Hi Mattijs,
>
> On 23/05/2024 10:50, Mattijs Korpershoek wrote:
>
> (...)
>
>> 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

Ah indeed, I missed that. Seems like a sane reason to not allow this.

>
> 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

>
> Cheers,
> Matt
> -- 
> Sponsored by the NGI0 Core fund.

Re: b4 prep-perpatch-check-cmd conf via .b4-config

[Konstantin Ryabitsev]

On Thu, May 23, 2024 at 11:58:12AM GMT, Mattijs Korpershoek wrote:
> >> 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
> 
> 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

> > 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

Re: b4 prep-perpatch-check-cmd conf via .b4-config

[Kernel.org Bugbot]

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.
-- 
Deet-doot-dot, I am a bot.
Kernel.org Bugzilla (peebz 0.1)

Re: b4 prep-perpatch-check-cmd conf via .b4-config

[Kernel.org Bugbot]

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.

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218877
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

(via https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=f00a9d14a57e)
-- 
Deet-doot-dot, I am a bot.
Kernel.org Bugzilla (peebz 0.1)