framework icon indicating copy to clipboard operation
framework copied to clipboard

Add QueryException message handling without replacing bindings.

Open tzmfreedom opened this issue 1 year ago • 6 comments

As mentioned in the discussion https://github.com/laravel/framework/discussions/41920, QueryException message that bind real SQL values is useful for development. But there is security risk, e.g. unintended personal information(email, user name, tel, ...) logging.

This pull request adds support for QueryException message handling with/without replacing bindings. If we put mask: true parameters to database config, ? masking is not replacing with real SQL value. I think this pull request help Laravel application more secure.

Example

config/database.php

<?php

return [
    'connections' => [
        'mysql' => [
            'driver' => 'mysql',
            // ...
            'mask' => true,
        ],

mask: true

(Connection: , SQL: SELECT * FROM users WHERE id = ?)

mask: false

(Connection: , SQL: SELECT * FROM users WHERE id = 1)

tzmfreedom avatar Jan 15 '25 13:01 tzmfreedom

In Eloquent we have toSql or toRawSql!

Maybe it would be interesting to change from mask to raw?

ezequidias avatar Jan 15 '25 16:01 ezequidias

<?php

return [
    'connections' => [
        'mysql' => [
            'driver' => 'mysql',
            // ...
            'mask' => true,
        ],

Just from reading this, to me, it's not entirely clear, that mask is only used for exceptions. Maybe, the key naming should reflect that better

shaedrich avatar Jan 15 '25 18:01 shaedrich

Yeah - naming is not super clear.

taylorotwell avatar Jan 15 '25 19:01 taylorotwell

Thank you for review!

How about the following name?

  • hide_bindings_on_exception_message
  • hide_bindings_on_error_message
  • hide_parameters_on_exception_message
  • hide_parameters_on_error_message

tzmfreedom avatar Jan 16 '25 00:01 tzmfreedom

Thank you for review!

How about the following name?

  • hide_bindings_on_exception_message
  • hide_bindings_on_error_message
  • hide_parameters_on_exception_message
  • hide_parameters_on_error_message

There's also mask_{bindings|parameters}_on_{error|exception}_message.

I'd say, using "bindings" and "exception" here feels like the most intuitive, but they are all way better than just "mask" 👍🏻

shaedrich avatar Jan 16 '25 06:01 shaedrich

Thank you for advice! I fixed the naming from mask to mask_bindings_on_exception_message.

tzmfreedom avatar Jan 16 '25 13:01 tzmfreedom