Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import java.util.HashMap;

import jakarta.annotation.security.RolesAllowed;

import org.apache.hugegraph.api.API;
import org.apache.hugegraph.config.HugeConfig;
import org.apache.hugegraph.config.ServerOptions;
Expand All @@ -38,6 +40,7 @@
@Path("arthas")
@Singleton
@Tag(name = "ArthasAPI")
@RolesAllowed("admin")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking: yes. Summary: This annotation does not protect PUT /arthas in the default no-auth configuration because anonymous requests are treated as admin. Please add an explicit localhost/no-auth guard here, or require authentication before calling ArthasAgent.attach(...). Evidence: GraphManager.requireAuthentication() returns false when no authenticator is configured; AuthenticationFilter.authenticate() then returns User.ANONYMOUS; HugeAuthenticator.User.ANONYMOUS is constructed with ROLE_ADMIN.

public class ArthasAPI extends API {

@Context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,15 +439,15 @@ public class ServerOptions extends OptionHolder {
"arthas.ip",
"arthas bound ip",
disallowEmpty(),
"0.0.0.0"
"127.0.0.1"
);

public static final ConfigOption<String> ARTHAS_DISABLED_COMMANDS =
new ConfigOption<>(
"arthas.disabledCommands",
"arthas disabled commands",
disallowEmpty(),
"jad"
"jad,ognl,vmtool"
Comment thread
imbajin marked this conversation as resolved.
Comment thread
imbajin marked this conversation as resolved.
);

public static final ConfigOption<Boolean> ALLOW_TRACE =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ batch.max_write_ratio=80
batch.max_write_threads=0

# configuration of arthas
arthas.telnet_port=8562
arthas.http_port=8561
arthas.telnetPort=8562
Comment thread
JisoLya marked this conversation as resolved.
arthas.httpPort=8561
arthas.ip=127.0.0.1
arthas.disabled_commands=jad
arthas.disabledCommands=jad,ognl,vmtool
Comment thread
JisoLya marked this conversation as resolved.

# authentication configs
#auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,10 @@ public class ArthasConfig {
@Value("${arthas.httpPort:8565}")
private String httpPort;

@Value("${arthas.ip:0.0.0.0}")
@Value("${arthas.ip:127.0.0.1}")
private String arthasip;

@Value("${arthas.disabledCommands:jad}")
@Value("${arthas.disabledCommands:jad,ognl,vmtool}")
private String disCmd;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.List;
import java.util.Map;

import javax.servlet.http.HttpServletRequest;

import org.apache.hugegraph.pd.common.PDException;
import org.apache.hugegraph.pd.grpc.Metapb;
import org.apache.hugegraph.rocksdb.access.RocksDBSession;
Expand All @@ -37,7 +39,9 @@
import org.apache.hugegraph.store.node.grpc.HgStoreNodeService;
import org.apache.hugegraph.util.Bytes;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
Expand All @@ -64,7 +68,7 @@ public class PartitionAPI {
AppConfig appConfig;

@GetMapping(value = "/partitions", produces = "application/json")
public Map<String, Object> getPartitions(
public ResponseEntity<Map<String, Object>> getPartitions(
@RequestParam(required = false, defaultValue = "") String flags) {

boolean accurate = false;
Expand Down Expand Up @@ -109,11 +113,11 @@ public Map<String, Object> getPartitions(
rafts.add(raft);
}

return okMap("partitions", rafts);
return ok("partitions", rafts);
}

@GetMapping(value = "/partition/{id}", produces = "application/json")
public Raft getPartition(@PathVariable(value = "id") int id) {
public ResponseEntity<Map<String, Object>> getPartition(@PathVariable(value = "id") int id) {

HgStoreEngine storeEngine = nodeService.getStoreEngine();

Expand All @@ -138,16 +142,15 @@ public Raft getPartition(@PathVariable(value = "id") int id) {
raft.getPartitions().add(partition);
}

return raft;
//return okMap("partition", rafts);
return ok("raft", raft);
Comment thread
JisoLya marked this conversation as resolved.
}

/**
* Print all keys in the partition
*/
@GetMapping(value = "/partition/dump/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
public Map<String, Object> dumpPartition(@PathVariable(value = "id") int id) throws
PDException {
public ResponseEntity<Map<String, Object>> dumpPartition(
@PathVariable(value = "id") int id) throws PDException {
HgStoreEngine storeEngine = nodeService.getStoreEngine();
BusinessHandler handler = storeEngine.getBusinessHandler();
InnerKeyCreator innerKeyCreator = new InnerKeyCreator(handler);
Expand All @@ -168,27 +171,40 @@ public Map<String, Object> dumpPartition(@PathVariable(value = "id") int id) thr
}
cfIterator.close();
});
return okMap("ok", null);
return ok("ok", "");
}

/**
* 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();
Comment thread
JisoLya marked this conversation as resolved.
BusinessHandler handler = storeEngine.getBusinessHandler();

storeEngine.getPartitionEngine(id).getPartitions().forEach((graph, partition) -> {
handler.cleanPartition(graph, id);
});
return okMap("ok", null);
return ok("ok", "");
}

@GetMapping(value = "/arthasstart", produces = "application/json")
Comment thread
JisoLya marked this conversation as resolved.
public Map<String, Object> arthasstart(
@RequestParam(required = false, defaultValue = "") String flags) {
public ResponseEntity<Map<String, Object>> arthasstart(
Comment thread
JisoLya marked this conversation as resolved.
@RequestParam(required = false, defaultValue = "") String flags,
HttpServletRequest request) {
// Ignore proxy headers to prevent IP spoofing.
// NOTE: If behind a reverse proxy (e.g., Nginx), getRemoteAddr() returns the proxy's IP.
// Ensure the proxy is configured to block untrusted external access.
String remoteAddr = getCleanIp(request.getRemoteAddr());
boolean isLocalRequest =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

High: Local proxies bypass the localhost restriction

hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/PartitionAPI.java:200

Evidence

  • getRemoteAddr() identifies the immediate peer, so an external request forwarded by a same-host reverse proxy appears as 127.0.0.1 and reaches ArthasAgent.attach() at line 213. No authentication interceptor or security configuration exists in hg-store-node.

Impact

  • Remote callers can still trigger dynamic agent attachment in a common reverse-proxy deployment despite the endpoint claiming localhost-only access.

Requested fix

  • Do not use peer IP as the sole authorization boundary; require authenticated administrative access or expose this endpoint through a separate listener explicitly bound to loopback.

"127.0.0.1".equals(remoteAddr) || "0:0:0:0:0:0:0:1".equals(remoteAddr) ||
"::1".equals(remoteAddr);
if (!isLocalRequest) {
List<String> ret = new ArrayList<>();
ret.add("Arthas start is ONLY allowed from localhost.");
return forbidden("arthasstart", ret);
}
HashMap<String, String> configMap = new HashMap<>();
configMap.put("arthas.telnetPort", appConfig.getArthasConfig().getTelnetPort());
configMap.put("arthas.httpPort", appConfig.getArthasConfig().getHttpPort());
Expand All @@ -198,11 +214,11 @@ public Map<String, Object> arthasstart(
// DashResponse retPose = new DashResponse();
List<String> ret = new ArrayList<>();
ret.add("Arthas started successfully");
return okMap("arthasstart", ret);
return ok("arthasstart", ret);
}

@PostMapping("/compat")
public Map<String, Object> compact(@RequestParam(value = "id") int id) {
public ResponseEntity<Map<String, Object>> compact(@RequestParam(value = "id") int id) {
boolean submitted =
nodeService.getStoreEngine().getBusinessHandler().blockingCompact("", id);
Map<String, Object> map = new HashMap<>();
Expand All @@ -215,14 +231,28 @@ public Map<String, Object> compact(@RequestParam(value = "id") int id) {
map.put("msg",
"compaction task fail to submit, and there could be another task in progress");
}
return map;
return ok("body", map);
Comment thread
JisoLya marked this conversation as resolved.
}

public Map<String, Object> okMap(String k, Object v) {
Map<String, Object> map = new HashMap<>();
map.put("status", 0);
map.put(k, v);
return map;
public ResponseEntity<Map<String, Object>> ok(String k, Object v) {
return ResponseEntity.ok(Map.of("status", 200, k, v));
Comment thread
JisoLya marked this conversation as resolved.
}

public ResponseEntity<Map<String, Object>> forbidden(String k, Object v) {
return ResponseEntity.status(HttpStatus.FORBIDDEN)
.body(Map.of("status", 403, k, v));
}

private String getCleanIp(String remoteAddr) {
if (remoteAddr != null) {
if (remoteAddr.startsWith("[")) {
remoteAddr = remoteAddr.substring(1);
}
if (remoteAddr.endsWith("]")) {
remoteAddr = remoteAddr.substring(0, remoteAddr.length() - 1);
}
}
return remoteAddr;
}

@Data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,10 @@ logging:
config: classpath:log4j2-dev.xml
level:
root: info

arthas:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking: no. Summary: These node-resource Arthas defaults are not reflected in the store distribution config, so packaged users will not see or tune the new settings in conf/application.yml. Please add the same arthas section to hugegraph-store/hg-store-dist/src/assembly/static/conf/application.yml. Evidence: the store assembly includes ${assembly.static.dir}/conf, and start-hugegraph-store.sh loads ${CONF}/application.yml; the dist application.yml currently ends at the logging section without any arthas keys.

telnetPort: 8566
httpPort: 8565
# Only allow starting arthas locally
ip: 127.0.0.1
disabledCommands: jad,ognl,vmtool
Loading