align allowed characters for option --value on console command config:delete to config:set
#20764
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'. |
|
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
|
@matomo-org/core-reviewers Can I get a review on this? |
…ig:delete to config:set The console command `config:set` allows any character for the `--value` option (see: `\Piwik\Plugins\CoreAdminHome\Commands\SetConfig\ConfigSettingManipulation::make`). The regex used for `config:set` is: `/^([a-zA-Z0-9_]+)\.([a-zA-Z0-9_]+)(\[\])?=(.*)/` The console command `config:delete` only allows characters a to z (upper and lower case), numbers and the underscore. This prevents one from deleting configs values like IPv4 or IPV6 addresses, mail addresses, networks or IP ranges and lists of values. The currently used regex is: `/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9_]+))?/` The following is a list of possible settings found in the wild. Those can be set with `console config:set` but cannot be deleted. ``` [General] login_allowlist_ip[] = "123.123.123.123" trusted_hosts = "[2001:858:6::186]:80" contact_email_address = "no-reply+with-extension@test.at" proxy_ips[] = "192.168.x.x/24" [HeatmapSessionRecording] session_recording_sample_limits = 50,100,250,500,1000,2000,5000 ``` This commit aligns the allowed characters for the option `--value` on the console command `config:delete` to the ones for `config:set` and changes the regex to `/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.(.+))?/`. Fixes: matomo-org#19966 Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
f8e65e0 to
3cda269
Compare
sgiehl
approved these changes
Jun 20, 2023
There was a problem hiding this comment.
Did some local testing and seems to fix the described problem. Well done @ziegenberg 🎉
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
The console command
config:setallows any character for the--valueoption (see:\Piwik\Plugins\CoreAdminHome\Commands\SetConfig\ConfigSettingManipulation::make).The regex used for
config:setis:/^([a-zA-Z0-9_]+)\.([a-zA-Z0-9_]+)(\[\])?=(.*)/The console command
config:deleteonly allows characters a to z (upper and lower case), numbers and the underscore. This prevents one from deleting configs values like IPv4 or IPV6 addresses, mail addresses, networks or IP ranges and lists of values. The currently used regex is:/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9_]+))?/The following is a list of possible settings found in the wild. Those can be set with
console config:setbut cannot be deleted.This commit aligns the allowed characters for the option
--valueon the console commandconfig:deleteto the ones forconfig:setand changes the regex to/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.(.+))?/.Fixes: #19966
If we do not want to accept all special characters we could also go with the following regex:
/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9\x28-\x2F\[\]_:@]+))?Besides the well-known character classes (
a-zA-Z0-9) this allows the following characters:(,),*,+,,,-,.,/[,],_,:,@Review