Skip to content

fix: cors wildcard#10956

Merged
abnegate merged 1 commit into1.8.xfrom
fix-cors-wildcard
Dec 15, 2025
Merged

fix: cors wildcard#10956
abnegate merged 1 commit into1.8.xfrom
fix-cors-wildcard

Conversation

@loks0n
Copy link
Copy Markdown
Member

@loks0n loks0n commented Dec 14, 2025

No description provided.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 14, 2025

📝 Walkthrough

Walkthrough

This pull request refactors the CORS hostname validation mechanism in Appwrite. The primary change replaces a direct array membership check (in_array) against allowedHosts with a validator-based approach using Utopia\Validator\Hostname. The hostname validator is instantiated with the allowed hosts list and validates incoming hosts against this list. A new unit test verifies that wildcard subdomain patterns (*.example.com) correctly allow subdomains (foo.example.com) and return the appropriate Allow-Origin header value. The change maintains existing lowercasing and wildcard-origin logic throughout the validation process.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify that the Hostname validator preserves the previous exact match behavior for non-wildcard hosts
  • Confirm that wildcard expansion logic (*.example.com → foo.example.com) functions identically to the prior implementation
  • Check that case-insensitive host comparison (lowercasing) is maintained by the validator
  • Review the new test method covers the documented wildcard subdomain behavior and integrates with existing test patterns

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether it relates to the changeset. Add a description explaining the CORS wildcard fix, the problem it addresses, and why the validator-based approach improves the implementation.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: cors wildcard' is directly related to the changeset, which modifies CORS hostname validation to handle wildcard subdomains correctly.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-cors-wildcard

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d2a95c and da0b2a7.

📒 Files selected for processing (2)
  • src/Appwrite/Network/Cors.php (2 hunks)
  • tests/unit/Network/CorsTest.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/unit/Network/CorsTest.php (1)
src/Appwrite/Network/Cors.php (2)
  • Cors (14-91)
  • headers (53-90)
src/Appwrite/Network/Cors.php (1)
src/Appwrite/Network/Validator/Origin.php (1)
  • isValid (31-62)
⏰ 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). (3)
  • GitHub Check: Benchmark
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan
🔇 Additional comments (2)
src/Appwrite/Network/Cors.php (1)

5-5: LGTM: Adding Hostname validator import.

The import is correctly added to support the new validator-based hostname matching logic.

tests/unit/Network/CorsTest.php (1)

39-52: The test appropriately validates subdomain wildcard matching. The Hostname validator uses fnmatch() with FNM_CASEFOLD for pattern matching, which already handles the edge cases correctly:

  • Base domain (example.com) correctly does NOT match *.example.com
  • Non-matching domains are properly rejected (verified by testUnlistedOriginReturnsStaticHeadersOnly)
  • Single-level subdomains (foo.example.com) match *.example.com as expected

Existing test coverage comprehensively validates CORS behavior including exact domain matches, unlisted origins, invalid URLs, and wildcard patterns. No additional edge-case tests are required.

@github-actions
Copy link
Copy Markdown

✨ Benchmark results

  • Requests per second: 1,202
  • Requests with 200 status code: 216,400
  • P99 latency: 0.161841462

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,202 1,255
200 216,400 225,996
P99 0.161841462 0.161197877

@abnegate abnegate merged commit e9d9aa6 into 1.8.x Dec 15, 2025
45 checks passed
@abnegate abnegate deleted the fix-cors-wildcard branch December 15, 2025 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants