-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#1930] Added custom trusted hosts support via .env.
#1931
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
Conversation
WalkthroughIntroduces DRUPAL_TRUSTED_HOSTS as a comma-separated env var, wires it through docker-compose, documents it, updates test fixtures, adds a Drupal settings include to append exact-match trusted_host_patterns from the env, and adds PHPUnit coverage for various input cases. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Operator as Operator
participant Env as .env / Compose Env
participant Drupal as Drupal Settings Loader
participant Include as settings.trusted_hosts.php
participant Kernel as Drupal Kernel
Operator->>Env: Set DRUPAL_TRUSTED_HOSTS
Env-->>Drupal: Provide env at boot
Drupal->>Include: Include settings.trusted_hosts.php
Include->>Include: Parse CSV, trim/lowercase, preg_quote
Include-->>Drupal: Append ^domain$ patterns to settings
Drupal-->>Kernel: trusted_host_patterns configured
Kernel->>Kernel: Enforce host checks on requests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2)tests/phpunit/Drupal/SwitchableSettingsTest.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information web/sites/default/includes/modules/settings.trusted_hosts.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1931 +/- ##
========================================
Coverage 78.69% 78.69%
========================================
Files 85 85
Lines 5027 5027
Branches 35 35
========================================
Hits 3956 3956
Misses 1071 1071 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6a28a9f to
a41e3c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (36)
.vortex/installer/tests/Fixtures/install/_baseline/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/includes/modules/settings.trusted_hosts.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_container_registry/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_ftp/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/db_download_source_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_circleci/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_gha/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_artifact/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_container_image/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_circleci/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_gha/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/modules/settings.trusted_hosts.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/web/sites/default/includes/modules/-settings.trusted_hosts.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_solr/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_valkey/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_valkey/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_absent/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_absent/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/web/themes/custom/light_saber/yarn.lockis excluded by!**/yarn.lock,!**/*.lock,!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (9)
.env(1 hunks).vortex/docs/content/workflows/variables.mdx(1 hunks).vortex/tests/bats/fixtures/docker-compose.env.json(7 hunks).vortex/tests/bats/fixtures/docker-compose.env_local.json(7 hunks).vortex/tests/bats/fixtures/docker-compose.env_mod.json(7 hunks).vortex/tests/bats/fixtures/docker-compose.noenv.json(7 hunks)docker-compose.yml(1 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(1 hunks)web/sites/default/includes/modules/settings.trusted_hosts.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
web/sites/default/includes/modules/settings.trusted_hosts.php (1)
.vortex/installer/src/Utils/Validator.php (1)
domain(22-26)
tests/phpunit/Drupal/SwitchableSettingsTest.php (1)
tests/phpunit/Drupal/SettingsTestCase.php (2)
setEnvVars(135-166)requireSettingsFile(205-224)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: vortex-test-workflow (1)
🔇 Additional comments (6)
.vortex/tests/bats/fixtures/docker-compose.env_mod.json (1)
42-42: Confirmed DRUPAL_TRUSTED_HOSTS in All Fixtures
- .vortex/tests/bats/fixtures/docker-compose.env.json: 7 occurrences
- .vortex/tests/bats/fixtures/docker-compose.env_local.json: 7 occurrences
- .vortex/tests/bats/fixtures/docker-compose.env_mod.json: 7 occurrences
- .vortex/tests/bats/fixtures/docker-compose.noenv.json: 7 occurrences
All fixtures consistently include
DRUPAL_TRUSTED_HOSTSacross the seven services (chrome, clamav, cli, database, nginx, php, solr). No further changes needed—LGTM..vortex/tests/bats/fixtures/docker-compose.env_local.json (1)
42-42: LGTM: local fixture mirrors env and mod variants.Consistent addition and ordering. Keeping this in chrome/database is harmless and aligns with the shared env anchor pattern used across services.
Also applies to: 102-102, 162-162, 220-220, 278-278, 345-345, 412-412
.vortex/tests/bats/fixtures/docker-compose.env.json (1)
42-42: LGTM: base fixture updated consistently.No issues spotted. Matches the distribution and key order used elsewhere.
Also applies to: 102-102, 162-162, 220-220, 278-278, 345-345, 412-412
web/sites/default/includes/modules/settings.trusted_hosts.php (1)
13-24: Solid, safe implementation (preg_quote + exact-match).The approach of lowercasing, escaping with preg_quote, and wrapping with ^...$ correctly prevents regex injection and enforces exact host matches. Appending to existing patterns preserves provider defaults. LGTM.
tests/phpunit/Drupal/SwitchableSettingsTest.php (1)
696-706: Nice, focused assertion for trusted_host_patterns.The equality assertion verifies both content and order, ensuring provider defaults are preserved and env-provided patterns are appended.
.vortex/tests/bats/fixtures/docker-compose.noenv.json (1)
42-42: DRUPAL_TRUSTED_HOSTS Consistency Confirmed Across All FixturesAll seven core services (chrome, clamav, cli, database, nginx, php, solr) in each of the BATS docker-compose fixtures now include the DRUPAL_TRUSTED_HOSTS environment variable, matching expectations in:
- .vortex/tests/bats/fixtures/docker-compose.noenv.json (lines 42, 102, 162, 220, 278, 345, 412)
- .vortex/tests/bats/fixtures/docker-compose.env.json (lines 42, 102, 162, 220, 278, 345, 412)
- .vortex/tests/bats/fixtures/docker-compose.env_local.json (lines 42, 102, 162, 220, 278, 345, 412)
- .vortex/tests/bats/fixtures/docker-compose.env_mod.json (lines 42, 102, 162, 220, 278, 345, 412)
The only services missing DRUPAL_TRUSTED_HOSTS ( valkey and wait_dependencies ) are outside the intended list and do not require host-based settings. No unexpected gaps were found—this is ready to merge.
| # Trusted host patterns. | ||
| # | ||
| # Comma-separated list of domains for trusted host patterns. These domains will | ||
| # be added to the trusted host patterns alongside any other routes defined by | ||
| # the hosting provider. | ||
| DRUPAL_TRUSTED_HOSTS=your-site-domain.example,www.your-site-domain.example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Clarify semantics: exact domains only; no wildcards/regex; trimming OK.
Docs here could be more explicit to prevent misconfiguration. Suggest refining the comments to state that entries are treated as exact domain matches (regex-escaped and anchored), spaces are trimmed, and subdomains must be listed explicitly.
Apply this diff to improve clarity without changing behavior:
# Trusted host patterns.
#
-# Comma-separated list of domains for trusted host patterns. These domains will
-# be added to the trusted host patterns alongside any other routes defined by
-# the hosting provider.
+# Comma-separated list of domains to trust. Each entry is treated as an exact
+# match (no wildcards or regex) and surrounding spaces are trimmed. To trust
+# subdomains, list each one explicitly (e.g., example.com,www.example.com).
+# These are appended to any routes/patterns defined by the hosting provider.
DRUPAL_TRUSTED_HOSTS=your-site-domain.example,www.your-site-domain.example📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Trusted host patterns. | |
| # | |
| # Comma-separated list of domains for trusted host patterns. These domains will | |
| # be added to the trusted host patterns alongside any other routes defined by | |
| # the hosting provider. | |
| DRUPAL_TRUSTED_HOSTS=your-site-domain.example,www.your-site-domain.example | |
| # Trusted host patterns. | |
| # | |
| # Comma-separated list of domains to trust. Each entry is treated as an exact | |
| # match (no wildcards or regex) and surrounding spaces are trimmed. To trust | |
| # subdomains, list each one explicitly (e.g., example.com,www.example.com). | |
| # These are appended to any routes/patterns defined by the hosting provider. | |
| DRUPAL_TRUSTED_HOSTS=your-site-domain.example,www.your-site-domain.example |
🤖 Prompt for AI Agents
In .env around lines 51 to 56, the trusted-hosts comment is ambiguous; update
the comment to explicitly state that entries are treated as exact domain matches
(they are regex-escaped and anchored), that wildcards or regex are not
supported, that surrounding whitespace will be trimmed, and that any subdomains
must be listed explicitly (e.g., example.com and www.example.com). Make the
wording brief, concrete, and place an example to show correct usage without
changing any logic or behavior.
| # Comma-separated list of domains for trusted host patterns. These domains will | ||
| # be added to the trusted host patterns alongside any other routes defined by | ||
| # the hosting provider. | ||
| DRUPAL_TRUSTED_HOSTS=your-site-domain.example,www.your-site-domain.example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider adding tests for edge cases (IDN/punycode, trailing dot, ports).
The parsing code (per PR summary) already trims, lowercases, dedupes, and escapes. Add test vectors for:
- IDN/punycode: xn--d1acpjx3f.xn--p1ai
- Trailing dot host: example.com.
- Accidental ports/paths: example.com:8443, example.com/foo (should be ignored or rejected)
If desired, I can open a follow-up to add these cases to tests/phpunit/Drupal/SwitchableSettingsTest.php.
🤖 Prompt for AI Agents
.env around line 56: add unit tests in
tests/phpunit/Drupal/SwitchableSettingsTest.php that assert host-parsing handles
edge cases: include a test vector for IDN/punycode ("xn--d1acpjx3f.xn--p1ai")
expecting it to be normalized and accepted, a trailing-dot host ("example.com.")
expecting the trailing dot to be trimmed and accepted, and inputs containing
ports or paths ("example.com:8443", "example.com/foo") expecting those to be
ignored or rejected by the parser; implement assertions to confirm trimming,
lowercasing, deduplication and escaping behavior for the valid cases and that
invalid port/path-bearing entries do not become trusted hosts.
| ### `DRUPAL_TRUSTED_HOSTS` | ||
|
|
||
| Trusted host patterns. | ||
|
|
||
| Comma-separated list of domains for trusted host patterns. These domains will<br/>be added to the trusted host patterns alongside any other routes defined by<br/>the hosting provider. | ||
|
|
||
| Default value: `your-site-domain.example,www.your-site-domain.example` | ||
|
|
||
| Defined in: `.env` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Clarify input expectations for DRUPAL_TRUSTED_HOSTS (exact match, hosts-only, no schemes/ports).
Good addition. To reduce misconfiguration, please explicitly state that values must be plain hostnames (no http/https, paths, or ports), are matched exactly (no wildcards), and that subdomains must be listed explicitly. Also call out IDNs and duplicates behavior.
Apply this doc tweak:
### `DRUPAL_TRUSTED_HOSTS`
-Trusted host patterns.
-
-Comma-separated list of domains for trusted host patterns. These domains will
-be added to the trusted host patterns alongside any other routes defined by
-the hosting provider.
+Trusted host patterns.
+
+Comma-separated list of hostnames to be added to Drupal’s trusted host patterns.
+Notes:
+- Use hostnames only (no scheme, path, or port). Example: `example.com,www.example.com`
+- Exact match only; wildcards are not supported. List each subdomain explicitly.
+- Values are lowercased and special characters are escaped.
+- Duplicates are preserved as provided.
+- For internationalized domains, provide the punycode form (e.g., `xn--...`).
Default value: `your-site-domain.example,www.your-site-domain.example`
Defined in: `.env`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### `DRUPAL_TRUSTED_HOSTS` | |
| Trusted host patterns. | |
| Comma-separated list of domains for trusted host patterns. These domains will<br/>be added to the trusted host patterns alongside any other routes defined by<br/>the hosting provider. | |
| Default value: `your-site-domain.example,www.your-site-domain.example` | |
| Defined in: `.env` | |
| ### `DRUPAL_TRUSTED_HOSTS` | |
| Trusted host patterns. | |
| Comma-separated list of hostnames to be added to Drupal’s trusted host patterns. | |
| Notes: | |
| - Use hostnames only (no scheme, path, or port). Example: `example.com,www.example.com` | |
| - Exact match only; wildcards are not supported. List each subdomain explicitly. | |
| - Values are lowercased and special characters are escaped. | |
| - Duplicates are preserved as provided. | |
| - For internationalized domains, provide the punycode form (e.g., `xn--...`). | |
| Default value: `your-site-domain.example,www.your-site-domain.example` | |
| Defined in: `.env` |
🤖 Prompt for AI Agents
In .vortex/docs/content/workflows/variables.mdx around lines 281 to 290, the
DRUPAL_TRUSTED_HOSTS description needs clarification of expected input format
and matching behavior: update the text to state values must be plain hostnames
only (no scheme like http/https, no paths, no ports), that entries are matched
exactly (no wildcards) so subdomains must be listed explicitly, and note how
IDNs should be provided (punycode or Unicode consistent with system handling)
and that duplicate entries are ignored or de-duplicated; make sure to add a
short example showing a valid comma-separated value and a brief note about
default value and where it is defined (.env).
| # Trusted host patterns. | ||
| DRUPAL_TRUSTED_HOSTS: ${DRUPAL_TRUSTED_HOSTS:-} | ||
| #;< DRUPAL_THEME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Good placement in default environment anchor; zero-risk default.
Passing DRUPAL_TRUSTED_HOSTS via x-environment with an empty default is correct and keeps it available to all services that rely on the shared anchor.
Optionally verify wiring end-to-end (env → PHP settings include → Drupal):
🏁 Script executed:
#!/bin/bash
# 1) Confirm the include file exists.
fd -a 'settings.trusted_hosts.php'
# 2) Confirm settings.php (or equivalent) loads the include.
rg -nP '(require|include)(_once)?\s*\(.+settings\.trusted_hosts\.php' -C2 --type=php
# 3) Spot-check that the include builds anchored, escaped patterns from DRUPAL_TRUSTED_HOSTS.
rg -n 'DRUPAL_TRUSTED_HOSTS|preg_quote|trusted_host_patterns|\^\S+\$' -C2 --type=php web 2>/dev/null || trueLength of output: 5949
🏁 Script executed:
#!/bin/bash
# 1) Print first 50 lines of the include file to confirm its contents and anchoring logic.
sed -n '1,50p' web/sites/default/includes/modules/settings.trusted_hosts.php
# 2) Search settings.php for any direct include/require of settings.trusted_hosts.php
rg -nP '(?:require|include)(_once)?\s*\(\s*[^)]*settings\.trusted_hosts\.php' -C2 --type=php web/sites/default/settings.php || true
# 3) Search settings.php for bulk-loading of all files under includes/modules
rg -nP 'foreach\s*\(\s*glob\([^)]*includes/modules/.*\.php\)' -C3 --type=php web/sites/default/settings.php || trueLength of output: 999
Include the trusted-hosts module in Drupal’s settings to activate DRUPAL_TRUSTED_HOSTS
Our checks confirm that:
- The file
web/sites/default/includes/modules/settings.trusted_hosts.phpexists and correctly reads, sanitizes, and appends patterns from theDRUPAL_TRUSTED_HOSTSenvironment variable. - However,
web/sites/default/settings.phpdoes notincludeorrequirethis file (nor does it bulk-load theincludes/modules/directory), so the module never runs and the environment setting has no effect.
Please add an include (or directory loader) in web/sites/default/settings.php, for example immediately after your provider includes:
// Load environment-driven provider settings.
require_once __DIR__ . '/includes/providers/settings.container.php';
require_once __DIR__ . '/includes/providers/settings.lagoon.php';
+
+ // Load custom modules (including trusted-hosts).
+ foreach (glob(__DIR__ . '/includes/modules/*.php') as $module) {
+ include_once $module;
+ }Or, to be more explicit:
+ // Activate DRUPAL_TRUSTED_HOSTS logic.
+ require_once __DIR__ . '/includes/modules/settings.trusted_hosts.php';This change is required to wire the DRUPAL_TRUSTED_HOSTS variable end-to-end.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docker-compose.yml around lines 50-52 the DRUPAL_TRUSTED_HOSTS env var is set
but web/sites/default/settings.php does not include the corresponding module
file, so the setting is never applied; open web/sites/default/settings.php and
immediately after the provider include(s) add an include (or require) of
web/sites/default/includes/modules/settings.trusted_hosts.php (or implement a
small directory loader to require all files in includes/modules/) so the
trusted-hosts module runs and DRUPAL_TRUSTED_HOSTS is wired end-to-end.
| public static function dataProviderTrustedHostPatterns(): array { | ||
| return [ | ||
| 'empty environment variable' => [ | ||
| [], | ||
| [ | ||
| '^localhost$', | ||
| ], | ||
| ], | ||
| 'single domain' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => 'example.com'], | ||
| [ | ||
| '^localhost$', | ||
| '^example\.com$', | ||
| ], | ||
| ], | ||
| 'multiple domains' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => 'example.com,www.example.com,cdn.example.org'], | ||
| [ | ||
| '^localhost$', | ||
| '^example\.com$', | ||
| '^www\.example\.com$', | ||
| '^cdn\.example\.org$', | ||
| ], | ||
| ], | ||
| 'whitespace and empty values' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => ' example.com , , www.example.com '], | ||
| [ | ||
| '^localhost$', | ||
| '^example\.com$', | ||
| '^www\.example\.com$', | ||
| ], | ||
| ], | ||
| 'special regex characters' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => 'sub-domain.example.com,test.example-site.org'], | ||
| [ | ||
| '^localhost$', | ||
| '^sub\-domain\.example\.com$', | ||
| '^test\.example\-site\.org$', | ||
| ], | ||
| ], | ||
| 'complex domains' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => 'api.v2.example.com,cdn-assets.example-site.co.uk'], | ||
| [ | ||
| '^localhost$', | ||
| '^api\.v2\.example\.com$', | ||
| '^cdn\-assets\.example\-site\.co\.uk$', | ||
| ], | ||
| ], | ||
| 'duplicates' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => 'example.com,test.org,example.com,another.com,test.org'], | ||
| [ | ||
| '^localhost$', | ||
| '^example\.com$', | ||
| '^test\.org$', | ||
| '^example\.com$', | ||
| '^another\.com$', | ||
| '^test\.org$', | ||
| ], | ||
| ], | ||
| 'uppercase hosts' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => 'EXAMPLE.COM,Test.ORG,www.EXAMPLE-SITE.CO.UK'], | ||
| [ | ||
| '^localhost$', | ||
| '^example\.com$', | ||
| '^test\.org$', | ||
| '^www\.example\-site\.co\.uk$', | ||
| ], | ||
| ], | ||
| 'explicit localhost' => [ | ||
| ['DRUPAL_TRUSTED_HOSTS' => 'localhost,example.com,localhost,test.org'], | ||
| [ | ||
| '^localhost$', | ||
| '^localhost$', | ||
| '^example\.com$', | ||
| '^localhost$', | ||
| '^test\.org$', | ||
| ], | ||
| ], | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Broaden datasets to cover explicit empty value and (if normalized) URLs/ports.
Good coverage: whitespace, special chars, case, duplicates, and localhost. Add two cases:
- Env var explicitly set to empty string to mirror docker-compose “noenv” fixture.
- Hosts pasted as URLs/with ports (only if you adopt normalization in settings include).
Proposed additions inside the data provider’s return array:
'uppercase hosts' => [
['DRUPAL_TRUSTED_HOSTS' => 'EXAMPLE.COM,Test.ORG,www.EXAMPLE-SITE.CO.UK'],
[
'^localhost$',
'^example\.com$',
'^test\.org$',
'^www\.example\-site\.co\.uk$',
],
],
+ 'explicit empty env var' => [
+ ['DRUPAL_TRUSTED_HOSTS' => ''],
+ [
+ '^localhost$',
+ ],
+ ],
+ // Enable this case if settings.trusted_hosts.php normalizes scheme/port.
+ 'hosts with scheme and port (normalized)' => [
+ ['DRUPAL_TRUSTED_HOSTS' => 'https://example.com:8443/foo,www.example.com:8080'],
+ [
+ '^localhost$',
+ '^example\.com$',
+ '^www\.example\.com$',
+ ],
+ ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static function dataProviderTrustedHostPatterns(): array { | |
| return [ | |
| 'empty environment variable' => [ | |
| [], | |
| [ | |
| '^localhost$', | |
| ], | |
| ], | |
| 'single domain' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'example.com'], | |
| [ | |
| '^localhost$', | |
| '^example\.com$', | |
| ], | |
| ], | |
| 'multiple domains' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'example.com,www.example.com,cdn.example.org'], | |
| [ | |
| '^localhost$', | |
| '^example\.com$', | |
| '^www\.example\.com$', | |
| '^cdn\.example\.org$', | |
| ], | |
| ], | |
| 'whitespace and empty values' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => ' example.com , , www.example.com '], | |
| [ | |
| '^localhost$', | |
| '^example\.com$', | |
| '^www\.example\.com$', | |
| ], | |
| ], | |
| 'special regex characters' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'sub-domain.example.com,test.example-site.org'], | |
| [ | |
| '^localhost$', | |
| '^sub\-domain\.example\.com$', | |
| '^test\.example\-site\.org$', | |
| ], | |
| ], | |
| 'complex domains' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'api.v2.example.com,cdn-assets.example-site.co.uk'], | |
| [ | |
| '^localhost$', | |
| '^api\.v2\.example\.com$', | |
| '^cdn\-assets\.example\-site\.co\.uk$', | |
| ], | |
| ], | |
| 'duplicates' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'example.com,test.org,example.com,another.com,test.org'], | |
| [ | |
| '^localhost$', | |
| '^example\.com$', | |
| '^test\.org$', | |
| '^example\.com$', | |
| '^another\.com$', | |
| '^test\.org$', | |
| ], | |
| ], | |
| 'uppercase hosts' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'EXAMPLE.COM,Test.ORG,www.EXAMPLE-SITE.CO.UK'], | |
| [ | |
| '^localhost$', | |
| '^example\.com$', | |
| '^test\.org$', | |
| '^www\.example\-site\.co\.uk$', | |
| ], | |
| ], | |
| 'explicit localhost' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'localhost,example.com,localhost,test.org'], | |
| [ | |
| '^localhost$', | |
| '^localhost$', | |
| '^example\.com$', | |
| '^localhost$', | |
| '^test\.org$', | |
| ], | |
| ], | |
| ]; | |
| } | |
| 'uppercase hosts' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'EXAMPLE.COM,Test.ORG,www.EXAMPLE-SITE.CO.UK'], | |
| [ | |
| '^localhost$', | |
| '^example\.com$', | |
| '^test\.org$', | |
| '^www\.example\-site\.co\.uk$', | |
| ], | |
| ], | |
| 'explicit empty env var' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => ''], | |
| [ | |
| '^localhost$', | |
| ], | |
| ], | |
| // Enable this case if settings.trusted_hosts.php normalizes scheme/port. | |
| 'hosts with scheme and port (normalized)' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'https://example.com:8443/foo,www.example.com:8080'], | |
| [ | |
| '^localhost$', | |
| '^example\.com$', | |
| '^www\.example\.com$', | |
| ], | |
| ], | |
| 'explicit localhost' => [ | |
| ['DRUPAL_TRUSTED_HOSTS' => 'localhost,example.com,localhost,test.org'], | |
| [ | |
| '^localhost$', | |
| '^localhost$', | |
| '^example\.com$', | |
| '^localhost$', | |
| '^test\.org$', | |
| ], | |
| ], |
🤖 Prompt for AI Agents
In tests/phpunit/Drupal/SwitchableSettingsTest.php around lines 711 to 790, add
two data-provider cases: one for an explicit empty environment variable (e.g.
['DRUPAL_TRUSTED_HOSTS' => ''] ) that should produce the same patterns as the
empty env case (only '^localhost$'), and another for hosts supplied as URLs/with
ports (e.g. 'http://example.com:8080,https://sub.example.org/path') only if your
settings code normalizes/strips schemes, ports and paths — that case should
assert the normalized host regexes (e.g. '^example\.com$',
'^sub\.example\.org$'). Insert them into the returned array alongside the other
scenarios so the provider covers the docker-compose “noenv” fixture and optional
URL/port normalization.
| $domains = array_map('trim', explode(',', $trusted_hosts)); | ||
| foreach ($domains as $domain) { | ||
| if (!empty($domain)) { | ||
| $domain = strtolower($domain); | ||
| $escaped_domain = preg_quote($domain, '/'); | ||
| $settings['trusted_host_patterns'][] = '^' . $escaped_domain . '$'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Initialize $settings['trusted_host_patterns'] before appending to avoid PHP notices.
If the base settings ever omit the key or set it to a non-array, $settings['trusted_host_patterns'][] = ... can emit a notice. Guard once before appending.
$domains = array_map('trim', explode(',', $trusted_hosts));
+ if (!isset($settings['trusted_host_patterns']) || !is_array($settings['trusted_host_patterns'])) {
+ $settings['trusted_host_patterns'] = [];
+ }
foreach ($domains as $domain) {
if (!empty($domain)) {
$domain = strtolower($domain);
$escaped_domain = preg_quote($domain, '/');
$settings['trusted_host_patterns'][] = '^' . $escaped_domain . '$';
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $domains = array_map('trim', explode(',', $trusted_hosts)); | |
| foreach ($domains as $domain) { | |
| if (!empty($domain)) { | |
| $domain = strtolower($domain); | |
| $escaped_domain = preg_quote($domain, '/'); | |
| $settings['trusted_host_patterns'][] = '^' . $escaped_domain . '$'; | |
| } | |
| } | |
| $domains = array_map('trim', explode(',', $trusted_hosts)); | |
| if (!isset($settings['trusted_host_patterns']) || !is_array($settings['trusted_host_patterns'])) { | |
| $settings['trusted_host_patterns'] = []; | |
| } | |
| foreach ($domains as $domain) { | |
| if (!empty($domain)) { | |
| $domain = strtolower($domain); | |
| $escaped_domain = preg_quote($domain, '/'); | |
| $settings['trusted_host_patterns'][] = '^' . $escaped_domain . '$'; | |
| } | |
| } |
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.trusted_hosts.php around lines 16
to 23, the code appends to $settings['trusted_host_patterns'] without ensuring
it exists as an array which can trigger PHP notices; before the foreach loop add
a guard that checks if (!isset($settings['trusted_host_patterns']) ||
!is_array($settings['trusted_host_patterns'])) {
$settings['trusted_host_patterns'] = []; } so subsequent
$settings['trusted_host_patterns'][] operations safely append while preserving
any existing array entries.
| foreach ($domains as $domain) { | ||
| if (!empty($domain)) { | ||
| $domain = strtolower($domain); | ||
| $escaped_domain = preg_quote($domain, '/'); | ||
| $settings['trusted_host_patterns'][] = '^' . $escaped_domain . '$'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Intentional duplicates vs. deduplication.
Tests assert that duplicates are preserved. If this is intentional (e.g., for deterministic merge behavior), consider a short code comment to make it explicit. Otherwise, deduplication via array_unique could reduce config noise.
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.trusted_hosts.php around lines
17–23, the loop that appends trusted_host_patterns preserves duplicate domains;
either add a one-line comment above the foreach documenting that duplicates are
intentionally preserved for deterministic merge behavior, or implement
deduplication by running $domains = array_values(array_unique($domains)) before
the foreach so duplicate entries are removed and config noise is reduced.
💡 Verification agent
🧩 Analysis chain
Optional: normalize accidental URLs/ports and IDNs to reduce footguns.
Users often paste full URLs (https://example.com:8443/foo) or host:port. Consider normalizing to a host-only value and trimming a trailing dot. If ext-intl is present, converting Unicode domains to punycode further hardens matching.
If you adopt this normalization, add corresponding PHPUnit datasets (see test file comment). Suggested change:
- foreach ($domains as $domain) {
+ foreach ($domains as $domain) {
if (!empty($domain)) {
- $domain = strtolower($domain);
- $escaped_domain = preg_quote($domain, '/');
+ $domain = strtolower($domain);
+ // Extract host if a scheme/port/path was provided.
+ $host = parse_url(strpos($domain, '://') !== false ? $domain : ('http://' . $domain), PHP_URL_HOST) ?? $domain;
+ // Remove trailing dot from FQDNs.
+ $host = rtrim($host, '.');
+ // Convert Unicode IDN to ASCII (punycode) if available.
+ if (function_exists('idn_to_ascii')) {
+ $converted = idn_to_ascii($host, 0);
+ if (is_string($converted) && $converted !== '') {
+ $host = $converted;
+ }
+ }
+ $escaped_domain = preg_quote($host, '/');
$settings['trusted_host_patterns'][] = '^' . $escaped_domain . '$';
}
}To verify impact, run a quick grep to confirm the base trusted_host_patterns default is defined before this include (ensuring order is safe):
🏁 Script executed:
#!/bin/bash
# Confirm where trusted_host_patterns are initialized in settings includes.
rg -nP -C2 '\$settings\s*\[\s*[\'"]trusted_host_patterns[\'"]\s*\]' web/sites/defaultLength of output: 116
I see the grep pattern broke due to nested single quotes. Let’s retry with proper quoting:
🏁 Script executed:
#!/bin/bash
# Confirm where trusted_host_patterns are initialized in settings includes.
rg -nC2 "\$settings\['trusted_host_patterns'\]" web/sites/defaultLength of output: 72
🏁 Script executed:
#!/bin/bash
# Search for any occurrences of trusted_host_patterns in web/sites/default
rg -n "trusted_host_patterns" -C2 web/sites/defaultLength of output: 4163
Normalization Safe – optional refactor recommended
Verified that $settings['trusted_host_patterns'] is initialized in web/sites/default/settings.php (lines 93–95) before any includes, so adding host‐normalization here won’t alter the intended order.
• In web/sites/default/settings.php at lines 93–95 you have:
$settings['trusted_host_patterns'] = [
'^localhost$',
];• In web/sites/default/includes/modules/settings.trusted_hosts.php (around lines 17–23), you can implement the optional normalization:
foreach ($domains as $domain) {
if (!empty($domain)) {
- $domain = strtolower($domain);
- $escaped_domain = preg_quote($domain, '/');
+ $domain = strtolower($domain);
+ // Extract host if a scheme/port/path was provided.
+ $host = parse_url(strpos($domain, '://') !== false ? $domain : ('http://' . $domain), PHP_URL_HOST) ?? $domain;
+ // Remove trailing dot from FQDNs.
+ $host = rtrim($host, '.');
+ // Convert Unicode IDN to ASCII (punycode) if available.
+ if (function_exists('idn_to_ascii')) {
+ $converted = idn_to_ascii($host, 0);
+ if (is_string($converted) && $converted !== '') {
+ $host = $converted;
+ }
+ }
+ $escaped_domain = preg_quote($host, '/');
$settings['trusted_host_patterns'][] = '^' . $escaped_domain . '$';
}
}Don’t forget to add PHPUnit datasets covering full URLs, ports, trailing dots, and Unicode domains as noted in the test‐file comment.
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.trusted_hosts.php around lines 17
to 23, implement host normalization before escaping and appending patterns: trim
whitespace, strip URL schemes and paths (accept full URLs), remove ports,
convert to lowercase, remove trailing dots, and handle IDN (convert Unicode to
punycode) then preg_quote the normalized host and push '^' . $escaped_domain .
'$' into $settings['trusted_host_patterns']; also add PHPUnit datasets for full
URLs, ports, trailing dots, and Unicode domains as noted in the test-file
comment.
Closes #1930
Summary by CodeRabbit