Tab characters in SSH key comments break 'rake gitlab shell setup'

When you run rake gitlab:shell:setup we send SSH public keys from one process (rake) to a gitlab-shell process. These keys are sent as 'id\tkey' lines, i.e. tab-separated values.

https://gitlab.com/gitlab-org/gitlab-ce/blob/d9078749a5a2d78ed6b8253184a1aacdf9193cdc/lib/gitlab/backend/shell.rb#L7

On the other end we turn these lines back into arrays. https://gitlab.com/gitlab-org/gitlab-shell/blob/a33818958222e5e6ef405a64608c37d7452a4b65/lib/gitlab_keys.rb#L60

Because somebody (ahem) made the parser in gitlab-shell pedantic about the number of tab-separated values per line, rebuilding authorized_keys can break if a user put a tab character in their SSH key comment. A tab in the comment makes it look like there are three instead of two values on the line sent to the key-adding process.

This bit us on gitlab.com a while ago. This problem will also affect any installation with tabs in SSH key comments during a backup restore.

Original issue: https://dev.gitlab.org/gitlab/gitlabhq/issues/2511

It somehow ended up in the 8.2 milestone.

Milestone changed to 8.2

cc @JobV

@jacobvosmaer Would it be enough to convert any tabs to spaces on the rake side?

io.puts("#{id}\t#{key.gsub(/[[:space:]]+/, ' ').strip}")

@rspeicher I think so. There should be exactly one tab per line. Replacing with spaces (as you suggest) is better then just removing the tabs probably, because you avoid accidentally joining non-whitespace strings together.

mentioned in commit 69b41ba0

mentioned in merge request !1552 (merged)

Simple fix, re-assigning to 8.1 /cc @JobV

Milestone changed to 8.1

Thanks for picking this up @rspeicher

Status changed to closed by commit 69b41ba0

mentioned in commit df3ae4ef

Reassigned to @rspeicher