1194767 - slugids starting with a '-' can cause problems in tools which use them as command line arguments

Status: RESOLVED FIXED

(Taskcluster :: Services, defect)

Description

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

Slug ids, such as those used to represent taskIds or taskGraphIds, are url-safe. They are also safe to use in AMQP routing keys.

However, when used as command line arguments, they can cause problems if they begin with a '-' which occurs one time in 64, on average. This can introduce an intermittent issue in a command line tool, whereby a failure occurs intermittently.

Tools should work around this, and correctly sanitise their args, however:

a) this can be easily overlooked
b) many command line tools that take slugids as arguments will be outside of our control (i.e. developed by external teams/individuals).

In order to reduce risk, we should prevent slugids that *we* generate starting with a '-' character. For slugids generated by external tools outside our control, (e.g. if a task id is not generating using a slugid utility method we provide) then it should still be allowed. This conforms with the [Robustness principle](https://en.wikipedia.org/wiki/Robustness_principle).

It would not make sense to stray from the base 64 encoding standard (RFC 4648) by using an alternative encoding. It would also not make sense to switch to an alternative encoding (such as hex) since generated ids would be considerably longer, which would negatively impact e.g. routing keys.

We should change this in all language implementations we have (node.js, python, java, go) and also make sure all tests respect use of said method.

Comment 1

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

The current method of generating a base64 encoding results in a 22 character string, giving a freedom of 22 * 6 = 132 bits. Since the uuid only carries 128 bits, the last character is zero padded with four trailing 0 bits.

The '-' character represents bits 0b111110 (value 62) in base64 and therefore since the last character of the slug is currently 0bxx0000 it means the last character can never be '-' (in reality, the last character must be one of 'A', 'Q', 'g', 'w').

Therefore, I propose a trivial solution to guarantee that the slugId does not being with a '-' character, which is to base64 encode the uuid, and then reverse the sequence of characters in the resulting string. This means all new slugIds will begin with either 'A', 'Q', 'g', or 'w'. This also means there is no loss in randomness (122 bits) by introducing this change.

Comment 2

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

Attachment: TaskCluster SlugID Github Pull Request

Comment 3

[Selena Deckelmann :selenamarie :selena]

+1 on fixing this. Thanks for diving in! I ran into this several times last week.

Comment 4

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

I've switched the implementation now back to using the old mechanism for generating slugids, but regenerating if a slugid starts with a `-` character. This is so that serialisation and deserialisation still works with existing slugids that were previously created, to avoid breaking anything that might need to decode/encode existing slugids that were generated before this change. It comes at a negligible cost to the randomisation (we lose 1/64th of all possible 122 bit permutations = (2^122)*(63/64) (which is significantly better than losing an entire bit which leave only (2^122)*(1/2) permutations).

Comment 5

[Jonas Finnemann Jensen (:jonasfj)]

I have mixed feelings about this... seems I can't really make up mind on what is sane.

The downside is that we make a intermittent error less frequent, but we don't fix it.
Ideally, people learn how to give command line arguments correctly, and we fix tools to do the right thing(tm).
Not doing this would force people to do so.

On the other hand this would make the bug a lot less likely.

Comment 6

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

I think we are free to define what the requirements of a slugid are, so I think we could even reject slugs (server side) that did not conform (i.e. they must match `[A-Za-z0-9_][A-Za-z0-9_-]{7}[Q-T][A-Za-z0-9_-][CGKOSWaeimquy26-][A-Za-z0-9_-]{10}[AQgw]`).

Then we rule out all bugs.

In the docs we could define slugId as a url-safe base64 encoded v4 uuid that does not begin with a `-`.

Would that be sufficient?

Being able to consistently use slugids as command line parameters without needing special handling or encoding is a huge plus, IMO, just like it is that we can use them in urls and routing keys.

Comment 7

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

Agree with pete -- if we could do it over again, I'd suggest hex encoding like version-control sha's, since it automatically avoids any potential punctuation issues.  That said, I don't see `_` causing an issue, so I think that just disallowing an initial `-` is sufficient.

There are a bunch more places where slugids are taken as command-line arguments, across a lot more languages.  Getting tests written to ensure that all of those are doing it "right" and consistently would be difficult.  On the other hand, there are only a few places where slugids are generated, and they're all pretty easy to test.

I would add a reference to Postel's law to the documentation and the implementation, too -- I don't think we should update the schemas to reject `-`-prefixed slugIds, as that will make 1.6% of existing tasks inaccessible.  Instead, the docs should just *recommend* generating slugIds that do not begin with `-` and follow this recommendation in the available implementations.

Comment 8

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8648742 [details] [review]
TaskCluster SlugID Github Pull Request

See comment on PR, there can be no looping here.
But other than that, I'm coming around to the idea.

Let's define slugids as url-safe base64 encoded v4 uuids stripped if == padding.
With the recommendation that when generating them slugids starting with - are avoided.

Note: We'll need to patch our python client to generate slugids like this too.

---
Once you have the comment addressed refile this and bump version number to say 1.1.0 perhaps...

Comment 9

[Greg Arndt [:garndt]]

We should change this as well: https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/taskcluster_graph/slugid.py#4

Comment 10

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

(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #8)
> Comment on attachment 8648742 [details] [review]
> Github Pull Request
> 
> See comment on PR, there can be no looping here.

See my response in the PR:
https://github.com/taskcluster/slugid/pull/1/files#r37629129

Unfortunately this reduced performance, and added complexity. Therefore I think we are better to leave it as is. For example, the chance of having three consecutive uuids that have to be rejected is 1 in 262144 (64^3). This is so remote that overall the performance is better with the loop implementation than the alternative implementation which imports the cryptography library and does the byte manipulation directly in the slugid library. It is arguably safer and cleaner too.


(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #8)
> Comment on attachment 8648742 [details] [review]
> Github Pull Request
> 
> See comment on PR, there can be no looping here.

See my response in the PR:
https://github.com/taskcluster/slugid/pull/1/files#r37629129

Unfortunately this reduced performance, and added complexity. Therefore I think we are better to leave it as is. For example, the chance of having three consecutive uuids that have to be rejected is 1 in 262144 (64^3). This is so remote that overall the performance is better with the loop implementation than the alternative implementation which imports the cryptography library and does the byte manipulation directly in the slugid library. It is arguably safer and cleaner too.

> Once you have the comment addressed refile this and bump version number to
> say 1.1.0 perhaps...

Good spot! Done. =)

Comment 11

[Jonas Finnemann Jensen (:jonasfj)]

See my reply on PR... Basically we should just mask + to something else... hardcoded...

Comment 12

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

Using a fixed character ('A') would introduce a skew in the distribution of slugids (twice as many would start with an 'A' as any other character) and would also unnecessarily reduce entropy. If there is a strong reason not to accept my version I will understand, but at the moment I don't understand what the objection to the loop implementation (as I believe we established it is not performance or code readability). Thanks.

Comment 13

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

I've updated the PR with an alternative non-looping version which (un)sets the first (most significant) bit of the uuid. This reduces entropy from 122 bits to 121, but avoids the loop. My preference would be the loop.... but you can't win them all. :p

Hoping this will be sufficient to get the issue resolved.

Comment 14

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

(In reply to Greg Arndt [:garndt] from comment #9)
> We should change this as well:
> https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/
> taskcluster_graph/slugid.py#4

Good spot! OK, as soon as we're all happy with the node version and it is accepted, I'm happy to port to python.

Jonas suggested we also add a utility method to the taskcluster clients that calls slugid.nice() - that seems to make sense to me too, so I'd like to do this too. However...

One possibility is that the slug creation is hidden by the client - i.e. if you call a method that requires a slug, it just generates one under the hood. But .... I'm guessing since the API methods expect a slug (rather than creating one themselves), we intended that the api consumer might wish to create his/her own slugs - and therefore it would not make sense to hide it in the client, since we intentionally expose it in the API.

For example, http://docs.taskcluster.net/queue/api-docs/#createTask requires that you specify a taskId in the URL you call, rather than assigning a slug and then telling you what the randomly generated slug was (taskId) is in the response. I can imagine this might be intentional so you can refer to entities that don't yet exist without needing to care too much on the order you create them in. However, if this isn't the case, we do have the possibility to hide the slugid generation from the consumer of taskcluster clients, by embedding the slug generation in the taskcluster client api methods that require slugs. Maybe this is a case of trying to simplify things too much, which results in a loss of power to the library user. Another option of course is making the slugid optional, and if you don't specify one, one gets generated for you.

Your thoughts please, good people of the world...

Comment 15

[Jonas Finnemann Jensen (:jonasfj)]

> One possibility is that the slug creation is hidden by the client
For the exact reasons you mention, this is a bad idea.
In particular idempotency is only fun if you can control the input.
Simply put you generate the slugid because that way you can have fault tolerance through eventual consistency.

Note, you should always create in correct order, to prevent an attacker from stealing a
slugid you intended to use. This could result in a security issue, in some cases.

------
On topic, I suggest we make it so that:
  var taskcluster = require('taskcluster-client');
  taskcluster.slugid() or taskcluster.newTaskId(), or taskcluster.generateId() or
  taskcluster.generateTaskId()

I suspect taskcluster.slugid() is the best method name. Then that can just run slugid.nice() under the
hood. That way devs only have to require the taskcluster-client library, and not the slugid library.
(so it also simplifies dependency management, pretty sure tc-client already needs slugid anyways).

I suggest we do exactly the same thing in both python, go and java clients. We don't need a separate
slugid library we can just make it part of the client library (assuming that's easier).

Comment 16

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

Comment on attachment 8648742 [details] [review]
TaskCluster SlugID Github Pull Request

Updating from r- to r+ to reflect Pull Request history.

Comment 17

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

Attachment: TaskCluster Scheduler Github Pull Request

Just a doc update to recommend use of slugid.nice() (previously slugid.v4()).

Comment 18

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

I've made the patch for gecko, but I haven't quite worked out how to run the python unit tests for taskcluster_graph so I haven't added tests yet. As it is end-of-day, I'll attach the patch without tests, and will add a second patch with tests tomorrow (once I work out how to run them - see https://irccloud.mozilla.com/pastebin/kbLfPIyG).

Comment 19

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

Attachment: Gecko patch (for taskcluster_graph) - without tests

Comment 20

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

Also need to land a patch for gaia - I'll do this tomorrow.

Comment 21

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

P.S. agree with comment 15, I'll proceed as suggested there.

Comment 22

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8654079 [details] [review]
TaskCluster Scheduler Github Pull Request

r+ with comment about leaving prototype warning in there.

Comment 23

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8654187 [details] [diff] [review]
Gecko patch (for taskcluster_graph) - without tests

Looks decent to me...

@pmoore, please add the same logic for python client.
I think it having a slugid() method makes sense too (if it doesn't already).
For consistency, let's call it slugid().

Comment 24

[Jonas Finnemann Jensen (:jonasfj)]

Attachment: github PR for tc-client

taskcluster.slugid() for taskcluster-client

Comment 25

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

Comment on attachment 8654247 [details] [review]
github PR for tc-client

r+ with the review comments addressed

Comment 26

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

I started work on a python port of slugid, since lots of places might want to use the library, including:

a) gecko
b) gaia
c) buildbot bridge
d) python taskcluster client
e) any project using the taskcluster-github integration that Morgan wrote

This would be preferable to having the logic in multiple python projects.

It's end-of-day for me now, so here is the work in progress for anyone interested:
https://github.com/taskcluster/slugid.py/compare/master...petemoore:initial-dev

Will raise the PR when finished with tests added etc.

Comment 27

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

Attachment: Gecko patch (for taskcluster_graph) to use new slugid.py library

Now that I've created a standalone slugid python library (in bug 1201062) I've redone this patch to use it.

See https://pypi.python.org/pypi/slugid.

Comment 28

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

Comment on attachment 8656536 [details] [diff] [review]
Gecko patch (for taskcluster_graph) to use new slugid.py library

Adding gps, based on:
https://hg.mozilla.org/mozilla-central/rev/21f18a7f5f9d

Comment 29

[Greg Arndt [:garndt]]

Comment on attachment 8656536 [details] [diff] [review]
Gecko patch (for taskcluster_graph) to use new slugid.py library

Moving the logic into it's own package so things like taskcluster-client.py, mozci and anything else python related generating slugids for tasks makes sense.

however, we need to figure out how to publish this and get it include when mach is invoked.  (discussions related to it on IRC).

Comment 30

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

Attachment: Github Pull Request for taskcluster-client.py

Adding Rail to f? as one of the primary consumers of taskcluster-client.py...

Comment 31

[Wander Lairson Costa]

Comment on attachment 8656536 [details] [diff] [review]
Gecko patch (for taskcluster_graph) to use new slugid.py library

Review of attachment 8656536 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the deployment issue fixed.

Comment 32

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 8656567 [details] [review]
Github Pull Request for taskcluster-client.py

Can we rename TestSlugIdNotNice to TestSlugIdV4?

Otherwise, looks good to me, r+

Comment 33

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

(In reply to John Ford [:jhford] from comment #32)
> Comment on attachment 8656567 [details] [review]
> Github Pull Request for taskcluster-client.py
> 
> Can we rename TestSlugIdNotNice to TestSlugIdV4?
> 
> Otherwise, looks good to me, r+

Yeah, the name isn't good, I agree. I called it "NotNice" to reflect that first bit is *set*, whereas with v4 the bit may or may not be set. A v4 slug can also be a nice slug (50% chance). Can you think of a another name we could give it that implies the first bit is set?

Comment 34

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

Attachment: Gecko patch (for taskcluster_graph) to use new slugid.py library

Updated mozilla-central patch to include slugid 1.0.6 library in /python.

Tested locally with:

  $ ./mach -v taskcluster-graph --pushlog-id='17316' --message=' ' --project='b2g-inbound' --owner='ffxbld' --revisionhash='f572e2439068601d4f7573955760d9ffa8a7442b' --extend-graph

which I stole from:

  * https://queue.taskcluster.net/v1/task/7TnNDeMzRHSKium5NF1ycQ

Comment 35

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

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

Comment 36

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

^^^ In this try run, I set all the taskcluster build tasks to be:

  command: ["/bin/bash", "-c", "true"]

in order that I could test creating a graph, without needing to run all the jobs and waste compute resources.

Comment 37

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

Yay, all tasksIds (and the group id) begin with [A-Za-f] \o/

https://tools.taskcluster.net/task-graph-inspector/#OgMQE_rXSbOzn0hcaFqjZg/

Comment 38

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

Attachment: bug1194767_mozilla-central_v2_with_commit_header.patch

Forgot to include the commit header - this patch is identical to last, just with hg commit header.

I'll ask a sheriff to land it. :)

Comment 39

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

"checkin-needed" only for the last patch (attachment 8657110 [details] [diff] [review]).

Comment 40

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

(In reply to John Ford [:jhford] from comment #32)
> Comment on attachment 8656567 [details] [review]
> Github Pull Request for taskcluster-client.py
> 
> Can we rename TestSlugIdNotNice to TestSlugIdV4?
> 
> Otherwise, looks good to me, r+

Discussed on IRC, we agreed "TestSlugIdNiceIsAlwaysNice". Will make the change, and merge the PR.

Comment 41

[Carsten Book [:Tomcat]]

Hey pete, this didn't apply to mozilla-inbound 

renamed 1194767 -> bug1194767_mozilla-central_v2_with_commit_header.patch
applying bug1194767_mozilla-central_v2_with_commit_header.patch
patching file testing/taskcluster/mach_commands.py
Hunk #1 FAILED at 251
1 out of 1 hunks FAILED -- saving rejects to file testing/taskcluster/mach_commands.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh bug1194767_mozilla-central_v2_with_commit_header.patch

maybe need a rebase ?

Comment 42

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

Attachment: bug1194767_mozilla-inbound_v1.patch

(In reply to Carsten Book [:Tomcat] from comment #41)
> Hey pete, this didn't apply to mozilla-inbound 
> 
> renamed 1194767 -> bug1194767_mozilla-central_v2_with_commit_header.patch
> applying bug1194767_mozilla-central_v2_with_commit_header.patch
> patching file testing/taskcluster/mach_commands.py
> Hunk #1 FAILED at 251
> 1 out of 1 hunks FAILED -- saving rejects to file
> testing/taskcluster/mach_commands.py.rej
> patch failed, unable to continue (try -v)
> patch failed, rejects left in working directory
> errors during apply, please fix and refresh
> bug1194767_mozilla-central_v2_with_commit_header.patch
> 
> maybe need a rebase ?

Rebased against mozilla-inbound. Sorry about that! This should be good to land against inbound now.

Comment 43

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

"checkin-needed" only for the last patch (attachment 8657173 [details] [diff] [review]).

Comment 44

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

Attachment: taskcluster-client PR with bumped version

Need this so we can republish to npm, and use that version e.g. in gaia.

Gaia is also using slugid library directly, but I'll be bumping both, to be safe and more future-proofed...

Please republish on npm if you merge it (I think I don't have permission to do that step?).

Thanks!

Comment 45

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

Comment on attachment 8657202 [details] [review]
taskcluster-client PR with bumped version

Apologies, I see it was already bumped since we updated. No need for a new release.

Comment 46

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3bd50974b08

Comment 47

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

Attachment: Github Pull Request Buildbot Bridge

Buildbot Bridge doesn't generate slugids during regular operation (listening only to notifications from preexisting tasks) but its tests do require slugids. This PR aligns the test slugs to how we now generate production slugs.

Comment 48

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e3bd50974b08

Comment 49

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

Attachment: Github Pull Request for taskcluster-client.py - release.sh enhancements

These are some enhancements to the release.sh script which I'd like to add before I make release 0.0.26.

Any questions, let me know. :)

Comment 50

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

Still need to update:

* https://github.com/mozilla/build-funsize
* https://github.com/taskcluster/mozilla-taskcluster
* https://github.com/taskcluster/taskcluster-github

This is turning out to be quite a long tail! :D

Comment 51

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

And while I think of it, also ...

* https://github.com/taskcluster/taskcluster-docs (everywhere where slugs are defined, and have regexs)
* https://github.com/taskcluster/taskcluster-tools (e.g. task creator)
* publish new schemas/references with updated comments for slug fields

Comment 52

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

> * https://github.com/mozilla/build-funsize

I *think* this has been superseded by https://github.com/mozilla/funsize

Rail, can you confirm? Thanks!

Comment 53

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

Attachment: Github Pull Request for funsize

Please don't land until:

  * https://pypi.python.org/pypi/taskcluster/0.0.26 and
  * http://pypi.pub.build.mozilla.org/pub/taskcluster-0.0.26.tar.gz

have been released.

Comment 54

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

Also it looks like it breaks docker worker, so no doubt we will need a fix for docker worker so that it can manage taskids / taskgroupids starting with a '-' even if we don't create them by default.

e.g. this failure I believe is because docker worker doesn't handle task group ids starting with a '-':

https://tools.taskcluster.net/task-inspector/#2AAxnGTzSeGLTX_Hwp_PVg

In other words, we shouldn't only fix clients to not use '-', we should also make sure we can handle them if we receive them (or block them via the API with appropriate regex).

Comment 55

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

Attachment: Github Pull Request for mozilla-taskcluster

Comment 56

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

Attachment: Github Pull Request for taskcluster-github

Comment 57

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

(In reply to Pete Moore [:pmoore][:pete] from comment #52)
> > * https://github.com/mozilla/build-funsize
> 
> I *think* this has been superseded by https://github.com/mozilla/funsize
> 
> Rail, can you confirm? Thanks!

Rail confirmed via IRC, see bug 1202457 about deprecating the old repo.

Comment 58

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

(In reply to Pete Moore [:pmoore][:pete] from comment #51)
> And while I think of it, also ...
> 
> * https://github.com/taskcluster/taskcluster-docs (everywhere where slugs
> are defined, and have regexs)
> * https://github.com/taskcluster/taskcluster-tools (e.g. task creator)
> * publish new schemas/references with updated comments for slug fields

Jonas, Dustin,

I'm tempted to suggest we update our regexes in our schemas/references to the 'nice' regex from taskcluster.github.io/slugid.py for creation of *new* objects:
[A-Za-z0-9_-]{8}[Q-T][A-Za-z0-9_-][CGKOSWaeimquy26-][A-Za-z0-9_-]{10}[AQgw]

... and to update the regexes to the 'v4' regex for api endpoints that query existing data:
[A-Za-f][A-Za-z0-9_-]{7}[Q-T][A-Za-z0-9_-][CGKOSWaeimquy26-][A-Za-z0-9_-]{10}[AQgw]

This should add some safety checks that provided slugids really are uuid v4 compliant, and that new entities are also 'nice'.

Currently we use:
[A-Za-z0-9_-]{22}

which is not a guarantee that the slug is v4 compliant, and thus could potentially introduce a security risk for a taskcluster component at a point in the future, which relies on it being so.

Comment 59

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

^^^ above, I put the regexes in the wrong order - of course the 'nice' slugs start with [A-Za-f] and the v4 ones start with [A-Za-z0-9_-]{8} ^^^

(was a long day) ;)

Comment 60

[Jonas Finnemann Jensen (:jonasfj)]

@pmoore,
I support upgrading schemas and regexp for URLs to v4-regexp:
  [A-Za-z0-9_-]{8}[Q-T][A-Za-z0-9_-][CGKOSWaeimquy26-][A-Za-z0-9_-]{10}[AQgw]
Instead of using:
  [A-Za-z0-9_-]{22}
Please go a head with this for queue, scheduler, etc..

But the definition of slugid remains the same. We just recommend that people use the "nice" form.
We don't require it -- some day if we ever make v2/, we can discuss a completely new encoding.
So I don't think we should use the "nice"-regexp, just the v4-regexp which is a good improvement.

Comment 61

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

(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #60)
> @pmoore,
> I support upgrading schemas and regexp for URLs to v4-regexp:
>   [A-Za-z0-9_-]{8}[Q-T][A-Za-z0-9_-][CGKOSWaeimquy26-][A-Za-z0-9_-]{10}[AQgw]
> Instead of using:
>   [A-Za-z0-9_-]{22}
> Please go a head with this for queue, scheduler, etc..

Created bug 1202573.

Comment 62

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

no info needed

Comment 63

[Greg Arndt [:garndt]]

Comment on attachment 8657858 [details] [review]
Github Pull Request for mozilla-taskcluster

Looks, good.  Merged and deployed.

Comment 64

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

(In reply to Dustin J. Mitchell [:dustin] from comment #62)
> no info needed

Sorry - this was my lazy way of asking if you think the suggestion makes sense. :)

Comment 65

[Rail Aliiev [:rail]]

Comment on attachment 8657856 [details] [review]
Github Pull Request for funsize

merged

Comment 66

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

Attachment: Github Pull Request for taskcluster/taskcluster-docs

Almost done now...

Comment 67

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

Attachment: Github Pull Request for taskcluster/taskcluster-tools

Hopefully this is the last one, and then we can close the bug! \o/

Comment 68

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

(In reply to Dustin J. Mitchell [:dustin] from comment #7)

> On the other hand, there are only a few places where slugids are generated,
> and they're all pretty easy to test.

Famous last words!! :D

Comment 69

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8659262 [details] [review]
Github Pull Request for taskcluster/taskcluster-docs

r+ with comments on quickly moving to taskcluster.slugid() instead of slugid.nice()

Comment 70

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8659263 [details] [review]
Github Pull Request for taskcluster/taskcluster-tools

Landed, thanks..

Comment 71

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

\o/

Comment 72

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

It seems we missed:

* https://github.com/taskcluster/gaia-taskcluster/blob/18d38dc6bdd28081b2c26306c571422067e8b2d5/routes/github_pr.js#L167
* https://github.com/mozilla/autolander/blob/dea6e36d1dcbbc252494512a61277d5deac2417b/lib/taskgraph.js#L270

Thanks to :garndt for the links above!

Will make patches for these two tools too.

Comment 73

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

Attachment: Github Pull Request for gaia-taskcluster

Comment 74

[GitHub Autolander Bot]

Attachment: [autolander] petemoore:bug1194767 > mozilla:master

Comment 75

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

Attachment: Github Pull Request for autolander

Comment 76

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

Comment on attachment 8678122 [details] [review]
Github Pull Request for autolander

Autolander also created an attachment, so obseleting mine (they had the same content).

Comment 77

[Greg Arndt [:garndt]]

Comment on attachment 8678113 [details] [review]
Github Pull Request for gaia-taskcluster

Looks reasonable to me.

Comment 78

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

Have redeployed gaia-taskcluster with the changes. N.B. there were a couple of commits in heroku that weren't on github. I checked they had no sensitive changes, and merged them in. The added commits were:

https://github.com/taskcluster/gaia-taskcluster/compare/18d38dc6bdd28081b2c26306c571422067e8b2d5...3329875f1481b9216dfeb35898945468c63fe1a4

These had not been published to github before.

Comment 79

[Kevin Grandon :kgrandon]

Comment on attachment 8678119 [details] [review]
[autolander] petemoore:bug1194767 > mozilla:master

Seems fine to me, thanks!

Comment 80

[Kevin Grandon :kgrandon]

Landed autolander changes in master: https://github.com/mozilla/autolander/commit/1549b2cb49de721c054e040b23dd93d2e03c843d

Comment 81

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

Awesome, thanks!

Comment 82

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

Attachment: Github Pull Request for taskcluster-cli