Skip to content

Fixed #476: Updated UUID source of randomness to random_bytes() - #675

Merged
danielmorell merged 2 commits into
masterfrom
fixed/issue-476-uuid-hash-colisions
Oct 10, 2026
Merged

danielmorell merged 2 commits into
masterfrom
fixed/issue-476-uuid-hash-colisions

Conversation

@danielmorell

Copy link
Copy Markdown
Collaborator

Description of the change

This fixes possible UUID collisions by improving the source of randomness for our UUID generation. It now uses random_bytes() which uses an OS cryptographically random source.

Because PHP will throw an exception if the runtime environment does not have a supported source of randomness, we have included a fallback to continue to relay on a simplified Mersenne Twister random number generator.

Type of change

  • Bug fix (non-breaking change that fixes an issue)

Related issues

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers assigned
  • Issue from task tracker has a link to this pull request
  • Changes have been reviewed by at least one other engineer

@danielmorell danielmorell added this to the v4.3.0 milestone Oct 9, 2026
@danielmorell
danielmorell requested a review from brianr October 9, 2026 21:20
@rollbar-circleci-machine

Copy link
Copy Markdown

AI Agent Review LGTM (openai, openai-astra)

LGTM. No blocking findings were found.

LGTM, no defects found. Utilities::uuid4() now builds an RFC 4122 v4 UUID from random_bytes(16), setting the version nibble on byte 6 and the variant bits on byte 8. src/Utilities.php:278-291 is correct, and the expected values in uuid4RandomBytesProvider (tests/UtilitiesTest.php:286-296) check out when worked through by hand.

PHP version. The minimum is 8.2 (composer.json:34, CI matrix at .github/workflows/ci.yml:39). So the non-capturing catch (Exception), the typed static \Closure property in the stub, and \Random\RandomException in the data provider (tests/UtilitiesTest.php:354) are all valid on every supported version. \Random\RandomException extends \Exception, so the fallback is reachable.

Test stub isolation. tests/TestHelpers/RandomBytesStub.php defines Rollbar\random_bytes. It is loaded lazily through the dev PSR-4 map (composer.json:27-31), is kept out of test discovery (phpunit.xml:12) and out of Psalm (psalm.xml:13). Only the @runInSeparateProcess tests reference it, so the main test process still uses the real random_bytes(). The only production caller, DataBuilder::getUuid() (src/DataBuilder.php:1142-1145), is unaffected, and Utilities is final, so the new : string return type is safe.

Design trade-off, not a defect. The mt_rand() fallback no longer calls mt_srand(), and tests/UtilitiesTest.php:343-347 checks that on purpose. This reverses the 0.14.0 fix in CHANGELOG.md:611, which made Rollbar reseed so apps that call mt_srand() with a fixed value would not get repeated UUIDs. If an app seeds mt_rand with a constant on every request and the host has no secure randomness source, fallback UUIDs will repeat across requests. Reseeding on every call would instead bring back the 32-bit seed-space collisions this PR is fixing, so not reseeding is defensible. Still, a short CHANGELOG note for 4.3.0 would help, since it partly undoes a documented fix.

Not verified here. No vendor/ directory and no shell, so I could not run phpcs, Psalm or PHPUnit. Nothing in this review claims the tests pass.

@danielmorell
danielmorell merged commit 69599da into master Oct 10, 2026
18 checks passed
@danielmorell
danielmorell deleted the fixed/issue-476-uuid-hash-colisions branch October 10, 2026 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hash collision generating the UUID4

3 participants