bitflicker64 commented on code in PR #2945:
URL: https://github.com/apache/hugegraph/pull/2945#discussion_r4298176792
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/arthas/ArthasAPI.java:
##########
@@ -17,48 +17,110 @@
package org.apache.hugegraph.api.arthas;
+import java.net.InetAddress;
import java.util.HashMap;
import org.apache.hugegraph.api.API;
+import org.apache.hugegraph.auth.HugeAuthenticator;
+import org.apache.hugegraph.config.ConfigOption;
import org.apache.hugegraph.config.HugeConfig;
import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.core.GraphManager;
+import org.apache.hugegraph.util.E;
import org.apache.hugegraph.util.JsonUtil;
+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
import com.codahale.metrics.annotation.Timed;
+import com.google.common.net.InetAddresses;
import com.taobao.arthas.agent.attach.ArthasAgent;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.annotation.security.RolesAllowed;
import jakarta.inject.Singleton;
+import jakarta.ws.rs.ForbiddenException;
import jakarta.ws.rs.PUT;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.Context;
+import jakarta.ws.rs.core.SecurityContext;
@Path("arthas")
@Singleton
@Tag(name = "ArthasAPI")
public class ArthasAPI extends API {
+ private static final Logger LOG = Log.logger(ArthasAPI.class);
+
@Context
private jakarta.inject.Provider<HugeConfig> configProvider;
@PUT
@Timed
@Produces(APPLICATION_JSON_WITH_CHARSET)
+ @RolesAllowed("admin")
@Operation(summary = "start arthas agent")
- public Object startArthas() {
+ public Object startArthas(@Context GraphManager manager,
+ @Context SecurityContext context) {
+ checkAccess(manager, context);
HugeConfig config = this.configProvider.get();
- HashMap<String, String> configMap = new HashMap<>(4);
- configMap.put("arthas.telnetPort",
config.get(ServerOptions.ARTHAS_TELNET_PORT));
- configMap.put("arthas.httpPort",
config.get(ServerOptions.ARTHAS_HTTP_PORT));
- configMap.put("arthas.ip", config.get(ServerOptions.ARTHAS_IP));
- configMap.put("arthas.disabledCommands",
- config.get(ServerOptions.ARTHAS_DISABLED_COMMANDS));
+ HashMap<String, String> configMap = arthasConfig(config);
String response = JsonUtil.toJson(configMap);
// Keep the optional MCP endpoint disabled without extending the
activation response.
configMap.put("arthas.mcpEndpoint", "");
+ // Do not let external Arthas properties override the validated local
listener.
+ configMap.put("arthas.config.overrideAll", "false");
+ configMap.put("arthas.tunnelServer", "");
ArthasAgent.attach(configMap);
return response;
}
+
+ static void checkAccess(GraphManager manager, SecurityContext context) {
+ // No-auth requests receive an anonymous admin role, so a role check
alone is insufficient.
+ if (!manager.requireAuthentication()) {
+ throw new ForbiddenException("HTTP Arthas activation requires
authentication; " +
+ "use bin/arthas.sh on the server host
instead");
+ }
+ if (context == null || context.getUserPrincipal() == null ||
+
HugeAuthenticator.USER_ANONY.equals(context.getUserPrincipal().getName()) ||
+ !context.isUserInRole("admin")) {
+ throw new ForbiddenException("Arthas activation requires an
authenticated administrator");
+ }
+ }
+
+ static HashMap<String, String> arthasConfig(HugeConfig config) {
+ HashMap<String, String> configMap = new HashMap<>(4);
+ configMap.put("arthas.telnetPort",
+ option(config, ServerOptions.ARTHAS_TELNET_PORT,
+ ServerOptions.ARTHAS_TELNET_PORT_LEGACY));
+ configMap.put("arthas.httpPort",
+ option(config, ServerOptions.ARTHAS_HTTP_PORT,
+ ServerOptions.ARTHAS_HTTP_PORT_LEGACY));
+ String ip = config.get(ServerOptions.ARTHAS_IP);
+ // Do not resolve arbitrary hostnames: their address may change after
validation.
+ ip = "localhost".equalsIgnoreCase(ip) ? "127.0.0.1" : ip;
+ E.checkArgument(InetAddresses.isInetAddress(ip) &&
+ InetAddresses.forString(ip).isLoopbackAddress(),
Review Comment:
Minor: `bin/arthas.sh` cannot reach or stop an API agent bound to `::1` (or
any loopback address other than `127.0.0.1`), although this check accepts it.
`arthasConfig` accepts any loopback literal, and
`ArthasConfigTest.testLoopbackAddresses` covers `::1` and `127.0.0.2`. The
helper hardcodes `127.0.0.1`: `arthas.sh:57` passes `--target-ip 127.0.0.1` to
arthas-boot, and `stop_agent` connects `arthas-client` to `127.0.0.1` (line
65). Yet `docs/server-runtime.md` tells an operator with an API-activated agent
to run `bin/arthas.sh --telnet-port 8562`, and to use `--command stop` before
switching to the helper defaults.
I ran the helper from this head (JDK 17, macOS) against a fixture JVM. The
fixture called `ArthasAgent.attach` with the same map `startArthas()` builds:
- `arthas.ip=127.0.0.1`: `--telnet-port 18562 --command version` printed
`4.1.5`, and `--command stop` shut the agent down.
- `arthas.ip=::1`: arthas-boot logged `The target process already listen
port 28562, skip attach`, then `Connect to telnet server error: 127.0.0.1
28562` (`Connection refused`). Both `--command version` and `--command stop`
exited 1, and `[::1]:28562` and `[::1]:28561` stayed open. Cleanup also took
the `Connection refused` branch silently.
Please either accept only `127.0.0.1` here (still mapping `localhost` to it)
and update the test, or give the helper a loopback-only target address and use
it for both arthas-boot and the cleanup client. If you keep the wider set, the
docs should say that the helper only reaches agents on `127.0.0.1`.
##########
hugegraph-store/hg-store-node/pom.xml:
##########
@@ -121,15 +121,10 @@
<artifactId>hg-store-core</artifactId>
</dependency>
- <dependency>
- <groupId>com.taobao.arthas</groupId>
- <artifactId>arthas-agent-attach</artifactId>
- <version>3.6.4</version>
- </dependency>
<dependency>
<groupId>com.taobao.arthas</groupId>
<artifactId>arthas-packaging</artifactId>
Review Comment:
Minor: hg-store-node no longer uses Arthas at runtime, but it still declares
`arthas-packaging` as a compile dependency, so the Store boot jar still carries
it.
This PR removes the only consumer (`PartitionAPI.arthasstart` and its
`ArthasAgent.attach` call) and drops `arthas-agent-attach`. After it, only the
two POMs under `hugegraph-store` reference `com.taobao.arthas`.
`arthas-packaging-4.1.5.jar` is 16.7 MB and holds only `arthas-bin.zip`. The
`repackage` execution in this POM puts it in `BOOT-INF/lib` of
`hg-store-node-*-exec.jar`, so every Store distribution and image keeps it on
the runtime classpath, next to the unpacked copy in `lib/arthas`. Its only
remaining reader is the `prepare-arthas` antrun step in `hg-store-dist`, which
reaches it transitively through the exec jar.
Please declare it in `hugegraph-store/hg-store-dist/pom.xml` instead (for
example with `<scope>provided</scope>`; the `dependencySet` only includes the
exec jar either way) and remove it here, then check that
`maven.dependency.com.taobao.arthas.arthas-packaging.jar.path` still resolves
for the unzip.
--
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]
