[grid] Add config blocked-routes and specific blocked-delete-session in Router

[VietND96]

User description

🔗 Related Issues

💥 What does this PR do?

1. Add a config to support #15808 (a flag to block session deletion - both on UI and API via route filter).

• This is a synchronized way to prevent deleting any session proactively via UI or API endpoint.
• Usage: Set CLI config --blocked-delete-session true when starting Router/Hub/Standalone component. (or similar config name in TOML)

2. Extend the feature for advanced users are able to define a list of routes to block.

• Grid support list of endpoints (https://www.selenium.dev/documentation/grid/advanced_features/endpoints/), which include delete session, delete node, or drain a node.
• For example, an advanced user who wants to block a few methods to route paths, they can define to this config.
• Usage: Route to block in format 'METHOD:path'. Can be specified multiple times.
• Example:

CLI options

--blocked-routes 'DELETE:/session/{session-id} --blocked-routes 'DELETE:/se/grid/distributor/node/{node-id}'

TOML configs

[router]
blocked-routes = ["DELETE:/session/{session-id}", "POST:/session"]

In logs, we can monitor the config apply

23:52:16.945 INFO [Hub.createHandlers] - Blocking 1 route(s): [DELETE:/session/{session-id}]
23:52:17.012 INFO [Hub.execute] - Started Selenium Hub 4.34.0-SNAPSHOT (revision Unknown): [http://172.18.0.2:4444⁠](http://172.18.0.2:4444/)

User who performs method to route path matched will see the response with status 403

curl -X DELETE http://localhost:4444/session/9da451a700131aac631c4547382661f9
Route blocked by configuration: DELETE /session/9da451a700131aac631c4547382661f9

From Router logs, we can also monitor how many blocked methods get hit

23:54:35.897 WARN [BlockedRoutesFilter.execute] - Blocked request: DELETE /session/9da451a700131aac631c4547382661f9 (matches blocked route: DELETE:/session/{session-id})

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Enhancement

---

Description

• Add configurable route blocking for Grid components
• Implement specific flag to block session deletion
• Support comma-separated list of blocked routes
• Add comprehensive test coverage for new functionality

---

Changes walkthrough 📝

	Relevant files
Enhancement	5 files Hub.java Apply blocked routes filter to Hub                                              +10/-0    Standalone.java Apply blocked routes filter to Standalone                                +10/-0    BlockedRoute.java Create BlockedRoute model class                                                    +101/-0  BlockedRoutesFilter.java Implement HTTP request filtering logic                                      +86/-0    RouterServer.java Apply blocked routes filter to RouterServer                            +8/-0
Configuration changes	3 files RouterFlags.java Add CLI flags for route blocking                                                  +21/-0    RouterOptions.java Parse blocked routes configuration                                              +50/-0    BUILD.bazel Add test build configuration                                                          +18/-0
Tests	2 files BlockedRoutesFilterTest.java Test blocked routes filter functionality                                  +213/-0  RouterOptionsTest.java Test router options configuration parsing                                +272/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Code Suggestions ✨

No code suggestions found for the PR.

[VietND96]

/review

[qodo-merge-pro]

PR Reviewer Guide 🔍

(Review updated until commit 24e5e23)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌ 5678 - Not compliant Non-compliant requirements: Fix ConnectFailure error when instantiating ChromeDriver Address connection refused errors for subsequent ChromeDriver instances Ensure first instance works without console errors 1234 - Not compliant Non-compliant requirements: Fix JavaScript execution in link's href on click() for Firefox Ensure test case alerts work with version 2.48+ as they did with 2.47.1 Address regression in Firefox 42.0 compatibility

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns Path traversal prevention: The BlockedRoute class implements path normalization to prevent path traversal attacks using techniques like '../' sequences and URL encoding. However, the complexity of this logic (lines 144-192) requires careful review to ensure all attack vectors are covered. The URL decoding and path resolution could potentially be bypassed with sophisticated encoding techniques or edge cases not covered by the current implementation.

⚡ Recommended focus areas for review Security Concern The path normalization logic in normalizePath method is complex and handles security-sensitive operations like path traversal prevention. The URL decoding and path resolution logic should be thoroughly reviewed for edge cases and potential bypasses. private String normalizePath(String path) { if (path == null || path.isEmpty()) { return "/"; } try { // URL decode the path to handle percent-encoded characters like %2F String decodedPath = URLDecoder.decode(path, StandardCharsets.UTF_8); // Normalize multiple consecutive slashes to single slashes String normalizedPath = decodedPath.replaceAll("/+", "/"); // Split into segments and resolve path traversal String[] segments = normalizedPath.split("/"); List<String> resolvedSegments = new ArrayList<>(); for (String segment : segments) { if (segment.isEmpty() || ".".equals(segment)) { // Skip empty segments and current directory references continue; } else if ("..".equals(segment)) { // Go up one directory level if (!resolvedSegments.isEmpty()) { resolvedSegments.remove(resolvedSegments.size() - 1); } else { // Attempting to go above root - this is a security violation throw new IllegalArgumentException("Path traversal attack detected: " + path); } } else { // Add normal segment resolvedSegments.add(segment); } } // Reconstruct the path StringBuilder result = new StringBuilder(); for (String segment : resolvedSegments) { result.append("/").append(segment); } // Ensure the result starts with / and handle empty path case String finalPath = result.toString(); return finalPath.isEmpty() ? "/" : finalPath; } catch (Exception e) { // If URL decoding fails or any other error occurs, throw security exception throw new IllegalArgumentException("Invalid path format: " + path, e); } } Performance Risk The matchesPathPattern method performs string splitting and iteration for every request match check. This could impact performance under high load, especially with many blocked routes configured. private boolean matchesPathPattern(String pattern, String requestPath) { // Normalize both paths to prevent path traversal attacks String normalizedPattern = normalizePath(pattern); String normalizedRequestPath = normalizePath(requestPath); // Split both paths into segments String[] patternSegments = normalizedPattern.split("/", -1); // keep trailing empty segments String[] requestSegments = normalizedRequestPath.split("/", -1); // Paths must have the same number of segments if (patternSegments.length != requestSegments.length) { return false; } // Compare each segment for (int i = 0; i < patternSegments.length; i++) { String patternSegment = patternSegments[i]; String requestSegment = requestSegments[i]; // If both are empty (leading/trailing slash), continue if (patternSegment.isEmpty() && requestSegment.isEmpty()) { continue; } // If pattern segment is a path parameter (enclosed in {}), it matches any non-empty segment if (isPathParameter(patternSegment)) { if (requestSegment.isEmpty()) { return false; } } else { // For literal segments, they must match exactly if (!patternSegment.equals(requestSegment)) { return false; } } } return true; } Logic Issue The duplicate prevention logic for DELETE session route uses stream operations that create new lists unnecessarily. The logic could be simplified and made more efficient. if (blockedDeleteSession) { BlockedRoute deleteSessionRoute = new BlockedRoute("DELETE", "/session/{session-id}"); // Only add if not already present if (routes.stream() .noneMatch( route -> "DELETE".equals(route.getMethod()) && "/session/{session-id}".equals(route.getPath()))) { routes = Stream.concat(routes.stream(), Stream.of(deleteSessionRoute)) .collect(Collectors.toList()); } }

[pujagani]

Looks good to me

[VietND96]

@pujagani thanks Puja for approval. In the last commit, I added a minor fix for AI review point "Path traversal vulnerability".
/review

[VietND96]

/review

[qodo-merge-pro]

Persistent review updated to latest commit 24e5e23