scopes may only contain printable ascii chars and space



3 years ago
8 months ago


(Reporter: jonasfj, Assigned: jonasfj)


Firefox Tracking Flags

(Not tracked)



(1 attachment)



3 years ago
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:
  (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:

The problem is that they all have:
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:
And then carry that across as many branches as possible.

Comment 1

3 years ago
Created attachment 8680332 [details] [review]
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.
Attachment #8680332 - Flags: review?(rwood)
@jonasfj Could we not just base64 encode scopes (and remove any line separators from encoded version) before hashing?
Flags: needinfo?(jopsen)

Comment 3

3 years ago
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?
Flags: needinfo?(garndt)
Attachment #8680332 - Flags: review?(rwood)

Comment 4

3 years ago
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.
Flags: needinfo?(garndt)

Comment 5

3 years ago
@pmoore, we discussed this today.
tl;dr: unicode is not desired and we can always add support later.
Flags: needinfo?(jopsen)
Jonas, this PR is now merged.  Can this bug be closed, and the patches in bug 1193607 re-landed?
Flags: needinfo?(jopsen)

Comment 8

3 years ago
I think this is fixed....
Last Resolved: 3 years ago
Flags: needinfo?(jopsen)
Resolution: --- → FIXED
Assignee: nobody → jopsen


8 months ago
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.