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.
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.
@jonasfj Could we not just base64 encode scopes (and remove any line separators from encoded version) before hashing?
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?
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.
@pmoore, we discussed this today. tl;dr: unicode is not desired and we can always add support later.
Jonas, this PR is now merged. Can this bug be closed, and the patches in bug 1193607 re-landed?
I think this is fixed....
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Assignee: nobody → jopsen
You need to log in before you can comment on or make changes to this bug.