.env (2)

125-125: Consider DNS server choice for different environments.

Google's public DNS (8.8.8.8) is appropriate for development, but production deployments might benefit from:

• Using system default DNS resolvers
• Configuring environment-specific DNS servers
• Consider privacy implications of using external DNS services

---

124-126: Address static analysis style issues.

The dotenv-linter has identified several style issues that should be addressed for consistency:

Apply this diff to fix the ordering and formatting issues:

# Move the variables to maintain alphabetical order
-_APP_STATS_USAGE_DUAL_WRITING_DBS=database_db_main
-_APP_DOMAINS_DNS=8.8.8.8
-_APP_DOMAIN_TARGET_CAA='0 issue "digicert.com"'
+_APP_DOMAIN_TARGET_CAA='0 issue "digicert.com"'
+_APP_DOMAINS_DNS=8.8.8.8
+_APP_STATS_USAGE_DUAL_WRITING_DBS=database_db_main
+

Note: Reorder these variables to maintain alphabetical sorting and add a blank line at the end of the file.

src/Appwrite/Utopia/Response/Model/ConsoleVariables.php (1)

31-42: LGTM! Consider adding format validation for CAA records.

The additions are well-structured and consistent with existing patterns. The default DNS server (8.8.8.8) is a good choice. However, since CAA records have a specific format (e.g., "0 issue "domain.com""), consider adding validation or documentation about the expected format to prevent configuration errors.

app/config/variables.php (1)

154-171: Improve environment variable descriptions for better clarity.

The additions are correctly placed and structured. However, the descriptions could be enhanced:

1. For _APP_DOMAIN_TARGET_CAA: The format example is helpful, but consider explaining what CAA records do (authorize certificate issuers).
2. For _APP_DOMAINS_DNS: Consider mentioning that an empty value will use the system's default DNS resolver.

Apply this diff to improve the descriptions:

-                'description' => 'A CAA record value that can be used to validate custom domains. Format: "0 issue \"certainly.com\""',
+                'description' => 'A CAA (Certification Authority Authorization) record value that specifies which certificate authorities are allowed to issue certificates for custom domains. Format: "0 issue \"certainly.com\""',

-                'description' => 'DNS server to use for domain validation. Default: 8.8.8.8',
+                'description' => 'DNS server to use for domain validation. Default: 8.8.8.8 (Google Public DNS). Leave empty to use system default DNS resolver.',

tests/unit/Network/Validators/DNSTest.php (1)

51-72: Well-structured CAA validation tests with good error handling.

The test implementation is solid with appropriate error handling for DNS resolution failures. Good coverage of positive and negative test cases.

Consider adding tests for:

1. Different CAA record formats (e.g., "0 iodef", "0 issuewild")
2. Invalid CAA record formats to ensure proper validation

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: stnguyen90
PR: appwrite/appwrite#10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, `_APP_DOMAIN` is a required environment variable that must always be set for the system to function properly.

Learnt from: stnguyen90
PR: appwrite/appwrite#10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, `_APP_DOMAIN` is a required environment variable that must always be set for the system to function properly.

.env (1)

24-32: Keys inserted out of alphabetical order – fails dotenv-linter & hurts merge-diff clarity

dotenv-linter already flags the new _APP_DNS and _APP_DOMAIN_TARGET_* keys. Re-ordering keeps the file deterministic and avoids future churn in unrelated PRs.

Diff to relocate the new keys near the other _APP_DOMAIN_* entries:

-_APP_DNS=8.8.8.8
-_APP_DOMAIN_TARGET_CNAME=test.localhost
-_APP_DOMAIN_TARGET_A=127.0.0.1
-_APP_DOMAIN_TARGET_AAAA=::1
-_APP_DOMAIN_TARGET_CAA=digicert.com
+_APP_DNS=8.8.8.8
+
+_APP_DOMAIN_TARGET_A=127.0.0.1
+_APP_DOMAIN_TARGET_AAAA=::1
+_APP_DOMAIN_TARGET_CAA=digicert.com
+_APP_DOMAIN_TARGET_CNAME=test.localhost

(Adapt positions to match the project’s exact ordering rules.)

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

src/Appwrite/Network/Validator/DNS.php (1)

56-57: Address the DNS server validation issue from previous review.

This is the same issue flagged in the previous review. The DNS server configuration should be validated before passing to the Client constructor to prevent runtime errors.

The previous review comment is still valid. Please implement the suggested validation:

-        $dnsServer = System::getEnv('_APP_DNS', '8.8.8.8');
-        $dns = new Client($dnsServer);
+        $dnsServer = System::getEnv('_APP_DNS', '8.8.8.8');
+        // Validate DNS server IP address
+        if (!filter_var($dnsServer, FILTER_VALIDATE_IP)) {
+            $dnsServer = '8.8.8.8'; // Fallback to Google DNS
+        }
+        $dns = new Client($dnsServer);

tests/unit/Network/Validators/DNSTest.php (3)

69-69: Fix typo in comment.

There's a typo in the comment: "becasue" should be "because".

-        // Valid becasue no CAA record configured
+        // Valid because no CAA record configured

---

73-73: Fix typo in comment.

Same typo here: "becasue" should be "because".

-        // Valid becasue no CAA record configured
+        // Valid because no CAA record configured

---

51-76: Consider test stability with real domains.

The test uses real external domains (github.com, test1.appwrite.org) which could make tests flaky if their DNS records change. Consider using mock DNS responses or dedicated test domains that you control.

For more stable tests, consider mocking the DNS client or using controlled test domains:

// Example with mocking approach
$mockDns = $this->createMock(Client::class);
$mockDns->method('query')
    ->willReturn([/* predictable test data */]);

src/Appwrite/Network/Validator/DNS.php (2)

76-82: Simplify CAA record counting logic.

The CAA record counting is unnecessary since you're already iterating through records. The count is only used once at line 101, which could be simplified.

Consider removing the counter and using a flag instead:

-        $caaCount = 0;
+        $foundCaaRecords = false;

-        foreach ($query as $record) {
-            if ($record->getTypeName() === self::RECORD_CAA) {
-                $caaCount++;
-            }
+        foreach ($query as $record) {
+            if ($record->getTypeName() === self::RECORD_CAA) {
+                $foundCaaRecords = true;
+            }

-        if ($this->type === self::RECORD_CAA && $caaCount === 0) {
+        if ($this->type === self::RECORD_CAA && !$foundCaaRecords) {

---

101-104: Redundant CAA validation logic.

This check for CAA records with count === 0 is redundant because it's already handled at lines 67-74 when the query is empty.

The empty query check at lines 67-74 already handles the case where no CAA records exist. This additional check is unnecessary and can be removed:

-        if ($this->type === self::RECORD_CAA && $caaCount === 0) {
-            // No CAA records, means anyone can issue certificate
-            return true;
-        }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: stnguyen90
PR: appwrite/appwrite#10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, `_APP_DOMAIN` is a required environment variable that must always be set for the system to function properly.