fix(docker): make auth bootstrap safe for mounted and upgraded configs

[Adarsh-Me]

What is changing

Closes #3133 (the parts still open on master 3681148, since #3119 landed the rest).

Properties rewriting now implements the Java grammar. docker-entrypoint.sh previously rewrote rest-server.properties / hugegraph.properties with grep/sed, which disagrees with HugeConfig on mounted or upgraded configs: backslash-escaped keys, :/whitespace separators, line continuations, and duplicate definitions are all parsed differently. The property logic moves to a new props.awk loaded by the entrypoint, which implements the java.util.Properties line grammar (comments, both separators, continuations, backslash escapes, first-definition-wins duplicates) and rewrites the first definition in place while keeping every untouched line byte-for-byte.

PASSWORD no longer appears in ps output. The old sed rewrite interpolated the encoded value into sed's command line, so when a key already existed (mounted or persisted config) the password was visible in ps. Values now travel through an environment variable into awk, never through argv.

enable-auth.sh appends are per-file guarded. The old script appended authentication definitions whenever conf-bak/ was absent. On a config it did not write, that created duplicate definitions that the properties parser (first definition wins) and snakeyaml (last definition wins) resolved in opposite directions — Gremlin and REST could land on different authenticators with no error from either. Now each append runs only when its file lacks the definition (or still has it commented out), re-runs are idempotent, custom gremlin.graph factories are preserved, and the authenticator class can be overridden via AUTHENTICATOR_CLASS.

The entrypoint aligns both sides before enabling auth. If the yaml declares an authenticator but the properties file does not (or vice versa), the entrypoint propagates it to the other side instead of letting the default StandardAuthenticator split the pair. When both sides name genuinely different authenticators, it logs a WARN and leaves both untouched instead of silently splitting them.

Implementation notes

• I chose a shell/awk implementation of the properties grammar over the issue's proposed Java ConfigTool CLI: it delivers the same parser-agreement contract with a much smaller footprint and no new build artifact. Happy to rework toward the ConfigTool if reviewers prefer that direction.
• props.awk ships in both server images (Dockerfile COPY) and in the test sandbox; CI already runs the unit suite via docker-build-ci.yml.

How was this tested

• docker/test/test-docker-entrypoint.sh extended with cases for: escaped-key definitions rewritten in place, continuation lines consumed with the key they belong to, get-mode separator/continuation/duplicate semantics, and appends when the key only exists commented out. All pass.
• docker-entrypoint-test.sh full harness passes end-to-end (secret round-trips incl. backslash/space/trailing-space secrets, enable-auth call counting unchanged).
• enable-auth.sh manually exercised against: fresh default config, idempotent re-run, mounted config with custom authenticator (no duplicates), and custom gremlin.graph factory (preserved).

Code Review Handbook

• props.awk is the core: block model (comment lines and logical entries), first-definition-wins, raw-value round-trip (get returns the on-disk escaped form so feeding it back into set is byte-exact).
• The value of get is intentionally not unescaped: the entrypoint re-writes secrets it just read, and unescape-then-re-encode would double-escape backslashes (caught by the complex-secret round-trip in docker-entrypoint-test.sh).
• The escaped-key unit case is the reported auth\.admin_pa scenario: the old grep could not match it, so the append created a duplicate and HugeConfig silently kept pa.

[bitflicker64]

Blocking: yes. Summary: props.awk is careful work and both shell suites this PR touches pass under the image's own mawk, but the new alignment layer does not hold on the mounted configs the PR targets. The yaml authenticator is copied into rest-server.properties without unquoting, a flow-style authentication: block reads as absent so REST silently falls back to the default, enable-auth.sh's guards accept only the key= spelling so duplicates are still appended, and the narrowed gremlin.graph guard no longer converts a CRLF config the old sed did convert. Two doc fixes as well. Evidence: measured at 698b0c3 in ubuntu:22.04 (GNU grep 3.7, GNU sed, mawk 1.3.4), the same toolchain eclipse-temurin:11-jre-jammy ships; test/test-docker-entrypoint.sh and docker-entrypoint-test.sh both exit 0 there; each finding below quotes the config the run produced. All seven workflow runs on this head are action_required and the combined status is pending with zero statuses, so there is no CI evidence for the image builds.

[bitflicker64]

⚠️ The yaml scalar goes into rest-server.properties as-is, so quotes and inline comments become part of the class name.

sub(/[,:].*$/, "", line) strips a trailing comma and nothing else, and line 123 hands the result straight to set_prop_encoded. snakeyaml strips quotes, comments and padding; java.util.Properties strips none of them. Measured at 698b0c3 on ubuntu:22.04:

yaml   authenticator: "com.example.MyAuth"         ->  auth.authenticator="com.example.MyAuth"
yaml   authenticator: com.example.MyAuth␣␣␣        ->  auth.authenticator=com.example.MyAuth␣␣␣
yaml   authenticator: com.example.MyAuth # custom  ->  auth.authenticator=com.example.MyAuth # custom

Gremlin resolves com.example.MyAuth in all three rows. REST gets a different string in all three, which is the split align_auth_config exists to prevent, and a quoted scalar is ordinary yaml.

Requested change: before returning, strip an inline # comment, then surrounding single or double quotes, then trailing blanks.

[bitflicker64]

⚠️ This next discards the rest of the authentication: line, so a single-line flow mapping reads as no authenticator and the alignment quietly falls back to the default.

Legal yaml that a hand-written or mounted file can carry:

authentication: {authenticator: com.example.MyAuth, authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler, config: {tokens: conf/rest-server.properties}}

At 698b0c3 on ubuntu:22.04, get_yaml_authenticator prints nothing, so align_auth_config takes the both-empty branch and exports StandardAuthenticator. After ./bin/enable-auth.sh:

rest: auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator
yaml: authentication: {authenticator: com.example.MyAuth, ...}

Gremlin on the mounted class, REST on the default, no WARN. On a first run (pre-PR the whole block was gated on conf-bak not existing) the old script appended a second authentication: block, so both sides came out on the default under the last-wins rule that your comment on line 106 cites.

Requested change: match authenticator: on the authentication: line as well, and when an authentication block is present but no authenticator can be read, take the WARN branch rather than the both-empty one.

[bitflicker64]

⚠️ These guards accept only the key= spelling, so the forms props.awk was added for still get a duplicate appended.

^[ \t]*auth\.authenticator[ \t]*= matches none of auth.authenticator : X, auth.authenticator X, auth\.authenticator=X, and java.util.Properties reads all three as a definition of auth.authenticator. Line 68 has the same shape. Note also that [ \t] in a GNU grep -E bracket is the set {space, backslash, t}, not a tab: under GNU grep 3.7 a tab-indented auth.authenticator=X fails this guard too.

Docker path at 698b0c3, mounted config using :, after align_auth_config && ./bin/enable-auth.sh:

auth.authenticator : com.example.MyAuth
auth.authenticator=com.example.MyAuth

Two definitions of one key, which the unit test you add at docker/test/test-docker-entrypoint.sh:76-79 names as the thing to avoid. Run standalone, with AUTHENTICATOR_CLASS unset, the same input gives those two lines with different values (: com.example.MyAuth, then =org.apache.hugegraph.auth.StandardAuthenticator) plus a yaml block on the default.

This is an incomplete fix rather than a regression, since the old code appended unconditionally. Requested change: use [[:blank:]], accept [:=] and the bare-whitespace separator, and cover the \-escaped key. Shipping props.awk under bin/ and querying it here would keep one grammar in one place.

[bitflicker64]

⚠️ Anchoring the flip to [ \t]*$ drops a CRLF config that the old unanchored sed did convert.

Measured at 698b0c3 on ubuntu:22.04 with conf/graphs/hugegraph.properties saved with CRLF line endings:

$ grep -Eq '^gremlin\.graph[ \t]*=org\.apache\.hugegraph\.HugeFactory[ \t]*$' hugegraph.properties; echo $?
1
$ sed 's/gremlin.graph=org.apache.hugegraph.HugeFactory/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/g' hugegraph.properties   # pre-PR
gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy

End to end the graph keeps gremlin.graph=org.apache.hugegraph.HugeFactory while rest-server.properties gains auth.authenticator and auth.graph_store and the yaml gains an authentication block, so the factory is never wrapped for auth although both servers now believe auth is on. A CRLF mounted config is squarely the case this PR is about. A tab before = fails for the same reason as line 64.

Requested change: use [[:blank:]] and allow an optional carriage return, for example ^gremlin\.graph[[:blank:]]*=org\.apache\.hugegraph\.HugeFactory[[:blank:]]*\r?$, with the sed pattern widened to match.

[bitflicker64]

🧹 The documented invocation does not work: the names are PROPS_MODE, PROPS_KEY, PROPS_FILE and PROPS_VALUE_ENCODED.

BEGIN at lines 214-223 reads ENVIRON["PROPS_MODE"], ENVIRON["PROPS_KEY"], ENVIRON["PROPS_FILE"] and ENVIRON["PROPS_VALUE_ENCODED"], so following this header verbatim gives props.awk: PROPS_FILE and PROPS_KEY must be set.

Suggested change

	# One invocation, selected with the `mode` environment variable:
	#
	# mode=get key=K file=F
	# print the value of K's first logical definition
	# mode=set key=K file=F
	# replace K's first definition in place, drop every other
	# definition of K, append one when the file has none. The new
	# value arrives pre-encoded in PROP_VALUE_ENCODED (an environment
	# One invocation, selected with the `PROPS_MODE` environment variable:
	#
	# PROPS_MODE=get PROPS_KEY=K PROPS_FILE=F
	# print the value of K's first logical definition
	# PROPS_MODE=set PROPS_KEY=K PROPS_FILE=F
	# replace K's first definition in place, drop every other
	# definition of K, append one when the file has none. The new
	# value arrives pre-encoded in PROPS_VALUE_ENCODED (an environment

[bitflicker64]

🧹 This comment describes an anchor the code does not use. Line 30 recomputes PROPS_AWK independently, and the extraction on lines 32-36 still starts at encode_prop_value and stops on a count of closing braces. No marker comment is involved.

The count is also newly load-bearing and unexplained: == 4 means "through get_prop_encoded", so a helper added anywhere between encode_prop_value and get_prop_encoded would cut the eval short and the suite would then fail with a confusing get_prop_encoded: command not found.

Requested change: drop the marker-comment sentence and say what the number selects, for example "stop after the fourth top-level function, get_prop_encoded".

[imbajin]

Blocking: yes. Summary: The new properties parser still mishandles valid indented keys, so mounted authentication values can remain stale after an environment override. Evidence: reproduced at the exact head with the PR helper; existing current-head comments cover other findings and are not duplicated.

[imbajin]

⚠️ Blocking: yes. java.util.Properties ignores leading spaces before a key, but this scan treats the first leading space as the separator. Reproduced at this head with auth.token_secret: old-secret: get_prop_encoded returns empty, and set_prop_encoded appends auth.token_secret=new-secret while retaining the old line, leaving two logical definitions and the old value effective in HugeConfig. Strip leading whitespace before scanning for the key separator and add an indented-key regression test.