Skip to content

Conversation

@amirhshokri
Copy link
Contributor

@amirhshokri amirhshokri commented Oct 3, 2025

Summary

This PR adds the checkIsReservedConnectionName method to prevent accidental use of the reserved horizon Redis connection. If horizon is passed as a connection name, or if a conflicting configuration exists, the method throws an InvalidArgumentException during the Laravel app boot process.

Why?

This change ensures safer configuration and prevents accidental misconfiguration.

image

@amirhshokri amirhshokri changed the title [5.x] Prevent using the reserved horizon Redis connection [5.x] Prevent using the reserved horizon Redis connection name Oct 3, 2025
@alexmontoanelli
Copy link
Contributor

alexmontoanelli commented Oct 3, 2025

Currently we've a db redis connection named horizon, and horizon.use pointing to 'horizon'.

As the image bellow, the method is just overriding the current config.

But, if your are using a redis db named horizon for other things, then the prefix key
is just begin override.

I' think this could introduce a infrastructure break for someone if you throw a exception.

Screenshot 2025-10-03 at 13 08 13

@taylorotwell
Copy link
Member

Can you just check in the service provider only?

@taylorotwell taylorotwell marked this pull request as draft October 3, 2025 21:14
@amirhshokri
Copy link
Contributor Author

amirhshokri commented Oct 4, 2025

Done @taylorotwell.
Please let me know if any other changes are needed.

@amirhshokri amirhshokri marked this pull request as ready for review October 4, 2025 16:18
@taylorotwell taylorotwell merged commit fc4904a into laravel:5.x Oct 4, 2025
14 checks passed
@amirhshokri amirhshokri deleted the 5.x-prevent-using-horizon-connection-name branch October 5, 2025 08:43
@patrickbrouwers
Copy link

Hello @amirhshokri this seems to be giving issues when using config:cache Any idea?

@donnyxray
Copy link

This PR should be reversed. There is no real way to fix this without busting or bypassing the cache.

Perhaps the config:cache code can allow for such checks to be registered somewhere and then executed upon caching.

amirhshokri added a commit to amirhshokri/laravel-horizon that referenced this pull request Oct 7, 2025
taylorotwell pushed a commit that referenced this pull request Oct 7, 2025
@thetheago
Copy link

Hi @amirhshokri. Thanks for this last commit, it was breaking my route:clear command.

@agustin-villalba
Copy link

@donnyxray is totally right, this should be reverted. Even version 5.35.1 is not fixing the problem when you have the config cached in production. Still the error The Redis connection name [horizon] is reserved for internal use.

@agustin-villalba
Copy link

It was using a hidden env variable, it's fine now. Thank you for the fix!

@gerardnll gerardnll mentioned this pull request Oct 8, 2025
amirhshokri added a commit to amirhshokri/laravel-horizon that referenced this pull request Oct 8, 2025
taylorotwell pushed a commit that referenced this pull request Oct 8, 2025
* revert #1615

* fix: style-ci
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.

7 participants