I really like this approach!

Use a scmpolicy plugin in postSCMCheckout callback. The pro is that we can do more checks after the source is initialized on builder, meanwhile, the con is that the source will be downloaded even it is denied by policy. It might be a potential risk?

In some cases (hello blender repo in fedora), the cloning itself can take hours and timeout so I'd prefer to not do it that way (+ you know, those vulnerabilities here and there).

Do the scm check in hub's make_task, this looks straightforward, but may lack some builder's information

Do you have some hint which one, for example?

Do the scm check in hub's make_task, this looks straightforward, but may lack some builder's information

Do you have some hint which one, for example?

Like which host the task is assigned to. This doesn't happen yet in make_task() 1. BTW, this info is not used by the current approach too.

And, we have to guess which argument is the source, especially for custom tasks.

What do you see as a benefit of checking it in make_task? Is it just that builder is making additional call here?

What do you see as a benefit of checking it in make_task? Is it just that builder is making additional call here?

And a faster response if the source is denied (no need to create a SCM-related task on the builder).

I would leave it in srpm task then. make_task is generic, so we would need to detect if it is some type of task involving SCM. For plugin tasks it would be unusable, as plugin will need to run the check anyway.

This is tricky and I think it might take a few iterations to get right.

I'm not sure I like the idea of working this into the existing assert_allowed call. It doesn't really get us much benefit as it still requires all invocations of the call to have significant updates to take advantage.

OTOH, maybe this is ok for now.

A couple things to consider:

• What we create here is likely to eventually replace the existing allowed_scms setting, so we should design it with that in mind
• Other tools will want to use this. Builder plugins, content generators, possibly more. We should make it as straightforward as possible. In particular, an external cg will not be able to use host.evalPolicy, so we might consider adding a hub call for this

When flattening the scm info, I think it would be better to use scm_ as the prefix.

The opts parameter is only used to update the policy_data, so it seems to be misnamed. It is not a general options parameter, but explicitly policy data info.

I'm not sure I like that the default policy doesn't work out of the box, but maybe that is ok? I guess our default allowed_scms='' also blocks builds out of the box.

@mikem, updated.

I'm not sure I like that the default policy doesn't work out of the box, but maybe that is ok? I guess our default allowed_scms='' also blocks builds out of the box.

The default policy only allows admin's builds. And although the default allowed_scms='' blocks all builds, the allowed_scms defined in the default kojid.conf contains some dummy items. That's almost equal to "all denied". So I believe it is ok in most cases.

Functionality is tested, I add testing-done tag. @julian8628 please, update policy hub documentation related to these changes.

doc updated

:thumbsup:

Commit 6d4abedb fixes this pull-request