From f796637919317786326375ee9373feeb42b8f57e Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Thu, 3 Sep 2026 11:21:25 +0530 Subject: [PATCH 01/28] fix(pd): validate REST credentials and return 401 on refusal The REST authentication interceptor had two defects (#3188): 1. Authentication.authenticate decoded the Basic credential but checked only the service name against the innerModules set, so any of the four public names with any password, including an empty one, was accepted, while the password was never read. 2. RestAuthentication.preHandle wrote an error body without calling setStatus, so success, refusal and missing credential all returned HTTP 200 and nothing keyed on a status code could see a refusal. Fix both together: - Compare the password of the Basic credential against the shared secret configured via auth.secret-key (constant-time comparison). A missing or empty secret refuses every request instead of falling back to name-only authentication. - Return 401 for a missing, malformed or refused credential. - Surface auth.secret-key in both shipped application.yml files with a change-in-production note, and let the Docker image set it through a new optional HG_PD_AUTH_SECRET_KEY env (never logged). - Wire the in-repo clients: wait-storage.sh now defaults its PD password to the shipped secret, and the Compose Hubble properties files carry a matching operations.pd.username/password pair. - Update the PD test credentials to the shipped secret and add REST tests asserting 401 for missing credential, wrong password, empty password and unknown service name. - Document the credential and the trusted-network requirement for port 8620 in the PD and Compose READMEs. Probes are unaffected: /v1/health, /actuator/* and /v1/prom/targets/* stay outside the interceptor. --- docker/README.md | 20 +++++- docker/conf/hubble/hstore-ha.properties | 4 ++ docker/conf/hubble/hstore.properties | 4 ++ hugegraph-pd/README.md | 14 ++++ .../hg-pd-dist/docker/docker-entrypoint.sh | 8 +++ .../src/assembly/static/conf/application.yml | 9 +++ .../rest/interceptor/RestAuthentication.java | 1 + .../service/interceptor/Authentication.java | 39 ++++++++--- .../src/main/resources/application.yml | 6 ++ .../org/apache/hugegraph/pd/BaseTest.java | 9 ++- .../hugegraph/pd/rest/BaseServerTest.java | 13 ++++ .../apache/hugegraph/pd/rest/RestApiTest.java | 67 ++++++++++++++++--- .../src/assembly/static/bin/wait-storage.sh | 4 +- 13 files changed, 177 insertions(+), 21 deletions(-) diff --git a/docker/README.md b/docker/README.md index 0bb74cf81f..e28afe0b05 100644 --- a/docker/README.md +++ b/docker/README.md @@ -66,6 +66,22 @@ For the verification commands below, set the password in your current shell: ADMIN_PASSWORD='the-same-password-used-in-.env' ``` +The PD REST API (port 8620, HStore topologies only) has its own credential: +requests other than health probes need HTTP Basic auth with an internal +service name (for example `hg`) and the PD secret as the password. PD ships +with a default secret in `conf/application.yml` (`auth.secret-key`), and the +Hubble files under `conf/hubble/` carry the matching `operations.pd.password`. +With the shipped default, list registered stores like this: + +```bash +curl -u hg:FXQXbJtbCLxODc6tGci732pkH1cyf8Qg http://localhost:8620/v1/stores +``` + +The default secret is public (it is in the source tree), so it only keeps +casual traffic out. On any shared network, change it: set +`HG_PD_AUTH_SECRET_KEY` on the PD services and put the same value in the +Hubble properties files, or do not publish port 8620 at all. + ### Standalone This is the recommended quickstart. @@ -312,7 +328,9 @@ docker compose -f docker-compose-hstore.yml up -d --wait ### Hubble configuration The three small files under `conf/hubble/` contain only topology-specific -discovery settings and container paths: +discovery settings, the PD REST credential (`operations.pd.username` and +`operations.pd.password`, which must match PD's `auth.secret-key`), and +container paths: - `conf/hubble/standalone.properties` uses direct Server mode. - `conf/hubble/hstore.properties` uses one PD and one Store REST target. diff --git a/docker/conf/hubble/hstore-ha.properties b/docker/conf/hubble/hstore-ha.properties index a50c52c96f..a9cb75d389 100644 --- a/docker/conf/hubble/hstore-ha.properties +++ b/docker/conf/hubble/hstore-ha.properties @@ -20,6 +20,10 @@ pd.enabled=true server.direct_url=http://server0:8080 pd.peers=pd0:8686,pd1:8686,pd2:8686 pd.server=pd0:8620 +# PD REST credential: the password must match PD's auth.secret-key (the +# value below is PD's shipped default). Change both together in production. +operations.pd.username=hubble +operations.pd.password=FXQXbJtbCLxODc6tGci732pkH1cyf8Qg operations.store.allowed_targets=[http://store0:8520,http://store1:8520,http://store2:8520] upload_file.location=/hubble/data/upload-files dashboard.address= diff --git a/docker/conf/hubble/hstore.properties b/docker/conf/hubble/hstore.properties index 5de43c212f..50541d9a0f 100644 --- a/docker/conf/hubble/hstore.properties +++ b/docker/conf/hubble/hstore.properties @@ -20,6 +20,10 @@ pd.enabled=true server.direct_url=http://server:8080 pd.peers=pd:8686 pd.server=pd:8620 +# PD REST credential: the password must match PD's auth.secret-key (the +# value below is PD's shipped default). Change both together in production. +operations.pd.username=hubble +operations.pd.password=FXQXbJtbCLxODc6tGci732pkH1cyf8Qg operations.store.allowed_targets=[http://store:8520] upload_file.location=/hubble/data/upload-files dashboard.address= diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 794dba9b98..ac9e91bb39 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -100,6 +100,7 @@ Key configuration file: `conf/application.yml` | `raft.address` | `127.0.0.1:8610` | Raft service address for this PD node | | `raft.peers-list` | `127.0.0.1:8610` | Comma-separated list of all PD nodes in the Raft cluster | | `pd.data-path` | `./pd_data` | Directory for storing PD metadata and Raft logs | +| `auth.secret-key` | (public default) | Password required by the REST API with an internal service name (`hg`, `store`, `hubble`, `vermeer`) via HTTP Basic auth. Change it in production and configure every REST client (e.g. Hubble's `operations.pd.password`) with the same value | #### Single-Node Example @@ -280,6 +281,19 @@ docker/docker-compose-3pd-3store-3server.yml - Ensure low latency (<5ms) between PD nodes for Raft consensus - Open required ports: `8620` (REST), `8686` (gRPC), `8610` (Raft) +### Security + +- Keep all three ports on a trusted network. The REST API on `8620` includes + management endpoints that mutate the cluster (peer changes, store removal, + data movement), and the gRPC and Raft ports carry no authentication. +- REST requests need HTTP Basic auth: one of the internal service names + (`hg`, `store`, `hubble`, `vermeer`) with the `auth.secret-key` value as + the password. Health probes (`/v1/health`, `/actuator/*`, + `/v1/prom/targets/*`) stay unauthenticated. +- The shipped `auth.secret-key` default is public. Change it in production + (config file, or `HG_PD_AUTH_SECRET_KEY` for the Docker image) and update + every REST client with the same value. + ### Monitoring PD exposes metrics via REST API at: diff --git a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh index 529936d06a..080c7e4260 100755 --- a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh +++ b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh @@ -57,8 +57,16 @@ require_env "HG_PD_INITIAL_STORE_LIST" : "${HG_PD_DATA_PATH:=/hugegraph-pd/pd_data}" : "${HG_PD_INITIAL_STORE_COUNT:=1}" +# Optional secret for REST Basic authentication (auth.secret-key). When unset, +# the value from conf/application.yml applies. Never logged. +AUTH_JSON="" +if [[ -n "${HG_PD_AUTH_SECRET_KEY:-}" ]]; then + AUTH_JSON="\"auth\": { \"secret-key\": \"$(json_escape "${HG_PD_AUTH_SECRET_KEY}")\" }," +fi + SPRING_APPLICATION_JSON="$(cat < - * WARNING: This class currently implements only basic internal authentication - * validation for internal modules (hg, store, hubble, vermeer). The authentication mechanism - * is designed for internal service-to-service communication only. + * WARNING: This class validates a Basic credential for internal modules + * (hg, store, hubble, vermeer): the service name must be one of the four and the + * password must match the shared secret configured via `auth.secret-key`. The + * mechanism is designed for internal service-to-service communication only. *

* *

Important SEC Considerations:

@@ -61,6 +65,9 @@ public class Authentication { private static final Set innerModules = Set.of("hg", "store", "hubble", "vermeer"); + @Autowired + private PDConfig pdConfig; + protected T authenticate(String authority, String token, Function tokenCall, Supplier call) { try { @@ -77,19 +84,33 @@ protected T authenticate(String authority, String token, Function } String name = info.substring(0, delim); - // TODO: password validation is skipped — only service name is checked against - // innerModules. Full credential validation should be added as part of the auth refactor. - //String pwd = info.substring(delim + 1); - if (innerModules.contains(name)) { - return call.get(); - } else { + String pwd = info.substring(delim + 1); + if (!innerModules.contains(name)) { throw new AccessDeniedException("invalid service name"); } + if (!verifySecret(pwd)) { + throw new BadCredentialsException("invalid credential"); + } + return call.get(); } catch (Exception e) { throw new RuntimeException(e); } } + /** + * Compare the password of the Basic credential with the shared secret + * configured via `auth.secret-key`. A missing or empty secret refuses every + * request instead of falling back to name-only authentication. + */ + private boolean verifySecret(String pwd) { + String secret = this.pdConfig == null ? null : this.pdConfig.getSecretKey(); + if (StringUtils.isEmpty(secret)) { + return false; + } + return MessageDigest.isEqual(pwd.getBytes(StandardCharsets.UTF_8), + secret.getBytes(StandardCharsets.UTF_8)); + } + public static String getTokenKey(String name) { return "PD/TOKEN/" + name; } diff --git a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml index 5a03595f7b..d4e83e04e1 100644 --- a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml +++ b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml @@ -43,6 +43,12 @@ license: server: port: 8620 +auth: + # Shared secret checked against the password of the Basic credential on every + # authenticated REST request. Change it in production and keep it in sync + # with every client that calls the PD REST API. + secret-key: FXQXbJtbCLxODc6tGci732pkH1cyf8Qg + pd: # Periodically check whether the cluster is healthy at intervals, in seconds patrol-interval: 300 diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/BaseTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/BaseTest.java index 0836120c73..dc8ac1a36e 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/BaseTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/BaseTest.java @@ -17,6 +17,9 @@ package org.apache.hugegraph.pd; +import java.nio.charset.StandardCharsets; +import java.util.Base64; + import org.apache.hugegraph.pd.client.PDConfig; public class BaseTest { @@ -24,9 +27,11 @@ public class BaseTest { protected static String pdGrpcAddr = "127.0.0.1:8686"; protected static String pdRestAddr = "http://127.0.0.1:8620"; protected static String user = "store"; - protected static String pwd = ""; + // Matches the auth.secret-key default that the PD under test runs with + protected static String pwd = "FXQXbJtbCLxODc6tGci732pkH1cyf8Qg"; protected static String key = "Authorization"; - protected static String value = "Basic c3RvcmU6YWRtaW4="; + protected static String value = "Basic " + Base64.getEncoder().encodeToString( + (user + ":" + pwd).getBytes(StandardCharsets.UTF_8)); protected PDConfig getPdConfig() { return PDConfig.of(pdGrpcAddr).setAuthority(user, pwd); diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/BaseServerTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/BaseServerTest.java index 4aff85d1e9..8204eb5410 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/BaseServerTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/BaseServerTest.java @@ -18,15 +18,28 @@ package org.apache.hugegraph.pd.rest; import java.net.http.HttpClient; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import org.junit.After; import org.junit.BeforeClass; public class BaseServerTest { + // Matches the auth.secret-key default that the PD under test runs with + protected static final String SECRET = "FXQXbJtbCLxODc6tGci732pkH1cyf8Qg"; + protected static final String AUTH_HEADER = "Authorization"; + protected static final String VALID_AUTH = basicAuth("store", SECRET); + protected static HttpClient client; protected static String pdRestAddr; + protected static String basicAuth(String name, String pwd) { + String credential = name + ":" + pwd; + return "Basic " + Base64.getEncoder() + .encodeToString(credential.getBytes(StandardCharsets.UTF_8)); + } + @BeforeClass public static void init() { client = HttpClient.newHttpClient(); diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java index fb2b71d480..3c0b6e475b 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java @@ -35,7 +35,7 @@ public void testQueryIndexInfo() throws URISyntaxException, IOException, Interru String url = pdRestAddr + "/"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); @@ -54,7 +54,7 @@ public void testQueryClusterInfo() throws URISyntaxException, IOException, Inter String url = pdRestAddr + "/v1/cluster"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); @@ -68,7 +68,7 @@ public void testQueryClusterMembers() throws URISyntaxException, IOException, String url = pdRestAddr + "/v1/members"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); @@ -82,7 +82,7 @@ public void testQueryStoresInfo() throws URISyntaxException, IOException, Interr String url = pdRestAddr + "/v1/stores"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); @@ -96,7 +96,7 @@ public void testQueryGraphsInfo() throws IOException, InterruptedException, JSON String url = pdRestAddr + "/v1/graphs"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); @@ -110,7 +110,7 @@ public void testQueryPartitionsInfo() throws IOException, InterruptedException, String url = pdRestAddr + "/v1/highLevelPartitions"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); @@ -124,7 +124,7 @@ public void testQueryDebugPartitionsInfo() throws URISyntaxException, IOExceptio String url = pdRestAddr + "/v1/partitions"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); @@ -137,11 +137,62 @@ public void testQueryShards() throws URISyntaxException, IOException, Interrupte String url = pdRestAddr + "/v1/shards"; HttpRequest request = HttpRequest.newBuilder() .uri(new URI(url)) - .header("Authorization", "Basic c3RvcmU6MTIz") + .header(AUTH_HEADER, VALID_AUTH) .GET() .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); JSONObject obj = new JSONObject(response.body()); assert obj.getInt("status") == 0; } + + @Test + public void testMissingCredentialGets401() throws URISyntaxException, IOException, + InterruptedException { + String url = pdRestAddr + "/v1/members"; + HttpRequest request = HttpRequest.newBuilder() + .uri(new URI(url)) + .GET() + .build(); + HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); + assert response.statusCode() == 401; + } + + @Test + public void testWrongPasswordGets401() throws URISyntaxException, IOException, + InterruptedException { + String url = pdRestAddr + "/v1/members"; + HttpRequest request = HttpRequest.newBuilder() + .uri(new URI(url)) + .header(AUTH_HEADER, basicAuth("store", "wrong-password")) + .GET() + .build(); + HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); + assert response.statusCode() == 401; + } + + @Test + public void testEmptyPasswordGets401() throws URISyntaxException, IOException, + InterruptedException { + String url = pdRestAddr + "/v1/members"; + HttpRequest request = HttpRequest.newBuilder() + .uri(new URI(url)) + .header(AUTH_HEADER, basicAuth("hg", "")) + .GET() + .build(); + HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); + assert response.statusCode() == 401; + } + + @Test + public void testUnknownServiceNameGets401() throws URISyntaxException, IOException, + InterruptedException { + String url = pdRestAddr + "/v1/members"; + HttpRequest request = HttpRequest.newBuilder() + .uri(new URI(url)) + .header(AUTH_HEADER, basicAuth("nobody", SECRET)) + .GET() + .build(); + HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); + assert response.statusCode() == 401; + } } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh index 93c3a19dd2..f7a871bc44 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh @@ -39,7 +39,9 @@ log() { echo "[wait-storage] $1" } -PD_AUTH_ARGS="-u ${PD_AUTH_USER:-store}:${PD_AUTH_PASSWORD:-admin}" +# PD validates the password against its auth.secret-key; the default below +# matches PD's shipped default. Override both when the PD secret is changed. +PD_AUTH_ARGS="-u ${PD_AUTH_USER:-store}:${PD_AUTH_PASSWORD:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg}" function key_exists { local key=$1 From 1e3b616f731aaed7db52df6bdb6f2123e3533c4d Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Thu, 3 Sep 2026 18:22:09 +0530 Subject: [PATCH 02/28] fix(pd): fail closed when auth.secret-key is absent, wire every client Review follow-ups on the REST credential change. The blocker: PDConfig declared the key as `@Value("${auth.secret-key: 'FXQ...'}")`. Spring takes the text after the first colon as a literal default, so a PD whose conf/application.yml has no auth block resolved the secret to " 'FXQ...'", quotes and leading space included. That is neither empty nor anything a client sends, so the fail-closed branch never ran and PD rejected the secret shipped in both config files, wait-storage.sh and the Hubble properties. Since start-hugegraph-pd.sh passes -Dspring.config.location, which replaces the default locations rather than adding to them, an upgrade that keeps an existing config file hit this and Server startup aborted after the 300s wait-storage timeout. Drop the default so an absent key yields "", and log an error naming the parameter once so the refusal is diagnosable. Also from the review: - Pass the secret to every consumer from one variable. Both Compose files now set HG_PD_AUTH_SECRET_KEY on each PD service and PD_AUTH_PASSWORD on each Server, so rotating one .env value keeps the Server's wait-storage.sh probe working. Hubble still needs a manual edit of its mounted properties file, which the docs now say. - Narrow the actuator exposure from "*" to health,metrics,prometheus in both configs. /actuator/* is excluded from the interceptor, so anything exposed there is anonymous on the port this change is hardening. - Send WWW-Authenticate on the 401, per RFC 7235. Without it clients that authenticate reactively never retry with credentials. - Record in the GRpcServerConfig TODO that the secret check now lives in the shared base class, so enabling the gRPC interceptor also requires giving the Server, Store and CLI clients the secret. - Document the credential in the PD configuration and API references, and credential the balanceLeaders rebalancing procedure in the Store operations guide, where a 401 reads as a no-op during an incident. Verified against the packaged dist: with the auth block removed from conf/application.yml every authenticated request is refused and the error names auth.secret-key; with it present the matrix is unchanged, the 401 carries the challenge header, and /actuator/env, /beans and /configprops no longer serve data. --- docker/README.md | 23 +++++++++++++-- docker/docker-compose-3pd-3store-3server.yml | 5 ++++ docker/docker-compose-hstore.yml | 3 ++ hugegraph-pd/README.md | 10 ++++++- hugegraph-pd/docs/api-reference.md | 16 ++++++++++ hugegraph-pd/docs/configuration.md | 29 +++++++++++++++++-- .../apache/hugegraph/pd/config/PDConfig.java | 6 +++- .../src/assembly/static/conf/application.yml | 4 ++- .../rest/interceptor/RestAuthentication.java | 3 ++ .../service/interceptor/Authentication.java | 12 ++++++++ .../pd/util/grpc/GRpcServerConfig.java | 7 ++++- .../src/main/resources/application.yml | 4 ++- hugegraph-store/docs/operations-guide.md | 22 ++++++++++++-- 13 files changed, 131 insertions(+), 13 deletions(-) diff --git a/docker/README.md b/docker/README.md index e28afe0b05..6861091a39 100644 --- a/docker/README.md +++ b/docker/README.md @@ -78,9 +78,26 @@ curl -u hg:FXQXbJtbCLxODc6tGci732pkH1cyf8Qg http://localhost:8620/v1/stores ``` The default secret is public (it is in the source tree), so it only keeps -casual traffic out. On any shared network, change it: set -`HG_PD_AUTH_SECRET_KEY` on the PD services and put the same value in the -Hubble properties files, or do not publish port 8620 at all. +casual traffic out. On any shared network, change it, or do not publish port +8620 at all. Three consumers read this credential, and all three have to +agree or startup fails: + +- PD itself, through `HG_PD_AUTH_SECRET_KEY`. +- The Server, whose `bin/wait-storage.sh` polls `/v1/stores` before the + Server starts. Both Compose files pass `PD_AUTH_PASSWORD` to it from the + same variable, so setting `HG_PD_AUTH_SECRET_KEY` in `.env` covers it. If + the Server sends the wrong secret it retries until + `WAIT_STORAGE_TIMEOUT_S` (300s) expires and the container exits with + `ERROR: Timeout waiting for storage backend`. +- Hubble, through `operations.pd.password` in the file under `conf/hubble/`. + That file is mounted read-only and is not templated, so edit it by hand to + match. + +Set the secret once in `.env` before the first start: + +```bash +printf "HG_PD_AUTH_SECRET_KEY='%s'\n" "$(openssl rand -hex 24)" >> .env +``` ### Standalone diff --git a/docker/docker-compose-3pd-3store-3server.yml b/docker/docker-compose-3pd-3store-3server.yml index 6f599c6870..34f6fab9cc 100644 --- a/docker/docker-compose-3pd-3store-3server.yml +++ b/docker/docker-compose-3pd-3store-3server.yml @@ -70,6 +70,8 @@ x-server-environment: &server-environment HG_SERVER_REQUIRE_AUTH_TOKEN_SECRET: "true" HG_SERVER_AUTH_TOKEN_SECRET: ${HUGEGRAPH_AUTH_TOKEN_SECRET:-} PASSWORD: ${HUGEGRAPH_ADMIN_PASSWORD:-} + # bin/wait-storage.sh polls the PD REST API, so it needs the same secret + PD_AUTH_PASSWORD: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} x-server-common: &server-common image: hugegraph/server:${HUGEGRAPH_VERSION:-latest} @@ -107,6 +109,7 @@ services: HG_PD_INITIAL_STORE_LIST: store0:8500,store1:8500,store2:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data HG_PD_INITIAL_STORE_COUNT: 3 + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} ports: ["8620:8620", "8686:8686"] volumes: - hg-pd0-data:/hugegraph-pd/pd_data @@ -125,6 +128,7 @@ services: HG_PD_INITIAL_STORE_LIST: store0:8500,store1:8500,store2:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data HG_PD_INITIAL_STORE_COUNT: 3 + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} ports: ["8621:8620", "8687:8686"] volumes: - hg-pd1-data:/hugegraph-pd/pd_data @@ -143,6 +147,7 @@ services: HG_PD_INITIAL_STORE_LIST: store0:8500,store1:8500,store2:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data HG_PD_INITIAL_STORE_COUNT: 3 + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} ports: ["8622:8620", "8688:8686"] volumes: - hg-pd2-data:/hugegraph-pd/pd_data diff --git a/docker/docker-compose-hstore.yml b/docker/docker-compose-hstore.yml index d201430692..aaca6df421 100644 --- a/docker/docker-compose-hstore.yml +++ b/docker/docker-compose-hstore.yml @@ -39,6 +39,7 @@ services: HG_PD_RAFT_PEERS_LIST: pd:8610 HG_PD_INITIAL_STORE_LIST: store:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} ports: - "8620:8620" volumes: @@ -94,6 +95,8 @@ services: HG_SERVER_INIT_STORE_ENABLED: "false" HG_SERVER_AUTH_TOKEN_SECRET: ${HUGEGRAPH_AUTH_TOKEN_SECRET:-} PASSWORD: ${HUGEGRAPH_ADMIN_PASSWORD:-} + # bin/wait-storage.sh polls the PD REST API, so it needs the same secret + PD_AUTH_PASSWORD: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} ports: - "8080:8080" healthcheck: diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index ac9e91bb39..324dbea389 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -292,7 +292,15 @@ docker/docker-compose-3pd-3store-3server.yml `/v1/prom/targets/*`) stay unauthenticated. - The shipped `auth.secret-key` default is public. Change it in production (config file, or `HG_PD_AUTH_SECRET_KEY` for the Docker image) and update - every REST client with the same value. + every REST client with the same value: the Server's `bin/wait-storage.sh` + reads `PD_AUTH_PASSWORD` (and `PD_AUTH_USER`, default `store`), and Hubble + reads `operations.pd.password`. A client left on the old secret gets 401, + and for `wait-storage.sh` that means Server startup aborts after + `WAIT_STORAGE_TIMEOUT_S`. +- An existing `conf/application.yml` carried over from an earlier release has + no `auth` block. PD then starts with an empty secret and refuses every + authenticated REST request, logging an error that names `auth.secret-key`. + Add the key to the file before upgrading. ### Monitoring diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index aa8cce8473..7535bc8f62 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -760,6 +760,22 @@ for (Map.Entry entry : results.entrySet()) { PD exposes a REST API for management and monitoring (default port: 8620). +### Authentication + +Every endpoint below except the probes needs HTTP Basic auth: one of the +internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, and +the `auth.secret-key` value from PD's `conf/application.yml` as the password. +A missing or wrong credential gets HTTP 401. The `curl` examples that follow +omit `-u` for readability; add it to every call except `/v1/health`, +`/actuator/*` and `/v1/prom/targets/*`, which stay unauthenticated for probes. + +```bash +curl -u hg: http://localhost:8620/v1/stores +``` + +Endpoints under `/v1` mutate the cluster (peer list changes, store removal, +partition balancing), so keep port 8620 on a trusted network regardless. + ### Health Check ```bash diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index e3ae4f6f25..ddc546fdbb 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -79,6 +79,31 @@ server: - Metrics: `http://:8620/actuator/metrics` - Prometheus: `http://:8620/actuator/prometheus` +### REST Authentication Settings + +Every REST request except the probes below must carry HTTP Basic auth: one of +the internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, +and the shared secret as the password. A missing or wrong credential gets +HTTP 401. Unauthenticated paths: `/v1/health`, `/actuator/*` and +`/v1/prom/targets/*`. + +```yaml +auth: + secret-key: FXQXbJtbCLxODc6tGci732pkH1cyf8Qg +``` + +| Parameter | Type | Default | Description | +|-----------|------|---------|-------------| +| `auth.secret-key` | String | (public default in the shipped `conf/application.yml`) | Password checked against the Basic credential. The shipped value is public, so change it in production. If the key is absent PD starts with an empty secret and refuses every authenticated REST request, logging an error that names this parameter. | + +Every REST client needs the same value: the Server's `bin/wait-storage.sh` +reads it from `PD_AUTH_PASSWORD`, Hubble from `operations.pd.password`, and +the Docker image takes `HG_PD_AUTH_SECRET_KEY`. + +```bash +curl -u hg: http://:8620/v1/stores +``` + ### Raft Consensus Settings Controls Raft consensus for PD cluster coordination. @@ -253,13 +278,13 @@ management: endpoints: web: exposure: - include: "*" # Expose all actuator endpoints + include: "health,metrics,prometheus" # Allowlist; see note below ``` | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `management.metrics.export.prometheus.enabled` | Boolean | `true` | Enable Prometheus-compatible metrics at `/actuator/prometheus`. | -| `management.endpoints.web.exposure.include` | String | `"*"` | Actuator endpoints to expose. `"*"` = all, or specify comma-separated list (e.g., `"health,metrics"`). | +| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. `/actuator/*` is excluded from the REST authentication interceptor, so every endpoint listed here is reachable without a credential on port 8620. Prefer an allowlist over `"*"`. | ## Deployment Scenarios diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java index acfc2ec290..3222f3e776 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java @@ -69,7 +69,11 @@ public class PDConfig { @Autowired private ThreadPoolGrpc threadPoolGrpc; - @Value("${auth.secret-key: 'FXQXbJtbCLxODc6tGci732pkH1cyf8Qg'}") + // No default: Spring takes the text after the first ':' literally, so a + // quoted default would resolve to a value including the quotes and the + // leading space, and no client would ever match it. An absent key must + // yield "" so the REST interceptor can refuse every request and say why. + @Value("${auth.secret-key:}") @ToString.Exclude private String secretKey; diff --git a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml index 5e4d59ca95..152eaa6eb7 100644 --- a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml +++ b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml @@ -27,7 +27,9 @@ management: endpoints: web: exposure: - include: "*" + # Allowlist, not "*": /actuator/* is excluded from the REST auth + # interceptor, so anything exposed here is anonymous on this port. + include: "health,metrics,prometheus" logging: config: 'file:./conf/log4j2.xml' diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java index 80db94a9e6..d10d2e9da8 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java @@ -60,6 +60,9 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons return authenticate(authority, token, tokenCall, DEFAULT_HANDLE); } catch (Exception e) { response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + // RFC 7235 requires a challenge on a 401; without it clients that + // authenticate reactively never retry with credentials + response.setHeader("WWW-Authenticate", "Basic realm=\"hugegraph-pd\""); response.setContentType("application/json"); response.getWriter().println(new API().toJSON(e)); response.getWriter().flush(); diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java index d609bb15e0..9774e7032f 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java @@ -21,6 +21,7 @@ import java.security.MessageDigest; import java.util.Base64; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.function.Supplier; @@ -31,6 +32,8 @@ import org.springframework.security.authentication.BadCredentialsException; import org.springframework.stereotype.Component; +import lombok.extern.slf4j.Slf4j; + /** * Simple internal authentication component for PD service. *

@@ -61,10 +64,13 @@ * and regular security audits. *

*/ +@Slf4j @Component public class Authentication { private static final Set innerModules = Set.of("hg", "store", "hubble", "vermeer"); + private static final AtomicBoolean missingSecretLogged = new AtomicBoolean(); + @Autowired private PDConfig pdConfig; @@ -105,6 +111,12 @@ protected T authenticate(String authority, String token, Function private boolean verifySecret(String pwd) { String secret = this.pdConfig == null ? null : this.pdConfig.getSecretKey(); if (StringUtils.isEmpty(secret)) { + // Logged once: this path is reachable by unauthenticated callers + if (missingSecretLogged.compareAndSet(false, true)) { + log.error("auth.secret-key is not configured, so every authenticated REST " + + "request is refused. Add it to conf/application.yml (or set " + + "HG_PD_AUTH_SECRET_KEY) and give every REST client the same value."); + } return false; } return MessageDigest.isEqual(pwd.getBytes(StandardCharsets.UTF_8), diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/util/grpc/GRpcServerConfig.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/util/grpc/GRpcServerConfig.java index 2b1103739b..224d417cc4 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/util/grpc/GRpcServerConfig.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/util/grpc/GRpcServerConfig.java @@ -41,7 +41,12 @@ public void configure(ServerBuilder serverBuilder) { poolGrpc.getQueue())); serverBuilder.maxInboundMessageSize(MAX_INBOUND_MESSAGE_SIZE); // TODO: GrpcAuthentication is instantiated as a Spring bean but never registered - // here — add serverBuilder.intercept(grpcAuthentication) once auth is refactored. + // here - add serverBuilder.intercept(grpcAuthentication) once auth is refactored. + // It extends Authentication, which now also checks the Basic password against + // auth.secret-key. Registering it therefore requires giving that value to every + // gRPC client first: ServiceConstant.AUTHORITY (Server) and DefaultPdProvider + // .authority (Store) are "" and "default" today, and hg-pd-cli sends "". + // Otherwise no store can register once the interceptor is enabled. } } diff --git a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml index d4e83e04e1..b0325e68a3 100644 --- a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml +++ b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml @@ -27,7 +27,9 @@ management: endpoints: web: exposure: - include: "*" + # Allowlist, not "*": /actuator/* is excluded from the REST auth + # interceptor, so anything exposed here is anonymous on this port. + include: "health,metrics,prometheus" grpc: port: 8686 diff --git a/hugegraph-store/docs/operations-guide.md b/hugegraph-store/docs/operations-guide.md index f46b5559d7..9b942683f0 100644 --- a/hugegraph-store/docs/operations-guide.md +++ b/hugegraph-store/docs/operations-guide.md @@ -2,6 +2,21 @@ This guide covers monitoring, troubleshooting, backup & recovery, and operational procedures for HugeGraph Store in production. +> **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than +> `/v1/health`, `/actuator/*` and `/v1/prom/targets/*`, need HTTP Basic auth: +> one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and +> PD's `auth.secret-key` value as the password. A call without it gets HTTP +> 401, which for a mutating step such as `balanceLeaders` means the step did +> nothing. Export the secret before following a procedure that uses +> `${PD_SECRET}`: +> +> ```bash +> read -rs PD_SECRET && export PD_SECRET +> ``` +> +> Store endpoints on port 8520 are unaffected. Some PD examples in this guide +> still omit the credential; add `-u hg:"${PD_SECRET}"` when a call returns 401. + ## Table of Contents - [Monitoring and Metrics](#monitoring-and-metrics) @@ -604,19 +619,20 @@ curl http://192.168.1.10:8620/v1/partitionsAndStatus 2. **Verify Registration**: ```bash - curl http://192.168.1.10:8620/v1/stores + curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/stores # New Store should appear ``` 3. **Trigger Rebalancing** (optional): ```bash - curl -X POST http://192.168.1.10:8620/v1/balanceLeaders + curl -u hg:"${PD_SECRET}" -X POST http://192.168.1.10:8620/v1/balanceLeaders ``` 4. **Monitor Rebalancing**: ```bash # Watch partition distribution - watch -n 10 'curl http://192.168.1.10:8620/v1/partitionsAndStatus' + watch -n 10 'curl -u hg:"${PD_SECRET}" \ + http://192.168.1.10:8620/v1/partitionsAndStatus' ``` 5. **Verify**: Wait for even distribution (may take hours) From 5c339c160356ad7683822826389953da542d31a6 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 01:22:17 +0530 Subject: [PATCH 03/28] fix(pd): ship no REST secret and keep the credential out of argv Addresses the two remaining review points. A published default is not a secret. The REST credential shipped with a fixed value that lives in this repository, on a port the HStore Compose files publish to the host, so anyone who could read the source could authenticate as an internal service against endpoints that rewrite the raft peer list, remove stores and move data. Requiring a deployment-provided secret is the only version of this check that means anything. - Both application.yml files now ship auth.secret-key empty, with a comment saying why there is no default and how to generate one. PD already refuses every authenticated REST request while it is empty, naming the key in an error. - PD refuses to start when auth.secret-key is set to the value earlier revisions carried as a placeholder, so a deployment that copied it does not quietly keep a well-known credential. - The Docker image requires HG_PD_AUTH_SECRET_KEY, and both Compose files fail fast when it is unset rather than falling back to a shared value. The .env recipe generates one alongside the JWT secret. The Hubble properties files ship the password empty, with the manual step documented. - travis/start-pd.sh supplies a test-only secret through SPRING_APPLICATION_JSON, matching what the PD suites send, since the shipped configuration no longer authenticates anything. wait-storage.sh no longer interpolates the credential into the inner bash -c string, where a secret containing a space, a backtick or $(...) would have split the arguments or run, and no longer passes it in argv where anything able to read /proc could see it. The inner shell reads the value from the environment and hands it to curl on stdin as a config file. Its test now asserts the credential is absent from argv and present in that config. Verified against the packaged dist: startup is refused with the published placeholder, the shipped configuration starts but answers 401 to every credential, and a deployment-provided secret restores the matrix with the challenge header. PDRestSuiteTest 17/17, PDClientSuiteTest 45/45, test-wait-storage.sh 5/5, compose renders pass and refuse to render without the secret. --- docker/README.md | 29 +++++++++---------- docker/conf/hubble/hstore-ha.properties | 7 +++-- docker/conf/hubble/hstore.properties | 7 +++-- docker/docker-compose-3pd-3store-3server.yml | 8 ++--- docker/docker-compose-hstore.yml | 4 +-- docker/test-compose.sh | 2 ++ hugegraph-pd/README.md | 20 ++++++++----- hugegraph-pd/docs/configuration.md | 4 +-- .../apache/hugegraph/pd/config/PDConfig.java | 22 +++++++++++++- .../hg-pd-dist/docker/docker-entrypoint.sh | 12 ++++---- .../src/assembly/static/conf/application.yml | 19 ++++++++---- .../src/main/resources/application.yml | 8 +++-- .../org/apache/hugegraph/pd/BaseTest.java | 5 ++-- .../hugegraph/pd/rest/BaseServerTest.java | 5 ++-- .../src/assembly/static/bin/wait-storage.sh | 28 ++++++++++++++---- .../src/assembly/travis/start-pd.sh | 5 ++++ .../src/assembly/travis/test-wait-storage.sh | 19 ++++++++++-- 17 files changed, 140 insertions(+), 64 deletions(-) diff --git a/docker/README.md b/docker/README.md index 6861091a39..a47c2f7c4a 100644 --- a/docker/README.md +++ b/docker/README.md @@ -39,8 +39,9 @@ contains a single quote or newline. echo ".env already exists; edit it instead of overwriting it" >&2 exit 1 } - printf "HUGEGRAPH_ADMIN_PASSWORD='%s'\nHUGEGRAPH_AUTH_TOKEN_SECRET='%s'\n" \ - 'replace-with-your-password' "${jwt_secret}" > .env + pd_secret="$(openssl rand -hex 24)" + printf "HUGEGRAPH_ADMIN_PASSWORD='%s'\nHUGEGRAPH_AUTH_TOKEN_SECRET='%s'\nHG_PD_AUTH_SECRET_KEY='%s'\n" \ + 'replace-with-your-password' "${jwt_secret}" "${pd_secret}" > .env ) ``` @@ -69,18 +70,16 @@ ADMIN_PASSWORD='the-same-password-used-in-.env' The PD REST API (port 8620, HStore topologies only) has its own credential: requests other than health probes need HTTP Basic auth with an internal service name (for example `hg`) and the PD secret as the password. PD ships -with a default secret in `conf/application.yml` (`auth.secret-key`), and the -Hubble files under `conf/hubble/` carry the matching `operations.pd.password`. -With the shipped default, list registered stores like this: +no default secret, so `HG_PD_AUTH_SECRET_KEY` is required and the HStore +Compose files refuse to start without it. The `.env` command above generates +one. To list registered stores: ```bash -curl -u hg:FXQXbJtbCLxODc6tGci732pkH1cyf8Qg http://localhost:8620/v1/stores +curl -u "hg:${HG_PD_AUTH_SECRET_KEY}" http://localhost:8620/v1/stores ``` -The default secret is public (it is in the source tree), so it only keeps -casual traffic out. On any shared network, change it, or do not publish port -8620 at all. Three consumers read this credential, and all three have to -agree or startup fails: +Three consumers read this credential, and all three have to agree or startup +fails: - PD itself, through `HG_PD_AUTH_SECRET_KEY`. - The Server, whose `bin/wait-storage.sh` polls `/v1/stores` before the @@ -90,13 +89,13 @@ agree or startup fails: `WAIT_STORAGE_TIMEOUT_S` (300s) expires and the container exits with `ERROR: Timeout waiting for storage backend`. - Hubble, through `operations.pd.password` in the file under `conf/hubble/`. - That file is mounted read-only and is not templated, so edit it by hand to - match. - -Set the secret once in `.env` before the first start: + That file is mounted read-only and is not templated, so write the same value + into it by hand. Until you do, Hubble's PD-backed views get 401 from PD; + everything else in Hubble works. ```bash -printf "HG_PD_AUTH_SECRET_KEY='%s'\n" "$(openssl rand -hex 24)" >> .env +sed -i.bak "s#^operations.pd.password=.*#operations.pd.password=${HG_PD_AUTH_SECRET_KEY}#" \ + conf/hubble/hstore.properties ``` ### Standalone diff --git a/docker/conf/hubble/hstore-ha.properties b/docker/conf/hubble/hstore-ha.properties index a9cb75d389..2a818d6fb2 100644 --- a/docker/conf/hubble/hstore-ha.properties +++ b/docker/conf/hubble/hstore-ha.properties @@ -20,10 +20,11 @@ pd.enabled=true server.direct_url=http://server0:8080 pd.peers=pd0:8686,pd1:8686,pd2:8686 pd.server=pd0:8620 -# PD REST credential: the password must match PD's auth.secret-key (the -# value below is PD's shipped default). Change both together in production. +# PD REST credential. The password must equal PD's auth.secret-key, which has +# no default: set it to the same value as HG_PD_AUTH_SECRET_KEY in .env. While +# it is empty, Hubble's PD-backed views get HTTP 401 from PD. operations.pd.username=hubble -operations.pd.password=FXQXbJtbCLxODc6tGci732pkH1cyf8Qg +operations.pd.password= operations.store.allowed_targets=[http://store0:8520,http://store1:8520,http://store2:8520] upload_file.location=/hubble/data/upload-files dashboard.address= diff --git a/docker/conf/hubble/hstore.properties b/docker/conf/hubble/hstore.properties index 50541d9a0f..55796d6e29 100644 --- a/docker/conf/hubble/hstore.properties +++ b/docker/conf/hubble/hstore.properties @@ -20,10 +20,11 @@ pd.enabled=true server.direct_url=http://server:8080 pd.peers=pd:8686 pd.server=pd:8620 -# PD REST credential: the password must match PD's auth.secret-key (the -# value below is PD's shipped default). Change both together in production. +# PD REST credential. The password must equal PD's auth.secret-key, which has +# no default: set it to the same value as HG_PD_AUTH_SECRET_KEY in .env. While +# it is empty, Hubble's PD-backed views get HTTP 401 from PD. operations.pd.username=hubble -operations.pd.password=FXQXbJtbCLxODc6tGci732pkH1cyf8Qg +operations.pd.password= operations.store.allowed_targets=[http://store:8520] upload_file.location=/hubble/data/upload-files dashboard.address= diff --git a/docker/docker-compose-3pd-3store-3server.yml b/docker/docker-compose-3pd-3store-3server.yml index 34f6fab9cc..ba02f27eb1 100644 --- a/docker/docker-compose-3pd-3store-3server.yml +++ b/docker/docker-compose-3pd-3store-3server.yml @@ -71,7 +71,7 @@ x-server-environment: &server-environment HG_SERVER_AUTH_TOKEN_SECRET: ${HUGEGRAPH_AUTH_TOKEN_SECRET:-} PASSWORD: ${HUGEGRAPH_ADMIN_PASSWORD:-} # bin/wait-storage.sh polls the PD REST API, so it needs the same secret - PD_AUTH_PASSWORD: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} + PD_AUTH_PASSWORD: ${HG_PD_AUTH_SECRET_KEY:?set HG_PD_AUTH_SECRET_KEY in .env; see docker/README.md} x-server-common: &server-common image: hugegraph/server:${HUGEGRAPH_VERSION:-latest} @@ -109,7 +109,7 @@ services: HG_PD_INITIAL_STORE_LIST: store0:8500,store1:8500,store2:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data HG_PD_INITIAL_STORE_COUNT: 3 - HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:?set HG_PD_AUTH_SECRET_KEY in .env; see docker/README.md} ports: ["8620:8620", "8686:8686"] volumes: - hg-pd0-data:/hugegraph-pd/pd_data @@ -128,7 +128,7 @@ services: HG_PD_INITIAL_STORE_LIST: store0:8500,store1:8500,store2:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data HG_PD_INITIAL_STORE_COUNT: 3 - HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:?set HG_PD_AUTH_SECRET_KEY in .env; see docker/README.md} ports: ["8621:8620", "8687:8686"] volumes: - hg-pd1-data:/hugegraph-pd/pd_data @@ -147,7 +147,7 @@ services: HG_PD_INITIAL_STORE_LIST: store0:8500,store1:8500,store2:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data HG_PD_INITIAL_STORE_COUNT: 3 - HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:?set HG_PD_AUTH_SECRET_KEY in .env; see docker/README.md} ports: ["8622:8620", "8688:8686"] volumes: - hg-pd2-data:/hugegraph-pd/pd_data diff --git a/docker/docker-compose-hstore.yml b/docker/docker-compose-hstore.yml index aaca6df421..ba5a421c2c 100644 --- a/docker/docker-compose-hstore.yml +++ b/docker/docker-compose-hstore.yml @@ -39,7 +39,7 @@ services: HG_PD_RAFT_PEERS_LIST: pd:8610 HG_PD_INITIAL_STORE_LIST: store:8500 HG_PD_DATA_PATH: /hugegraph-pd/pd_data - HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:?set HG_PD_AUTH_SECRET_KEY in .env; see docker/README.md} ports: - "8620:8620" volumes: @@ -96,7 +96,7 @@ services: HG_SERVER_AUTH_TOKEN_SECRET: ${HUGEGRAPH_AUTH_TOKEN_SECRET:-} PASSWORD: ${HUGEGRAPH_ADMIN_PASSWORD:-} # bin/wait-storage.sh polls the PD REST API, so it needs the same secret - PD_AUTH_PASSWORD: ${HG_PD_AUTH_SECRET_KEY:-FXQXbJtbCLxODc6tGci732pkH1cyf8Qg} + PD_AUTH_PASSWORD: ${HG_PD_AUTH_SECRET_KEY:?set HG_PD_AUTH_SECRET_KEY in .env; see docker/README.md} ports: - "8080:8080" healthcheck: diff --git a/docker/test-compose.sh b/docker/test-compose.sh index ecd5ab5b2d..0e9ddc76bb 100644 --- a/docker/test-compose.sh +++ b/docker/test-compose.sh @@ -21,6 +21,7 @@ set -Eeuo pipefail DOCKER_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PASSWORD="ci-compose-password" SECRET="0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" +PD_SECRET="ci-compose-pd-secret" VERSION="ci-version" RENDER_HUBBLE_IMAGE="example.invalid/hugegraph/hubble:ci" DATASOURCE="jdbc:h2:file:/hubble/data/hubble;DB_CLOSE_ON_EXIT=FALSE" @@ -34,6 +35,7 @@ compose_auth() { HUBBLE_IMAGE="${RENDER_HUBBLE_IMAGE}" \ HUGEGRAPH_ADMIN_PASSWORD="${PASSWORD}" \ HUGEGRAPH_AUTH_TOKEN_SECRET="${SECRET}" \ + HG_PD_AUTH_SECRET_KEY="${PD_SECRET}" \ docker compose "$@" } diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 324dbea389..7aa3ff9168 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -100,7 +100,7 @@ Key configuration file: `conf/application.yml` | `raft.address` | `127.0.0.1:8610` | Raft service address for this PD node | | `raft.peers-list` | `127.0.0.1:8610` | Comma-separated list of all PD nodes in the Raft cluster | | `pd.data-path` | `./pd_data` | Directory for storing PD metadata and Raft logs | -| `auth.secret-key` | (public default) | Password required by the REST API with an internal service name (`hg`, `store`, `hubble`, `vermeer`) via HTTP Basic auth. Change it in production and configure every REST client (e.g. Hubble's `operations.pd.password`) with the same value | +| `auth.secret-key` | none (required) | Password required by the REST API with an internal service name (`hg`, `store`, `hubble`, `vermeer`) via HTTP Basic auth. No default is shipped; generate one per deployment and configure every REST client (e.g. Hubble's `operations.pd.password`) with the same value | #### Single-Node Example @@ -242,6 +242,7 @@ docker run -d \ -p 8620:8620 \ -p 8686:8686 \ -p 8610:8610 \ + -e HG_PD_AUTH_SECRET_KEY="$(openssl rand -hex 24)" \ -e HG_PD_GRPC_HOST= \ -e HG_PD_RAFT_ADDRESS=:8610 \ -e HG_PD_RAFT_PEERS_LIST=:8610 \ @@ -290,17 +291,20 @@ docker/docker-compose-3pd-3store-3server.yml (`hg`, `store`, `hubble`, `vermeer`) with the `auth.secret-key` value as the password. Health probes (`/v1/health`, `/actuator/*`, `/v1/prom/targets/*`) stay unauthenticated. -- The shipped `auth.secret-key` default is public. Change it in production - (config file, or `HG_PD_AUTH_SECRET_KEY` for the Docker image) and update - every REST client with the same value: the Server's `bin/wait-storage.sh` - reads `PD_AUTH_PASSWORD` (and `PD_AUTH_USER`, default `store`), and Hubble - reads `operations.pd.password`. A client left on the old secret gets 401, - and for `wait-storage.sh` that means Server startup aborts after +- `auth.secret-key` has no shipped default, because a secret in the source + tree is published to everyone. Generate one per deployment (`openssl rand + -hex 24`) and set it in the config file, or through + `HG_PD_AUTH_SECRET_KEY`, which the Docker image requires. Give every REST + client the same value: the Server's `bin/wait-storage.sh` reads + `PD_AUTH_PASSWORD` (and `PD_AUTH_USER`, default `store`), and Hubble reads + `operations.pd.password`. A client left on a stale secret gets 401, and for + `wait-storage.sh` that means Server startup aborts after `WAIT_STORAGE_TIMEOUT_S`. - An existing `conf/application.yml` carried over from an earlier release has no `auth` block. PD then starts with an empty secret and refuses every authenticated REST request, logging an error that names `auth.secret-key`. - Add the key to the file before upgrading. + Add the key before upgrading. PD refuses to start if the key is set to the + placeholder value that earlier revisions of this repository carried. ### Monitoring diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index ddc546fdbb..e3cbd1ed3a 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -89,12 +89,12 @@ HTTP 401. Unauthenticated paths: `/v1/health`, `/actuator/*` and ```yaml auth: - secret-key: FXQXbJtbCLxODc6tGci732pkH1cyf8Qg + secret-key: ``` | Parameter | Type | Default | Description | |-----------|------|---------|-------------| -| `auth.secret-key` | String | (public default in the shipped `conf/application.yml`) | Password checked against the Basic credential. The shipped value is public, so change it in production. If the key is absent PD starts with an empty secret and refuses every authenticated REST request, logging an error that names this parameter. | +| `auth.secret-key` | String | none (required) | Password checked against the Basic credential. There is no default: a secret shipped in the source tree would be published to everyone. While it is empty PD refuses every authenticated REST request and logs an error naming this parameter, and PD refuses to start at all if it is set to the value that earlier revisions carried as a placeholder. | Every REST client needs the same value: the Server's `bin/wait-storage.sh` reads it from `PD_AUTH_PASSWORD`, Hubble from `operations.pd.password`, and diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java index 3222f3e776..1eece31655 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java @@ -25,6 +25,7 @@ import org.apache.hugegraph.pd.ConfigService; import org.apache.hugegraph.pd.IdService; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Configuration; @@ -38,7 +39,15 @@ */ @Data @Component -public class PDConfig { +public class PDConfig implements InitializingBean { + + /** + * The secret that earlier revisions carried as the placeholder default for + * `auth.secret-key`. It is published in this repository, so a deployment + * still using it authenticates anyone who can read the source. Refuse to + * start rather than let a well-known string look like authentication. + */ + private static final String PUBLISHED_SECRET_KEY = "FXQXbJtbCLxODc6tGci732pkH1cyf8Qg"; // cluster ID @Value("${pd.cluster_id:1}") @@ -89,6 +98,17 @@ public class PDConfig { private ConfigService configService; private IdService idService; + @Override + public void afterPropertiesSet() { + if (PUBLISHED_SECRET_KEY.equals(this.secretKey)) { + throw new IllegalStateException( + "auth.secret-key is set to the value published in the HugeGraph source " + + "tree, which authenticates anyone who can read it. Set a " + + "deployment-specific secret in conf/application.yml, or through the " + + "HG_PD_AUTH_SECRET_KEY environment variable for the Docker image."); + } + } + public Map getInitialStoreMap() { if (initialStoreMap == null) { initialStoreMap = new HashMap<>(); diff --git a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh index 080c7e4260..495be026eb 100755 --- a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh +++ b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh @@ -51,18 +51,18 @@ require_env "HG_PD_GRPC_HOST" require_env "HG_PD_RAFT_ADDRESS" require_env "HG_PD_RAFT_PEERS_LIST" require_env "HG_PD_INITIAL_STORE_LIST" +# The REST API refuses every authenticated request without this, and the image +# ships no default because a published secret is not a secret. +require_env "HG_PD_AUTH_SECRET_KEY" : "${HG_PD_GRPC_PORT:=8686}" : "${HG_PD_REST_PORT:=8620}" : "${HG_PD_DATA_PATH:=/hugegraph-pd/pd_data}" : "${HG_PD_INITIAL_STORE_COUNT:=1}" -# Optional secret for REST Basic authentication (auth.secret-key). When unset, -# the value from conf/application.yml applies. Never logged. -AUTH_JSON="" -if [[ -n "${HG_PD_AUTH_SECRET_KEY:-}" ]]; then - AUTH_JSON="\"auth\": { \"secret-key\": \"$(json_escape "${HG_PD_AUTH_SECRET_KEY}")\" }," -fi +# Secret for REST Basic authentication (auth.secret-key). Required above and +# never logged. +AUTH_JSON="\"auth\": { \"secret-key\": \"$(json_escape "${HG_PD_AUTH_SECRET_KEY}")\" }," SPRING_APPLICATION_JSON="$(cat < /dev/null; then check_any_pd_stores() { for peer in \$(echo \"\$PD_REST_LIST\" | tr ',' ' '); do - if curl ${PD_AUTH_ARGS} -f -s \ + if printf 'user = \"%s:%s\"\n' \ + \"\$PD_AUTH_CURL_USER\" \"\$PD_AUTH_CURL_PASSWORD\" | \ + curl -K - -f -s \ --connect-timeout ${WAIT_STORAGE_PD_CONNECT_TIMEOUT_S} \ --max-time ${WAIT_STORAGE_PD_MAX_TIMEOUT_S} \ - http://\${peer}/v1/stores 2>/dev/null | \ + \"http://\${peer}/v1/stores\" 2>/dev/null | \ grep -qi '\"state\"[[:space:]]*:[[:space:]]*\"Up\"'; then echo \"\$peer\" return 0 diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh index 0c137489e1..7e3198bedb 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh @@ -31,6 +31,11 @@ fi PD_DIR=$HOME_DIR/hugegraph-pd/apache-hugegraph-pd-$VersionInBash +# conf/application.yml ships auth.secret-key empty on purpose, so PD would +# refuse every authenticated REST request. Supply a test-only secret; it must +# match the value the PD test suites send. +export SPRING_APPLICATION_JSON='{"auth":{"secret-key":"pd-ci-test-secret-not-for-production"}}' + pushd $PD_DIR . bin/start-hugegraph-pd.sh sleep 10 diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh index da6a008f05..8764806519 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh @@ -25,6 +25,7 @@ DIST_ROOT="${TMP_DIR}/dist" MOCK_BIN="${TMP_DIR}/mock-bin" CALL_LOG="${TMP_DIR}/curl-calls" ARGS_LOG="${TMP_DIR}/curl-args" +CONFIG_LOG="${TMP_DIR}/curl-config" COUNT_FILE="${TMP_DIR}/store-call-count" TIMEOUT_LOG="${TMP_DIR}/timeout-arg" CASE_OUTPUT="" @@ -54,8 +55,12 @@ assert_contract() { ! grep -q '/v1/health' "${CALL_LOG}" || \ fail "/v1/health must not gate readiness" [[ -s "${ARGS_LOG}" ]] || fail "curl was not called" - if grep -Fv -- '-u test-user:test-password' "${ARGS_LOG}" | grep -q .; then - fail "authentication arguments were not preserved" + if grep -Fq -- 'test-password' "${ARGS_LOG}"; then + fail "credential leaked into curl argv" + fi + [[ -s "${CONFIG_LOG}" ]] || fail "curl was not given a credential config" + if grep -Fv -- 'user = "test-user:test-password"' "${CONFIG_LOG}" | grep -q .; then + fail "authentication credential was not preserved" fi if grep -Fv -- '--connect-timeout 2' "${ARGS_LOG}" | grep -q .; then fail "per-peer connect timeout was not preserved" @@ -70,6 +75,7 @@ run_case() { local scenario="$1" peers="$2" abort_after="$3" : > "${CALL_LOG}" : > "${ARGS_LOG}" + : > "${CONFIG_LOG}" : > "${COUNT_FILE}" : > "${TIMEOUT_LOG}" : > "${DIST_ROOT}/conf/graphs/hugegraph.properties" @@ -80,6 +86,7 @@ run_case() { MOCK_ABORT_AFTER="${abort_after}" \ MOCK_CALL_LOG="${CALL_LOG}" \ MOCK_ARGS_LOG="${ARGS_LOG}" \ + MOCK_CONFIG_LOG="${CONFIG_LOG}" \ MOCK_COUNT_FILE="${COUNT_FILE}" \ MOCK_TIMEOUT_LOG="${TIMEOUT_LOG}" \ HG_SERVER_PD_REST_ENDPOINT="${peers}" \ @@ -132,6 +139,14 @@ url="${!#}" printf '%s\n' "$*" >> "${MOCK_ARGS_LOG}" printf '%s\n' "${url}" >> "${MOCK_CALL_LOG}" +# The credential must arrive as a config file on stdin, never in argv +for arg in "$@"; do + if [[ "${arg}" == "-K" ]]; then + cat >> "${MOCK_CONFIG_LOG}" + break + fi +done + if [[ "${url}" == */v1/health ]]; then printf '{}\n' exit 0 From 3b3913229d9e408af5e2dba84f2fe6d969adc2c1 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:09:14 +0530 Subject: [PATCH 04/28] fix(pd): unbreak the hstore smoke, decode credentials as UTF-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups, including the CI failure the previous commit caused. The blocker was mine: requiring HG_PD_AUTH_SECRET_KEY in the Compose files without giving it to every caller of docker compose. Only compose_auth passed it, so the auth-on hstore smoke, which goes through compose_active, could not interpolate the file and failed before it started the stack. compose_active now passes it too. The Basic credential was decoded with the platform default charset while the secret it is compared against comes from the UTF-8 YAML, so a non-ASCII secret authenticated or not depending on the host locale. PD targets JDK 11, where the default charset still follows the locale, and the start script sets no -Dfile.encoding, so a PD started with LANG unset rejected a secret that worked from the operator's terminal, reporting only "invalid credential". Decode as UTF-8, which is what RFC 7617 specifies. Confirmed both ways: the old decode rejects `sécrèt-2026` under -Dfile.encoding=US-ASCII and the new one accepts it, and they agree under UTF-8. json_escape in the PD entrypoint escaped backslash and quote and dropped LF, leaving CR and TAB to produce invalid SPRING_APPLICATION_JSON and a container that failed before startup. It now escapes every C0 control character as \uXXXX. travis/test-pd-docker-entrypoint.sh covers the override: eight secrets including CR, TAB, quote, backslash and non-ASCII round-trip through the generated JSON, the missing-secret case is refused, and the secret never reaches the log. Wired into the pd CI job. Both README recipes were wrong in the same way, generating or expecting a secret the operator never has in their shell: - docker/README.md now loads .env before the commands that use the value, so the curl example stops sending an empty password and the Hubble sed stops rewriting the empty value to itself. It refuses to write an empty secret and names hstore-ha.properties for the HA topology. - hugegraph-pd/README.md generates the secret into a variable first, then passes it to docker run, instead of minting one inside the run line that only the container ever sees. --- .github/workflows/pd-store-ci.yml | 4 + docker/README.md | 19 ++- docker/test-compose.sh | 1 + hugegraph-pd/README.md | 7 +- .../hg-pd-dist/docker/docker-entrypoint.sh | 27 ++++- .../service/interceptor/Authentication.java | 5 +- .../travis/test-pd-docker-entrypoint.sh | 114 ++++++++++++++++++ 7 files changed, 169 insertions(+), 8 deletions(-) create mode 100755 hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh diff --git a/.github/workflows/pd-store-ci.yml b/.github/workflows/pd-store-ci.yml index 1a6825e7e4..f0c2231a4f 100644 --- a/.github/workflows/pd-store-ci.yml +++ b/.github/workflows/pd-store-ci.yml @@ -134,6 +134,10 @@ jobs: done echo "can_run=true" >> "$GITHUB_OUTPUT" + - name: Run PD docker entrypoint secret override tests + run: | + $TRAVIS_DIR/test-pd-docker-entrypoint.sh + - name: Run start-hugegraph-pd.sh foreground mode tests if: steps.pd-preflight.outputs.can_run == 'true' run: | diff --git a/docker/README.md b/docker/README.md index a47c2f7c4a..b26d0b8060 100644 --- a/docker/README.md +++ b/docker/README.md @@ -61,12 +61,17 @@ behind an HTTPS reverse proxy and trusted network controls. first authenticated startup. Changing `.env` does not rotate an existing administrator password; use the HugeGraph user API for credential changes. -For the verification commands below, set the password in your current shell: +For the verification commands below, load `.env` into your current shell and +set the password: ```bash +set -a; . ./.env; set +a ADMIN_PASSWORD='the-same-password-used-in-.env' ``` +Compose reads `.env` on its own; the line above is so that the `curl` and +`sed` commands on this page can use `${HG_PD_AUTH_SECRET_KEY}` too. + The PD REST API (port 8620, HStore topologies only) has its own credential: requests other than health probes need HTTP Basic auth with an internal service name (for example `hg`) and the PD secret as the password. PD ships @@ -93,9 +98,17 @@ fails: into it by hand. Until you do, Hubble's PD-backed views get 401 from PD; everything else in Hubble works. +Write it in, after loading `.env` as above. Use `hstore.properties` for the +Minimal HStore topology and `hstore-ha.properties` for HA: + ```bash -sed -i.bak "s#^operations.pd.password=.*#operations.pd.password=${HG_PD_AUTH_SECRET_KEY}#" \ - conf/hubble/hstore.properties +if [ -n "${HG_PD_AUTH_SECRET_KEY:-}" ]; then + sed -i.bak \ + "s#^operations.pd.password=.*#operations.pd.password=${HG_PD_AUTH_SECRET_KEY}#" \ + conf/hubble/hstore.properties +else + echo 'HG_PD_AUTH_SECRET_KEY is empty; load .env first' >&2 +fi ``` ### Standalone diff --git a/docker/test-compose.sh b/docker/test-compose.sh index 0e9ddc76bb..aa74a88a30 100644 --- a/docker/test-compose.sh +++ b/docker/test-compose.sh @@ -273,6 +273,7 @@ compose_active() { HUBBLE_IMAGE="${HUBBLE_IMAGE:-hugegraph/hubble:latest}" \ HUGEGRAPH_ADMIN_PASSWORD="${PASSWORD}" \ HUGEGRAPH_AUTH_TOKEN_SECRET="${SECRET}" \ + HG_PD_AUTH_SECRET_KEY="${PD_SECRET}" \ COMPOSE_PROGRESS=plain \ docker compose -p "${ACTIVE_PROJECT}" "${ACTIVE_FILES[@]}" "$@" } diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 7aa3ff9168..fa5d8de86c 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -237,12 +237,17 @@ Build PD Docker image: # From project root docker build -f hugegraph-pd/Dockerfile -t hugegraph/pd:latest . +# Generate the REST secret once and keep it: every PD REST client needs this +# same value, and a new one silently breaks the clients already using the old +# one. Store it somewhere durable rather than only in this shell. +export HG_PD_AUTH_SECRET_KEY="$(openssl rand -hex 24)" + # Run container docker run -d \ -p 8620:8620 \ -p 8686:8686 \ -p 8610:8610 \ - -e HG_PD_AUTH_SECRET_KEY="$(openssl rand -hex 24)" \ + -e HG_PD_AUTH_SECRET_KEY="${HG_PD_AUTH_SECRET_KEY}" \ -e HG_PD_GRPC_HOST= \ -e HG_PD_RAFT_ADDRESS=:8610 \ -e HG_PD_RAFT_PEERS_LIST=:8610 \ diff --git a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh index 495be026eb..d38230b37d 100755 --- a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh +++ b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh @@ -26,10 +26,31 @@ require_env() { fi } +# Escape a value for use inside a JSON string: backslash and quote, then every +# remaining C0 control character as \uXXXX. Dropping only LF, as an earlier +# version did, left CR and TAB to produce invalid JSON and a container that +# failed before startup. json_escape() { - local s="$1" - s=${s//\\/\\\\}; s=${s//\"/\\\"}; s=${s//$'\n'/} - printf "%s" "$s" + local s="$1" out="" i c + s=${s//\\/\\\\} + s=${s//\"/\\\"} + for (( i = 0; i < ${#s}; i++ )); do + c=${s:i:1} + case "$c" in + $'\n') out+='\n' ;; + $'\r') out+='\r' ;; + $'\t') out+='\t' ;; + $'\b') out+='\b' ;; + $'\f') out+='\f' ;; + *) + if [[ "$c" < $'\x20' || "$c" == $'\x7f' ]]; then + printf -v c '\\u%04x' "'$c" + fi + out+="$c" + ;; + esac + done + printf "%s" "$out" } migrate_env() { diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java index 9774e7032f..4a8d73fa7e 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/interceptor/Authentication.java @@ -83,7 +83,10 @@ protected T authenticate(String authority, String token, Function } byte[] bytes = authority.getBytes(StandardCharsets.UTF_8); byte[] decode = Base64.getDecoder().decode(bytes); - String info = new String(decode); + // RFC 7617: Basic credentials are UTF-8. Decoding with the platform + // default would compare against a UTF-8 secret only when the host + // locale happens to agree. + String info = new String(decode, StandardCharsets.UTF_8); int delim = info.indexOf(':'); if (delim == -1) { throw new BadCredentialsException(invalidBasicInfo); diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh new file mode 100755 index 0000000000..ea8874dc37 --- /dev/null +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Checks that the PD Docker entrypoint turns HG_PD_AUTH_SECRET_KEY into valid +# SPRING_APPLICATION_JSON, whatever the secret contains, and that the value +# Spring would read back is the secret that went in. + +set -euo pipefail + +ENTRYPOINT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../../.." && pwd)/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh" +TMP_DIR=$(mktemp -d "${TMPDIR:-/tmp}/pd-entrypoint-test.XXXXXX") +trap 'rm -rf "${TMP_DIR}"' EXIT + +PASS=0 +FAIL=0 + +[[ -f "${ENTRYPOINT}" ]] || { echo "entrypoint not found at ${ENTRYPOINT}" >&2; exit 1; } +command -v python3 >/dev/null || { echo "python3 required" >&2; exit 1; } + +mkdir -p "${TMP_DIR}/bin" +cp "${ENTRYPOINT}" "${TMP_DIR}/docker-entrypoint.sh" +# Stand in for the launcher: record the generated config instead of starting PD +cat > "${TMP_DIR}/bin/start-hugegraph-pd.sh" <<'STUB' +#!/usr/bin/env bash +printf '%s' "${SPRING_APPLICATION_JSON}" > ./spring.json +STUB +chmod +x "${TMP_DIR}/bin/start-hugegraph-pd.sh" "${TMP_DIR}/docker-entrypoint.sh" + +run_case() { + local name="$1" secret="$2" + local out + if ! out=$(cd "${TMP_DIR}" && env \ + HG_PD_GRPC_HOST=pd0 \ + HG_PD_RAFT_ADDRESS=pd0:8610 \ + HG_PD_RAFT_PEERS_LIST=pd0:8610 \ + HG_PD_INITIAL_STORE_LIST=store0:8500 \ + HG_PD_AUTH_SECRET_KEY="${secret}" \ + ./docker-entrypoint.sh 2>&1); then + echo " FAIL ${name}: entrypoint exited non-zero" + printf '%s\n' "${out}" | tail -3 + FAIL=$((FAIL + 1)) + return + fi + + if ! SECRET="${secret}" python3 - "${TMP_DIR}/spring.json" <<'PY' +import json, os, sys +with open(sys.argv[1], encoding="utf-8") as fh: + doc = json.load(fh) +got = doc["auth"]["secret-key"] +want = os.environ["SECRET"] +if got != want: + print(" round-trip mismatch: %r != %r" % (got, want)) + sys.exit(1) +PY + then + echo " FAIL ${name}: invalid JSON or secret did not round-trip" + FAIL=$((FAIL + 1)) + return + fi + echo " PASS ${name}" + PASS=$((PASS + 1)) +} + +echo "PD docker-entrypoint secret override" +run_case "plain secret" 'aVerySecretValue123' +run_case "carriage return" "$(printf 'a\rb')" +run_case "tab" "$(printf 'a\tb')" +run_case "double quote" 'a"b' +run_case "backslash" 'a\b' +run_case "backslash and quote" 'a\"b' +run_case "non-ascii" 'sécrèt-2026' +run_case "spaces" 'two words' + +# The secret is required, and must never be echoed to the log +if (cd "${TMP_DIR}" && env \ + HG_PD_GRPC_HOST=pd0 HG_PD_RAFT_ADDRESS=pd0:8610 \ + HG_PD_RAFT_PEERS_LIST=pd0:8610 HG_PD_INITIAL_STORE_LIST=store0:8500 \ + ./docker-entrypoint.sh >/dev/null 2>&1); then + echo " FAIL missing secret: entrypoint started without HG_PD_AUTH_SECRET_KEY" + FAIL=$((FAIL + 1)) +else + echo " PASS missing secret is refused" + PASS=$((PASS + 1)) +fi + +log_out=$(cd "${TMP_DIR}" && env \ + HG_PD_GRPC_HOST=pd0 HG_PD_RAFT_ADDRESS=pd0:8610 \ + HG_PD_RAFT_PEERS_LIST=pd0:8610 HG_PD_INITIAL_STORE_LIST=store0:8500 \ + HG_PD_AUTH_SECRET_KEY='do-not-log-this-value' \ + ./docker-entrypoint.sh 2>&1) +if printf '%s' "${log_out}" | grep -q 'do-not-log-this-value'; then + echo " FAIL secret was written to the log" + FAIL=$((FAIL + 1)) +else + echo " PASS secret is not logged" + PASS=$((PASS + 1)) +fi + +echo "${PASS} passed, ${FAIL} failed" +[[ "${FAIL}" -eq 0 ]] From b352e8c105cf722a626a2b1e7b0861c4ca628eff Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:18:43 +0530 Subject: [PATCH 05/28] chore(ci): retrigger build-commons after a port-collision flake ServerClientTest.testServiceProxy failed with "bind(..) failed: Address already in use" on port 8090 in hugegraph-commons/hugegraph-rpc, a module this branch does not touch. Empty commit, no source change. From ace8913a079810503810584cdc552bb710fe601a Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:37:13 +0530 Subject: [PATCH 06/28] test(pd): cover the REST credential check in process The suites that exercise this talk to a PD in another JVM, so none of the new branches were attributed to the build and codecov/patch read 0%. AuthenticationTest runs the check directly: each inner module accepted with the secret, wrong and empty passwords refused, unknown names refused, a malformed credential refused, an unset secret refusing everyone rather than falling back to a name check, a non-ASCII secret that would fail if the credential were decoded with the platform charset, and PDConfig refusing to start on the published placeholder. Added to PDCoreSuiteTest, which needs no running PD. --- .../hugegraph/pd/core/PDCoreSuiteTest.java | 2 + .../interceptor/AuthenticationTest.java | 127 ++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index 95b044c76b..a65b53e886 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -17,6 +17,7 @@ package org.apache.hugegraph.pd.core; +import org.apache.hugegraph.pd.service.interceptor.AuthenticationTest; import org.apache.hugegraph.pd.core.meta.MetadataKeyHelperTest; import org.apache.hugegraph.pd.core.store.HgKVStoreImplTest; import org.apache.hugegraph.pd.raft.IpAuthHandlerTest; @@ -32,6 +33,7 @@ MetadataKeyHelperTest.class, HgKVStoreImplTest.class, PDConfigTest.class, + AuthenticationTest.class, ConfigServiceTest.class, IdServiceTest.class, KvServiceTest.class, diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java new file mode 100644 index 0000000000..5bb801f6d1 --- /dev/null +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.pd.service.interceptor; + +import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.util.Base64; + +import org.apache.hugegraph.pd.config.PDConfig; +import org.junit.Assert; +import org.junit.Test; + +/** + * In-process cover for the REST credential check. The suites that exercise it + * over HTTP talk to a PD in another JVM, so nothing here is covered by them. + */ +public class AuthenticationTest { + + private static final String SECRET = "unit-test-secret"; + + private static Authentication authWithSecret(String secret) throws Exception { + Authentication auth = new Authentication(); + PDConfig config = new PDConfig(); + config.setSecretKey(secret); + Field field = Authentication.class.getDeclaredField("pdConfig"); + field.setAccessible(true); + field.set(auth, config); + return auth; + } + + private static String credential(String name, String pwd) { + return Base64.getEncoder().encodeToString( + (name + ":" + pwd).getBytes(StandardCharsets.UTF_8)); + } + + private static boolean accepts(Authentication auth, String authority) { + try { + return auth.authenticate(authority, null, t -> Boolean.TRUE, () -> Boolean.TRUE); + } catch (RuntimeException e) { + return false; + } + } + + @Test + public void testEveryInnerModuleIsAcceptedWithTheSecret() throws Exception { + Authentication auth = authWithSecret(SECRET); + for (String name : new String[]{"hg", "store", "hubble", "vermeer"}) { + Assert.assertTrue(name + " should be accepted with the right secret", + accepts(auth, credential(name, SECRET))); + } + } + + @Test + public void testPasswordIsActuallyChecked() throws Exception { + Authentication auth = authWithSecret(SECRET); + Assert.assertFalse("wrong password must be refused", + accepts(auth, credential("hg", "wrong-password"))); + Assert.assertFalse("empty password must be refused", + accepts(auth, credential("hg", ""))); + Assert.assertFalse("secret as the name must not help", + accepts(auth, credential(SECRET, SECRET))); + } + + @Test + public void testUnknownServiceNameIsRefused() throws Exception { + Authentication auth = authWithSecret(SECRET); + Assert.assertFalse(accepts(auth, credential("nobody", SECRET))); + Assert.assertFalse(accepts(auth, credential("admin", SECRET))); + } + + @Test + public void testMissingOrMalformedCredentialIsRefused() throws Exception { + Authentication auth = authWithSecret(SECRET); + Assert.assertFalse(accepts(auth, null)); + Assert.assertFalse(accepts(auth, "")); + // no colon + Assert.assertFalse(accepts(auth, Base64.getEncoder().encodeToString( + "hg".getBytes(StandardCharsets.UTF_8)))); + } + + @Test + public void testUnconfiguredSecretRefusesEveryone() throws Exception { + for (String secret : new String[]{null, ""}) { + Authentication auth = authWithSecret(secret); + Assert.assertFalse("an unset secret must not fall back to a name check", + accepts(auth, credential("hg", ""))); + Assert.assertFalse(accepts(auth, credential("hg", SECRET))); + } + } + + @Test + public void testNonAsciiSecretDoesNotDependOnTheDefaultCharset() throws Exception { + String secret = "sécrèt-2026"; + Authentication auth = authWithSecret(secret); + Assert.assertTrue(accepts(auth, credential("hg", secret))); + Assert.assertFalse(accepts(auth, credential("hg", "secret-2026"))); + } + + @Test + public void testPublishedSecretRefusesStartup() { + PDConfig config = new PDConfig(); + config.setSecretKey("FXQXbJtbCLxODc6tGci732pkH1cyf8Qg"); + Assert.assertThrows(IllegalStateException.class, config::afterPropertiesSet); + } + + @Test + public void testOwnSecretStarts() { + PDConfig config = new PDConfig(); + config.setSecretKey(SECRET); + config.afterPropertiesSet(); + } +} From 94b15664890b419f50b00514caf5ca476cab8857 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sat, 5 Sep 2026 11:09:53 +0530 Subject: [PATCH 07/28] fix(pd): move the auth test where it compiles, close review gaps The CI cascade at ace8913 was one mistake: AuthenticationTest lived in hg-pd-test, which declares hg-pd-service but cannot compile against it. The Spring Boot repackage replaces that module's main artifact with a fat jar whose classes sit under BOOT-INF/classes, invisible to javac, so every job that runs `mvn install` (commons, store, hstore, the Docker matrix, CodeQL, macOS) failed before any test ran. It only passed locally because `-am` builds from the reactor's target/classes. The test now lives in hg-pd-service's own test source set, with junit added there at test scope. Verified against the same `mvn install -Dmaven.test.skip=true` path CI uses. Also from the review: - The interceptor excluded /actuator/*, which Spring does not match against /actuator/metrics/{name} or /actuator/health/{group}, so those exposed probe paths returned 401. Now /actuator/**; what is reachable there is bounded by the exposure allowlist. RestApiTest asserts the nested metrics path answers without a credential and that /actuator/env stays closed. - application.yml.template still exposed "*" and carried no auth block, so an operator starting from the packaged template got neither hardening. travis/test-pd-shipped-config.sh now checks every shipped variant for an allowlisted exposure, an empty auth.secret-key and no published secret, wired into the pd job. - wait-storage.sh escaped only backslash and quote for the curl -K config, which is line-delimited: a secret containing a line break ended the option early and sent a truncated credential. LF, CR, TAB and VT are now escaped the way curl's quoted strings expect, and test-wait-storage.sh covers a secret containing CR, LF, backslash and a quote. - The docker/README Hubble step used the secret as a sed replacement, where & # and backslash are special. docker/set-hubble-pd-password.sh writes the value without sed, doubles backslashes for .properties, refuses an empty value or a line break, and keeps the file mode that the read-only mount depends on. test-compose.sh render exercises it with a non-hex secret. - Copy-paste deployment snippets in hugegraph-pd/README.md and hugegraph-store/docs/deployment-guide.md omitted the now-required HG_PD_AUTH_SECRET_KEY, so following them failed on the missing variable. --- .github/workflows/pd-store-ci.yml | 4 ++ docker/README.md | 22 ++++---- docker/set-hubble-pd-password.sh | 43 +++++++++++++++ docker/test-compose.sh | 24 +++++++++ hugegraph-pd/README.md | 5 +- .../static/conf/application.yml.template | 12 ++++- hugegraph-pd/hg-pd-service/pom.xml | 6 +++ .../interceptor/AuthenticationConfigurer.java | 6 ++- .../interceptor/AuthenticationTest.java | 0 .../hugegraph/pd/core/PDCoreSuiteTest.java | 2 - .../apache/hugegraph/pd/rest/RestApiTest.java | 25 +++++++++ .../src/assembly/static/bin/wait-storage.sh | 12 ++++- .../assembly/travis/test-pd-shipped-config.sh | 53 +++++++++++++++++++ .../src/assembly/travis/test-wait-storage.sh | 14 +++-- hugegraph-store/docs/deployment-guide.md | 5 ++ 15 files changed, 212 insertions(+), 21 deletions(-) create mode 100755 docker/set-hubble-pd-password.sh rename hugegraph-pd/{hg-pd-test/src/main => hg-pd-service/src/test}/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java (100%) create mode 100755 hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh diff --git a/.github/workflows/pd-store-ci.yml b/.github/workflows/pd-store-ci.yml index f0c2231a4f..fce7168e05 100644 --- a/.github/workflows/pd-store-ci.yml +++ b/.github/workflows/pd-store-ci.yml @@ -138,6 +138,10 @@ jobs: run: | $TRAVIS_DIR/test-pd-docker-entrypoint.sh + - name: Check every shipped PD config carries the REST hardening + run: | + $TRAVIS_DIR/test-pd-shipped-config.sh + - name: Run start-hugegraph-pd.sh foreground mode tests if: steps.pd-preflight.outputs.can_run == 'true' run: | diff --git a/docker/README.md b/docker/README.md index b26d0b8060..1d8f87f532 100644 --- a/docker/README.md +++ b/docker/README.md @@ -69,8 +69,8 @@ set -a; . ./.env; set +a ADMIN_PASSWORD='the-same-password-used-in-.env' ``` -Compose reads `.env` on its own; the line above is so that the `curl` and -`sed` commands on this page can use `${HG_PD_AUTH_SECRET_KEY}` too. +Compose reads `.env` on its own; the line above is so that the `curl` command and +the Hubble helper on this page can use `${HG_PD_AUTH_SECRET_KEY}` too. The PD REST API (port 8620, HStore topologies only) has its own credential: requests other than health probes need HTTP Basic auth with an internal @@ -98,19 +98,19 @@ fails: into it by hand. Until you do, Hubble's PD-backed views get 401 from PD; everything else in Hubble works. -Write it in, after loading `.env` as above. Use `hstore.properties` for the -Minimal HStore topology and `hstore-ha.properties` for HA: +Write it in with the helper, after loading `.env` as above. Use +`hstore.properties` for the Minimal HStore topology and `hstore-ha.properties` +for HA: ```bash -if [ -n "${HG_PD_AUTH_SECRET_KEY:-}" ]; then - sed -i.bak \ - "s#^operations.pd.password=.*#operations.pd.password=${HG_PD_AUTH_SECRET_KEY}#" \ - conf/hubble/hstore.properties -else - echo 'HG_PD_AUTH_SECRET_KEY is empty; load .env first' >&2 -fi +./set-hubble-pd-password.sh conf/hubble/hstore.properties ``` +The helper refuses an empty value, writes the secret without passing it +through a `sed` replacement (where `&`, `#` and backslashes are special), and +doubles backslashes for the `.properties` format. The generated hex secret +needs none of that, but a hand-chosen one might. + ### Standalone This is the recommended quickstart. diff --git a/docker/set-hubble-pd-password.sh b/docker/set-hubble-pd-password.sh new file mode 100755 index 0000000000..aa44d4b1be --- /dev/null +++ b/docker/set-hubble-pd-password.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Write PD's REST secret into a Hubble properties file as operations.pd.password. +# +# usage: set-hubble-pd-password.sh [secret] +# +# The secret defaults to $HG_PD_AUTH_SECRET_KEY. The value never goes through a +# sed replacement, where & # and backslash are special, and backslashes are +# doubled for the .properties format. The file keeps its mode, which matters +# because Compose mounts it read-only into the Hubble container. +set -euo pipefail + +file=${1:?usage: $0 [secret]} +secret=${2:-${HG_PD_AUTH_SECRET_KEY:-}} + +[[ -f "$file" ]] || { echo "no such file: $file" >&2; exit 1; } +[[ -n "$secret" ]] || { echo "secret is empty; load .env first (set -a; . ./.env; set +a)" >&2; exit 1; } +case "$secret" in + *$'\n'*|*$'\r'*) echo "secret contains a line break, which a .properties value cannot hold" >&2; exit 1 ;; +esac + +escaped=${secret//\\/\\\\} +tmp=$(mktemp "${file}.XXXXXX") +trap 'rm -f "$tmp"' EXIT +grep -v '^operations\.pd\.password=' "$file" > "$tmp" || true +printf 'operations.pd.password=%s\n' "$escaped" >> "$tmp" +# cat, not mv: keep the file's inode and mode +cat "$tmp" > "$file" diff --git a/docker/test-compose.sh b/docker/test-compose.sh index aa74a88a30..d363e3cb9d 100644 --- a/docker/test-compose.sh +++ b/docker/test-compose.sh @@ -245,6 +245,29 @@ cleanup() { [[ -z "${RENDER_DIR}" ]] || rm -rf "${RENDER_DIR}" } +# set-hubble-pd-password.sh must survive the characters a sed replacement +# would mangle, keep the rest of the file, and keep the file's mode. +hubble_password_helper_check() { + local tmp + tmp=$(mktemp "${TMPDIR:-/tmp}/hubble-props.XXXXXX") + cp "${DOCKER_DIR}/conf/hubble/hstore.properties" "${tmp}" + chmod 644 "${tmp}" + "${DOCKER_DIR}/set-hubble-pd-password.sh" "${tmp}" 'a&b#c\d' >/dev/null + local line mode + line=$(grep '^operations\.pd\.password=' "${tmp}") + mode=$(stat -c '%a' "${tmp}" 2>/dev/null || stat -f '%Lp' "${tmp}") + rm -f "${tmp}" + [[ "${line}" == 'operations.pd.password=a&b#c\\d' ]] || { + echo "set-hubble-pd-password.sh mangled the secret: ${line}" >&2; exit 1; } + [[ "${mode}" == "644" ]] || { + echo "set-hubble-pd-password.sh changed the file mode to ${mode}" >&2; exit 1; } + if ! "${DOCKER_DIR}/set-hubble-pd-password.sh" "${DOCKER_DIR}/conf/hubble/hstore.properties" '' 2>/dev/null; then + : + else + echo "set-hubble-pd-password.sh accepted an empty secret" >&2; exit 1 + fi +} + run_render() { RENDER_DIR="$(mktemp -d)" trap cleanup EXIT INT TERM @@ -265,6 +288,7 @@ run_render() { assert_ha "${RENDER_DIR}/ha.json" assert_dev_override "${RENDER_DIR}/dev.json" \ "${RENDER_DIR}/override.json" + hubble_password_helper_check echo "Compose render contracts passed" } diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index fa5d8de86c..a11959aa37 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -158,11 +158,12 @@ For detailed configuration options and production tuning, see [Configuration Gui #### Docker Bridge Network Example -When running PD in Docker with bridge networking (e.g., `docker/docker-compose-3pd-3store-3server.yml`), configuration is injected via environment variables instead of editing `application.yml` directly. Container hostnames are used instead of IP addresses: +When running PD in Docker with bridge networking (e.g., `docker/docker-compose-3pd-3store-3server.yml`), configuration is injected via environment variables instead of editing `application.yml` directly. Container hostnames are used instead of IP addresses. `HG_PD_AUTH_SECRET_KEY` is required by the image and must be the same value on every PD node and every PD REST client; generate it once (`openssl rand -hex 24`) and keep it: **pd0** container: ```bash HG_PD_GRPC_HOST=pd0 +HG_PD_AUTH_SECRET_KEY= HG_PD_RAFT_ADDRESS=pd0:8610 HG_PD_RAFT_PEERS_LIST=pd0:8610,pd1:8610,pd2:8610 HG_PD_INITIAL_STORE_LIST=store0:8500,store1:8500,store2:8500 @@ -171,6 +172,7 @@ HG_PD_INITIAL_STORE_LIST=store0:8500,store1:8500,store2:8500 **pd1** container: ```bash HG_PD_GRPC_HOST=pd1 +HG_PD_AUTH_SECRET_KEY= HG_PD_RAFT_ADDRESS=pd1:8610 HG_PD_RAFT_PEERS_LIST=pd0:8610,pd1:8610,pd2:8610 HG_PD_INITIAL_STORE_LIST=store0:8500,store1:8500,store2:8500 @@ -179,6 +181,7 @@ HG_PD_INITIAL_STORE_LIST=store0:8500,store1:8500,store2:8500 **pd2** container: ```bash HG_PD_GRPC_HOST=pd2 +HG_PD_AUTH_SECRET_KEY= HG_PD_RAFT_ADDRESS=pd2:8610 HG_PD_RAFT_PEERS_LIST=pd0:8610,pd1:8610,pd2:8610 HG_PD_INITIAL_STORE_LIST=store0:8500,store1:8500,store2:8500 diff --git a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template index 8b8f0d63c5..96211154a3 100644 --- a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template +++ b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template @@ -29,7 +29,9 @@ management: endpoints: web: exposure: - include: "*" + # Allowlist, not "*": /actuator/** is excluded from the REST auth + # interceptor, so anything exposed here is anonymous on this port. + include: "health,metrics,prometheus" grpc: port: $GRPC_PORT$ @@ -42,6 +44,14 @@ grpc: server: port : $SERVER_PORT$ +auth: + # Shared secret checked against the password of the Basic credential on every + # authenticated REST request. Required and deliberately empty: a secret in the + # source tree is published to everyone. Generate one per deployment and give + # every REST client the same value. While empty, PD refuses every + # authenticated REST request. + secret-key: + pd: # Cluster ID: to distinguish different PD clusters diff --git a/hugegraph-pd/hg-pd-service/pom.xml b/hugegraph-pd/hg-pd-service/pom.xml index ee78863f35..acbe344e06 100644 --- a/hugegraph-pd/hg-pd-service/pom.xml +++ b/hugegraph-pd/hg-pd-service/pom.xml @@ -51,6 +51,12 @@ 5.8.3 + + junit + junit + ${junit.version} + test + org.apache.hugegraph hg-pd-core diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java index 7d10416967..0100d4e4d4 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java @@ -32,6 +32,10 @@ public class AuthenticationConfigurer implements WebMvcConfigurer { public void addInterceptors(InterceptorRegistry registry) { registry.addInterceptor(restAuthentication) .addPathPatterns("/**") - .excludePathPatterns("/actuator/*", "/v1/health", "/v1/prom/targets/*"); + // /actuator/** rather than /actuator/*: Spring's matcher does not + // treat /actuator/metrics/{name} or /actuator/health/{group} as one + // segment, and both are legitimate probe paths. What is reachable + // there is bounded by management.endpoints.web.exposure.include. + .excludePathPatterns("/actuator/**", "/v1/health", "/v1/prom/targets/*"); } } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java b/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java similarity index 100% rename from hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java rename to hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index a65b53e886..95b044c76b 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -17,7 +17,6 @@ package org.apache.hugegraph.pd.core; -import org.apache.hugegraph.pd.service.interceptor.AuthenticationTest; import org.apache.hugegraph.pd.core.meta.MetadataKeyHelperTest; import org.apache.hugegraph.pd.core.store.HgKVStoreImplTest; import org.apache.hugegraph.pd.raft.IpAuthHandlerTest; @@ -33,7 +32,6 @@ MetadataKeyHelperTest.class, HgKVStoreImplTest.class, PDConfigTest.class, - AuthenticationTest.class, ConfigServiceTest.class, IdServiceTest.class, KvServiceTest.class, diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java index 3c0b6e475b..fce5d56857 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java @@ -183,6 +183,31 @@ public void testEmptyPasswordGets401() throws URISyntaxException, IOException, assert response.statusCode() == 401; } + private int statusWithoutCredential(String path) throws URISyntaxException, IOException, + InterruptedException { + HttpRequest request = HttpRequest.newBuilder() + .uri(new URI(pdRestAddr + path)) + .GET() + .build(); + return client.send(request, HttpResponse.BodyHandlers.ofString()).statusCode(); + } + + @Test + public void testProbePathsNeedNoCredential() throws URISyntaxException, IOException, + InterruptedException { + assert statusWithoutCredential("/v1/health") == 200; + assert statusWithoutCredential("/actuator/health") == 200; + // nested actuator paths are probe surface too; /actuator/* would 401 them + assert statusWithoutCredential("/actuator/metrics/jvm.memory.used") == 200; + } + + @Test + public void testUnexposedActuatorEndpointIsClosed() throws URISyntaxException, IOException, + InterruptedException { + // not in management.endpoints.web.exposure.include, so it must never serve data + assert statusWithoutCredential("/actuator/env") != 200; + } + @Test public void testUnknownServiceNameGets401() throws URISyntaxException, IOException, InterruptedException { diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh index 6842cb5563..ffc8e0ec11 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh @@ -49,11 +49,19 @@ PD_AUTH_PASSWORD="${PD_AUTH_PASSWORD:-}" if [ -z "${PD_AUTH_PASSWORD}" ]; then log "WARN: PD_AUTH_PASSWORD is empty; PD will answer 401 unless it runs without auth" fi -# curl -K takes a quoted string, so escape backslash first and then quote +# curl -K reads one option per line and takes the value as a quoted string +# whose only escapes are \\ \" \t \n \r \v. Backslash first, then the rest; +# an unescaped line break would end the option early and send a truncated +# credential (curl then warns that the remainder is an unknown option). escape_curlrc() { local v=$1 v=${v//\\/\\\\} - printf '%s' "${v//\"/\\\"}" + v=${v//\"/\\\"} + v=${v//$'\n'/\\n} + v=${v//$'\r'/\\r} + v=${v//$'\t'/\\t} + v=${v//$'\v'/\\v} + printf '%s' "$v" } PD_AUTH_CURL_USER=$(escape_curlrc "${PD_AUTH_USER}") PD_AUTH_CURL_PASSWORD=$(escape_curlrc "${PD_AUTH_PASSWORD}") diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh new file mode 100755 index 0000000000..4dfc141cfa --- /dev/null +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Every PD configuration that ships in the archive or the jar must carry the +# same REST hardening: no wildcard actuator exposure (that path is anonymous), +# an auth.secret-key that is present and empty, and no copy of the secret that +# earlier revisions published. A fix applied to one variant and not the others +# is what this catches. + +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../../.." && pwd)" +PUBLISHED_SECRET='FXQXbJtbCLxODc6tGci732pkH1cyf8Qg' +FAIL=0 + +check() { + local file="$1" rel="${1#"${ROOT}/"}" + [[ -f "$file" ]] || { echo " FAIL ${rel}: missing"; FAIL=1; return; } + + local exposure + exposure=$(sed -n 's/^[[:space:]]*include:[[:space:]]*//p' "$file" | head -1) + if [[ "$exposure" == *'*'* ]]; then + echo " FAIL ${rel}: actuator exposure is a wildcard (${exposure})"; FAIL=1 + fi + if ! grep -qE '^[[:space:]]*secret-key:[[:space:]]*$' "$file"; then + echo " FAIL ${rel}: auth.secret-key must be present and empty"; FAIL=1 + fi + if grep -q "${PUBLISHED_SECRET}" "$file"; then + echo " FAIL ${rel}: contains the published secret"; FAIL=1 + fi + echo " ok ${rel}" +} + +echo "PD shipped configuration hardening" +for f in "${ROOT}"/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml* \ + "${ROOT}"/hugegraph-pd/hg-pd-service/src/main/resources/application.yml; do + check "$f" +done +[[ "$FAIL" -eq 0 ]] && echo "all shipped PD configs pass" || { echo "shipped PD config check failed"; exit 1; } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh index 8764806519..6e64ff5d59 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh @@ -72,7 +72,7 @@ assert_contract() { } run_case() { - local scenario="$1" peers="$2" abort_after="$3" + local scenario="$1" peers="$2" abort_after="$3" password="${4:-test-password}" : > "${CALL_LOG}" : > "${ARGS_LOG}" : > "${CONFIG_LOG}" @@ -91,7 +91,7 @@ run_case() { MOCK_TIMEOUT_LOG="${TIMEOUT_LOG}" \ HG_SERVER_PD_REST_ENDPOINT="${peers}" \ PD_AUTH_USER="test-user" \ - PD_AUTH_PASSWORD="test-password" \ + PD_AUTH_PASSWORD="${password}" \ 'hugegraph.backend=hstore' \ 'hugegraph.pd.peers=config-only:8686' \ "${DIST_ROOT}/bin/wait-storage.sh" 2>&1) @@ -225,4 +225,12 @@ assert_output "ERROR: Timeout waiting for storage backend" assert_contract echo " PASS all-unready timeout" -echo "5 passed, 0 failed" +# A secret with CR/LF must reach curl as one escaped config line, not two. +run_case "pd1-up" "pd0:8620,pd1:8620" 6 "$(printf 'a\r\nb\\c"d')" +assert_equal "line-break secret rc" "0" "${CASE_RC}" +if grep -Fv -- 'user = "test-user:a\r\nb\\c\"d"' "${CONFIG_LOG}" | grep -q .; then + fail "line break or quote in the secret was not escaped for curl -K" +fi +echo " PASS line break in secret" + +echo "6 passed, 0 failed" diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index de07904d64..c08f7d787b 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -678,6 +678,10 @@ For a production-like 3-node distributed deployment, use the compose file at `do ```bash cd docker +# The PD REST secret is required; the Compose file refuses to start without +# it. Generate it once and keep it, every PD node and PD client needs the +# same value (docker/README.md has the full .env recipe). +export HG_PD_AUTH_SECRET_KEY="$(openssl rand -hex 24)" HUGEGRAPH_VERSION=1.7.0 docker compose -f docker-compose-3pd-3store-3server.yml up -d ``` @@ -695,6 +699,7 @@ environment: HG_PD_INITIAL_STORE_LIST: store0:8500,store1:8500,store2:8500 # maps to pd.initial-store-list HG_PD_DATA_PATH: /hugegraph-pd/pd_data # maps to pd.data-path HG_PD_INITIAL_STORE_COUNT: 3 # maps to pd.initial-store-count + HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:?} # maps to auth.secret-key; required ``` **Store environment variables** (per node): From e964b680e0100a59ab76122275377b07e8d0bd67 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sat, 5 Sep 2026 11:19:04 +0530 Subject: [PATCH 08/28] fix(pd): generate Hubble's PD credential, abort wait-storage on 401 Four more review items. Hubble had no path from .env to operations.pd.password: Compose does not interpolate properties files and the Hubble image has no entrypoint that reads the environment, so both HStore stacks mounted a tracked file with an empty password and the documented remedy was to sed a production secret into that tracked file. The tracked files are now conf/hubble/{hstore,hstore-ha}.properties.example, the Compose files mount the generated conf/hubble/.local.properties, that pattern is in .gitignore next to docker/.env, and set-hubble-pd-password.sh [secret] generates the file from the example. The .env recipe runs it, the deployment guide runs it before `up`, and test-compose.sh generates both files before every render and smoke (a developer's own local files are restored afterwards) and checks the generator with a non-hex secret, the resulting mode, and its refusals. wait-storage.sh could not tell a 401 from an unreachable PD or a cluster with no Up store: curl sat inside a grep pipeline with no pipefail, so a wrong or missing secret retried for 300s and then reported a storage failure. curl now runs alone with -w '\n%{http_code}', a 401 aborts at once with a message naming PD_AUTH_PASSWORD and auth.secret-key, and the outer error distinguishes a timeout from an abort. The test mock honours -w and a new case asserts the abort happens on the first call. PDConfig now logs the empty auth.secret-key at boot, not only on the first refused request, since /v1/health keeps answering 200 in between and the shipped configs leave the key empty on purpose. The 401 body is the constant {"status":-1,"error":"Unauthorized"}. The old body carried the exception's toString, which named internal classes and told an unauthenticated caller whether the service name or the password was wrong; the reason now goes to a debug log with the method and path. --- .gitignore | 2 + docker/README.md | 41 ++++++---- ...roperties => hstore-ha.properties.example} | 5 +- ...e.properties => hstore.properties.example} | 5 +- docker/docker-compose-3pd-3store-3server.yml | 2 +- docker/docker-compose-hstore.yml | 2 +- docker/set-hubble-pd-password.sh | 39 ++++++---- docker/test-compose.sh | 75 +++++++++++++------ .../apache/hugegraph/pd/config/PDConfig.java | 10 +++ .../rest/interceptor/RestAuthentication.java | 9 ++- .../src/assembly/static/bin/wait-storage.sh | 33 ++++++-- .../src/assembly/travis/test-wait-storage.sh | 34 +++++++-- hugegraph-store/docs/deployment-guide.md | 3 + 13 files changed, 190 insertions(+), 70 deletions(-) rename docker/conf/hubble/{hstore-ha.properties => hstore-ha.properties.example} (86%) rename docker/conf/hubble/{hstore.properties => hstore.properties.example} (85%) diff --git a/.gitignore b/.gitignore index e1f546810f..e9d96893cd 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,8 @@ build/ .env.test.local .env.production.local docker/.env +# generated by docker/set-hubble-pd-password.sh, carries the PD REST secret +docker/conf/hubble/*.local.properties *.orig *.rej diff --git a/docker/README.md b/docker/README.md index 1d8f87f532..d5f1433a53 100644 --- a/docker/README.md +++ b/docker/README.md @@ -42,10 +42,15 @@ contains a single quote or newline. pd_secret="$(openssl rand -hex 24)" printf "HUGEGRAPH_ADMIN_PASSWORD='%s'\nHUGEGRAPH_AUTH_TOKEN_SECRET='%s'\nHG_PD_AUTH_SECRET_KEY='%s'\n" \ 'replace-with-your-password' "${jwt_secret}" "${pd_secret}" > .env + # Hubble reads the PD secret from a file, not from .env: generate the + # untracked properties files the HStore topologies mount + ./set-hubble-pd-password.sh hstore "${pd_secret}" + ./set-hubble-pd-password.sh hstore-ha "${pd_secret}" ) ``` -Do not commit `.env`. Keeping the same JWT secret preserves authentication +Do not commit `.env` or `conf/hubble/*.local.properties`; both are in +`.gitignore`. Keeping the same JWT secret preserves authentication tokens when containers are recreated. For authenticated topologies with multiple Server replicas, all replicas receive this same secret. The HA topology fails fast if authentication is enabled without this shared secret. @@ -93,23 +98,26 @@ fails: the Server sends the wrong secret it retries until `WAIT_STORAGE_TIMEOUT_S` (300s) expires and the container exits with `ERROR: Timeout waiting for storage backend`. -- Hubble, through `operations.pd.password` in the file under `conf/hubble/`. - That file is mounted read-only and is not templated, so write the same value - into it by hand. Until you do, Hubble's PD-backed views get 401 from PD; - everything else in Hubble works. - -Write it in with the helper, after loading `.env` as above. Use -`hstore.properties` for the Minimal HStore topology and `hstore-ha.properties` -for HA: +- Hubble, through `operations.pd.password` in + `conf/hubble/hstore.local.properties` (Minimal HStore) or + `conf/hubble/hstore-ha.local.properties` (HA). Compose mounts those files + read-only and does not template them, and the Hubble image has no + entrypoint that reads the environment, so they are generated from the + tracked `*.properties.example` files by `set-hubble-pd-password.sh`. The + `.env` recipe above already runs it. To regenerate after loading `.env`: ```bash -./set-hubble-pd-password.sh conf/hubble/hstore.properties +./set-hubble-pd-password.sh hstore # or hstore-ha ``` +Run it before `docker compose up`: if the file is missing, Docker creates an +empty directory at the bind path and Hubble starts with no configuration. The helper refuses an empty value, writes the secret without passing it through a `sed` replacement (where `&`, `#` and backslashes are special), and doubles backslashes for the `.properties` format. The generated hex secret -needs none of that, but a hand-chosen one might. +needs none of that, but a hand-chosen one might. Until the file carries the +right value, Hubble's PD-backed views get 401 from PD; everything else in +Hubble works. ### Standalone @@ -362,9 +370,14 @@ discovery settings, the PD REST credential (`operations.pd.username` and container paths: - `conf/hubble/standalone.properties` uses direct Server mode. -- `conf/hubble/hstore.properties` uses one PD and one Store REST target. -- `conf/hubble/hstore-ha.properties` uses all three PD peers and all three - allowed Store REST targets. +- `conf/hubble/hstore.properties.example` uses one PD and one Store REST + target. +- `conf/hubble/hstore-ha.properties.example` uses all three PD peers and all + three allowed Store REST targets. + +The two HStore topologies mount the generated `*.local.properties` next to +these examples (see `set-hubble-pd-password.sh`), never the examples +themselves, so the PD secret stays out of tracked files. Hubble detects Server authentication through the Server API. Do not add an `auth.enabled` property or duplicate auth-on/auth-off configurations. diff --git a/docker/conf/hubble/hstore-ha.properties b/docker/conf/hubble/hstore-ha.properties.example similarity index 86% rename from docker/conf/hubble/hstore-ha.properties rename to docker/conf/hubble/hstore-ha.properties.example index 2a818d6fb2..82c32248d9 100644 --- a/docker/conf/hubble/hstore-ha.properties +++ b/docker/conf/hubble/hstore-ha.properties.example @@ -21,8 +21,9 @@ server.direct_url=http://server0:8080 pd.peers=pd0:8686,pd1:8686,pd2:8686 pd.server=pd0:8620 # PD REST credential. The password must equal PD's auth.secret-key, which has -# no default: set it to the same value as HG_PD_AUTH_SECRET_KEY in .env. While -# it is empty, Hubble's PD-backed views get HTTP 401 from PD. +# no default. Do not edit this tracked example: docker/set-hubble-pd-password.sh +# generates the untracked .local.properties that Compose mounts, with the value +# from HG_PD_AUTH_SECRET_KEY in .env. operations.pd.username=hubble operations.pd.password= operations.store.allowed_targets=[http://store0:8520,http://store1:8520,http://store2:8520] diff --git a/docker/conf/hubble/hstore.properties b/docker/conf/hubble/hstore.properties.example similarity index 85% rename from docker/conf/hubble/hstore.properties rename to docker/conf/hubble/hstore.properties.example index 55796d6e29..271ef9039f 100644 --- a/docker/conf/hubble/hstore.properties +++ b/docker/conf/hubble/hstore.properties.example @@ -21,8 +21,9 @@ server.direct_url=http://server:8080 pd.peers=pd:8686 pd.server=pd:8620 # PD REST credential. The password must equal PD's auth.secret-key, which has -# no default: set it to the same value as HG_PD_AUTH_SECRET_KEY in .env. While -# it is empty, Hubble's PD-backed views get HTTP 401 from PD. +# no default. Do not edit this tracked example: docker/set-hubble-pd-password.sh +# generates the untracked .local.properties that Compose mounts, with the value +# from HG_PD_AUTH_SECRET_KEY in .env. operations.pd.username=hubble operations.pd.password= operations.store.allowed_targets=[http://store:8520] diff --git a/docker/docker-compose-3pd-3store-3server.yml b/docker/docker-compose-3pd-3store-3server.yml index ba02f27eb1..22ec9733b9 100644 --- a/docker/docker-compose-3pd-3store-3server.yml +++ b/docker/docker-compose-3pd-3store-3server.yml @@ -241,7 +241,7 @@ services: - "${HUBBLE_PUBLISH_HOST:-127.0.0.1}:8088:8088" volumes: - hubble-data:/hubble/data - - ./conf/hubble/hstore-ha.properties:/hubble/conf/hugegraph-hubble.properties:ro + - ./conf/hubble/hstore-ha.local.properties:/hubble/conf/hugegraph-hubble.properties:ro healthcheck: test: - CMD-SHELL diff --git a/docker/docker-compose-hstore.yml b/docker/docker-compose-hstore.yml index ba5a421c2c..3f3a7c8395 100644 --- a/docker/docker-compose-hstore.yml +++ b/docker/docker-compose-hstore.yml @@ -120,7 +120,7 @@ services: - "${HUBBLE_PUBLISH_HOST:-127.0.0.1}:8088:8088" volumes: - hubble-data:/hubble/data - - ./conf/hubble/hstore.properties:/hubble/conf/hugegraph-hubble.properties:ro + - ./conf/hubble/hstore.local.properties:/hubble/conf/hugegraph-hubble.properties:ro healthcheck: test: - CMD-SHELL diff --git a/docker/set-hubble-pd-password.sh b/docker/set-hubble-pd-password.sh index aa44d4b1be..535e847a28 100755 --- a/docker/set-hubble-pd-password.sh +++ b/docker/set-hubble-pd-password.sh @@ -15,29 +15,42 @@ # See the License for the specific language governing permissions and # limitations under the License. # -# Write PD's REST secret into a Hubble properties file as operations.pd.password. +# Generate the Hubble properties file a Compose topology mounts, with PD's +# REST secret written in as operations.pd.password. # -# usage: set-hubble-pd-password.sh [secret] +# usage: set-hubble-pd-password.sh [secret] # -# The secret defaults to $HG_PD_AUTH_SECRET_KEY. The value never goes through a -# sed replacement, where & # and backslash are special, and backslashes are -# doubled for the .properties format. The file keeps its mode, which matters -# because Compose mounts it read-only into the Hubble container. +# Reads conf/hubble/.properties.example (tracked) and writes +# conf/hubble/.local.properties (ignored by git), so the secret never +# lands in a tracked file. The secret defaults to $HG_PD_AUTH_SECRET_KEY. The +# value never goes through a sed replacement, where & # and backslash are +# special, and backslashes are doubled for the .properties format. Run this +# before `docker compose up`: if the target is missing Docker creates an empty +# directory at the bind path and Hubble starts with no configuration. set -euo pipefail -file=${1:?usage: $0 [secret]} +name=${1:?usage: $0 [secret]} secret=${2:-${HG_PD_AUTH_SECRET_KEY:-}} +dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/conf/hubble" +example="${dir}/${name}.properties.example" +out="${dir}/${name}.local.properties" -[[ -f "$file" ]] || { echo "no such file: $file" >&2; exit 1; } +[[ -f "$example" ]] || { echo "no such topology: ${name} (expected ${example})" >&2; exit 1; } [[ -n "$secret" ]] || { echo "secret is empty; load .env first (set -a; . ./.env; set +a)" >&2; exit 1; } case "$secret" in *$'\n'*|*$'\r'*) echo "secret contains a line break, which a .properties value cannot hold" >&2; exit 1 ;; esac escaped=${secret//\\/\\\\} -tmp=$(mktemp "${file}.XXXXXX") +tmp=$(mktemp "${out}.XXXXXX") trap 'rm -f "$tmp"' EXIT -grep -v '^operations\.pd\.password=' "$file" > "$tmp" || true -printf 'operations.pd.password=%s\n' "$escaped" >> "$tmp" -# cat, not mv: keep the file's inode and mode -cat "$tmp" > "$file" +{ + printf '# Generated from %s by set-hubble-pd-password.sh; not tracked by git.\n' "$(basename "$example")" + grep -v '^operations\.pd\.password=' "$example" || true + printf 'operations.pd.password=%s\n' "$escaped" +} > "$tmp" +# Hubble runs unprivileged and the mount is read-only, so the file must be world-readable +chmod 644 "$tmp" +mv "$tmp" "$out" +trap - EXIT +echo "wrote ${out}" diff --git a/docker/test-compose.sh b/docker/test-compose.sh index d363e3cb9d..d4988a9d51 100644 --- a/docker/test-compose.sh +++ b/docker/test-compose.sh @@ -138,7 +138,7 @@ assert_hstore() { assert_common "${rendered}" \ '["hubble","pd","server","store"]' \ '["hubble-data","pd-data","store-data"]' - assert_hubble "${rendered}" "hstore.properties" server + assert_hubble "${rendered}" "hstore.local.properties" server jq -e ' .services.pd.image == "hugegraph/pd:ci-version" and .services.store.image == "hugegraph/store:ci-version" and @@ -160,9 +160,9 @@ assert_hstore() { .source == "store-data" and .target == "/hugegraph-store/storage") ' "${rendered}" >/dev/null - assert_file_property "${DOCKER_DIR}/conf/hubble/hstore.properties" \ + assert_file_property "${DOCKER_DIR}/conf/hubble/hstore.local.properties" \ "pd.peers=pd:8686" - assert_file_property "${DOCKER_DIR}/conf/hubble/hstore.properties" \ + assert_file_property "${DOCKER_DIR}/conf/hubble/hstore.local.properties" \ "operations.store.allowed_targets=[http://store:8520]" } @@ -171,7 +171,7 @@ assert_ha() { assert_common "${rendered}" \ '["hubble","pd0","pd1","pd2","server0","server1","server2","store0","store1","store2"]' \ '["hg-pd0-data","hg-pd1-data","hg-pd2-data","hg-store0-data","hg-store1-data","hg-store2-data","hubble-data"]' - assert_hubble "${rendered}" "hstore-ha.properties" \ + assert_hubble "${rendered}" "hstore-ha.local.properties" \ server0 server1 server2 jq -e ' all([.services.pd0, .services.pd1, .services.pd2][]; @@ -205,9 +205,9 @@ assert_ha() { .healthcheck.retries == 30 and .healthcheck.start_period == "1m0s") ' "${rendered}" >/dev/null - assert_file_property "${DOCKER_DIR}/conf/hubble/hstore-ha.properties" \ + assert_file_property "${DOCKER_DIR}/conf/hubble/hstore-ha.local.properties" \ "pd.peers=pd0:8686,pd1:8686,pd2:8686" - assert_file_property "${DOCKER_DIR}/conf/hubble/hstore-ha.properties" \ + assert_file_property "${DOCKER_DIR}/conf/hubble/hstore-ha.local.properties" \ "operations.store.allowed_targets=[http://store0:8520,http://store1:8520,http://store2:8520]" } @@ -242,35 +242,66 @@ cleanup() { if [[ -n "${ACTIVE_PROJECT}" ]]; then compose_active down -v --remove-orphans >/dev/null 2>&1 || true fi + restore_hubble_configs [[ -z "${RENDER_DIR}" ]] || rm -rf "${RENDER_DIR}" } +# The HStore topologies mount conf/hubble/.local.properties, which is +# generated and untracked. Generate both with the CI secret before any render +# or `up`; a missing file would make Docker create an empty directory at the +# bind path. A developer's own local files are put back afterwards. +HUBBLE_BACKUP_DIR="" +prepare_hubble_configs() { + local name + HUBBLE_BACKUP_DIR="$(mktemp -d)" + for name in hstore hstore-ha; do + local f="${DOCKER_DIR}/conf/hubble/${name}.local.properties" + [[ ! -f "${f}" ]] || cp -p "${f}" "${HUBBLE_BACKUP_DIR}/${name}.local.properties" + "${DOCKER_DIR}/set-hubble-pd-password.sh" "${name}" "${PD_SECRET}" >/dev/null + done +} +restore_hubble_configs() { + [[ -n "${HUBBLE_BACKUP_DIR}" ]] || return 0 + local name + for name in hstore hstore-ha; do + local f="${DOCKER_DIR}/conf/hubble/${name}.local.properties" + if [[ -f "${HUBBLE_BACKUP_DIR}/${name}.local.properties" ]]; then + cp -p "${HUBBLE_BACKUP_DIR}/${name}.local.properties" "${f}" + else + rm -f "${f}" + fi + done + rm -rf "${HUBBLE_BACKUP_DIR}" + HUBBLE_BACKUP_DIR="" +} + # set-hubble-pd-password.sh must survive the characters a sed replacement -# would mangle, keep the rest of the file, and keep the file's mode. +# would mangle, keep the rest of the example, produce a world-readable file +# for the read-only mount, and refuse an empty secret or unknown topology. hubble_password_helper_check() { - local tmp - tmp=$(mktemp "${TMPDIR:-/tmp}/hubble-props.XXXXXX") - cp "${DOCKER_DIR}/conf/hubble/hstore.properties" "${tmp}" - chmod 644 "${tmp}" - "${DOCKER_DIR}/set-hubble-pd-password.sh" "${tmp}" 'a&b#c\d' >/dev/null + local f="${DOCKER_DIR}/conf/hubble/hstore.local.properties" + "${DOCKER_DIR}/set-hubble-pd-password.sh" hstore 'a&b#c\d' >/dev/null local line mode - line=$(grep '^operations\.pd\.password=' "${tmp}") - mode=$(stat -c '%a' "${tmp}" 2>/dev/null || stat -f '%Lp' "${tmp}") - rm -f "${tmp}" + line=$(grep '^operations\.pd\.password=' "${f}") + mode=$(stat -c '%a' "${f}" 2>/dev/null || stat -f '%Lp' "${f}") [[ "${line}" == 'operations.pd.password=a&b#c\\d' ]] || { echo "set-hubble-pd-password.sh mangled the secret: ${line}" >&2; exit 1; } [[ "${mode}" == "644" ]] || { - echo "set-hubble-pd-password.sh changed the file mode to ${mode}" >&2; exit 1; } - if ! "${DOCKER_DIR}/set-hubble-pd-password.sh" "${DOCKER_DIR}/conf/hubble/hstore.properties" '' 2>/dev/null; then - : - else - echo "set-hubble-pd-password.sh accepted an empty secret" >&2; exit 1 - fi + echo "set-hubble-pd-password.sh wrote mode ${mode}, Hubble could not read it" >&2; exit 1; } + grep -q '^pd.server=pd:8620$' "${f}" || { + echo "set-hubble-pd-password.sh dropped the example's other properties" >&2; exit 1; } + ! "${DOCKER_DIR}/set-hubble-pd-password.sh" hstore '' 2>/dev/null || { + echo "set-hubble-pd-password.sh accepted an empty secret" >&2; exit 1; } + ! "${DOCKER_DIR}/set-hubble-pd-password.sh" nope 'x' 2>/dev/null || { + echo "set-hubble-pd-password.sh accepted an unknown topology" >&2; exit 1; } + # put the CI value back for the render/smoke that follows + "${DOCKER_DIR}/set-hubble-pd-password.sh" hstore "${PD_SECRET}" >/dev/null } run_render() { RENDER_DIR="$(mktemp -d)" trap cleanup EXIT INT TERM + prepare_hubble_configs render "${RENDER_DIR}/standalone.json" \ -f "${DOCKER_DIR}/docker-compose.yml" render "${RENDER_DIR}/hstore.json" \ @@ -400,6 +431,7 @@ smoke() { run_smoke() { trap cleanup EXIT INT TERM + prepare_hubble_configs smoke standalone false true "${DOCKER_DIR}/docker-compose.yml" smoke hstore true true "${DOCKER_DIR}/docker-compose-hstore.yml" } @@ -407,6 +439,7 @@ run_smoke() { run_smoke_auth_off() { PASSWORD="" trap cleanup EXIT INT TERM + prepare_hubble_configs smoke standalone-anon false false "${DOCKER_DIR}/docker-compose.yml" smoke hstore-anon true false "${DOCKER_DIR}/docker-compose-hstore.yml" } diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java index 1eece31655..56bd58b34d 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java @@ -33,10 +33,12 @@ import lombok.Data; import lombok.ToString; +import lombok.extern.slf4j.Slf4j; /** * PD profile */ +@Slf4j @Data @Component public class PDConfig implements InitializingBean { @@ -107,6 +109,14 @@ public void afterPropertiesSet() { "deployment-specific secret in conf/application.yml, or through the " + "HG_PD_AUTH_SECRET_KEY environment variable for the Docker image."); } + // The shipped configs leave this empty on purpose. Say so in the boot log: + // the REST interceptor also logs it, but only on the first refused request, + // and /v1/health keeps answering 200 in the meantime. + if (this.secretKey == null || this.secretKey.isEmpty()) { + log.error("auth.secret-key is not configured, so every authenticated REST " + + "request will be refused. Add it to conf/application.yml (or set " + + "HG_PD_AUTH_SECRET_KEY) and give every REST client the same value."); + } } public Map getInitialStoreMap() { diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java index d10d2e9da8..3aeeb48da9 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java @@ -24,7 +24,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.hugegraph.pd.rest.API; import org.apache.hugegraph.pd.service.interceptor.Authentication; import org.springframework.lang.Nullable; import org.springframework.stereotype.Service; @@ -38,6 +37,7 @@ public class RestAuthentication extends Authentication implements HandlerInterceptor { private static final String TOKEN_KEY = "Pd-Token"; + private static final String UNAUTHORIZED_BODY = "{\"status\":-1,\"error\":\"Unauthorized\"}"; private static final Supplier DEFAULT_HANDLE = () -> true; @Override @@ -64,8 +64,13 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons // authenticate reactively never retry with credentials response.setHeader("WWW-Authenticate", "Basic realm=\"hugegraph-pd\""); response.setContentType("application/json"); - response.getWriter().println(new API().toJSON(e)); + // Constant body: the exception text named internal classes and told an + // unauthenticated caller whether the name or the password was wrong. + response.getWriter().println(UNAUTHORIZED_BODY); response.getWriter().flush(); + Throwable reason = e.getCause() != null ? e.getCause() : e; + log.debug("REST authentication refused for {} {}: {}", request.getMethod(), + request.getRequestURI(), reason.getMessage()); return false; } } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh index ffc8e0ec11..71b6a61fed 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh @@ -125,15 +125,23 @@ if env | grep '^hugegraph\.' > /dev/null; then log() { echo '[wait-storage] '\"\$1\"; } + # curl stays out of the grep pipeline so its status code is + # readable: a 401 is a wrong secret, not a storage problem, and + # retrying it for 300s only hides that. check_any_pd_stores() { for peer in \$(echo \"\$PD_REST_LIST\" | tr ',' ' '); do - if printf 'user = \"%s:%s\"\n' \ - \"\$PD_AUTH_CURL_USER\" \"\$PD_AUTH_CURL_PASSWORD\" | \ - curl -K - -f -s \ - --connect-timeout ${WAIT_STORAGE_PD_CONNECT_TIMEOUT_S} \ - --max-time ${WAIT_STORAGE_PD_MAX_TIMEOUT_S} \ - \"http://\${peer}/v1/stores\" 2>/dev/null | \ - grep -qi '\"state\"[[:space:]]*:[[:space:]]*\"Up\"'; then + body=\$(printf 'user = \"%s:%s\"\n' \ + \"\$PD_AUTH_CURL_USER\" \"\$PD_AUTH_CURL_PASSWORD\" | \ + curl -K - -s -w '\n%{http_code}' \ + --connect-timeout ${WAIT_STORAGE_PD_CONNECT_TIMEOUT_S} \ + --max-time ${WAIT_STORAGE_PD_MAX_TIMEOUT_S} \ + \"http://\${peer}/v1/stores\" 2>/dev/null) + code=\${body##*\$'\n'} + if [ \"\$code\" = 401 ]; then + log \"ERROR: PD at \${peer} refused the credential (401): PD_AUTH_PASSWORD must match PD's auth.secret-key\" >&2 + return 2 + fi + if printf '%s' \"\$body\" | grep -qi '\"state\"[[:space:]]*:[[:space:]]*\"Up\"'; then echo \"\$peer\" return 0 fi @@ -142,12 +150,21 @@ if env | grep '^hugegraph\.' > /dev/null; then } until PD_REST=\$(check_any_pd_stores); do + if [ \$? -eq 2 ]; then exit 2; fi log 'No Up store yet, retrying in 5s' sleep 5 done log \"Store registration check PASSED via \$PD_REST\" log 'Storage backend is VIABLE' - " || { echo "[wait-storage] ERROR: Timeout waiting for storage backend"; exit 1; } + " || { + rc=$? + if [ "$rc" -eq 124 ]; then + echo "[wait-storage] ERROR: Timeout waiting for storage backend" + else + echo "[wait-storage] ERROR: storage wait aborted, see the message above" + fi + exit 1 + } else log "No pd.peers configured, skipping storage wait" diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh index 6e64ff5d59..3238a2fbff 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh @@ -147,6 +147,18 @@ for arg in "$@"; do fi done +# Honour -w like curl: append the write-out with %{http_code} substituted +fmt="" +prev="" +for arg in "$@"; do + [[ "${prev}" == "-w" ]] && fmt="${arg}" + prev="${arg}" +done +respond() { + printf '%s\n' "$1" + [[ -z "${fmt}" ]] || printf '%s' "${fmt//\\n/$'\n'}" | sed "s/%{http_code}/$2/" +} + if [[ "${url}" == */v1/health ]]; then printf '{}\n' exit 0 @@ -156,9 +168,11 @@ count=$(cat "${MOCK_COUNT_FILE}" 2>/dev/null || true) count=$((${count:-0} + 1)) printf '%s\n' "${count}" > "${MOCK_COUNT_FILE}" -if [[ "${MOCK_SCENARIO}" == "pd1-up" && \ +if [[ "${MOCK_SCENARIO}" == "auth-401" ]]; then + respond '{"status":-1,"error":"Unauthorized"}' 401 +elif [[ "${MOCK_SCENARIO}" == "pd1-up" && \ "${url}" == "http://pd1:8620/v1/stores" ]]; then - printf '{"stores":[{"state":"Up"}]}\n' + respond '{"stores":[{"state":"Up"}]}' 200 elif [[ "${MOCK_SCENARIO}" == "hanging-first" && \ "${url}" == "http://pd0:8620/v1/stores" ]]; then if [[ " $* " == *" --connect-timeout 2 "* && \ @@ -170,15 +184,15 @@ elif [[ "${MOCK_SCENARIO}" == "hanging-first" && \ exit 28 elif [[ "${MOCK_SCENARIO}" == "hanging-first" && \ "${url}" == "http://pd1:8620/v1/stores" ]]; then - printf '{"stores":[{"state":"Up"}]}\n' + respond '{"stores":[{"state":"Up"}]}' 200 elif [[ "${MOCK_SCENARIO}" == "retry" && "${count}" -eq 3 && \ "${url}" == "http://pd0:8620/v1/stores" ]]; then exit 7 elif [[ "${MOCK_SCENARIO}" == "retry" && "${count}" -eq 4 && \ "${url}" == "http://pd1:8620/v1/stores" ]]; then - printf '{"stores":[{"state":"Up"}]}\n' + respond '{"stores":[{"state":"Up"}]}' 200 else - printf '{"stores":[]}\n' + respond '{"stores":[]}' 200 fi EOF @@ -233,4 +247,12 @@ if grep -Fv -- 'user = "test-user:a\r\nb\\c\"d"' "${CONFIG_LOG}" | grep -q .; th fi echo " PASS line break in secret" -echo "6 passed, 0 failed" +# A 401 is a wrong secret, not a storage problem: abort at once, name the cause. +run_case "auth-401" "pd0:8620,pd1:8620" 9 +[[ "${CASE_RC}" -ne 0 ]] || fail "a 401 from PD must abort" +assert_output "refused the credential (401)" +assert_equal "no retry after 401" "${PD0}" "$(cat "${CALL_LOG}")" +[[ "${CASE_OUTPUT}" != *"Timeout waiting"* ]] || fail "401 was reported as a timeout" +echo " PASS 401 aborts without retry" + +echo "7 passed, 0 failed" diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index c08f7d787b..4af3832d85 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -682,6 +682,9 @@ cd docker # it. Generate it once and keep it, every PD node and PD client needs the # same value (docker/README.md has the full .env recipe). export HG_PD_AUTH_SECRET_KEY="$(openssl rand -hex 24)" +# Hubble reads the secret from a generated, untracked properties file that the +# Compose file mounts; create it before `up` or Hubble starts unconfigured. +./set-hubble-pd-password.sh hstore-ha HUGEGRAPH_VERSION=1.7.0 docker compose -f docker-compose-3pd-3store-3server.yml up -d ``` From 94a4a0ff6231797a07553883efb0d452145c46e7 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sun, 6 Sep 2026 12:31:59 +0530 Subject: [PATCH 09/28] fix(pd): ignore the generator's temp file, correct the 401 docs Four follow-ups on the previous commit. The .gitignore pattern missed the file the Hubble generator actually writes through: set-hubble-pd-password.sh writes to a mktemp name derived from the output, so the plaintext secret first lands in conf/hubble/hstore.local.properties.aB3xY9, which *.local.properties does not match. A trailing * covers the base name and every suffixed form, while the tracked examples and standalone.properties stay tracked. Both READMEs still told operators that a wrong secret waits out WAIT_STORAGE_TIMEOUT_S and exits with "Timeout waiting for storage backend". The previous commit made a 401 abort on the first attempt with a different message, so the documented string could not appear. They now describe the abort and name the two messages the script really prints. test-pd-shipped-config.sh printed "ok" for a file that had just failed, because the checks only set the global FAIL and the ok line ran unconditionally. It now compares FAIL against its value on entry, using an if rather than a && so the function does not return non-zero under set -e and abort the loop at the first bad file. Its exposure lookup also reads the include: under the actuator exposure block rather than the first include: in the file, so a config that grows an unrelated one cannot pass silently. --- .gitignore | 2 +- docker/README.md | 7 ++++--- hugegraph-pd/README.md | 6 +++--- .../src/assembly/travis/test-pd-shipped-config.sh | 12 +++++++++--- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index e9d96893cd..edfe89f7d2 100644 --- a/.gitignore +++ b/.gitignore @@ -45,7 +45,7 @@ build/ .env.production.local docker/.env # generated by docker/set-hubble-pd-password.sh, carries the PD REST secret -docker/conf/hubble/*.local.properties +docker/conf/hubble/*.local.properties* *.orig *.rej diff --git a/docker/README.md b/docker/README.md index d5f1433a53..e3da7f6cdb 100644 --- a/docker/README.md +++ b/docker/README.md @@ -95,9 +95,10 @@ fails: - The Server, whose `bin/wait-storage.sh` polls `/v1/stores` before the Server starts. Both Compose files pass `PD_AUTH_PASSWORD` to it from the same variable, so setting `HG_PD_AUTH_SECRET_KEY` in `.env` covers it. If - the Server sends the wrong secret it retries until - `WAIT_STORAGE_TIMEOUT_S` (300s) expires and the container exits with - `ERROR: Timeout waiting for storage backend`. + the Server sends the wrong secret `wait-storage.sh` aborts on the first + 401 rather than waiting out `WAIT_STORAGE_TIMEOUT_S`, and the container + exits with `ERROR: storage wait aborted, see the message above` after + logging `ERROR: PD at refused the credential (401)`. - Hubble, through `operations.pd.password` in `conf/hubble/hstore.local.properties` (Minimal HStore) or `conf/hubble/hstore-ha.local.properties` (HA). Compose mounts those files diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index a11959aa37..089bbc6a0b 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -305,9 +305,9 @@ docker/docker-compose-3pd-3store-3server.yml `HG_PD_AUTH_SECRET_KEY`, which the Docker image requires. Give every REST client the same value: the Server's `bin/wait-storage.sh` reads `PD_AUTH_PASSWORD` (and `PD_AUTH_USER`, default `store`), and Hubble reads - `operations.pd.password`. A client left on a stale secret gets 401, and for - `wait-storage.sh` that means Server startup aborts after - `WAIT_STORAGE_TIMEOUT_S`. + `operations.pd.password`. A client left on a stale secret gets 401, and + `wait-storage.sh` aborts the Server's startup on the first one rather than + waiting out `WAIT_STORAGE_TIMEOUT_S`. - An existing `conf/application.yml` carried over from an earlier release has no `auth` block. PD then starts with an empty secret and refuses every authenticated REST request, logging an error that names `auth.secret-key`. diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh index 4dfc141cfa..b7b9ea734a 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh @@ -28,11 +28,14 @@ PUBLISHED_SECRET='FXQXbJtbCLxODc6tGci732pkH1cyf8Qg' FAIL=0 check() { - local file="$1" rel="${1#"${ROOT}/"}" + local file="$1" rel="${1#"${ROOT}/"}" before="$FAIL" [[ -f "$file" ]] || { echo " FAIL ${rel}: missing"; FAIL=1; return; } + # The actuator exposure specifically: a config that grows an unrelated + # include: above this block must not satisfy the check by accident. local exposure - exposure=$(sed -n 's/^[[:space:]]*include:[[:space:]]*//p' "$file" | head -1) + exposure=$(awk '/^[[:space:]]*exposure:/ {found = 1; next} + found && /^[[:space:]]*include:/ {sub(/^[[:space:]]*include:[[:space:]]*/, ""); print; exit}' "$file") if [[ "$exposure" == *'*'* ]]; then echo " FAIL ${rel}: actuator exposure is a wildcard (${exposure})"; FAIL=1 fi @@ -42,7 +45,10 @@ check() { if grep -q "${PUBLISHED_SECRET}" "$file"; then echo " FAIL ${rel}: contains the published secret"; FAIL=1 fi - echo " ok ${rel}" + # Only when nothing above raised FAIL, or the file contradicts its own report + if [[ "$FAIL" == "$before" ]]; then + echo " ok ${rel}" + fi } echo "PD shipped configuration hardening" From a890122c6aa3ebf5151c462335ba6a972503f8ef Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sun, 6 Sep 2026 21:12:40 +0530 Subject: [PATCH 10/28] docs(pd): match the actuator exclusion the interceptor ships AuthenticationConfigurer excludes /actuator/** so that nested probe paths such as /actuator/metrics/{name} stay reachable, but seven prose and comment lines still named /actuator/*. The table row in hugegraph-pd/docs/configuration.md was the misleading one: it claimed every exposed actuator endpoint is reachable without a credential, which only holds for the double-star pattern. Correct all seven to /actuator/**, matching application.yml.template, which already used it. Documentation only; no behaviour change. --- hugegraph-pd/README.md | 2 +- hugegraph-pd/docs/api-reference.md | 2 +- hugegraph-pd/docs/configuration.md | 4 ++-- .../hg-pd-dist/src/assembly/static/conf/application.yml | 2 +- hugegraph-pd/hg-pd-service/src/main/resources/application.yml | 2 +- hugegraph-store/docs/operations-guide.md | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 089bbc6a0b..470c4b9864 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -297,7 +297,7 @@ docker/docker-compose-3pd-3store-3server.yml data movement), and the gRPC and Raft ports carry no authentication. - REST requests need HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) with the `auth.secret-key` value as - the password. Health probes (`/v1/health`, `/actuator/*`, + the password. Health probes (`/v1/health`, `/actuator/**`, `/v1/prom/targets/*`) stay unauthenticated. - `auth.secret-key` has no shipped default, because a secret in the source tree is published to everyone. Generate one per deployment (`openssl rand diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 7535bc8f62..2632c26dac 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -767,7 +767,7 @@ internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, and the `auth.secret-key` value from PD's `conf/application.yml` as the password. A missing or wrong credential gets HTTP 401. The `curl` examples that follow omit `-u` for readability; add it to every call except `/v1/health`, -`/actuator/*` and `/v1/prom/targets/*`, which stay unauthenticated for probes. +`/actuator/**` and `/v1/prom/targets/*`, which stay unauthenticated for probes. ```bash curl -u hg: http://localhost:8620/v1/stores diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index e3cbd1ed3a..bacaaa46b2 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -84,7 +84,7 @@ server: Every REST request except the probes below must carry HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, and the shared secret as the password. A missing or wrong credential gets -HTTP 401. Unauthenticated paths: `/v1/health`, `/actuator/*` and +HTTP 401. Unauthenticated paths: `/v1/health`, `/actuator/**` and `/v1/prom/targets/*`. ```yaml @@ -284,7 +284,7 @@ management: | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `management.metrics.export.prometheus.enabled` | Boolean | `true` | Enable Prometheus-compatible metrics at `/actuator/prometheus`. | -| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. `/actuator/*` is excluded from the REST authentication interceptor, so every endpoint listed here is reachable without a credential on port 8620. Prefer an allowlist over `"*"`. | +| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. `/actuator/**` is excluded from the REST authentication interceptor, so every endpoint listed here is reachable without a credential on port 8620. Prefer an allowlist over `"*"`. | ## Deployment Scenarios diff --git a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml index a929c876ea..9fdb7d0f1b 100644 --- a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml +++ b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml @@ -27,7 +27,7 @@ management: endpoints: web: exposure: - # Allowlist, not "*": /actuator/* is excluded from the REST auth + # Allowlist, not "*": /actuator/** is excluded from the REST auth # interceptor, so anything exposed here is anonymous on this port. include: "health,metrics,prometheus" diff --git a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml index 44adc2ed1f..0d0a0449cb 100644 --- a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml +++ b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml @@ -27,7 +27,7 @@ management: endpoints: web: exposure: - # Allowlist, not "*": /actuator/* is excluded from the REST auth + # Allowlist, not "*": /actuator/** is excluded from the REST auth # interceptor, so anything exposed here is anonymous on this port. include: "health,metrics,prometheus" diff --git a/hugegraph-store/docs/operations-guide.md b/hugegraph-store/docs/operations-guide.md index 9b942683f0..2d6501c696 100644 --- a/hugegraph-store/docs/operations-guide.md +++ b/hugegraph-store/docs/operations-guide.md @@ -3,7 +3,7 @@ This guide covers monitoring, troubleshooting, backup & recovery, and operational procedures for HugeGraph Store in production. > **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than -> `/v1/health`, `/actuator/*` and `/v1/prom/targets/*`, need HTTP Basic auth: +> `/v1/health`, `/actuator/**` and `/v1/prom/targets/*`, need HTTP Basic auth: > one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and > PD's `auth.secret-key` value as the password. A call without it gets HTTP > 401, which for a mutating step such as `balanceLeaders` means the step did From fdec006b1b4321a67bfe382470909d3e1b1e76bf Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sun, 6 Sep 2026 21:49:26 +0530 Subject: [PATCH 11/28] docs(pd): correct why actuator paths stay anonymous Three comments added by this branch claimed that /actuator/* would make the REST auth interceptor refuse nested actuator paths, and that /actuator/** is what keeps them open. That is not how the request reaches them. Actuator endpoints are served by WebMvcEndpointHandlerMapping. Like every AbstractHandlerMapping it auto-detects MappedInterceptor beans only, and an interceptor registered through WebMvcConfigurer.addInterceptors goes into the MVC registry rather than the bean factory, so it is attached to the MVC handler mappings alone. The interceptor never runs on an actuator path with or without the exclusion. Checked against the versions hg-pd-service resolves, spring-boot-actuator 2.5.14 and spring-webmvc 5.3.20, and reproduced on Boot 2.5.14 with Jetty: excluding /actuator/* and excluding a pattern that matches nothing both left /actuator/metrics/jvm.memory.used at 200 with the interceptor logging only the /v1 request, while the same interceptor published as a MappedInterceptor bean did 401 the actuator paths. /actuator/** stays in the exclusion list. It states the intent for nested probe paths and costs nothing; only the rationale was wrong. The exposure allowlist is what bounds which endpoints exist there. Comments and prose only; no behaviour change and no test assertion change. --- hugegraph-pd/docs/configuration.md | 2 +- .../rest/interceptor/AuthenticationConfigurer.java | 12 ++++++++---- .../org/apache/hugegraph/pd/rest/RestApiTest.java | 4 +++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index bacaaa46b2..092793ae49 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -284,7 +284,7 @@ management: | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `management.metrics.export.prometheus.enabled` | Boolean | `true` | Enable Prometheus-compatible metrics at `/actuator/prometheus`. | -| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. `/actuator/**` is excluded from the REST authentication interceptor, so every endpoint listed here is reachable without a credential on port 8620. Prefer an allowlist over `"*"`. | +| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. Actuator is served by its own handler mapping, which the REST authentication interceptor is not attached to, so every endpoint listed here is reachable without a credential on port 8620. This allowlist is what bounds which endpoints exist there, so prefer it over `"*"`. The interceptor's `/actuator/**` exclusion records the same intent but is not what makes these paths anonymous. | ## Deployment Scenarios diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java index 0100d4e4d4..8b94f41d20 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java @@ -32,10 +32,14 @@ public class AuthenticationConfigurer implements WebMvcConfigurer { public void addInterceptors(InterceptorRegistry registry) { registry.addInterceptor(restAuthentication) .addPathPatterns("/**") - // /actuator/** rather than /actuator/*: Spring's matcher does not - // treat /actuator/metrics/{name} or /actuator/health/{group} as one - // segment, and both are legitimate probe paths. What is reachable - // there is bounded by management.endpoints.web.exposure.include. + // /actuator/** states the intent for nested probe paths such as + // /actuator/metrics/{name}, but it is not what keeps them open. + // Actuator is served by WebMvcEndpointHandlerMapping, which only + // picks up MappedInterceptor beans; an interceptor added through + // this registry is not one, so it is attached to the MVC handler + // mappings alone and never sees an actuator request either way. + // What is reachable there is bounded by + // management.endpoints.web.exposure.include. .excludePathPatterns("/actuator/**", "/v1/health", "/v1/prom/targets/*"); } } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java index fce5d56857..42d04eae97 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java @@ -197,7 +197,9 @@ public void testProbePathsNeedNoCredential() throws URISyntaxException, IOExcept InterruptedException { assert statusWithoutCredential("/v1/health") == 200; assert statusWithoutCredential("/actuator/health") == 200; - // nested actuator paths are probe surface too; /actuator/* would 401 them + // Nested actuator paths are probe surface too. They stay open because + // actuator has its own handler mapping that the auth interceptor is not + // attached to, not because of the /actuator/** exclusion pattern. assert statusWithoutCredential("/actuator/metrics/jvm.memory.used") == 200; } From ffd88d31c913a54a10704d0b0c8e90e04ba8b068 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sun, 6 Sep 2026 21:59:00 +0530 Subject: [PATCH 12/28] docs(pd): correct the same rationale in the shipped conf comments The three config comments above management.endpoints.web.exposure.include still said the /actuator/** exclusion is what makes those endpoints anonymous. It is not: actuator is served by its own handler mapping that the REST auth interceptor is never attached to, so the exclusion pattern records intent rather than causing the behaviour. Two of these files ship to users in the dist conf, so they are more likely to be read than the docs table that was corrected earlier. Also says "is never a bean" where the interceptor comment said "is not one", which could be read as claiming the registry does not build a MappedInterceptor. It does; it just never registers one as a bean, and bean detection is what the actuator mapping requires. --- .../hg-pd-dist/src/assembly/static/conf/application.yml | 5 +++-- .../src/assembly/static/conf/application.yml.template | 5 +++-- .../pd/rest/interceptor/AuthenticationConfigurer.java | 4 ++-- .../hg-pd-service/src/main/resources/application.yml | 5 +++-- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml index 9fdb7d0f1b..ada9791922 100644 --- a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml +++ b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml @@ -27,8 +27,9 @@ management: endpoints: web: exposure: - # Allowlist, not "*": /actuator/** is excluded from the REST auth - # interceptor, so anything exposed here is anonymous on this port. + # Allowlist, not "*": actuator has its own handler mapping, which + # the REST auth interceptor is not attached to, so anything exposed + # here is anonymous on this port. This list is what bounds it. include: "health,metrics,prometheus" logging: diff --git a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template index 96211154a3..43bdf99fc4 100644 --- a/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template +++ b/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml.template @@ -29,8 +29,9 @@ management: endpoints: web: exposure: - # Allowlist, not "*": /actuator/** is excluded from the REST auth - # interceptor, so anything exposed here is anonymous on this port. + # Allowlist, not "*": actuator has its own handler mapping, which + # the REST auth interceptor is not attached to, so anything exposed + # here is anonymous on this port. This list is what bounds it. include: "health,metrics,prometheus" grpc: diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java index 8b94f41d20..762aebaaf5 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java @@ -35,8 +35,8 @@ public void addInterceptors(InterceptorRegistry registry) { // /actuator/** states the intent for nested probe paths such as // /actuator/metrics/{name}, but it is not what keeps them open. // Actuator is served by WebMvcEndpointHandlerMapping, which only - // picks up MappedInterceptor beans; an interceptor added through - // this registry is not one, so it is attached to the MVC handler + // picks up MappedInterceptor beans; one added through this + // registry is never a bean, so it is attached to the MVC handler // mappings alone and never sees an actuator request either way. // What is reachable there is bounded by // management.endpoints.web.exposure.include. diff --git a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml index 0d0a0449cb..f8c1ecea84 100644 --- a/hugegraph-pd/hg-pd-service/src/main/resources/application.yml +++ b/hugegraph-pd/hg-pd-service/src/main/resources/application.yml @@ -27,8 +27,9 @@ management: endpoints: web: exposure: - # Allowlist, not "*": /actuator/** is excluded from the REST auth - # interceptor, so anything exposed here is anonymous on this port. + # Allowlist, not "*": actuator has its own handler mapping, which + # the REST auth interceptor is not attached to, so anything exposed + # here is anonymous on this port. This list is what bounds it. include: "health,metrics,prometheus" grpc: From 10f590a02f91258accd99b672805eaa073521af8 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:01:08 +0530 Subject: [PATCH 13/28] fix(ci): keep PD's test secret out of the Store launch install-hstore.sh sources start-pd.sh and start-store.sh in the same shell, so the SPRING_APPLICATION_JSON that start-pd.sh exported stayed in the environment and reached Store's Spring context as well. Move the export into a subshell around the PD launch so it applies to PD only. --- .../hugegraph-dist/src/assembly/travis/start-pd.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh index 7e3198bedb..94d4f82fed 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh @@ -31,12 +31,16 @@ fi PD_DIR=$HOME_DIR/hugegraph-pd/apache-hugegraph-pd-$VersionInBash +pushd $PD_DIR # conf/application.yml ships auth.secret-key empty on purpose, so PD would # refuse every authenticated REST request. Supply a test-only secret; it must -# match the value the PD test suites send. -export SPRING_APPLICATION_JSON='{"auth":{"secret-key":"pd-ci-test-secret-not-for-production"}}' - -pushd $PD_DIR -. bin/start-hugegraph-pd.sh +# match the value the PD test suites send. Keep it inside this subshell: +# install-hstore.sh sources this script and start-store.sh in the same shell, +# and an exported SPRING_APPLICATION_JSON would reach Store's Spring context +# too. +( + export SPRING_APPLICATION_JSON='{"auth":{"secret-key":"pd-ci-test-secret-not-for-production"}}' + . bin/start-hugegraph-pd.sh +) sleep 10 popd From 687b90e2f21910b3c84d2a529833f5d2982cbfb7 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:01:08 +0530 Subject: [PATCH 14/28] fix(pd): harden the cluster-test PD config template The template PDNodeWrapper copies onto every PD node still had management.endpoints.web.exposure.include: "*" and no auth.secret-key, so REST credentials failed closed while /actuator/env and its neighbours stayed anonymously exposed. Give it the same allowlist and empty-secret contract as the other shipped PD configs, and add it to the shipped config check, whose header already claimed to cover it. From 8344ec9c251ba5238f9fff44a82fe772f0a61276 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:01:08 +0530 Subject: [PATCH 15/28] fix(pd): pin the actuator allowlist for carried-over configs Actuator has its own handler mapping that the REST auth interceptor is not attached to, so the exposure allowlist is what bounds the anonymous surface on 8620. A conf/application.yml carried over from an earlier release brings back include: "*", which also hands back the secret: the /env sanitizer keys off the property name, redacts auth.secret-key, and returns the SPRING_APPLICATION_JSON environment entry verbatim. Emit the allowlist from the Docker entrypoint in the same JSON document, where it outranks any mounted config file, and cover it in the entrypoint test. Name the same setting in the README upgrade note, which previously mentioned only auth.secret-key. From 470f4a7f5a17c6192c3da950bb33c56213f1b3a6 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:01:09 +0530 Subject: [PATCH 16/28] fix(docker): stop Compose creating the Hubble config path Both Hubble config files are generated by set-hubble-pd-password.sh and gitignored. With the short bind syntax, a first `docker compose up` that skipped the generator had Docker create an empty directory at that path, mount it over Hubble's config, and boot Hubble unconfigured. Switch both mounts to the long syntax with create_host_path disabled so Compose refuses to start instead, and assert it in the render contracts, which always ran the generator first and so could never catch this. From 504ab01b31609185ddea0fb8932c30fb65aa7d7b Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:01:09 +0530 Subject: [PATCH 17/28] fix(server): tidy wait-storage's PD credential handling The empty-password warning sat above the guard that decides whether PD is polled at all, so the standalone RocksDB topology logged it and then said it was skipping the storage wait two lines later. Move it next to the PD REST peer log. The 401 fast-abort also depended on $? being read as the very first statement in the loop body, with nothing saying so; any command inserted in front of it turned the abort back into a 300s retry. Read the status into a named copy and branch on that. From e420f9ab27f6c3019fe5cb1d43611a88753ac47b Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:04:02 +0530 Subject: [PATCH 18/28] fix(pd): harden the cluster-test PD config template The template PDNodeWrapper copies onto every PD node still had management.endpoints.web.exposure.include: "*" and no auth.secret-key, so REST credentials failed closed while /actuator/env and its neighbours stayed anonymously exposed. Give it the same allowlist and empty-secret contract as the other shipped PD configs, and add it to the shipped config check, whose header already claimed to cover it. --- .../static/conf/pd-application.yml.template | 13 ++++++++++++- .../src/assembly/travis/test-pd-shipped-config.sh | 8 +++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/hugegraph-cluster-test/hugegraph-clustertest-dist/src/assembly/static/conf/pd-application.yml.template b/hugegraph-cluster-test/hugegraph-clustertest-dist/src/assembly/static/conf/pd-application.yml.template index 87229aabcf..55a1afcbdb 100644 --- a/hugegraph-cluster-test/hugegraph-clustertest-dist/src/assembly/static/conf/pd-application.yml.template +++ b/hugegraph-cluster-test/hugegraph-clustertest-dist/src/assembly/static/conf/pd-application.yml.template @@ -27,7 +27,10 @@ management: endpoints: web: exposure: - include: "*" + # Allowlist, not "*": actuator has its own handler mapping, which + # the REST auth interceptor is not attached to, so anything exposed + # here is anonymous on this port. This list is what bounds it. + include: "health,metrics,prometheus" logging: config: 'file:./conf/log4j2.xml' @@ -43,6 +46,14 @@ server: # REST service port number port : $REST_PORT$ +auth: + # Shared secret checked against the password of the Basic credential on every + # authenticated REST request. Required and deliberately empty: a secret in the + # source tree is published to everyone. Generate one per deployment and give + # every REST client the same value. While empty, PD refuses every + # authenticated REST request. + secret-key: + pd: # Storage path data-path: ./pd_data diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh index b7b9ea734a..049de0f8c3 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh @@ -15,11 +15,12 @@ # See the License for the specific language governing permissions and # limitations under the License. # -# Every PD configuration that ships in the archive or the jar must carry the +# Every PD configuration that ships in an archive or in the jar must carry the # same REST hardening: no wildcard actuator exposure (that path is anonymous), # an auth.secret-key that is present and empty, and no copy of the secret that # earlier revisions published. A fix applied to one variant and not the others -# is what this catches. +# is what this catches, so the list below covers the PD distribution, the +# service jar, and the template the cluster test writes onto each PD node. set -euo pipefail @@ -53,7 +54,8 @@ check() { echo "PD shipped configuration hardening" for f in "${ROOT}"/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml* \ - "${ROOT}"/hugegraph-pd/hg-pd-service/src/main/resources/application.yml; do + "${ROOT}"/hugegraph-pd/hg-pd-service/src/main/resources/application.yml \ + "${ROOT}"/hugegraph-cluster-test/hugegraph-clustertest-dist/src/assembly/static/conf/pd-application.yml.template; do check "$f" done [[ "$FAIL" -eq 0 ]] && echo "all shipped PD configs pass" || { echo "shipped PD config check failed"; exit 1; } From edc95180c3b2d434966852eea95a700779464f5a Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:04:02 +0530 Subject: [PATCH 19/28] fix(pd): pin the actuator allowlist for carried-over configs Actuator has its own handler mapping that the REST auth interceptor is not attached to, so the exposure allowlist is what bounds the anonymous surface on 8620. A conf/application.yml carried over from an earlier release brings back include: "*", which also hands back the secret: the /env sanitizer keys off the property name, redacts auth.secret-key, and returns the SPRING_APPLICATION_JSON environment entry verbatim. Emit the allowlist from the Docker entrypoint in the same JSON document, where it outranks any mounted config file, and cover it in the entrypoint test. Name the same setting in the README upgrade note, which previously mentioned only auth.secret-key. --- hugegraph-pd/README.md | 10 +++++++--- .../hg-pd-dist/docker/docker-entrypoint.sh | 9 +++++++++ .../assembly/travis/test-pd-docker-entrypoint.sh | 15 ++++++++++++--- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 470c4b9864..6d4bdedc7f 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -309,9 +309,13 @@ docker/docker-compose-3pd-3store-3server.yml `wait-storage.sh` aborts the Server's startup on the first one rather than waiting out `WAIT_STORAGE_TIMEOUT_S`. - An existing `conf/application.yml` carried over from an earlier release has - no `auth` block. PD then starts with an empty secret and refuses every - authenticated REST request, logging an error that names `auth.secret-key`. - Add the key before upgrading. PD refuses to start if the key is set to the + no `auth` block, and still carries + `management.endpoints.web.exposure.include: "*"`. PD then starts with an + empty secret and refuses every authenticated REST request, logging an error + that names `auth.secret-key`, while `/actuator/env`, `/actuator/configprops` + and `/actuator/beans` stay anonymously readable on `8620`. Before upgrading, + add `auth.secret-key` and narrow that exposure to + `health,metrics,prometheus`. PD refuses to start if the key is set to the placeholder value that earlier revisions of this repository carried. ### Monitoring diff --git a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh index d38230b37d..991d69f6bd 100755 --- a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh +++ b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh @@ -85,9 +85,18 @@ require_env "HG_PD_AUTH_SECRET_KEY" # never logged. AUTH_JSON="\"auth\": { \"secret-key\": \"$(json_escape "${HG_PD_AUTH_SECRET_KEY}")\" }," +# The secret above lands in SPRING_APPLICATION_JSON, and actuator's /env +# sanitizer keys off the property name: it redacts auth.secret-key but returns +# the SPRING_APPLICATION_JSON environment entry verbatim, secret included. The +# image's own conf/application.yml already narrows the exposure, but a +# bind-mounted pre-1.8 config brings back include: "*". SPRING_APPLICATION_JSON +# outranks the config file, so pin the allowlist here too. +MANAGEMENT_JSON="\"management\": { \"endpoints\": { \"web\": { \"exposure\": { \"include\": \"health,metrics,prometheus\" } } } }," + SPRING_APPLICATION_JSON="$(cat < Date: Tue, 8 Sep 2026 00:04:02 +0530 Subject: [PATCH 20/28] fix(docker): stop Compose creating the Hubble config path Both Hubble config files are generated by set-hubble-pd-password.sh and gitignored. With the short bind syntax, a first `docker compose up` that skipped the generator had Docker create an empty directory at that path, mount it over Hubble's config, and boot Hubble unconfigured. Switch both mounts to the long syntax with create_host_path disabled so Compose refuses to start instead, and assert it in the render contracts, which always ran the generator first and so could never catch this. --- docker/docker-compose-3pd-3store-3server.yml | 10 +++++++++- docker/docker-compose-hstore.yml | 10 +++++++++- docker/test-compose.sh | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/docker/docker-compose-3pd-3store-3server.yml b/docker/docker-compose-3pd-3store-3server.yml index 22ec9733b9..5632eb6726 100644 --- a/docker/docker-compose-3pd-3store-3server.yml +++ b/docker/docker-compose-3pd-3store-3server.yml @@ -241,7 +241,15 @@ services: - "${HUBBLE_PUBLISH_HOST:-127.0.0.1}:8088:8088" volumes: - hubble-data:/hubble/data - - ./conf/hubble/hstore-ha.local.properties:/hubble/conf/hugegraph-hubble.properties:ro + - type: bind + source: ./conf/hubble/hstore-ha.local.properties + target: /hubble/conf/hugegraph-hubble.properties + read_only: true + bind: + # The file is generated by set-hubble-pd-password.sh and is + # gitignored. Without this, Docker would create an empty + # directory at that path and Hubble would boot unconfigured. + create_host_path: false healthcheck: test: - CMD-SHELL diff --git a/docker/docker-compose-hstore.yml b/docker/docker-compose-hstore.yml index 3f3a7c8395..6080049f96 100644 --- a/docker/docker-compose-hstore.yml +++ b/docker/docker-compose-hstore.yml @@ -120,7 +120,15 @@ services: - "${HUBBLE_PUBLISH_HOST:-127.0.0.1}:8088:8088" volumes: - hubble-data:/hubble/data - - ./conf/hubble/hstore.local.properties:/hubble/conf/hugegraph-hubble.properties:ro + - type: bind + source: ./conf/hubble/hstore.local.properties + target: /hubble/conf/hugegraph-hubble.properties + read_only: true + bind: + # The file is generated by set-hubble-pd-password.sh and is + # gitignored. Without this, Docker would create an empty + # directory at that path and Hubble would boot unconfigured. + create_host_path: false healthcheck: test: - CMD-SHELL diff --git a/docker/test-compose.sh b/docker/test-compose.sh index d4988a9d51..5e9abfea06 100644 --- a/docker/test-compose.sh +++ b/docker/test-compose.sh @@ -112,6 +112,20 @@ assert_hubble() { ' "${rendered}" >/dev/null } +# The two *.local.properties files are generated by set-hubble-pd-password.sh +# and gitignored. Their binds must disable host-path creation, so a first +# `docker compose up` that skipped the generator fails instead of getting an +# empty directory mounted over Hubble's config. +assert_hubble_bind_pinned() { + local rendered="$1" + jq -e ' + any(.services.hubble.volumes[]; + .type == "bind" and + .target == "/hubble/conf/hugegraph-hubble.properties" and + .bind.create_host_path == false) + ' "${rendered}" >/dev/null +} + assert_standalone() { local rendered="$1" assert_common "${rendered}" \ @@ -139,6 +153,7 @@ assert_hstore() { '["hubble","pd","server","store"]' \ '["hubble-data","pd-data","store-data"]' assert_hubble "${rendered}" "hstore.local.properties" server + assert_hubble_bind_pinned "${rendered}" jq -e ' .services.pd.image == "hugegraph/pd:ci-version" and .services.store.image == "hugegraph/store:ci-version" and @@ -173,6 +188,7 @@ assert_ha() { '["hg-pd0-data","hg-pd1-data","hg-pd2-data","hg-store0-data","hg-store1-data","hg-store2-data","hubble-data"]' assert_hubble "${rendered}" "hstore-ha.local.properties" \ server0 server1 server2 + assert_hubble_bind_pinned "${rendered}" jq -e ' all([.services.pd0, .services.pd1, .services.pd2][]; .image == "hugegraph/pd:ci-version" and From 63a54e972e96516443f8e507fbc025533941cce8 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:04:02 +0530 Subject: [PATCH 21/28] fix(server): tidy wait-storage's PD credential handling The empty-password warning sat above the guard that decides whether PD is polled at all, so the standalone RocksDB topology logged it and then said it was skipping the storage wait two lines later. Move it next to the PD REST peer log. The 401 fast-abort also depended on $? being read as the very first statement in the loop body, with nothing saying so; any command inserted in front of it turned the abort back into a 300s retry. Read the status into a named copy and branch on that. --- .../src/assembly/static/bin/wait-storage.sh | 14 ++++++++++---- .../src/assembly/travis/test-wait-storage.sh | 16 +++++++++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh index 71b6a61fed..a282037621 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh @@ -46,9 +46,6 @@ log() { # curl on stdin as a config file. PD_AUTH_USER="${PD_AUTH_USER:-store}" PD_AUTH_PASSWORD="${PD_AUTH_PASSWORD:-}" -if [ -z "${PD_AUTH_PASSWORD}" ]; then - log "WARN: PD_AUTH_PASSWORD is empty; PD will answer 401 unless it runs without auth" -fi # curl -K reads one option per line and takes the value as a quoted string # whose only escapes are \\ \" \t \n \r \v. Backslash first, then the rest; # an unescaped line break would end the option early and send a truncated @@ -119,6 +116,11 @@ if env | grep '^hugegraph\.' > /dev/null; then export PD_REST_LIST log "PD REST peers = $PD_REST_LIST" + # Only worth saying where PD is actually polled: topologies without + # pd.peers never send this credential anywhere. + if [ -z "${PD_AUTH_PASSWORD}" ]; then + log "WARN: PD_AUTH_PASSWORD is empty; PD will answer 401 unless it runs without auth" + fi log "Timeout = ${WAIT_STORAGE_TIMEOUT_S}s" timeout "${WAIT_STORAGE_TIMEOUT_S}s" bash -c " @@ -150,7 +152,11 @@ if env | grep '^hugegraph\.' > /dev/null; then } until PD_REST=\$(check_any_pd_stores); do - if [ \$? -eq 2 ]; then exit 2; fi + # Must stay the first statement in the loop: any command in + # front of it would overwrite \$? and turn the 401 abort back + # into a 300s retry. + rc=\$? + if [ \"\$rc\" -eq 2 ]; then exit 2; fi log 'No Up store yet, retrying in 5s' sleep 5 done diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh index 3238a2fbff..a78f6e60ab 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh @@ -255,4 +255,18 @@ assert_equal "no retry after 401" "${PD0}" "$(cat "${CALL_LOG}")" [[ "${CASE_OUTPUT}" != *"Timeout waiting"* ]] || fail "401 was reported as a timeout" echo " PASS 401 aborts without retry" -echo "7 passed, 0 failed" +# The standalone RocksDB topology never reaches PD, so it must not warn about +# a PD credential it will not send. +: > "${DIST_ROOT}/conf/graphs/hugegraph.properties" +CASE_OUTPUT=$(env \ + PD_AUTH_PASSWORD="" \ + 'hugegraph.backend=rocksdb' \ + "${DIST_ROOT}/bin/wait-storage.sh" 2>&1) +CASE_RC=$? +assert_equal "no-PD topology rc" "0" "${CASE_RC}" +assert_output "No pd.peers configured, skipping storage wait" +[[ "${CASE_OUTPUT}" != *"PD_AUTH_PASSWORD is empty"* ]] || \ + fail "warned about an unused PD credential with no pd.peers configured" +echo " PASS no credential warning without pd.peers" + +echo "8 passed, 0 failed" From 61ae6f5434c45b2815afeb120f7736807e24dc32 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:47:25 +0530 Subject: [PATCH 22/28] docs(pd): note that the Docker image pins the actuator allowlist The entrypoint emits management.endpoints.web.exposure.include in SPRING_APPLICATION_JSON, which outranks a mounted conf/application.yml, so an operator who edits that key in a mounted config gets no effect and no message. The table still presented it as freely settable. --- hugegraph-pd/docs/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index 092793ae49..9639167b9a 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -284,7 +284,7 @@ management: | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `management.metrics.export.prometheus.enabled` | Boolean | `true` | Enable Prometheus-compatible metrics at `/actuator/prometheus`. | -| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. Actuator is served by its own handler mapping, which the REST authentication interceptor is not attached to, so every endpoint listed here is reachable without a credential on port 8620. This allowlist is what bounds which endpoints exist there, so prefer it over `"*"`. The interceptor's `/actuator/**` exclusion records the same intent but is not what makes these paths anonymous. | +| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. Actuator is served by its own handler mapping, which the REST authentication interceptor is not attached to, so every endpoint listed here is reachable without a credential on port 8620. This allowlist is what bounds which endpoints exist there, so prefer it over `"*"`. The interceptor's `/actuator/**` exclusion records the same intent but is not what makes these paths anonymous. In the PD Docker image this key is pinned: the entrypoint emits it in `SPRING_APPLICATION_JSON`, which outranks a mounted `conf/application.yml`, so changing it there has no effect. | ## Deployment Scenarios From b98db4188c72463e5a7c2e4376b55680026e8637 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 16:29:55 +0530 Subject: [PATCH 23/28] fix(ci): unbreak the Compose render contracts and name failures assert_hubble_bind_pinned read create_host_path back out of `docker compose config --format json`, but compose-go tags ServiceVolumeBind.CreateHostPath omitempty, so a false is dropped from the render and the long syntax it pins looks identical to the short syntax it pins away from. Compose 2.38.2, which the ubuntu-24.04 runner ships, does exactly that, and build-server (rocksdb, 11) has failed on it since the assertion landed. Read the Compose file instead, where the contract is actually written. Every other assertion is a bare `jq -e ... >/dev/null` under `set -e`, so that failure printed nothing at all and the job log said only "exit code 1". An ERR trap now names the file, line and command that failed. Also reject a non-ASCII secret in set-hubble-pd-password.sh, which Hubble decodes as ISO-8859-1 while PD compares UTF-8 bytes, and escape a leading space, which java.util.Properties would otherwise discard so that Hubble alone read a shortened secret. The README recipe now passes the secret in the environment rather than in argv, where ps shows it to every local account for as long as the helper runs. --- docker/README.md | 8 +++-- docker/set-hubble-pd-password.sh | 24 ++++++++++++++ docker/test-compose.sh | 54 ++++++++++++++++++++++++++------ 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/docker/README.md b/docker/README.md index ffb10e6554..f8b990dba0 100644 --- a/docker/README.md +++ b/docker/README.md @@ -43,9 +43,11 @@ contains a single quote or newline. printf "HUGEGRAPH_ADMIN_PASSWORD='%s'\nHUGEGRAPH_AUTH_TOKEN_SECRET='%s'\nHG_PD_AUTH_SECRET_KEY='%s'\n" \ 'replace-with-your-password' "${jwt_secret}" "${pd_secret}" > .env # Hubble reads the PD secret from a file, not from .env: generate the - # untracked properties files the HStore topologies mount - ./set-hubble-pd-password.sh hstore "${pd_secret}" - ./set-hubble-pd-password.sh hstore-ha "${pd_secret}" + # untracked properties files the HStore topologies mount. Passed in the + # environment rather than as an argument, which `ps` shows to every local + # account for as long as the helper runs. + HG_PD_AUTH_SECRET_KEY="${pd_secret}" ./set-hubble-pd-password.sh hstore + HG_PD_AUTH_SECRET_KEY="${pd_secret}" ./set-hubble-pd-password.sh hstore-ha ) ``` diff --git a/docker/set-hubble-pd-password.sh b/docker/set-hubble-pd-password.sh index 535e847a28..85f410e58c 100755 --- a/docker/set-hubble-pd-password.sh +++ b/docker/set-hubble-pd-password.sh @@ -27,6 +27,12 @@ # special, and backslashes are doubled for the .properties format. Run this # before `docker compose up`: if the target is missing Docker creates an empty # directory at the bind path and Hubble starts with no configuration. +# +# The secret must be printable ASCII. PD compares it as UTF-8 bytes +# (Authentication.verifySecret), while Hubble reads this file through +# commons-configuration2, whose DEFAULT_ENCODING is ISO-8859-1, so a non-ASCII +# secret decodes to different bytes on the two sides and gives a permanent 401 +# with no diagnostic anywhere. The README recipe generates hex, which is safe. set -euo pipefail name=${1:?usage: $0 [secret]} @@ -40,8 +46,26 @@ out="${dir}/${name}.local.properties" case "$secret" in *$'\n'*|*$'\r'*) echo "secret contains a line break, which a .properties value cannot hold" >&2; exit 1 ;; esac +# LC_ALL=C so the range is ordinal and the walk byte-wise: under the caller's +# collation a non-ASCII character can sort inside \x20-\x7e and slip through. +is_printable_ascii() { + local LC_ALL=C + case "$1" in + *[!$'\x20'-$'\x7e']*) return 1 ;; + esac +} +is_printable_ascii "$secret" || { + echo "secret must be printable ASCII: Hubble reads .properties as ISO-8859-1, PD compares as UTF-8" >&2 + exit 1 +} escaped=${secret//\\/\\\\} +# java.util.Properties skips whitespace between the separator and the value, so +# a secret that starts with a space would reach Hubble shortened while PD and +# the Server kept the original. A backslash before it keeps that first byte. +case "$escaped" in + [$' \t']*) escaped="\\${escaped}" ;; +esac tmp=$(mktemp "${out}.XXXXXX") trap 'rm -f "$tmp"' EXIT { diff --git a/docker/test-compose.sh b/docker/test-compose.sh index 8ddd6a10bc..6f455a0d4a 100644 --- a/docker/test-compose.sh +++ b/docker/test-compose.sh @@ -18,6 +18,12 @@ set -Eeuo pipefail +# Most assertions below are a bare `jq -e ... >/dev/null`, so `set -e` used to +# end the run with no output at all and a CI log that said only "exit code 1". +# Name the command and line that failed instead; the trap fires once per frame, +# so the innermost assertion comes first and its callers follow. +trap 'echo "FAILED at ${BASH_SOURCE[0]}:${LINENO}: ${BASH_COMMAND}" >&2' ERR + DOCKER_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PASSWORD="ci-compose-password" SECRET="0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" @@ -150,14 +156,30 @@ assert_hubble() { # and gitignored. Their binds must disable host-path creation, so a first # `docker compose up` that skipped the generator fails instead of getting an # empty directory mounted over Hubble's config. +# +# This reads the Compose file rather than the render on purpose. compose-go +# tags ServiceVolumeBind.CreateHostPath `json:"create_host_path,omitempty"` +# (checked in compose-go v2.7.0, the version inside the Compose 2.38.2 that +# ubuntu-24.04 runners ship), and omitempty drops a false bool, so +# `config --format json` emits the same `"bind": {}` for the long syntax with +# create_host_path disabled and for the short syntax this pins away from. The +# render simply cannot carry this contract on that version. assert_hubble_bind_pinned() { - local rendered="$1" - jq -e ' - any(.services.hubble.volumes[]; - .type == "bind" and - .target == "/hubble/conf/hugegraph-hubble.properties" and - .bind.create_host_path == false) - ' "${rendered}" >/dev/null + local file="$1" properties="$2" + grep -Fq "source: ./conf/hubble/${properties}" "${file}" || { + echo "${file}: the ${properties} mount is not in long bind syntax" >&2 + return 1 + } + grep -Fq "create_host_path: false" "${file}" || { + echo "${file}: the ${properties} bind does not disable" \ + "create_host_path" >&2 + return 1 + } + if grep -Fq "./conf/hubble/${properties}:/hubble/conf" "${file}"; then + echo "${file}: the ${properties} mount is back on the short bind" \ + "syntax, which lets Docker create an empty directory there" >&2 + return 1 + fi } assert_standalone() { @@ -187,7 +209,8 @@ assert_hstore() { '["hubble","pd","server","store"]' \ '["hubble-data","pd-data","store-data"]' assert_hubble "${rendered}" "hstore.local.properties" server - assert_hubble_bind_pinned "${rendered}" + assert_hubble_bind_pinned "${DOCKER_DIR}/docker-compose-hstore.yml" \ + "hstore.local.properties" jq -e ' .services.pd.image == "hugegraph/pd:ci-version" and .services.store.image == "hugegraph/store:ci-version" and @@ -222,7 +245,9 @@ assert_ha() { '["hg-pd0-data","hg-pd1-data","hg-pd2-data","hg-store0-data","hg-store1-data","hg-store2-data","hubble-data"]' assert_hubble "${rendered}" "hstore-ha.local.properties" \ server0 server1 server2 - assert_hubble_bind_pinned "${rendered}" + assert_hubble_bind_pinned \ + "${DOCKER_DIR}/docker-compose-3pd-3store-3server.yml" \ + "hstore-ha.local.properties" jq -e ' all([.services.pd0, .services.pd1, .services.pd2][]; .image == "hugegraph/pd:ci-version" and @@ -344,6 +369,17 @@ hubble_password_helper_check() { echo "set-hubble-pd-password.sh accepted an empty secret" >&2; exit 1; } ! "${DOCKER_DIR}/set-hubble-pd-password.sh" nope 'x' 2>/dev/null || { echo "set-hubble-pd-password.sh accepted an unknown topology" >&2; exit 1; } + # Hubble decodes this file as ISO-8859-1 and PD compares UTF-8 bytes, so a + # non-ASCII secret would be a permanent 401 with nothing in any log. + ! "${DOCKER_DIR}/set-hubble-pd-password.sh" hstore 'pässwörd' 2>/dev/null || { + echo "set-hubble-pd-password.sh accepted a non-ASCII secret" >&2; exit 1; } + # java.util.Properties drops whitespace after the separator, so a leading + # space has to survive as an escape or Hubble reads a shortened secret. + "${DOCKER_DIR}/set-hubble-pd-password.sh" hstore ' lead' >/dev/null + line=$(grep '^operations\.pd\.password=' "${f}") + [[ "${line}" == 'operations.pd.password=\ lead' ]] || { + echo "set-hubble-pd-password.sh left a leading space unescaped: ${line}" >&2 + exit 1; } # put the CI value back for the render/smoke that follows "${DOCKER_DIR}/set-hubble-pd-password.sh" hstore "${PD_SECRET}" >/dev/null } From ae2a39a5ff6154cb771988000e6c05e0e290cc9e Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 16:32:46 +0530 Subject: [PATCH 24/28] fix(pd): close the remaining review gaps on the REST credential wait-storage.sh returned 2 on the first 401, so one refusing peer ended the wait before the rest of PD_REST_LIST was tried: a Server died during a rolling secret rotation, or against a pre-1.8 peer that answers 200 to any password, even though the next peer would have accepted it. Remember the refusal, finish the pass, and return 2 only when no peer produced an Up store, which keeps the fail-fast for a fleet-wide wrong secret. The Docker image no longer pins the actuator allowlist out of reach. HG_PD_ACTUATOR_EXPOSURE defaults to it and refuses a wildcard, so an operator can expose /actuator/loggers without giving up the hardening that keeps the secret out of /actuator/env. json_escape pins LC_ALL=C. The base image sets LC_ALL=en_US.UTF-8, under which the walk counts characters rather than bytes and the comparison sorts by collation rather than by code point. RestApiTest asserts != 401 on the two health probes. They also answer 503 when PD is merely unhealthy, and == 200 read that as an auth regression. The shipped-config check now compares the actuator allowlist exactly instead of only rejecting a wildcard, so a reordered, duplicated or extra endpoint fails too. The Store deployment guide and README carry the credential note and the -u that the operations guide already had, and both guides say how to keep the secret out of argv. --- hugegraph-pd/docs/configuration.md | 12 +++- .../hg-pd-dist/docker/docker-entrypoint.sh | 27 +++++++- .../apache/hugegraph/pd/rest/RestApiTest.java | 10 ++- .../src/assembly/static/bin/wait-storage.sh | 17 ++++- .../travis/test-pd-docker-entrypoint.sh | 63 +++++++++++++++++-- .../assembly/travis/test-pd-shipped-config.sh | 19 +++++- .../src/assembly/travis/test-wait-storage.sh | 28 +++++++-- hugegraph-store/README.md | 6 +- hugegraph-store/docs/deployment-guide.md | 36 +++++++++-- 9 files changed, 190 insertions(+), 28 deletions(-) diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index 9639167b9a..39c0e0b92d 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -104,6 +104,16 @@ the Docker image takes `HG_PD_AUTH_SECRET_KEY`. curl -u hg: http://:8620/v1/stores ``` +`-u` puts the secret in curl's process arguments, where any local account can +read it while the call runs, and PD REST is plain HTTP. On a shared host, or +across a network you do not control, keep the secret out of `argv` by reading +it from a file mode 0600: + +```bash +printf 'user = "hg:%s"\n' "${PD_SECRET}" > pd.curlrc && chmod 600 pd.curlrc +curl -K pd.curlrc http://:8620/v1/stores +``` + ### Raft Consensus Settings Controls Raft consensus for PD cluster coordination. @@ -284,7 +294,7 @@ management: | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `management.metrics.export.prometheus.enabled` | Boolean | `true` | Enable Prometheus-compatible metrics at `/actuator/prometheus`. | -| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. Actuator is served by its own handler mapping, which the REST authentication interceptor is not attached to, so every endpoint listed here is reachable without a credential on port 8620. This allowlist is what bounds which endpoints exist there, so prefer it over `"*"`. The interceptor's `/actuator/**` exclusion records the same intent but is not what makes these paths anonymous. In the PD Docker image this key is pinned: the entrypoint emits it in `SPRING_APPLICATION_JSON`, which outranks a mounted `conf/application.yml`, so changing it there has no effect. | +| `management.endpoints.web.exposure.include` | String | `"health,metrics,prometheus"` | Actuator endpoints to expose. Actuator is served by its own handler mapping, which the REST authentication interceptor is not attached to, so every endpoint listed here is reachable without a credential on port 8620. This allowlist is what bounds which endpoints exist there, so prefer it over `"*"`. The interceptor's `/actuator/**` exclusion records the same intent but is not what makes these paths anonymous. In the PD Docker image the entrypoint emits this key in `SPRING_APPLICATION_JSON`, which outranks a mounted `conf/application.yml`, so editing it there has no effect; set `HG_PD_ACTUATOR_EXPOSURE` on the container instead. That variable defaults to the same allowlist and refuses a value containing `*`, because `/actuator/env` returns the `SPRING_APPLICATION_JSON` entry verbatim, PD's REST secret included. | ## Deployment Scenarios diff --git a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh index 991d69f6bd..77af2af89d 100755 --- a/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh +++ b/hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh @@ -30,7 +30,16 @@ require_env() { # remaining C0 control character as \uXXXX. Dropping only LF, as an earlier # version did, left CR and TAB to produce invalid JSON and a container that # failed before startup. +# +# LC_ALL=C is what makes the walk deterministic. eclipse-temurin:11-jre-jammy +# sets LC_ALL=en_US.UTF-8, and under that locale `${#s}`/`${s:i:1}` iterate +# characters rather than bytes and `<` sorts by collation rather than by code +# point. Pinned to C, iteration is byte-wise, the comparison is ordinal, and +# the bytes of a non-ASCII character (0x80 and up) pass through untouched, +# which is still valid JSON. The escape branch then only sees single-byte +# ASCII, so this pin is the only behavioral change. json_escape() { + local LC_ALL=C local s="$1" out="" i c s=${s//\\/\\\\} s=${s//\"/\\\"} @@ -80,6 +89,16 @@ require_env "HG_PD_AUTH_SECRET_KEY" : "${HG_PD_REST_PORT:=8620}" : "${HG_PD_DATA_PATH:=/hugegraph-pd/pd_data}" : "${HG_PD_INITIAL_STORE_COUNT:=1}" +# Actuator endpoints reachable without a credential. Hardened by default; an +# operator who needs /actuator/info or /actuator/loggers from this image opts +# in deliberately instead of losing the endpoint. "*" is refused: /actuator/env +# returns the SPRING_APPLICATION_JSON entry below verbatim, secret included. +: "${HG_PD_ACTUATOR_EXPOSURE:=health,metrics,prometheus}" +if [[ "${HG_PD_ACTUATOR_EXPOSURE}" == *'*'* ]]; then + echo "ERROR: HG_PD_ACTUATOR_EXPOSURE must not be a wildcard;" \ + "/actuator/env would publish the PD REST secret" >&2 + exit 2 +fi # Secret for REST Basic authentication (auth.secret-key). Required above and # never logged. @@ -90,8 +109,11 @@ AUTH_JSON="\"auth\": { \"secret-key\": \"$(json_escape "${HG_PD_AUTH_SECRET_KEY} # the SPRING_APPLICATION_JSON environment entry verbatim, secret included. The # image's own conf/application.yml already narrows the exposure, but a # bind-mounted pre-1.8 config brings back include: "*". SPRING_APPLICATION_JSON -# outranks the config file, so pin the allowlist here too. -MANAGEMENT_JSON="\"management\": { \"endpoints\": { \"web\": { \"exposure\": { \"include\": \"health,metrics,prometheus\" } } } }," +# outranks the config file, so pin the allowlist here too, from the env-driven +# default above rather than from a literal: every other PD setting in this +# entrypoint is env-driven, and pinning a literal took away the operator's +# only way to expose another endpoint from this image. +MANAGEMENT_JSON="\"management\": { \"endpoints\": { \"web\": { \"exposure\": { \"include\": \"$(json_escape "${HG_PD_ACTUATOR_EXPOSURE}")\" } } } }," SPRING_APPLICATION_JSON="$(cat < /dev/null; then # curl stays out of the grep pipeline so its status code is # readable: a 401 is a wrong secret, not a storage problem, and # retrying it for 300s only hides that. + # + # A 401 is remembered rather than returned at once, so one + # refusing peer no longer ends the wait before the rest of + # PD_REST_LIST is tried. That case is real: during a rolling + # secret rotation, or against a pre-1.8 PD that answers 200 to + # any password, a Server used to die even though the next peer + # would have accepted it. Returning 2 only when no peer produced + # an Up store keeps the fail-fast for a fleet-wide wrong secret, + # which still aborts on the first pass instead of retrying 300s. check_any_pd_stores() { + refused= for peer in \$(echo \"\$PD_REST_LIST\" | tr ',' ' '); do body=\$(printf 'user = \"%s:%s\"\n' \ \"\$PD_AUTH_CURL_USER\" \"\$PD_AUTH_CURL_PASSWORD\" | \ @@ -140,14 +150,17 @@ if env | grep '^hugegraph\.' > /dev/null; then \"http://\${peer}/v1/stores\" 2>/dev/null) code=\${body##*\$'\n'} if [ \"\$code\" = 401 ]; then - log \"ERROR: PD at \${peer} refused the credential (401): PD_AUTH_PASSWORD must match PD's auth.secret-key\" >&2 - return 2 + log \"ERROR: PD at \${peer} refused the credential (401):\" >&2 + log ' PD_AUTH_PASSWORD must match PD auth.secret-key' >&2 + refused=1 + continue fi if printf '%s' \"\$body\" | grep -qi '\"state\"[[:space:]]*:[[:space:]]*\"Up\"'; then echo \"\$peer\" return 0 fi done + [ -z \"\$refused\" ] || return 2 return 1 } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh index 2a038272a1..ed39b67492 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-docker-entrypoint.sh @@ -45,9 +45,13 @@ STUB chmod +x "${TMP_DIR}/bin/start-hugegraph-pd.sh" "${TMP_DIR}/docker-entrypoint.sh" run_case() { - local name="$1" secret="$2" + # run_case [expected exposure] [extra NAME=VALUE ...] + local name="$1" secret="$2" expected_exposure="${3:-health,metrics,prometheus}" + shift 2 + (( $# == 0 )) || shift # drop the exposure argument; the rest is extra env local out if ! out=$(cd "${TMP_DIR}" && env \ + "$@" \ HG_PD_GRPC_HOST=pd0 \ HG_PD_RAFT_ADDRESS=pd0:8610 \ HG_PD_RAFT_PEERS_LIST=pd0:8610 \ @@ -60,7 +64,8 @@ run_case() { return fi - if ! SECRET="${secret}" python3 - "${TMP_DIR}/spring.json" <<'PY' + if ! SECRET="${secret}" EXPOSURE="${expected_exposure}" \ + python3 - "${TMP_DIR}/spring.json" <<'PY' import json, os, sys with open(sys.argv[1], encoding="utf-8") as fh: doc = json.load(fh) @@ -71,12 +76,13 @@ if got != want: sys.exit(1) exposure = doc.get("management", {}).get("endpoints", {}).get("web", {}) exposure = exposure.get("exposure", {}).get("include") -if exposure != "health,metrics,prometheus": - print(" actuator exposure is not pinned to the allowlist: %r" % (exposure,)) +if exposure != os.environ["EXPOSURE"]: + print(" actuator exposure is %r, expected %r" + % (exposure, os.environ["EXPOSURE"])) sys.exit(1) PY then - echo " FAIL ${name}: invalid JSON, secret did not round-trip, or exposure is not pinned" + echo " FAIL ${name}: invalid JSON, secret did not round-trip, or exposure is wrong" FAIL=$((FAIL + 1)) return fi @@ -94,6 +100,53 @@ run_case "backslash and quote" 'a\"b' run_case "non-ascii" 'sécrèt-2026' run_case "spaces" 'two words' +# json_escape walks the secret with ${#s} and ${s:i:1} and compares with `<`. +# Both are locale-sensitive, and the base image (eclipse-temurin:11-jre-jammy) +# sets LC_ALL=en_US.UTF-8, so the same secret has to survive a UTF-8 locale +# exactly as it does under C. Pick a UTF-8 locale the host actually has: +# an ungenerated one silently falls back to C and makes the case vacuous. +# Captured rather than piped into grep: `grep -q` exits at the first match, and +# under pipefail the SIGPIPE that gives `locale -a` fails the whole pipeline. +AVAILABLE_LOCALES=$(locale -a 2>/dev/null || true) +UTF8_LOCALE="" +for candidate in C.UTF-8 en_US.UTF-8 en_US.utf8; do + if grep -Fqix -- "${candidate}" <<<"${AVAILABLE_LOCALES}"; then + UTF8_LOCALE="${candidate}" + break + fi +done +if [[ -n "${UTF8_LOCALE}" ]]; then + run_case "non-ascii under ${UTF8_LOCALE}" 'sécrèt-2026' \ + 'health,metrics,prometheus' "LC_ALL=${UTF8_LOCALE}" + run_case "control chars under ${UTF8_LOCALE}" "$(printf 'a\rb\tc')" \ + 'health,metrics,prometheus' "LC_ALL=${UTF8_LOCALE}" +else + echo " SKIP UTF-8 locale cases: no UTF-8 locale on this host" +fi + +# The allowlist is the default, not a pin: an operator who needs another +# endpoint from this image opts in rather than losing the endpoint entirely. +run_case "actuator exposure override" 'aVerySecretValue123' \ + 'health,metrics,prometheus,loggers' \ + 'HG_PD_ACTUATOR_EXPOSURE=health,metrics,prometheus,loggers' + +# ...but not all the way back to the hole this closed: /actuator/env returns +# the SPRING_APPLICATION_JSON entry verbatim, secret included. +for wildcard in '*' 'health,*'; do + if (cd "${TMP_DIR}" && env \ + HG_PD_GRPC_HOST=pd0 HG_PD_RAFT_ADDRESS=pd0:8610 \ + HG_PD_RAFT_PEERS_LIST=pd0:8610 HG_PD_INITIAL_STORE_LIST=store0:8500 \ + HG_PD_AUTH_SECRET_KEY='aVerySecretValue123' \ + HG_PD_ACTUATOR_EXPOSURE="${wildcard}" \ + ./docker-entrypoint.sh >/dev/null 2>&1); then + echo " FAIL wildcard exposure '${wildcard}' was accepted" + FAIL=$((FAIL + 1)) + else + echo " PASS wildcard exposure '${wildcard}' is refused" + PASS=$((PASS + 1)) + fi +done + # The secret is required, and must never be echoed to the log if (cd "${TMP_DIR}" && env \ HG_PD_GRPC_HOST=pd0 HG_PD_RAFT_ADDRESS=pd0:8610 \ diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh index 049de0f8c3..623fd1efb4 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh @@ -26,6 +26,11 @@ set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../../.." && pwd)" PUBLISHED_SECRET='FXQXbJtbCLxODc6tGci732pkH1cyf8Qg' +# The allowlist these files must carry, spelled out. An exact comparison rather +# than "no wildcard": a missing include:, a reordered or duplicated entry, and +# an extra endpoint are all changes to what this port serves anonymously, and +# each of them used to pass. +EXPECTED_EXPOSURE='health,metrics,prometheus' FAIL=0 check() { @@ -36,9 +41,17 @@ check() { # include: above this block must not satisfy the check by accident. local exposure exposure=$(awk '/^[[:space:]]*exposure:/ {found = 1; next} - found && /^[[:space:]]*include:/ {sub(/^[[:space:]]*include:[[:space:]]*/, ""); print; exit}' "$file") - if [[ "$exposure" == *'*'* ]]; then - echo " FAIL ${rel}: actuator exposure is a wildcard (${exposure})"; FAIL=1 + found && /^[[:space:]]*include:/ { + sub(/^[[:space:]]*include:[[:space:]]*/, "") + sub(/[[:space:]]+$/, "") + print; exit + }' "$file") + # YAML quoting is the file's business, not this contract's + exposure=${exposure#\"}; exposure=${exposure%\"} + exposure=${exposure#\'}; exposure=${exposure%\'} + if [[ "$exposure" != "${EXPECTED_EXPOSURE}" ]]; then + echo " FAIL ${rel}: actuator exposure must be exactly" \ + "'${EXPECTED_EXPOSURE}', got '${exposure}'"; FAIL=1 fi if ! grep -qE '^[[:space:]]*secret-key:[[:space:]]*$' "$file"; then echo " FAIL ${rel}: auth.secret-key must be present and empty"; FAIL=1 diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh index a78f6e60ab..dde216a26e 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh @@ -170,6 +170,13 @@ printf '%s\n' "${count}" > "${MOCK_COUNT_FILE}" if [[ "${MOCK_SCENARIO}" == "auth-401" ]]; then respond '{"status":-1,"error":"Unauthorized"}' 401 +elif [[ "${MOCK_SCENARIO}" == "one-401" && \ + "${url}" == "http://pd0:8620/v1/stores" ]]; then + # One stale peer mid-rotation, or a pre-1.8 peer that took any password + respond '{"status":-1,"error":"Unauthorized"}' 401 +elif [[ "${MOCK_SCENARIO}" == "one-401" && \ + "${url}" == "http://pd1:8620/v1/stores" ]]; then + respond '{"stores":[{"state":"Up"}]}' 200 elif [[ "${MOCK_SCENARIO}" == "pd1-up" && \ "${url}" == "http://pd1:8620/v1/stores" ]]; then respond '{"stores":[{"state":"Up"}]}' 200 @@ -247,13 +254,24 @@ if grep -Fv -- 'user = "test-user:a\r\nb\\c\"d"' "${CONFIG_LOG}" | grep -q .; th fi echo " PASS line break in secret" -# A 401 is a wrong secret, not a storage problem: abort at once, name the cause. +# A fleet-wide wrong secret is not a storage problem: finish the pass so every +# refusal is named, then abort rather than retrying it for the full 300s. run_case "auth-401" "pd0:8620,pd1:8620" 9 -[[ "${CASE_RC}" -ne 0 ]] || fail "a 401 from PD must abort" +[[ "${CASE_RC}" -ne 0 ]] || fail "a 401 from every peer must abort" assert_output "refused the credential (401)" -assert_equal "no retry after 401" "${PD0}" "$(cat "${CALL_LOG}")" +assert_equal "one pass, no retry after 401" "${TWO_CALLS}" "$(cat "${CALL_LOG}")" [[ "${CASE_OUTPUT}" != *"Timeout waiting"* ]] || fail "401 was reported as a timeout" -echo " PASS 401 aborts without retry" +echo " PASS 401 from every peer aborts without retry" + +# One refusing peer must not cost the Server: during a rolling secret rotation, +# or against a pre-1.8 peer that answers 200 to any password, the next peer in +# PD_REST_LIST would have accepted it. The abort is for the fleet, not the peer. +run_case "one-401" "pd0:8620,pd1:8620" 6 +assert_equal "one refusing peer rc" "0" "${CASE_RC}" +assert_equal "kept going past the 401" "${TWO_CALLS}" "$(cat "${CALL_LOG}")" +assert_output "refused the credential (401)" +assert_output "Store registration check PASSED via pd1:8620" +echo " PASS one refusing peer does not end the wait" # The standalone RocksDB topology never reaches PD, so it must not warn about # a PD credential it will not send. @@ -269,4 +287,4 @@ assert_output "No pd.peers configured, skipping storage wait" fail "warned about an unused PD credential with no pd.peers configured" echo " PASS no credential warning without pd.peers" -echo "8 passed, 0 failed" +echo "9 passed, 0 failed" diff --git a/hugegraph-store/README.md b/hugegraph-store/README.md index 4e6bc8aca2..647971740c 100644 --- a/hugegraph-store/README.md +++ b/hugegraph-store/README.md @@ -259,8 +259,10 @@ curl http://localhost:8520/v1/health # Check logs tail -f logs/hugegraph-store.log -# Verify registration with PD (from PD node) -curl http://localhost:8620/v1/stores +# Verify registration with PD (from PD node). PD REST needs HTTP Basic auth: +# an internal service name (hg, store, hubble, vermeer) and PD's +# auth.secret-key value. Without it this returns 401, not the store list. +curl -u hg:"${PD_SECRET}" http://localhost:8620/v1/stores ``` For production deployment, see [Deployment Guide](docs/deployment-guide.md) and [Best Practices](docs/best-practices.md). diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index 4af3832d85..f19b287b87 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -2,6 +2,21 @@ This guide provides comprehensive instructions for deploying HugeGraph Store in various environments, from development to production clusters. +> **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than +> `/v1/health`, `/actuator/**` and `/v1/prom/targets/*`, need HTTP Basic auth: +> one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and +> PD's `auth.secret-key` value as the password. A call without it gets HTTP 401 +> and a `{"status":-1,"error":"Unauthorized"}` body, not the payloads shown +> below. Export the secret before following a step that uses `${PD_SECRET}`: +> +> ```bash +> read -rs PD_SECRET && export PD_SECRET +> ``` +> +> Store endpoints on port 8520 are unaffected. `-u` puts the secret in curl's +> process arguments; on a shared host pass it in a `curl -K` file mode 0600 +> instead, as `hugegraph-pd/docs/configuration.md` shows. + ## Table of Contents - [Deployment Topologies](#deployment-topologies) @@ -472,7 +487,7 @@ curl http://localhost:8620/actuator/health ```bash # Check cluster members -curl http://192.168.1.10:8620/v1/members +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/members # Expected output: { @@ -586,7 +601,7 @@ curl http://localhost:8520/v1/health ```bash # Query PD for registered stores -curl http://192.168.1.10:8620/v1/stores +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/stores # Expected output: { @@ -703,8 +718,17 @@ environment: HG_PD_DATA_PATH: /hugegraph-pd/pd_data # maps to pd.data-path HG_PD_INITIAL_STORE_COUNT: 3 # maps to pd.initial-store-count HG_PD_AUTH_SECRET_KEY: ${HG_PD_AUTH_SECRET_KEY:?} # maps to auth.secret-key; required + # optional; maps to management.endpoints.web.exposure.include + HG_PD_ACTUATOR_EXPOSURE: health,metrics,prometheus ``` +`HG_PD_ACTUATOR_EXPOSURE` is the only way to change the actuator allowlist in +this image: the entrypoint emits it in `SPRING_APPLICATION_JSON`, which outranks +a mounted `conf/application.yml`. Add an endpoint here to expose it, for example +`health,metrics,prometheus,loggers`. A value containing `*` is refused, because +every actuator endpoint is anonymous on port 8620 and `/actuator/env` returns the +`SPRING_APPLICATION_JSON` entry verbatim, PD's REST secret included. + **Store environment variables** (per node): ```yaml @@ -874,16 +898,16 @@ curl http://192.168.1.20:8520/v1/health ```bash # PD cluster members -curl http://192.168.1.10:8620/v1/members +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/members # Registered stores -curl http://192.168.1.10:8620/v1/stores +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/stores # Partitions -curl http://192.168.1.10:8620/v1/partitions +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/partitions # Graph list -curl http://192.168.1.10:8620/v1/graphs +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/graphs ``` ### Basic Operations Test From cf50aa5206de6afa37c811aa2d1a1c7d5832cc62 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Wed, 9 Sep 2026 01:08:26 +0530 Subject: [PATCH 25/28] fix(server): abort the storage wait only when every PD peer refuses wait-storage.sh flagged a 401 rather than counting it, so a single refusing peer turned a "nothing Up yet" pass into return 2 and killed the Server. PD serves /v1/stores well before stores register, so the first pass of a rolling secret rotation finds the stale peer refusing while the healthy peers are merely storeless, and the Server died instead of retrying. Count refusals and return 2 only when every peer polled refused, which keeps the fail-fast for a fleet-wide wrong secret aborting on pass one. Cover the case that was missing: one refusing peer with no Up store anywhere must retry to the outer timeout, not abort. Also report the shipped PD config check per file. It compared the global FAIL against its entry value, which only separates 0 from 1 on the first failing file, so every later failing file printed ok as well. Correct the Hubble mount wording in three places. Both HStore Compose files pin the bind with create_host_path: false, so a missing *.local.properties makes Compose refuse to start rather than mounting an empty directory over Hubble's config, and add the printable-ASCII rejection to the helper's documented guarantees. --- docker/README.md | 22 ++++++++++------ docker/set-hubble-pd-password.sh | 4 +-- docker/test-compose.sh | 5 ++-- .../src/assembly/static/bin/wait-storage.sh | 25 ++++++++++-------- .../assembly/travis/test-pd-shipped-config.sh | 15 ++++++----- .../src/assembly/travis/test-wait-storage.sh | 26 +++++++++++++++++-- 6 files changed, 66 insertions(+), 31 deletions(-) diff --git a/docker/README.md b/docker/README.md index 6bdf970d6d..7380905683 100644 --- a/docker/README.md +++ b/docker/README.md @@ -113,14 +113,20 @@ fails: ./set-hubble-pd-password.sh hstore # or hstore-ha ``` -Run it before `docker compose up`: if the file is missing, Docker creates an -empty directory at the bind path and Hubble starts with no configuration. -The helper refuses an empty value, writes the secret without passing it -through a `sed` replacement (where `&`, `#` and backslashes are special), and -doubles backslashes for the `.properties` format. The generated hex secret -needs none of that, but a hand-chosen one might. Until the file carries the -right value, Hubble's PD-backed views get 401 from PD; everything else in -Hubble works. +Run it before `docker compose up`: both HStore Compose files pin the mount with `create_host_path: false`, so a missing +file makes Compose refuse to start rather than mounting an empty directory over Hubble's config. + +
What the helper guarantees, and why Hubble 401s until it has run + +- Refuses an empty secret. +- Refuses a secret that is not printable ASCII. Hubble reads `.properties` as ISO-8859-1 while PD compares UTF-8 bytes, + so a non-ASCII secret gives a permanent 401 with nothing logged on either side. The generated hex is safe. +- Writes the value without passing it through a `sed` replacement (where `&`, `#` and backslashes are special) and + doubles backslashes for the `.properties` format. The generated hex needs none of that; a hand-chosen one might. + +Until the file carries the right value, Hubble's PD-backed views get 401 from PD; everything else in Hubble works. + +
### Standalone diff --git a/docker/set-hubble-pd-password.sh b/docker/set-hubble-pd-password.sh index 85f410e58c..73d3f18a63 100755 --- a/docker/set-hubble-pd-password.sh +++ b/docker/set-hubble-pd-password.sh @@ -25,8 +25,8 @@ # lands in a tracked file. The secret defaults to $HG_PD_AUTH_SECRET_KEY. The # value never goes through a sed replacement, where & # and backslash are # special, and backslashes are doubled for the .properties format. Run this -# before `docker compose up`: if the target is missing Docker creates an empty -# directory at the bind path and Hubble starts with no configuration. +# before `docker compose up`: the bind pins create_host_path: false, so a +# missing target makes Compose refuse to start. # # The secret must be printable ASCII. PD compares it as UTF-8 bytes # (Authentication.verifySecret), while Hubble reads this file through diff --git a/docker/test-compose.sh b/docker/test-compose.sh index 6f455a0d4a..54418d413a 100644 --- a/docker/test-compose.sh +++ b/docker/test-compose.sh @@ -323,8 +323,9 @@ cleanup() { # The HStore topologies mount conf/hubble/.local.properties, which is # generated and untracked. Generate both with the CI secret before any render -# or `up`; a missing file would make Docker create an empty directory at the -# bind path. A developer's own local files are put back afterwards. +# or `up`; the binds pin create_host_path: false, so a missing file makes +# Compose refuse to start. A developer's own local files are put back +# afterwards. HUBBLE_BACKUP_DIR="" prepare_hubble_configs() { local name diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh index 39afa70543..168efe8979 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh @@ -131,17 +131,20 @@ if env | grep '^hugegraph\.' > /dev/null; then # readable: a 401 is a wrong secret, not a storage problem, and # retrying it for 300s only hides that. # - # A 401 is remembered rather than returned at once, so one - # refusing peer no longer ends the wait before the rest of - # PD_REST_LIST is tried. That case is real: during a rolling - # secret rotation, or against a pre-1.8 PD that answers 200 to - # any password, a Server used to die even though the next peer - # would have accepted it. Returning 2 only when no peer produced - # an Up store keeps the fail-fast for a fleet-wide wrong secret, - # which still aborts on the first pass instead of retrying 300s. + # 401s are counted, not flagged, so the abort is for the fleet + # and never for one peer. That distinction is real: PD serves + # /v1/stores well before stores finish registering, so the first + # pass of a rolling secret rotation can find one stale peer + # refusing while the healthy peers are merely storeless. A flag + # turned that into a dead Server. Returning 2 only when every + # peer polled refused keeps the fail-fast for a fleet-wide wrong + # secret, which still aborts on pass one instead of retrying for + # the full 300s. check_any_pd_stores() { - refused= + refused=0 + peers=0 for peer in \$(echo \"\$PD_REST_LIST\" | tr ',' ' '); do + peers=\$((peers + 1)) body=\$(printf 'user = \"%s:%s\"\n' \ \"\$PD_AUTH_CURL_USER\" \"\$PD_AUTH_CURL_PASSWORD\" | \ curl -K - -s -w '\n%{http_code}' \ @@ -152,7 +155,7 @@ if env | grep '^hugegraph\.' > /dev/null; then if [ \"\$code\" = 401 ]; then log \"ERROR: PD at \${peer} refused the credential (401):\" >&2 log ' PD_AUTH_PASSWORD must match PD auth.secret-key' >&2 - refused=1 + refused=\$((refused + 1)) continue fi if printf '%s' \"\$body\" | grep -qi '\"state\"[[:space:]]*:[[:space:]]*\"Up\"'; then @@ -160,7 +163,7 @@ if env | grep '^hugegraph\.' > /dev/null; then return 0 fi done - [ -z \"\$refused\" ] || return 2 + [ \"\$peers\" -gt 0 ] && [ \"\$refused\" -eq \"\$peers\" ] && return 2 return 1 } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh index 623fd1efb4..46521dd05e 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh @@ -34,7 +34,7 @@ EXPECTED_EXPOSURE='health,metrics,prometheus' FAIL=0 check() { - local file="$1" rel="${1#"${ROOT}/"}" before="$FAIL" + local file="$1" rel="${1#"${ROOT}/"}" bad=0 [[ -f "$file" ]] || { echo " FAIL ${rel}: missing"; FAIL=1; return; } # The actuator exposure specifically: a config that grows an unrelated @@ -51,16 +51,19 @@ check() { exposure=${exposure#\'}; exposure=${exposure%\'} if [[ "$exposure" != "${EXPECTED_EXPOSURE}" ]]; then echo " FAIL ${rel}: actuator exposure must be exactly" \ - "'${EXPECTED_EXPOSURE}', got '${exposure}'"; FAIL=1 + "'${EXPECTED_EXPOSURE}', got '${exposure}'"; FAIL=1; bad=1 fi if ! grep -qE '^[[:space:]]*secret-key:[[:space:]]*$' "$file"; then - echo " FAIL ${rel}: auth.secret-key must be present and empty"; FAIL=1 + echo " FAIL ${rel}: auth.secret-key must be present and empty"; FAIL=1; bad=1 fi if grep -q "${PUBLISHED_SECRET}" "$file"; then - echo " FAIL ${rel}: contains the published secret"; FAIL=1 + echo " FAIL ${rel}: contains the published secret"; FAIL=1; bad=1 fi - # Only when nothing above raised FAIL, or the file contradicts its own report - if [[ "$FAIL" == "$before" ]]; then + # Per file, not the global FAIL: once an earlier file has set that, every + # later file matches it again and a failing file reports itself ok. Kept as + # an if rather than a bare test-and-echo, which would be the function's last + # command and return 1 on a failing file, aborting the loop under set -e. + if [[ "$bad" -eq 0 ]]; then echo " ok ${rel}" fi } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh index dde216a26e..6d7c4ce8d0 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh @@ -177,6 +177,13 @@ elif [[ "${MOCK_SCENARIO}" == "one-401" && \ elif [[ "${MOCK_SCENARIO}" == "one-401" && \ "${url}" == "http://pd1:8620/v1/stores" ]]; then respond '{"stores":[{"state":"Up"}]}' 200 +elif [[ "${MOCK_SCENARIO}" == "one-401-pending" && \ + "${url}" == "http://pd0:8620/v1/stores" ]]; then + # Same stale peer, but caught before any store has finished registering + respond '{"status":-1,"error":"Unauthorized"}' 401 +elif [[ "${MOCK_SCENARIO}" == "one-401-pending" && \ + "${url}" == "http://pd1:8620/v1/stores" ]]; then + respond '{"stores":[{"state":"Pending"}]}' 200 elif [[ "${MOCK_SCENARIO}" == "pd1-up" && \ "${url}" == "http://pd1:8620/v1/stores" ]]; then respond '{"stores":[{"state":"Up"}]}' 200 @@ -265,7 +272,8 @@ echo " PASS 401 from every peer aborts without retry" # One refusing peer must not cost the Server: during a rolling secret rotation, # or against a pre-1.8 peer that answers 200 to any password, the next peer in -# PD_REST_LIST would have accepted it. The abort is for the fleet, not the peer. +# PD_REST_LIST accepts the same credential. Only a refusal from every peer that +# answered is a fleet-wide wrong secret, so a lone 401 stays a retry. run_case "one-401" "pd0:8620,pd1:8620" 6 assert_equal "one refusing peer rc" "0" "${CASE_RC}" assert_equal "kept going past the 401" "${TWO_CALLS}" "$(cat "${CALL_LOG}")" @@ -273,6 +281,20 @@ assert_output "refused the credential (401)" assert_output "Store registration check PASSED via pd1:8620" echo " PASS one refusing peer does not end the wait" +# The same lone 401 caught before any store is Up. PD serves /v1/stores well +# ahead of store registration, so the first pass of a rolling rotation sees the +# stale peer refuse and the healthy peers still storeless. That must retry, not +# abort: the store is on its way, and the accepting peer is right there. +run_case "one-401-pending" "pd0:8620,pd1:8620" 4 +[[ "${CASE_RC}" -ne 0 ]] || fail "a storeless fleet must still fail closed" +assert_equal "retried past a lone 401" "${FOUR_CALLS}" "$(cat "${CALL_LOG}")" +assert_output "refused the credential (401)" +assert_output "No Up store yet, retrying in 5s" +assert_output "ERROR: Timeout waiting for storage backend" +[[ "${CASE_OUTPUT}" != *"storage wait aborted"* ]] || \ + fail "a lone 401 aborted the wait with no Up store anywhere" +echo " PASS one refusing peer with no Up store retries" + # The standalone RocksDB topology never reaches PD, so it must not warn about # a PD credential it will not send. : > "${DIST_ROOT}/conf/graphs/hugegraph.properties" @@ -287,4 +309,4 @@ assert_output "No pd.peers configured, skipping storage wait" fail "warned about an unused PD credential with no pd.peers configured" echo " PASS no credential warning without pd.peers" -echo "9 passed, 0 failed" +echo "10 passed, 0 failed" From 23284b76150a8cfac8383fdaf751427e548c702b Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 9 Sep 2026 14:34:25 +0800 Subject: [PATCH 26/28] fix(pd): remove duplicate junit dependency in hg-pd-service pom --- hugegraph-pd/hg-pd-service/pom.xml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hugegraph-pd/hg-pd-service/pom.xml b/hugegraph-pd/hg-pd-service/pom.xml index 5cb87618f5..eef38b4073 100644 --- a/hugegraph-pd/hg-pd-service/pom.xml +++ b/hugegraph-pd/hg-pd-service/pom.xml @@ -51,12 +51,6 @@ 5.8.3 - - junit - junit - ${junit.version} - test - org.apache.hugegraph hg-pd-core From 30754877dcf6c5dc843690e49936d6264d2f409d Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 9 Sep 2026 14:47:06 +0800 Subject: [PATCH 27/28] fix(pd): handle basic auth scheme case-insensitively and align docs - In RestAuthentication, check the Basic scheme prefix case-insensitively via regionMatches, strip the prefix, and throw BadCredentialsException on non-Basic or malformed schemes instead of doing a naive string replacement. - Add unit tests in AuthenticationTest covering case-insensitive schemes ('Basic ', 'basic ', 'BASIC ', 'BaSiC ') and refusal of non-Basic/malformed schemes. - Add /v1/ready to the unauthenticated endpoint lists in configuration.md, api-reference.md, hugegraph-pd/README.md, and operations-guide.md. - In integration-guide.md, add the PD REST credential callout banner and add '-u hg:"${PD_SECRET}"' to the troubleshooting curl examples. - Streamline docker/README.md for high information density, removing verbose step-by-step logs, and keep markdown line wrapping >120 characters across all touched documentation. --- docker/README.md | 75 +++-------------- hugegraph-pd/README.md | 29 +------ hugegraph-pd/docs/api-reference.md | 10 +-- hugegraph-pd/docs/configuration.md | 15 +--- .../rest/interceptor/RestAuthentication.java | 8 +- .../interceptor/AuthenticationTest.java | 83 +++++++++++++++++++ hugegraph-store/README.md | 4 +- hugegraph-store/docs/deployment-guide.md | 25 ++---- hugegraph-store/docs/integration-guide.md | 14 +++- hugegraph-store/docs/operations-guide.md | 14 +--- 10 files changed, 129 insertions(+), 148 deletions(-) diff --git a/docker/README.md b/docker/README.md index 7380905683..762fb7782a 100644 --- a/docker/README.md +++ b/docker/README.md @@ -42,20 +42,13 @@ contains a single quote or newline. pd_secret="$(openssl rand -hex 24)" printf "HUGEGRAPH_ADMIN_PASSWORD='%s'\nHUGEGRAPH_AUTH_TOKEN_SECRET='%s'\nHG_PD_AUTH_SECRET_KEY='%s'\n" \ 'replace-with-your-password' "${jwt_secret}" "${pd_secret}" > .env - # Hubble reads the PD secret from a file, not from .env: generate the - # untracked properties files the HStore topologies mount. Passed in the - # environment rather than as an argument, which `ps` shows to every local - # account for as long as the helper runs. + # Hubble reads the PD secret from a file, not from .env: generate the untracked properties files the HStore topologies mount. HG_PD_AUTH_SECRET_KEY="${pd_secret}" ./set-hubble-pd-password.sh hstore HG_PD_AUTH_SECRET_KEY="${pd_secret}" ./set-hubble-pd-password.sh hstore-ha ) ``` -Do not commit `.env` or `conf/hubble/*.local.properties`; both are in -`.gitignore`. Keeping the same JWT secret preserves authentication -tokens when containers are recreated. For authenticated topologies with -multiple Server replicas, all replicas receive this same secret. The HA -topology fails fast if authentication is enabled without this shared secret. +Do not commit `.env` or `conf/hubble/*.local.properties`; both are in `.gitignore`. Keeping the same JWT secret preserves authentication tokens when containers are recreated. For authenticated topologies with multiple Server replicas, all replicas receive this same secret. The HA topology fails fast if authentication is enabled without this shared secret. A non-empty `HUGEGRAPH_ADMIN_PASSWORD` enables Server authentication, and Hubble detects that mode automatically. Omitting the variable or setting it to @@ -68,66 +61,27 @@ behind an HTTPS reverse proxy and trusted network controls. first authenticated startup. Changing `.env` does not rotate an existing administrator password; use the HugeGraph user API for credential changes. -For the verification commands below, load `.env` into your current shell and -set the password: +For the verification commands below, load `.env` into your current shell and set the password: ```bash set -a; . ./.env; set +a ADMIN_PASSWORD='the-same-password-used-in-.env' ``` -Compose reads `.env` on its own; the line above is so that the `curl` command and -the Hubble helper on this page can use `${HG_PD_AUTH_SECRET_KEY}` too. +The PD REST API (port 8620, HStore topologies only) requires HTTP Basic auth (`hg:${HG_PD_AUTH_SECRET_KEY}`) for all endpoints except health/readiness probes (`/v1/health`, `/v1/ready`). `HG_PD_AUTH_SECRET_KEY` is shared across PD, Server (`bin/wait-storage.sh`), and Hubble (`conf/hubble/*.local.properties` generated by `./set-hubble-pd-password.sh`). -The PD REST API (port 8620, HStore topologies only) has its own credential: -requests other than health probes need HTTP Basic auth with an internal -service name (for example `hg`) and the PD secret as the password. PD ships -no default secret, so `HG_PD_AUTH_SECRET_KEY` is required and the HStore -Compose files refuse to start without it. The `.env` command above generates -one. To list registered stores: +Verify registered stores: ```bash curl -u "hg:${HG_PD_AUTH_SECRET_KEY}" http://localhost:8620/v1/stores ``` -Three consumers read this credential, and all three have to agree or startup -fails: - -- PD itself, through `HG_PD_AUTH_SECRET_KEY`. -- The Server, whose `bin/wait-storage.sh` polls `/v1/stores` before the - Server starts. Both Compose files pass `PD_AUTH_PASSWORD` to it from the - same variable, so setting `HG_PD_AUTH_SECRET_KEY` in `.env` covers it. If - the Server sends the wrong secret `wait-storage.sh` aborts on the first - 401 rather than waiting out `WAIT_STORAGE_TIMEOUT_S`, and the container - exits with `ERROR: storage wait aborted, see the message above` after - logging `ERROR: PD at refused the credential (401)`. -- Hubble, through `operations.pd.password` in - `conf/hubble/hstore.local.properties` (Minimal HStore) or - `conf/hubble/hstore-ha.local.properties` (HA). Compose mounts those files - read-only and does not template them, and the Hubble image has no - entrypoint that reads the environment, so they are generated from the - tracked `*.properties.example` files by `set-hubble-pd-password.sh`. The - `.env` recipe above already runs it. To regenerate after loading `.env`: +To regenerate Hubble configuration after modifying `.env`: ```bash ./set-hubble-pd-password.sh hstore # or hstore-ha ``` -Run it before `docker compose up`: both HStore Compose files pin the mount with `create_host_path: false`, so a missing -file makes Compose refuse to start rather than mounting an empty directory over Hubble's config. - -
What the helper guarantees, and why Hubble 401s until it has run - -- Refuses an empty secret. -- Refuses a secret that is not printable ASCII. Hubble reads `.properties` as ISO-8859-1 while PD compares UTF-8 bytes, - so a non-ASCII secret gives a permanent 401 with nothing logged on either side. The generated hex is safe. -- Writes the value without passing it through a `sed` replacement (where `&`, `#` and backslashes are special) and - doubles backslashes for the `.properties` format. The generated hex needs none of that; a hand-chosen one might. - -Until the file carries the right value, Hubble's PD-backed views get 401 from PD; everything else in Hubble works. - -
- ### Standalone This is the recommended quickstart. @@ -410,20 +364,13 @@ docker compose -f docker-compose-hstore.yml up -d --wait ### Hubble configuration -The three small files under `conf/hubble/` contain only topology-specific -discovery settings, the PD REST credential (`operations.pd.username` and -`operations.pd.password`, which must match PD's `auth.secret-key`), and -container paths: +The three small files under `conf/hubble/` contain only topology-specific discovery settings, the PD REST credential (`operations.pd.username` and `operations.pd.password`, which must match PD's `auth.secret-key`), and container paths: - `conf/hubble/standalone.properties` uses direct Server mode. -- `conf/hubble/hstore.properties.example` uses one PD and one Store REST - target. -- `conf/hubble/hstore-ha.properties.example` uses all three PD peers and all - three allowed Store REST targets. - -The two HStore topologies mount the generated `*.local.properties` next to -these examples (see `set-hubble-pd-password.sh`), never the examples -themselves, so the PD secret stays out of tracked files. +- `conf/hubble/hstore.properties.example` uses one PD and one Store REST target. +- `conf/hubble/hstore-ha.properties.example` uses all three PD peers and all three allowed Store REST targets. + +The two HStore topologies mount the generated `*.local.properties` next to these examples (see `set-hubble-pd-password.sh`), never the examples themselves, so the PD secret stays out of tracked files. Hubble detects Server authentication through the Server API. Do not add an `auth.enabled` property or duplicate auth-on/auth-off configurations. diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 87dfec0e9e..35ad023918 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -292,31 +292,10 @@ docker/docker-compose-3pd-3store-3server.yml ### Security -- Keep all three ports on a trusted network. The REST API on `8620` includes - management endpoints that mutate the cluster (peer changes, store removal, - data movement), and the gRPC and Raft ports carry no authentication. -- REST requests need HTTP Basic auth: one of the internal service names - (`hg`, `store`, `hubble`, `vermeer`) with the `auth.secret-key` value as - the password. Health probes (`/v1/health`, `/actuator/**`, - `/v1/prom/targets/*`) stay unauthenticated. -- `auth.secret-key` has no shipped default, because a secret in the source - tree is published to everyone. Generate one per deployment (`openssl rand - -hex 24`) and set it in the config file, or through - `HG_PD_AUTH_SECRET_KEY`, which the Docker image requires. Give every REST - client the same value: the Server's `bin/wait-storage.sh` reads - `PD_AUTH_PASSWORD` (and `PD_AUTH_USER`, default `store`), and Hubble reads - `operations.pd.password`. A client left on a stale secret gets 401, and - `wait-storage.sh` aborts the Server's startup on the first one rather than - waiting out `WAIT_STORAGE_TIMEOUT_S`. -- An existing `conf/application.yml` carried over from an earlier release has - no `auth` block, and still carries - `management.endpoints.web.exposure.include: "*"`. PD then starts with an - empty secret and refuses every authenticated REST request, logging an error - that names `auth.secret-key`, while `/actuator/env`, `/actuator/configprops` - and `/actuator/beans` stay anonymously readable on `8620`. Before upgrading, - add `auth.secret-key` and narrow that exposure to - `health,metrics,prometheus`. PD refuses to start if the key is set to the - placeholder value that earlier revisions of this repository carried. +- Keep all three ports on a trusted network. The REST API on `8620` includes management endpoints that mutate the cluster (peer changes, store removal, data movement), and the gRPC and Raft ports carry no authentication. +- REST requests need HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) with the `auth.secret-key` value as the password. Health probes (`/v1/health`, `/v1/ready`, `/actuator/**`, `/v1/prom/targets/*`) stay unauthenticated. +- `auth.secret-key` has no shipped default, because a secret in the source tree is published to everyone. Generate one per deployment (`openssl rand -hex 24`) and set it in the config file, or through `HG_PD_AUTH_SECRET_KEY`, which the Docker image requires. Give every REST client the same value: the Server's `bin/wait-storage.sh` reads `PD_AUTH_PASSWORD` (and `PD_AUTH_USER`, default `store`), and Hubble reads `operations.pd.password`. A client left on a stale secret gets 401, and `wait-storage.sh` aborts the Server's startup on the first one rather than waiting out `WAIT_STORAGE_TIMEOUT_S`. +- An existing `conf/application.yml` carried over from an earlier release has no `auth` block, and still carries `management.endpoints.web.exposure.include: "*"`. PD then starts with an empty secret and refuses every authenticated REST request, logging an error that names `auth.secret-key`, while `/actuator/env`, `/actuator/configprops` and `/actuator/beans` stay anonymously readable on `8620`. Before upgrading, add `auth.secret-key` and narrow that exposure to `health,metrics,prometheus`. PD refuses to start if the key is set to the placeholder value that earlier revisions of this repository carried. ### Monitoring diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 07b3d8abf9..03297d9569 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -762,19 +762,13 @@ PD exposes a REST API for management and monitoring (default port: 8620). ### Authentication -Every endpoint below except the probes needs HTTP Basic auth: one of the -internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, and -the `auth.secret-key` value from PD's `conf/application.yml` as the password. -A missing or wrong credential gets HTTP 401. The `curl` examples that follow -omit `-u` for readability; add it to every call except `/v1/health`, -`/actuator/**` and `/v1/prom/targets/*`, which stay unauthenticated for probes. +Every endpoint below except the probes needs HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, and the `auth.secret-key` value from PD's `conf/application.yml` as the password. A missing or wrong credential gets HTTP 401. The `curl` examples that follow omit `-u` for readability; add it to every call except `/v1/health`, `/v1/ready`, `/actuator/**` and `/v1/prom/targets/*`, which stay unauthenticated for probes. ```bash curl -u hg: http://localhost:8620/v1/stores ``` -Endpoints under `/v1` mutate the cluster (peer list changes, store removal, -partition balancing), so keep port 8620 on a trusted network regardless. +Endpoints under `/v1` mutate the cluster (peer list changes, store removal, partition balancing), so keep port 8620 on a trusted network regardless. ### Health Check diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index 39c0e0b92d..4e60d8e8be 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -81,11 +81,7 @@ server: ### REST Authentication Settings -Every REST request except the probes below must carry HTTP Basic auth: one of -the internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, -and the shared secret as the password. A missing or wrong credential gets -HTTP 401. Unauthenticated paths: `/v1/health`, `/actuator/**` and -`/v1/prom/targets/*`. +Every REST request except the probes below must carry HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) as the user, and the shared secret as the password. A missing or wrong credential gets HTTP 401. Unauthenticated paths: `/v1/health`, `/v1/ready`, `/actuator/**` and `/v1/prom/targets/*`. ```yaml auth: @@ -96,18 +92,13 @@ auth: |-----------|------|---------|-------------| | `auth.secret-key` | String | none (required) | Password checked against the Basic credential. There is no default: a secret shipped in the source tree would be published to everyone. While it is empty PD refuses every authenticated REST request and logs an error naming this parameter, and PD refuses to start at all if it is set to the value that earlier revisions carried as a placeholder. | -Every REST client needs the same value: the Server's `bin/wait-storage.sh` -reads it from `PD_AUTH_PASSWORD`, Hubble from `operations.pd.password`, and -the Docker image takes `HG_PD_AUTH_SECRET_KEY`. +Every REST client needs the same value: the Server's `bin/wait-storage.sh` reads it from `PD_AUTH_PASSWORD`, Hubble from `operations.pd.password`, and the Docker image takes `HG_PD_AUTH_SECRET_KEY`. ```bash curl -u hg: http://:8620/v1/stores ``` -`-u` puts the secret in curl's process arguments, where any local account can -read it while the call runs, and PD REST is plain HTTP. On a shared host, or -across a network you do not control, keep the secret out of `argv` by reading -it from a file mode 0600: +`-u` puts the secret in curl's process arguments, where any local account can read it while the call runs, and PD REST is plain HTTP. On a shared host, or across a network you do not control, keep the secret out of `argv` by reading it from a file mode 0600: ```bash printf 'user = "hg:%s"\n' "${PD_SECRET}" > pd.curlrc && chmod 600 pd.curlrc diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java index 3aeeb48da9..9d28e7d75d 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/RestAuthentication.java @@ -26,6 +26,7 @@ import org.apache.hugegraph.pd.service.interceptor.Authentication; import org.springframework.lang.Nullable; +import org.springframework.security.authentication.BadCredentialsException; import org.springframework.stereotype.Service; import org.springframework.web.servlet.HandlerInterceptor; import org.springframework.web.servlet.ModelAndView; @@ -49,14 +50,17 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons String authority = request.getHeader("Authorization"); if (authority == null) { - throw new Exception("Unauthorized!"); + throw new BadCredentialsException("Unauthorized!"); + } + if (!authority.regionMatches(true, 0, "Basic ", 0, 6)) { + throw new BadCredentialsException("invalid basic authentication info"); } Function tokenCall = t -> { response.addHeader(TOKEN_KEY, t); return true; }; - authority = authority.replace("Basic ", ""); + authority = authority.substring(6); return authenticate(authority, token, tokenCall, DEFAULT_HANDLE); } catch (Exception e) { response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); diff --git a/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java b/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java index 5bb801f6d1..688743ee0d 100644 --- a/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java +++ b/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java @@ -17,11 +17,18 @@ package org.apache.hugegraph.pd.service.interceptor; +import java.io.PrintWriter; +import java.io.StringWriter; import java.lang.reflect.Field; +import java.lang.reflect.Proxy; import java.nio.charset.StandardCharsets; import java.util.Base64; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + import org.apache.hugegraph.pd.config.PDConfig; +import org.apache.hugegraph.pd.rest.interceptor.RestAuthentication; import org.junit.Assert; import org.junit.Test; @@ -43,6 +50,50 @@ private static Authentication authWithSecret(String secret) throws Exception { return auth; } + private static RestAuthentication restAuthWithSecret(String secret) throws Exception { + RestAuthentication auth = new RestAuthentication(); + PDConfig config = new PDConfig(); + config.setSecretKey(secret); + Field field = Authentication.class.getDeclaredField("pdConfig"); + field.setAccessible(true); + field.set(auth, config); + return auth; + } + + private static boolean acceptsRest(RestAuthentication auth, String authHeader) { + try { + HttpServletRequest req = (HttpServletRequest) Proxy.newProxyInstance( + HttpServletRequest.class.getClassLoader(), + new Class[]{HttpServletRequest.class}, + (proxy, method, args) -> { + if ("getHeader".equals(method.getName())) { + return "Authorization".equalsIgnoreCase((String) args[0]) ? authHeader : null; + } + if ("getMethod".equals(method.getName())) { + return "GET"; + } + if ("getRequestURI".equals(method.getName())) { + return "/v1/stores"; + } + return null; + }); + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + HttpServletResponse resp = (HttpServletResponse) Proxy.newProxyInstance( + HttpServletResponse.class.getClassLoader(), + new Class[]{HttpServletResponse.class}, + (proxy, method, args) -> { + if ("getWriter".equals(method.getName())) { + return pw; + } + return null; + }); + return auth.preHandle(req, resp, null); + } catch (Exception e) { + return false; + } + } + private static String credential(String name, String pwd) { return Base64.getEncoder().encodeToString( (name + ":" + pwd).getBytes(StandardCharsets.UTF_8)); @@ -124,4 +175,36 @@ public void testOwnSecretStarts() { config.setSecretKey(SECRET); config.afterPropertiesSet(); } + + @Test + public void testRestAuthenticationAcceptsCaseInsensitiveBasicScheme() throws Exception { + RestAuthentication auth = restAuthWithSecret(SECRET); + String cred = credential("hg", SECRET); + Assert.assertTrue("Basic with uppercase B should be accepted", + acceptsRest(auth, "Basic " + cred)); + Assert.assertTrue("basic with all lowercase should be accepted", + acceptsRest(auth, "basic " + cred)); + Assert.assertTrue("BASIC with all uppercase should be accepted", + acceptsRest(auth, "BASIC " + cred)); + Assert.assertTrue("BaSiC with mixed case should be accepted", + acceptsRest(auth, "BaSiC " + cred)); + } + + @Test + public void testRestAuthenticationRefusesNonBasicOrMalformedSchemes() throws Exception { + RestAuthentication auth = restAuthWithSecret(SECRET); + String cred = credential("hg", SECRET); + Assert.assertFalse("null Authorization header must be refused", + acceptsRest(auth, null)); + Assert.assertFalse("empty Authorization header must be refused", + acceptsRest(auth, "")); + Assert.assertFalse("Bearer scheme must be refused", + acceptsRest(auth, "Bearer " + cred)); + Assert.assertFalse("Digest scheme must be refused", + acceptsRest(auth, "Digest " + cred)); + Assert.assertFalse("Basic without trailing space must be refused", + acceptsRest(auth, "Basic" + cred)); + Assert.assertFalse("invalid credential with basic prefix must be refused", + acceptsRest(auth, "basic " + credential("hg", "wrong-secret"))); + } } diff --git a/hugegraph-store/README.md b/hugegraph-store/README.md index 647971740c..f12df4427e 100644 --- a/hugegraph-store/README.md +++ b/hugegraph-store/README.md @@ -259,9 +259,7 @@ curl http://localhost:8520/v1/health # Check logs tail -f logs/hugegraph-store.log -# Verify registration with PD (from PD node). PD REST needs HTTP Basic auth: -# an internal service name (hg, store, hubble, vermeer) and PD's -# auth.secret-key value. Without it this returns 401, not the store list. +# Verify registration with PD (from PD node). PD REST needs HTTP Basic auth: an internal service name (hg, store, hubble, vermeer) and PD's auth.secret-key value. Without it this returns 401, not the store list. curl -u hg:"${PD_SECRET}" http://localhost:8620/v1/stores ``` diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index d23300b812..cfe47d977f 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -2,20 +2,13 @@ This guide provides comprehensive instructions for deploying HugeGraph Store in various environments, from development to production clusters. -> **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than -> `/v1/health`, `/actuator/**` and `/v1/prom/targets/*`, need HTTP Basic auth: -> one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and -> PD's `auth.secret-key` value as the password. A call without it gets HTTP 401 -> and a `{"status":-1,"error":"Unauthorized"}` body, not the payloads shown -> below. Export the secret before following a step that uses `${PD_SECRET}`: +> **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than `/v1/health`, `/v1/ready`, `/actuator/**` and `/v1/prom/targets/*`, need HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and PD's `auth.secret-key` value as the password. A call without it gets HTTP 401 and a `{"status":-1,"error":"Unauthorized"}` body, not the payloads shown below. Export the secret before following a step that uses `${PD_SECRET}`: > > ```bash > read -rs PD_SECRET && export PD_SECRET > ``` > -> Store endpoints on port 8520 are unaffected. `-u` puts the secret in curl's -> process arguments; on a shared host pass it in a `curl -K` file mode 0600 -> instead, as `hugegraph-pd/docs/configuration.md` shows. +> Store endpoints on port 8520 are unaffected. `-u` puts the secret in curl's process arguments; on a shared host pass it in a `curl -K` file mode 0600 instead, as `hugegraph-pd/docs/configuration.md` shows. ## Table of Contents @@ -693,12 +686,9 @@ For a production-like 3-node distributed deployment, use the compose file at `do ```bash cd docker -# The PD REST secret is required; the Compose file refuses to start without -# it. Generate it once and keep it, every PD node and PD client needs the -# same value (docker/README.md has the full .env recipe). +# The PD REST secret is required; the Compose file refuses to start without it. Generate it once and keep it, every PD node and PD client needs the same value (docker/README.md has the full .env recipe). export HG_PD_AUTH_SECRET_KEY="$(openssl rand -hex 24)" -# Hubble reads the secret from a generated, untracked properties file that the -# Compose file mounts; create it before `up` or Hubble starts unconfigured. +# Hubble reads the secret from a generated, untracked properties file that the Compose file mounts; create it before `up` or Hubble starts unconfigured. ./set-hubble-pd-password.sh hstore-ha HUGEGRAPH_VERSION=1.7.0 docker compose -f docker-compose-3pd-3store-3server.yml up -d ``` @@ -722,12 +712,7 @@ environment: HG_PD_ACTUATOR_EXPOSURE: health,metrics,prometheus ``` -`HG_PD_ACTUATOR_EXPOSURE` is the only way to change the actuator allowlist in -this image: the entrypoint emits it in `SPRING_APPLICATION_JSON`, which outranks -a mounted `conf/application.yml`. Add an endpoint here to expose it, for example -`health,metrics,prometheus,loggers`. A value containing `*` is refused, because -every actuator endpoint is anonymous on port 8620 and `/actuator/env` returns the -`SPRING_APPLICATION_JSON` entry verbatim, PD's REST secret included. +`HG_PD_ACTUATOR_EXPOSURE` is the only way to change the actuator allowlist in this image: the entrypoint emits it in `SPRING_APPLICATION_JSON`, which outranks a mounted `conf/application.yml`. Add an endpoint here to expose it, for example `health,metrics,prometheus,loggers`. A value containing `*` is refused, because every actuator endpoint is anonymous on port 8620 and `/actuator/env` returns the `SPRING_APPLICATION_JSON` entry verbatim, PD's REST secret included. **Store environment variables** (per node): diff --git a/hugegraph-store/docs/integration-guide.md b/hugegraph-store/docs/integration-guide.md index f35669c698..13164872e0 100644 --- a/hugegraph-store/docs/integration-guide.md +++ b/hugegraph-store/docs/integration-guide.md @@ -2,6 +2,14 @@ This guide explains how to integrate HugeGraph Store with HugeGraph Server, use the client library, and migrate from other storage backends. +> **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than `/v1/health`, `/v1/ready`, `/actuator/**` and `/v1/prom/targets/*`, need HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and PD's `auth.secret-key` value as the password. A call without it gets HTTP 401. Export the secret before following a procedure that uses `${PD_SECRET}`: +> +> ```bash +> read -rs PD_SECRET && export PD_SECRET +> ``` +> +> Store endpoints on port 8520 are unaffected. + ## Table of Contents - [Backend Configuration](#backend-configuration) @@ -706,7 +714,7 @@ tail -f logs/hugegraph-server.log | grep PD curl http://192.168.1.20:8520/v1/health # Check partition distribution -curl http://192.168.1.10:8620/v1/partitions +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/partitions # Check if queries are using indexes # (Enable query logging in Server) @@ -733,10 +741,10 @@ ERROR o.a.h.b.s.h.HstoreSession - Write operation failed: Raft leader not found tail -f logs/hugegraph-store.log | grep Raft # Check partition leaders -curl http://192.168.1.10:8620/v1/partitions | grep leader +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/partitions | grep leader # Check Store node states -curl http://192.168.1.10:8620/v1/stores +curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/stores ``` **Solutions**: diff --git a/hugegraph-store/docs/operations-guide.md b/hugegraph-store/docs/operations-guide.md index 2d6501c696..e94bbf1555 100644 --- a/hugegraph-store/docs/operations-guide.md +++ b/hugegraph-store/docs/operations-guide.md @@ -2,20 +2,13 @@ This guide covers monitoring, troubleshooting, backup & recovery, and operational procedures for HugeGraph Store in production. -> **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than -> `/v1/health`, `/actuator/**` and `/v1/prom/targets/*`, need HTTP Basic auth: -> one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and -> PD's `auth.secret-key` value as the password. A call without it gets HTTP -> 401, which for a mutating step such as `balanceLeaders` means the step did -> nothing. Export the secret before following a procedure that uses -> `${PD_SECRET}`: +> **PD REST credential.** Calls to a PD REST endpoint on port 8620, other than `/v1/health`, `/v1/ready`, `/actuator/**` and `/v1/prom/targets/*`, need HTTP Basic auth: one of the internal service names (`hg`, `store`, `hubble`, `vermeer`) and PD's `auth.secret-key` value as the password. A call without it gets HTTP 401, which for a mutating step such as `balanceLeaders` means the step did nothing. Export the secret before following a procedure that uses `${PD_SECRET}`: > > ```bash > read -rs PD_SECRET && export PD_SECRET > ``` > -> Store endpoints on port 8520 are unaffected. Some PD examples in this guide -> still omit the credential; add `-u hg:"${PD_SECRET}"` when a call returns 401. +> Store endpoints on port 8520 are unaffected. Some PD examples in this guide still omit the credential; add `-u hg:"${PD_SECRET}"` when a call returns 401. ## Table of Contents @@ -631,8 +624,7 @@ curl http://192.168.1.10:8620/v1/partitionsAndStatus 4. **Monitor Rebalancing**: ```bash # Watch partition distribution - watch -n 10 'curl -u hg:"${PD_SECRET}" \ - http://192.168.1.10:8620/v1/partitionsAndStatus' + watch -n 10 'curl -u hg:"${PD_SECRET}" http://192.168.1.10:8620/v1/partitionsAndStatus' ``` 5. **Verify**: Wait for even distribution (may take hours) From ee98ef9024a9afbce3637d1c46e06bc0019bcbd1 Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 9 Sep 2026 14:53:50 +0800 Subject: [PATCH 28/28] fix(pd): streamline docker README, unwrap markdown lines, and expand auth test cases --- docker/README.md | 63 ++++--------------- hugegraph-pd/README.md | 4 +- hugegraph-pd/docs/api-reference.md | 48 +++----------- .../interceptor/AuthenticationTest.java | 28 ++++++--- hugegraph-store/docs/deployment-guide.md | 13 +--- 5 files changed, 44 insertions(+), 112 deletions(-) diff --git a/docker/README.md b/docker/README.md index 762fb7782a..081fa36dc8 100644 --- a/docker/README.md +++ b/docker/README.md @@ -50,16 +50,9 @@ contains a single quote or newline. Do not commit `.env` or `conf/hubble/*.local.properties`; both are in `.gitignore`. Keeping the same JWT secret preserves authentication tokens when containers are recreated. For authenticated topologies with multiple Server replicas, all replicas receive this same secret. The HA topology fails fast if authentication is enabled without this shared secret. -A non-empty `HUGEGRAPH_ADMIN_PASSWORD` enables Server authentication, and -Hubble detects that mode automatically. Omitting the variable or setting it to -an empty value disables authentication. Auth-off is only suitable for a -trusted local environment; never expose it to a public or untrusted network. -Hubble listens on host loopback by default. Set `HUBBLE_PUBLISH_HOST` only -behind an HTTPS reverse proxy and trusted network controls. +A non-empty `HUGEGRAPH_ADMIN_PASSWORD` enables Server authentication, and Hubble detects that mode automatically. Omitting the variable or setting it to an empty value disables authentication. Auth-off is only suitable for a trusted local environment; never expose it to a public or untrusted network. Hubble listens on host loopback by default. Set `HUBBLE_PUBLISH_HOST` only behind an HTTPS reverse proxy and trusted network controls. -`HUGEGRAPH_ADMIN_PASSWORD` initializes the built-in `admin` account on its -first authenticated startup. Changing `.env` does not rotate an existing -administrator password; use the HugeGraph user API for credential changes. +`HUGEGRAPH_ADMIN_PASSWORD` initializes the built-in `admin` account on its first authenticated startup. Changing `.env` does not rotate an existing administrator password; use the HugeGraph user API for credential changes. For the verification commands below, load `.env` into your current shell and set the password: @@ -197,8 +190,7 @@ Status: docker compose -f docker-compose-3pd-3store-3server.yml ps ``` -Verify all published PD, Store, and Server endpoints, Server authentication, -and Hubble: +Verify all published PD, Store, and Server endpoints, Server authentication, and Hubble: ```bash for port in 8620 8621 8622; do @@ -218,34 +210,11 @@ done curl -fsS http://localhost:8088/about ``` -PD answers two unauthenticated probe endpoints. `/v1/health` is liveness only: -it returns `200` as soon as the REST listener is up, even when the PD has no -raft leader. `/v1/ready` returns `200` only while the PD sees a raft leader, -and `503` otherwise. Each PD answers for itself: a single PD elects itself, and -in a three-PD group the two that can reach each other elect a leader and turn -ready, while a partitioned third keeps answering `503` until it sees that -leader. - -The healthchecks in these files still gate on `/v1/health`, because -`/v1/ready` ships from the next release onwards while the files run published -images. Two things to know before pointing them at readiness: - -- Match on the body, not the status code. As of 1.7.0 PD answers `200` with - `{"status":-1,"error":"Unauthorized!"}` on every path its auth interceptor - does not exclude, a path that does not exist included, so a status-only - probe reads a PD too old to have `/v1/ready` as ready. The body match holds - whichever status a refusal carries. Gate with - `curl -fsS http://localhost:8620/v1/ready | grep -q '"ready":true'` instead. -- Pin `HUGEGRAPH_VERSION` to a release that carries the endpoint, or build the - images from source with `docker-compose.dev.yml`. - -The `HEALTHCHECK` baked into `hugegraph-pd/Dockerfile` is `/v1/health` as well. -Both compose files override it, so it governs `docker run` and anything else -inheriting the image probe, and those keep reading a PD without a quorum as -healthy. +PD answers two unauthenticated probe endpoints: `/v1/health` for liveness (returns 200 once the REST listener is up, regardless of raft state), and `/v1/ready` for readiness (returns 200 only when PD sees a raft leader, 503 otherwise). -Open `http://localhost:8088` and sign in as `admin` with the password from -`.env`. +Compose healthchecks currently gate on `/v1/health` for compatibility with published images. When targeting readiness on newer releases or source builds (`docker-compose.dev.yml`), match on the response body (`curl -fsS http://localhost:8620/v1/ready | grep -q '"ready":true'`). + +Open `http://localhost:8088` and sign in as `admin` with the password from `.env`. Stop containers while keeping them: @@ -281,9 +250,7 @@ HUBBLE_IMAGE=hugegraph/hubble:latest \ docker compose -f docker-compose.yml up -d ``` -The Hubble `latest` image is expected to work with HugeGraph Server 1.7 and -Server `latest`; compatibility with versions older than 1.7 is not promised. -Pin immutable image references when reproducibility is required. +The Hubble `latest` image is expected to work with HugeGraph Server 1.7 and Server `latest`; compatibility with versions older than 1.7 is not promised. Pin immutable image references when reproducibility is required. ### Server startup timeout @@ -372,8 +339,7 @@ The three small files under `conf/hubble/` contain only topology-specific discov The two HStore topologies mount the generated `*.local.properties` next to these examples (see `set-hubble-pd-password.sh`), never the examples themselves, so the PD secret stays out of tracked files. -Hubble detects Server authentication through the Server API. Do not add an -`auth.enabled` property or duplicate auth-on/auth-off configurations. +Hubble detects Server authentication through the Server API. Do not add an `auth.enabled` property or duplicate auth-on/auth-off configurations. ### Render and smoke checks @@ -383,12 +349,9 @@ Render every topology with auth-on inputs before submitting a change: bash test-compose.sh render ``` -The HA render is mandatory even when local resources are insufficient to start -its ten containers. +The HA render is mandatory even when local resources are insufficient to start its ten containers. -Run focused auth-on smoke checks for standalone and minimal HStore with the -corresponding `up -d --wait`, status, authentication, Hubble `/about`, and -`down -v` commands from the Users section: +Run focused auth-on smoke checks for standalone and minimal HStore with the corresponding `up -d --wait`, status, authentication, Hubble `/about`, and `down -v` commands from the Users section: ```bash bash test-compose.sh smoke @@ -400,6 +363,4 @@ Run the required local auth-off checks separately: bash test-compose.sh smoke-auth-off ``` -The auth-off mode is intentionally excluded from the default CI matrix and must -remain on a trusted local machine. Both smoke modes remove only the isolated -Compose projects and volumes that they create. +The auth-off mode is intentionally excluded from the default CI matrix and must remain on a trusted local machine. Both smoke modes remove only the isolated Compose projects and volumes that they create. diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 35ad023918..5050025565 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -240,9 +240,7 @@ Build PD Docker image: # From project root docker build -f hugegraph-pd/Dockerfile -t hugegraph/pd:latest . -# Generate the REST secret once and keep it: every PD REST client needs this -# same value, and a new one silently breaks the clients already using the old -# one. Store it somewhere durable rather than only in this shell. +# Generate the REST secret once and keep it: every PD REST client needs this same value, and a new one silently breaks the clients already using the old one. Store it somewhere durable rather than only in this shell. export HG_PD_AUTH_SECRET_KEY="$(openssl rand -hex 24)" # Run container diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 03297d9569..f7c37be9d5 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -806,33 +806,13 @@ curl -i http://localhost:8620/v1/ready } ``` -A follower reports `"state": "STATE_FOLLOWER"` with `"isLeader": false`. When -the quorum is lost the PD keeps answering `/v1/health` with `200` but -`/v1/ready` turns into `503` with `"ready": false`. Being unauthenticated, the -body carries no cluster addresses; the leader's address stays on `/v1/members`. - -The answer is served from state the raft callbacks maintain rather than from -the raft node, so it stays prompt while an election is running and never waits -on the node lock. `state` is therefore the last change raft announced. A PD -reports `STATE_UNINITIALIZED` with `"ready": false` from process start until -its first raft callback, which is the ordinary startup window before a quorum -first forms, and jraft emits no callback for candidacy or leadership transfer, -so a candidate reports `STATE_FOLLOWER` with `"ready": false`. - -Point Kubernetes readiness probes, `depends_on` healthchecks and any -"wait for PD" script at `/v1/ready`; keep liveness probes on `/v1/health` -so a PD that merely lost its leader is not restarted. - -Match on the body rather than on the status code alone. A PD that predates this -endpoint does not reliably answer `404` for it: `RestAuthentication` refuses a -request it does not exclude by writing an error envelope, and as of 1.7.0 it -does so without setting a status, so an unknown path answers `200` with -`{"status":-1,"error":"Unauthorized!"}`. A status-only probe therefore reads -such a PD as ready. The body match holds whichever status a refusal carries: a -shell gate should use -`curl -fsS http://:8620/v1/ready | grep -q '"ready":true'`, and a -Kubernetes `httpGet` probe should be paired with a PD image that carries the -endpoint. +A follower reports `"state": "STATE_FOLLOWER"` with `"isLeader": false`. When the quorum is lost the PD keeps answering `/v1/health` with `200` but `/v1/ready` turns into `503` with `"ready": false`. Being unauthenticated, the body carries no cluster addresses; the leader's address stays on `/v1/members`. + +The answer is served from state the raft callbacks maintain rather than from the raft node, so it stays prompt while an election is running and never waits on the node lock. `state` is therefore the last change raft announced. A PD reports `STATE_UNINITIALIZED` with `"ready": false` from process start until its first raft callback, which is the ordinary startup window before a quorum first forms, and jraft emits no callback for candidacy or leadership transfer, so a candidate reports `STATE_FOLLOWER` with `"ready": false`. + +Point Kubernetes readiness probes, `depends_on` healthchecks and any "wait for PD" script at `/v1/ready`; keep liveness probes on `/v1/health` so a PD that merely lost its leader is not restarted. + +Match on the body rather than on the status code alone. A PD that predates this endpoint does not reliably answer `404` for it: `RestAuthentication` refuses a request it does not exclude by writing an error envelope, and as of 1.7.0 it does so without setting a status, so an unknown path answers `200` with `{"status":-1,"error":"Unauthorized!"}`. A status-only probe therefore reads such a PD as ready. The body match holds whichever status a refusal carries: a shell gate should use `curl -fsS http://:8620/v1/ready | grep -q '"ready":true'`, and a Kubernetes `httpGet` probe should be paired with a PD image that carries the endpoint. ### Metrics @@ -866,19 +846,11 @@ Exported on `/actuator/prometheus` for alerting on quorum loss: | `hg_raft_has_leader` | `1` while this PD sees a leader (is inside a quorum), `0` otherwise | | `hg_raft_alive_peers` | Number of alive peers on the leader, itself included; `NaN` elsewhere | -`hg_raft_alive_peers` counts the peers the leader has heard from within the -leader lease timeout, which jraft derives as 90% of the election timeout by -default. +`hg_raft_alive_peers` counts the peers the leader has heard from within the leader lease timeout, which jraft derives as 90% of the election timeout by default. -A cluster has lost its quorum when `sum(hg_raft_leader) == 0` or when -`hg_raft_has_leader == 0` on every member. Both are briefly true during a -normal election, so alert on them with a `for:` clause longer than the -election timeout rather than on the instantaneous value. +A cluster has lost its quorum when `sum(hg_raft_leader) == 0` or when `hg_raft_has_leader == 0` on every member. Both are briefly true during a normal election, so alert on them with a `for:` clause longer than the election timeout rather than on the instantaneous value. -Do not aggregate `hg_raft_alive_peers` across instances: it is `NaN` on every -node but the leader, and one `NaN` sample turns the result of `sum()` or -`avg()` into `NaN` as well. Select the leader's series instead, for example -`hg_raft_alive_peers and on(instance) (hg_raft_leader == 1)`. +Do not aggregate `hg_raft_alive_peers` across instances: it is `NaN` on every node but the leader, and one `NaN` sample turns the result of `sum()` or `avg()` into `NaN` as well. Select the leader's series instead, for example `hg_raft_alive_peers and on(instance) (hg_raft_leader == 1)`. ### Partition API diff --git a/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java b/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java index 688743ee0d..8793642d35 100644 --- a/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java +++ b/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/service/interceptor/AuthenticationTest.java @@ -179,15 +179,17 @@ public void testOwnSecretStarts() { @Test public void testRestAuthenticationAcceptsCaseInsensitiveBasicScheme() throws Exception { RestAuthentication auth = restAuthWithSecret(SECRET); - String cred = credential("hg", SECRET); - Assert.assertTrue("Basic with uppercase B should be accepted", - acceptsRest(auth, "Basic " + cred)); - Assert.assertTrue("basic with all lowercase should be accepted", - acceptsRest(auth, "basic " + cred)); - Assert.assertTrue("BASIC with all uppercase should be accepted", - acceptsRest(auth, "BASIC " + cred)); - Assert.assertTrue("BaSiC with mixed case should be accepted", - acceptsRest(auth, "BaSiC " + cred)); + for (String name : new String[]{"hg", "store", "hubble", "vermeer"}) { + String cred = credential(name, SECRET); + Assert.assertTrue("Basic with uppercase B should be accepted for " + name, + acceptsRest(auth, "Basic " + cred)); + Assert.assertTrue("basic with all lowercase should be accepted for " + name, + acceptsRest(auth, "basic " + cred)); + Assert.assertTrue("BASIC with all uppercase should be accepted for " + name, + acceptsRest(auth, "BASIC " + cred)); + Assert.assertTrue("BaSiC with mixed case should be accepted for " + name, + acceptsRest(auth, "BaSiC " + cred)); + } } @Test @@ -198,6 +200,10 @@ public void testRestAuthenticationRefusesNonBasicOrMalformedSchemes() throws Exc acceptsRest(auth, null)); Assert.assertFalse("empty Authorization header must be refused", acceptsRest(auth, "")); + Assert.assertFalse("Basic with empty credentials must be refused", + acceptsRest(auth, "Basic ")); + Assert.assertFalse("basic with empty credentials must be refused", + acceptsRest(auth, "basic ")); Assert.assertFalse("Bearer scheme must be refused", acceptsRest(auth, "Bearer " + cred)); Assert.assertFalse("Digest scheme must be refused", @@ -206,5 +212,9 @@ public void testRestAuthenticationRefusesNonBasicOrMalformedSchemes() throws Exc acceptsRest(auth, "Basic" + cred)); Assert.assertFalse("invalid credential with basic prefix must be refused", acceptsRest(auth, "basic " + credential("hg", "wrong-secret"))); + Assert.assertFalse("unknown service name with basic prefix must be refused", + acceptsRest(auth, "basic " + credential("unknown", SECRET))); + Assert.assertFalse("non-base64 token with basic prefix must be refused", + acceptsRest(auth, "basic ???")); } } diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index cfe47d977f..ca39ae509c 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -740,10 +740,7 @@ environment: 2. Store nodes start after all PD nodes are healthy 3. Server nodes start after all Store nodes are healthy -`/v1/health` answers `200` as soon as the PD REST listener is up, so step 1 does -not wait for a raft quorum to form. PD also serves `/v1/ready`, which answers -`200` only while the PD sees a raft leader; `docker/README.md` covers what -pointing the healthchecks at it requires. +`/v1/health` answers `200` as soon as the PD REST listener is up, so step 1 does not wait for a raft quorum to form. PD also serves `/v1/ready`, which answers `200` only while the PD sees a raft leader; `docker/README.md` covers what pointing the healthchecks at it requires. > **Note**: The deprecated env var names (`GRPC_HOST`, `RAFT_ADDRESS`, `RAFT_PEERS`, `PD_ADDRESS`, `BACKEND`, `PD_PEERS`) still work but log a warning. Use the `HG_*` prefixed names for new deployments. @@ -887,13 +884,7 @@ curl -i http://192.168.1.10:8620/v1/ready curl http://192.168.1.20:8520/v1/health ``` -> **Note**: `/v1/ready` ships from the release after `1.7.0`, so the Docker -> examples above, which pin `HUGEGRAPH_VERSION=1.7.0`, need a newer tag or -> images built from source before this check means anything. On `1.7.0` the PD -> answers `200` with `{"status":-1,"error":"Unauthorized!"}` on any path its -> auth interceptor does not exclude, `/v1/ready` included, so match on the body -> rather than the status code. See -> [docker/README.md](../../docker/README.md) for the details. +> **Note**: `/v1/ready` ships from the release after `1.7.0`, so the Docker examples above, which pin `HUGEGRAPH_VERSION=1.7.0`, need a newer tag or images built from source before this check means anything. On `1.7.0` the PD answers `200` with `{"status":-1,"error":"Unauthorized!"}` on any path its auth interceptor does not exclude, `/v1/ready` included, so match on the body rather than the status code. See [docker/README.md](../../docker/README.md) for the details. ### Cluster Status