bitflicker64 commented on code in PR #3189:
URL: https://github.com/apache/hugegraph/pull/3189#discussion_r3928159161


##########
hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml:
##########
@@ -44,6 +44,15 @@ server:
   # REST service port number
   port: 8620
 
+auth:
+  # Shared secret checked against the password of the Basic credential on every
+  # authenticated REST request. Clients must send one of the internal service
+  # names (hg, store, hubble, vermeer) with this value as the password.
+  # CHANGE THIS IN PRODUCTION and keep it in sync with every client that calls
+  # the PD REST API (e.g. Hubble's operations.pd.password). Do not expose the
+  # REST port (8620) outside a trusted network.
+  secret-key: FXQXbJtbCLxODc6tGci732pkH1cyf8Qg

Review Comment:
   You are right, and this is fixed in 5c339c1. Both halves of what you asked 
for:
   
   Required, not defaulted. Both `application.yml` files now ship 
`auth.secret-key` empty. The Docker image requires `HG_PD_AUTH_SECRET_KEY`, and 
both HStore Compose files fail to render without it rather than falling back to 
a shared value; the `.env` recipe generates one next to the JWT secret. The 
Hubble properties ship the password empty with the manual step documented.
   
   Fails startup on the published default. PD refuses to boot if 
`auth.secret-key` equals the value earlier revisions carried, so a deployment 
that copied it does not silently keep a well-known credential.
   
   Verified on the packaged dist: startup refused with the published value, 
shipped config starts but answers 401 to every credential while naming the key 
in an error, and a deployment-provided secret restores normal access.
   
   Unrelated but worth flagging: `hugegraph-struct/.../AuthOptions.java:51` 
uses the same published string as the default for the Server's 
`auth.token_secret`. Out of scope here; happy to file it separately.



##########
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}"

Review Comment:
   Fixed in 5c339c1 after all, rather than deferring it. The inner shell now 
reads the credential from the environment and passes it to curl on stdin as a 
config file, so it is neither part of the `bash -c` source text nor visible in 
argv. Values are escaped for the `-K` quoted-string form, backslash first. 
`PD_AUTH_PASSWORD` no longer carries a default, since PD ships no secret. 
`test-wait-storage.sh` now asserts the credential is absent from argv and 
present in the config the mock reads from stdin; 5/5 still pass.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to