1219487 - scopes may only contain printable ascii chars and space

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Jonas Finnemann Jensen (:jonasfj)]

My willingness not to break gaia is blocking roll-out of some low priority
security patches.. And a few other issues.

List of all violating tasks out of the 4.5k tasks I inspected:
  https://gist.github.com/jonasfj/14e65603539ff54e11e4
  (warning big file)

All of them have:
workerType: gaia
treeherder groupSymbol: BOOTSTRAP GB Gij Gip GU JSM LINT
image: taskcluster/gaia-taskenv:1.2.1 or taskcluster/npm-cache:0.1.10

List of all task names is present here:
https://public.etherpad-mozilla.org/p/tc-scopes-breakages-on-newline#lineNumber=25

The problem is that they all have:
  "docker-worker:image:taskcluster/gaia-taskenv\n*"
in task.scopes.
This scope makes no difference as it doesn't protect anything.
We should remove it.

---------
Let's fix this asap, so we can roll-out checks on queue that prevents people
from making these dumb scopes, and eliminate related security issues.

This bug is intended to give some decent warning and help migrate most tasks.
I'm okay if somethings breaks as a result of this. Clearly people haven't paid
much attention when they create a scope with \n in the name of a docker image :)


Probably we just need to string IMAGE here:
https://github.com/mozilla-b2g/gaia/blob/01ffe82cf088ca8fda9fe6783dc5cad2c3dde01c/tests/taskcluster/lib/decorate_task.js#L96
And then carry that across as many branches as possible.

Comment 1

[Jonas Finnemann Jensen (:jonasfj)]

Attachment: Github PR for gaia master

Not sure if you're the right reviewer...
But you are listed in the history of the file. I know nothing of gaia, but
figured I would try to avoid breaking you when I roll out a minor breaking API
change on taskcluster, such that task.scopes with non-printable chars won't
be allowed.

Comment 2

[Pete Moore [:pmoore][:pete]]

@jonasfj Could we not just base64 encode scopes (and remove any line separators from encoded version) before hashing?

Comment 3

[Robert Wood [:rwood]]

Comment on attachment 8680332 [details] [review]
Github PR for gaia master

No sorry I'm not familiar with this :(  and haven't touched TC in ages. Maybe :garndt will know who?

Comment 4

[Greg Arndt [:garndt]]

I'm not an expert with the gaia integration, but looking over these changes, they seem like they are sane and wouldn't cause an issue.

Comment 5

[Jonas Finnemann Jensen (:jonasfj)]

@pmoore, we discussed this today.
tl;dr: unicode is not desired and we can always add support later.

Comment 6

[Jonas Finnemann Jensen (:jonasfj)]

Will be fixed in:
https://github.com/mozilla-b2g/gaia/pull/32870

Comment 7

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

Jonas, this PR is now merged.  Can this bug be closed, and the patches in bug 1193607 re-landed?

Comment 8

[Jonas Finnemann Jensen (:jonasfj)]

I think this is fixed....