[core] Augment browser request validation using CORS header checks

[edoakes]

Rejects requests containing "Access-Control-Request-Method" or "Access-Control-Request-Headers" headers.

[gemini-code-assist]

Code Review

This pull request enhances the detection of browser requests by checking for additional CORS-related headers. This improves security by making it harder for non-browser clients to be misidentified. The changes are accompanied by a comprehensive set of new test cases that cover various browsers and scenarios.

My review focuses on improving code maintainability. I've suggested refactoring the header checks into a constant for clarity and moving the large set of test data into a separate file to make the test code more readable.

[gemini-code-assist]

For improved readability and a minor performance gain, consider defining these headers as a module-level constant set instead of a tuple literal inside the function. This avoids recreating the collection on every call and makes the list of headers easier to manage.

For example, at the module level:

_BROWSER_HEADERS = {
    "Origin",
    "Sec-Fetch-Mode",
    "Sec-Fetch-Dest",
    "Sec-Fetch-Site",
    "Sec-Fetch-User",
    "Access-Control-Request-Method",
    "Access-Control-Request-Headers",
}

Then the function body can be simplified to:

return req.headers.get("User-Agent", "").startswith("Mozilla") or any(
    h in req.headers for h in _BROWSER_HEADERS
)

[gemini-code-assist]

This large block of test case data makes the test function hard to read and maintain. It's good practice to separate test data from test logic.

Consider moving this testcases tuple into a separate data file (e.g., a JSON file) and loading it at the start of the test. The comments in the code (e.g., # chrome-invalid-tls.json) suggest this data may have originated from JSON files anyway.

For example, you could create a test_dashboard_browser_headers.json file:

[
    {
        "name": "chrome-invalid-tls.json",
        "headers": {
            "Host": "localtest.me",
            "Connection": "keep-alive",
            "Content-Length": "0",
            "Origin": "https://localtest.me"
        }
    },
    {
        "name": "chrome-localhost-notls.json",
        "headers": {
            "Host": "localhost:5000",
            "Connection": "keep-alive",
            "Origin": "http://localhost:5000"
        }
    }
]

And then in your test:

import json
import os

# ...

def test_browser_no_post_no_put(enable_test_module, ray_start_with_dashboard):
    # ...
    test_data_path = os.path.join(os.path.dirname(__file__), "test_dashboard_browser_headers.json")
    with open(test_data_path) as f:
        testcases_data = json.load(f)
    testcases = [item['headers'] for item in testcases_data]

    # ... rest of the test logic

This would make the test function much cleaner.