Skip to content

Remove an old potentially misuable random string method

Nick Malcolm requested to merge nmalcolm-random-string into master

What does this MR do and why?

This removes a barely used, but potentially misusable, method: Gitlab::Utils#random_string. Ruby's random method is not appropriate for security contexts, which is why we have SecureRandom and friends. In 2017 we added random_string and while it's probably fine (it's a really large value) it's only used in one location.

As a defense in depth measure to prevent us from using it incorrectly in a secure context, I have removed it.

In the one place it was used, Internationalization Linter code, I have switched it for SecureRandom.alphanumeric SecureRandom.uuid. Length didn't seem to matter. Hopefully dashes don't matter.

Since it's do do with i18n I'm assuming it's ~"group::import" responsibility.

Random.rand(Float::MAX.to_i).to_s(36)
=> "jxeahfd2mxp60koy3qrxvawdn7uepj5vp6nrg5pt86y5cn5xf82npzvwcld8tf7jmyqk4a09jb6sc0vgn1375cbnhjx4gasgf2c0hzs01iliifpjtg9b7ocjoz491h9cxpi9k7shuizq0pncjy7po0yeksvr01rant0y0qhzaexmv591f6keo3m5kyo4kaapb2guz0"

SecureRandom.alphanumeric(64)
=> "hKKLOI8mMjp1AtEppnKdgqm9oZ1euIcGgxi1NH5fbT8QtoskYhL2AdgNlAttMYu9"

# Previously:
SecureRandom.uuid
=> "d09e1f86-2d62-40d8-ad20-d0e5c376d2c9"

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Peter Leitzen

Merge request reports