imbajin commented on code in PR #2945:
URL:
https://github.com/apache/incubator-hugegraph/pull/2945#discussion_r2740395698
##########
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/PartitionAPI.java:
##########
@@ -168,27 +172,45 @@ public Map<String, Object>
dumpPartition(@PathVariable(value = "id") int id) thr
}
cfIterator.close();
});
- return okMap("ok", null);
+ return ResponseEntity.status(HttpStatus.OK).body(okMap("ok", null));
}
/**
* Print all keys in the partition
*/
@GetMapping(value = "/partition/clean/{id}", produces =
MediaType.APPLICATION_JSON_VALUE)
- public Map<String, Object> cleanPartition(@PathVariable(value = "id") int
id) throws
-
PDException {
+ public ResponseEntity<Map<String, Object>> cleanPartition(
+ @PathVariable(value = "id") int id) throws PDException {
HgStoreEngine storeEngine = nodeService.getStoreEngine();
Review Comment:
‼️ **Critical: Inconsistent IPv6 localhost handling**
The localhost check uses string comparison which doesn't handle IPv6
properly:
```java
boolean isLocalRequest = "127.0.0.1".equals(remoteAddr) ||
"[0:0:0:0:0:0:0:1]".equals(remoteAddr);
```
Problems:
1. IPv6 localhost can be represented as `::1` (without brackets in
`getRemoteAddr()`)
2. The bracket format `[0:0:0:0:0:0:0:1]` is typically only in HTTP headers,
not `getRemoteAddr()`
3. Proxy headers might return IPv6 in different formats
**Suggested fix:**
```suggestion
boolean isLocalRequest = "127.0.0.1".equals(remoteAddr) ||
"::1".equals(remoteAddr) ||
"0:0:0:0:0:0:0:1".equals(remoteAddr);
```
##########
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/PartitionAPI.java:
##########
@@ -225,6 +247,34 @@ public Map<String, Object> okMap(String k, Object v) {
return map;
}
+ public Map<String, Object> forbiddenMap(String k, Object v) {
+ HashMap<String, Object> map = new HashMap<>();
+ map.put("status", 403);
+ map.put(k, v);
+ return map;
+ }
+
+ private String getClientIp(HttpServletRequest request) {
Review Comment:
⚠️ **Security consideration: X-Forwarded-For can be spoofed**
The current implementation trusts proxy headers unconditionally. If there's
a reverse proxy in front, attackers can spoof these headers to bypass localhost
restriction.
**Recommendation:**
- Document that this endpoint should never be exposed through reverse
proxies (nginx, etc.)
- Or add configuration to enable/disable proxy header trust
- Consider using `request.getRemoteAddr()` directly if not behind a proxy
Example improvement:
```java
// Only trust X-Forwarded-For if explicitly configured
String ip = appConfig.isTrustProxyHeaders()
? getProxiedIp(request)
: request.getRemoteAddr();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
