1491186 - validate that non-generic actions are not run with action_perm = generic

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

It's hard to see why someone would do this, but let's make such actions fail on contact anyway.

I think this means that ci-admin needs to configure *generic* hooks to pass some indication to the task that this is a generic run; and then the in-tree action code needs to fail fast if that indication is present but cb_name points to a non-generic action.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aef106176be54bb9f4438438adc78e61f598ab74

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1491186: sanity check action scopes

This provides a modicum of assurance that, for example, a non-generic action is
not being run with generic scopes. While scopes would prevent any serious
damage from such an action, this provides an extra layer of security to prevent
such abuse.

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 9010299 [details]
Bug 1491186: sanity check action scopes

Aki Sasaki [:aki] has approved the revision.

Comment 4

[Tom Prince [:tomprince]]

Comment on attachment 9010299 [details]
Bug 1491186: sanity check action scopes

Tom Prince [:tomprince] has approved the revision.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/090051ce866f
sanity check action scopes r=aki,tomprince

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/090051ce866f

Comment 7

[[e-mail] (:bhearsum)]

This appears to have broken release promotion on Maple. From https://tools.taskcluster.net/groups/PC32PCpzQy2U_aK7vpxKpA/tasks/bE30WPeoT_i5me8Pp-5ZIw/details:
[task 2018-09-25T00:18:01.880Z] + ./mach --log-no-times taskgraph action-callback
[task 2018-09-25T00:18:02.404Z] loading config from `taskcluster/ci/config.yml`
[task 2018-09-25T00:18:03.236Z] Traceback (most recent call last):
[task 2018-09-25T00:18:03.236Z]   File "/builds/worker/checkouts/gecko/taskcluster/mach_commands.py", line 252, in action_callback
[task 2018-09-25T00:18:03.236Z]     test=False)
[task 2018-09-25T00:18:03.236Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/registry.py", line 353, in trigger_action_callback
[task 2018-09-25T00:18:03.236Z]     sanity_check_task_scope(callback, parameters, graph_config)
[task 2018-09-25T00:18:03.236Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/registry.py", line 332, in sanity_check_task_scope
[task 2018-09-25T00:18:03.236Z]     raise Exception('Expected task scope {} for this action'.format(expected_scope))
[task 2018-09-25T00:18:03.236Z] Exception: Expected task scope assume:repo:hg.mozilla.org/projects/maple:action:generic for this action


AFAICT, maple is configured the same way as mozilla-beta, so this is likely to break when this is uplifted. I'm guessing we need to change something in tree or in Ship It to fix it?

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

How are you running that action?  The task has scopes

  assume:repo:hg.mozilla.org/projects/maple:branch:default

https://hg.mozilla.org/projects/maple/file/tip/taskcluster/taskgraph/actions/registry.py#l203
                # tasks get all of the scopes the original push did, yuck; this is not
                # done with kind = hook.
                repo_scope = 'assume:repo:{}/{}:branch:default'.format(
                    match.group(1), match.group(2))
                action['repo_scope'] = repo_scope

so, it sounds like we should make this particular scope check only apply for `kind: hook`.

Comment 9

[[e-mail] (:bhearsum)]

(In reply to Dustin J. Mitchell [:dustin] pronoun: he from comment #8)
> How are you running that action?  The task has scopes
> 
>   assume:repo:hg.mozilla.org/projects/maple:branch:default
> 
> https://hg.mozilla.org/projects/maple/file/tip/taskcluster/taskgraph/actions/
> registry.py#l203
>                 # tasks get all of the scopes the original push did, yuck;
> this is not
>                 # done with kind = hook.
>                 repo_scope = 'assume:repo:{}/{}:branch:default'.format(
>                     match.group(1), match.group(2))
>                 action['repo_scope'] = repo_scope
> 
> so, it sounds like we should make this particular scope check only apply for
> `kind: hook`.

The action is run by Ship It, which looks like it uses https://github.com/mozilla/release-services/blob/master/src/shipit/api/shipit_api/models.py#L133 to generate the action task.

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1491186: don't sanity check scopes for kind=task

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

Ship-it looks like it's doing the right thing.

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 9011778 [details]
Bug 1491186: don't sanity check scopes for kind=task

Ben Hearsum (:bhearsum) has approved the revision.

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

I landed this since tomprince is away.  Tom, feel free to re-open for an r- :)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a765e968a4a
don't sanity check scopes for kind=task r=bhearsum

Comment 15

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/9a765e968a4a

Comment 16

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/22d8343c70a0

Comment 17

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/d2538a8c3b47
https://hg.mozilla.org/releases/mozilla-esr60/rev/f6871fe7112b

Comment 18

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/5f9fa0298fc3