Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(setupcheck): Check several config values to be valid URLs (and more) #45382

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickvergessen
Copy link
Member

TODO

  • Validate that several configs are valid URLs or of hardcoded other format
  • Make sure overwrite.cli.url is within trusted_domains
  • Mark more of those configs as sensitive

Checklist

}

if (filter_var($parts[0], FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
if (!is_numeric($parts[1]) || $parts[1] > 32 || $parts[1] < 1) {

Check notice

Code scanning / Psalm

PossiblyUndefinedArrayOffset Note

Possibly undefined array key $parts[1] on list{0: string, 1?: string}
return SetupResult::error($this->l10n->t('Config value %s is not an valid IP address or IP range.', [$configKey]));
}
} elseif (filter_var($parts[0], FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
if (!is_numeric($parts[1]) || $parts[1] > 128 || $parts[1] < 1) {

Check notice

Code scanning / Psalm

PossiblyUndefinedArrayOffset Note

Possibly undefined array key $parts[1] on list{0: string, 1?: string}
@kesselb
Copy link
Contributor

kesselb commented May 17, 2024

Good idea 👍

Some people were confused with the well-known setup check because it stops on the first failure.

I fear this could be confusing here as well if you encounter many issues in the configuration file.

This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 14, 2024
@skjnldsv skjnldsv marked this pull request as draft November 15, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: occ config:list system does not redact trusted_domains and overwrite.cli.url
4 participants